Skip to content

test: Add externalIPs tests to K8sServicesTest and disable K8sKubeProxyFreeMatrix#11696

Merged
aanm merged 3 commits intomasterfrom
pr/brb/ci-rm-kube-proxy-free-suite
May 27, 2020
Merged

test: Add externalIPs tests to K8sServicesTest and disable K8sKubeProxyFreeMatrix#11696
aanm merged 3 commits intomasterfrom
pr/brb/ci-rm-kube-proxy-free-suite

Conversation

@brb
Copy link
Copy Markdown
Member

@brb brb commented May 26, 2020

See commit msgs.

test-net-next should run ~20min faster.

Fix #11442.

@brb brb added pending-review area/CI Continuous Integration testing issue or flake area/CI-improvement Topic or proposal to improve the Continuous Integration workflow release-note/ci This PR makes changes to the CI. ci/flake This is a known failure that occurs in the tree. Please investigate me! labels May 26, 2020
@brb brb requested a review from a team as a code owner May 26, 2020 16:32
@brb brb force-pushed the pr/brb/ci-rm-kube-proxy-free-suite branch from f247f50 to 2659f43 Compare May 26, 2020 16:34
@brb brb requested a review from aanm May 26, 2020 16:35
@brb
Copy link
Copy Markdown
Member Author

brb commented May 26, 2020

test-me-please

brb added 3 commits May 26, 2020 18:35
It covers both - bpf_host and bpf_sock.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
The new name describes better what the function supposed to test.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
The suite does not provide much value because of the following reasons:

- It does not test the kube-proxy replacement from outside, so only
  bpf_sock is tested.
- K8sServicesTest should provide the same coverage.
- It takes 20min to run the suite.

Signed-off-by: Martynas Pumputis <m@lambda.lt>
Comment thread test/k8sT/external_ips.go
Comment on lines +37 to +42
func skipSuite(name string, t func()) bool {
return false
}

// Replace "skipSuite" with "Describe" to enable the suite.
var _ = skipSuite("K8sKubeProxyFreeMatrix tests", func() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are we gaining by disabling this entire suite? This is testing 396 different test cases in 15 minutes which is roughly 2.7 seconds per test case.

Copy link
Copy Markdown
Member Author

@brb brb May 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

20min for each test-net-next run to test just bpf_sock implementation is imho too much, considering that K8sServicesTest already does it.
If we want to test with all possible permutations, we can run the K8sKubeProxyFreeMatrix suite manually, same as we do e.g. with -race.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the K8sServicesTest is only a subset of all tests written in K8sKubeProxyFreeMatrix

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but K8sServicesTest should have the full coverage of bpf_sock.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.007%) to 36.881% when pulling 2659f43 on pr/brb/ci-rm-kube-proxy-free-suite into 84db4f6 on master.

@brb
Copy link
Copy Markdown
Member Author

brb commented May 27, 2020

retest-4.19

@brb brb requested a review from aanm May 27, 2020 15:40
@brb
Copy link
Copy Markdown
Member Author

brb commented May 27, 2020

4.19 failed due to L7+NodePort flake.

@aanm aanm merged commit e09d991 into master May 27, 2020
@aanm aanm deleted the pr/brb/ci-rm-kube-proxy-free-suite branch May 27, 2020 16:16
@pchaigno
Copy link
Copy Markdown
Member

pchaigno commented Aug 5, 2020

Marking for backports mostly to disable the K8sKubeProxyFreeMatrix tests in the v1.7 branch. The two first commits can probably be skipped if they generate conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/CI Continuous Integration testing issue or flake area/CI-improvement Topic or proposal to improve the Continuous Integration workflow ci/flake This is a known failure that occurs in the tree. Please investigate me! release-note/ci This PR makes changes to the CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: k8s-1.11 / net-next test run times out before completing

6 participants