Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add quarantine mechanism to test suite #11981

Merged
merged 3 commits into from Jun 15, 2020
Merged

Add quarantine mechanism to test suite #11981

merged 3 commits into from Jun 15, 2020

Conversation

nebril
Copy link
Member

@nebril nebril commented Jun 9, 2020

There is a new cli options for our test suite - cilium.runQuarantined, which tells ginkgo whether to skip tests which are marked as quarantined.

Test is marked as being under quarantine when its It(... call is replaced by SkipItIf(helpers.SkipQuarantined, ...

@nebril nebril requested a review from a team as a code owner June 9, 2020 12:47
@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

@nebril
Copy link
Member Author

nebril commented Jun 9, 2020

test-runtime

@nebril
Copy link
Member Author

nebril commented Jun 9, 2020

test-4.9

@nebril nebril added the release-note/ci This PR makes changes to the CI. label Jun 9, 2020
@coveralls
Copy link

coveralls commented Jun 9, 2020

Coverage Status

Coverage increased (+0.03%) to 37.064% when pulling 1131776 on pr/quarantine-jobs into f3b474a on master.

@pchaigno
Copy link
Member

pchaigno commented Jun 9, 2020

Is that for flakes? Just to make it easier to see which tests are disabled compared to PIt, PDescribe, etc.?

It would be nice to have the same for Context and Describe as we sometimes disable whole contexts.

@nebril
Copy link
Member Author

nebril commented Jun 10, 2020

@pchaigno this is for flakes, but also to allow running flaky tests within "quarantine" pipelines. Whole contexts will be skipped by using SkipContextIf, similarly to how It blocks became quarantined in this change.

@pchaigno
Copy link
Member

this is for flakes, but also to allow running flaky tests within "quarantine" pipelines.

Ah, nice!

Whole contexts will be skipped by using SkipContextIf, similarly to how It blocks became quarantined in this change.

Of course 🤦 I read the code a bit too quickly.

test/config/config.go Show resolved Hide resolved
test/k8sT/Policies.go Show resolved Hide resolved
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.0 Jun 11, 2020
@aanm
Copy link
Member

aanm commented Jun 11, 2020

@nebril I assume this is supposed to be backported?

@aanm aanm added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jun 11, 2020
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
@nebril nebril requested a review from a team as a code owner June 15, 2020 11:44
@nebril
Copy link
Member Author

nebril commented Jun 15, 2020

test-runtime

@nebril
Copy link
Member Author

nebril commented Jun 15, 2020

test-4.9

@nebril nebril requested a review from pchaigno June 15, 2020 11:50
@qmonnet
Copy link
Member

qmonnet commented Jun 15, 2020

Note: We could maybe quarantine the check disabled in this commit for Linux 4.19, but I can do that in a follow-up if necessary.

@nebril
Copy link
Member Author

nebril commented Jun 15, 2020

@qmonnet interesting case, so you're saying that it should be quarantined only on 4.19? It should be easy enough to do.

@@ -37,6 +37,10 @@ pipeline {
returnStdout: true,
script: 'if [ "${JobKernelVersion}" = "net-next" ]; then echo -n "0"; else echo -n ""; fi'
)}"""
RUN_QUARANTINED="""${sh(
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need to update other Jenkinsfile as well? Also (unrelated to your PR), but can we somehow share some sections among all Jenkinsfiles (e.g. ENV vars)?

Copy link
Member Author

Choose a reason for hiding this comment

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

@brb this is only for running https://jenkins.cilium.io/view/Quarantine%20Pipelines/ which for now only use this jenkinsfile.

@nebril nebril merged commit 17b597d into master Jun 15, 2020
1.8.0 automation moved this from In progress to Merged Jun 15, 2020
@nebril nebril deleted the pr/quarantine-jobs branch June 15, 2020 15:10
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.0 Jun 16, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.0 Jun 16, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.8 to Backport done to v1.8 in 1.8.0 Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. release-note/ci This PR makes changes to the CI.
Projects
No open projects
1.8.0
  
Merged
1.8.0
Backport done to v1.8
Development

Successfully merging this pull request may close these issues.

None yet

7 participants