MCO-1652: Add MCO disruptive suite#29776
MCO-1652: Add MCO disruptive suite#29776openshift-merge-bot[bot] merged 2 commits intoopenshift:mainfrom
Conversation
|
@yuqi-zhang: This pull request references MCO-1652 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
|
/hold Not 100% decided on whether we'd like to move existing tests here yet, or we should just set this up for layering |
openshift-ci
Bot
added
the
do-not-merge/hold
yuqi-zhang
force-pushed
the
add-mco-disruptive-suite
branch
from
ebeaa62 to
f67ce4d
Compare
yuqi-zhang
force-pushed
the
add-mco-disruptive-suite
branch
from
f67ce4d to
edf191f
Compare
|
Job Failure Risk Analysis for sha: edf191f
|
| ) | ||
|
|
||
| var _ = g.Describe("[sig-mco][OCPFeatureGate:MachineConfigNodes]", func() { | ||
| var _ = g.Describe("[Suite:openshift/machine-config-operator/disruptive][sig-mco][OCPFeatureGate:MachineConfigNodes]", func() { |
There was a problem hiding this comment.
As far as we can tell, this would cause the test to only run in the new MCO suite, and not for serial runs anymore.
We're depending on this for signal, so while we're transitioning between the tests, would it be possible to run the test on both Serial and the new MCO suite? Would we have to define this test twice with different labels?
openshift-merge-robot
added
the
needs-rebase
yuqi-zhang
force-pushed
the
add-mco-disruptive-suite
branch
from
edf191f to
41dc8ba
Compare
openshift-merge-robot
removed
the
needs-rebase
|
Job Failure Risk Analysis for sha: 41dc8ba
|
|
Job Failure Risk Analysis for sha: 8cefbf9
Risk analysis has seen new tests most likely introduced by this PR. New Test Risks for sha: 8cefbf9
New tests seen in this PR at sha: 8cefbf9
|
yuqi-zhang
force-pushed
the
add-mco-disruptive-suite
branch
from
8cefbf9 to
b625c64
Compare
|
Job Failure Risk Analysis for sha: b625c64
Risk analysis has seen new tests most likely introduced by this PR. New Test Risks for sha: b625c64
New tests seen in this PR at sha: b625c64
|
| g.It("[Suite:openshift/conformance/serial][Serial][Slow]Should properly report MCN conditions on node degrade [apigroup:machineconfiguration.openshift.io]", func() { | ||
| if IsSingleNode(oc) { //handle SNO clusters | ||
| ValidateMCNConditionOnNodeDegrade(oc, invalidMasterMCFixture, true) | ||
| } else { //handle standard, non-SNO, clusters | ||
| ValidateMCNConditionOnNodeDegrade(oc, invalidWorkerMCFixture, false) | ||
| } | ||
| }) | ||
|
|
||
| g.It("[Serial][Slow]Should properly create and remove MCN on node creation and deletion [apigroup:machineconfiguration.openshift.io]", func() { | ||
| g.It("[Suite:openshift/conformance/serial][Serial][Slow]Should properly create and remove MCN on node creation and deletion [apigroup:machineconfiguration.openshift.io]", func() { |
There was a problem hiding this comment.
These two tests don't currently run in the default serial suite (or any default suite) since they are labeled with slow, as can be seen here. So if it's possible to run some in the new suite and not others, these would be good candidates since they are not currently being used for our component readiness signal.
There was a problem hiding this comment.
ack, thanks! Should I remove the serial suite label entirely then? Theoretically it shouldn't make a difference? Or would this actually cause it to run in serial instead.
There was a problem hiding this comment.
Should I remove the serial suite label entirely then? Theoretically it shouldn't make a difference?
I don't think the serial suite label would do anything here, but I'm not 100% sure. We can maybe do a rehearsal payload job and see if this test shows up.
There was a problem hiding this comment.
Have you tried building locally and using --dry-run to see what tests are included with which suites?
I feel like if you don't expect it to be included in serial suite due to the [slow] annotation (which I believe is correct) you shouldn't add [Suite:openshift/conformance/serial]. Adding extra annotation now will just make it harder to rename later if you go back and cleanup and want to matchup the old tests with the new names.
Also I'm wondering if you need both [Suite:openshift/conformance/serial] and [Serial] or [Serial] alone will do it.
It's an open question since you are adding [Suite:openshift/machine-config-operator/disruptive] but you can test locally with something like
./openshift-tests run "openshift/conformance/serial" -o "${ARTIFACT_DIR}/e2e.log" --junit-dir "${ARTIFACT_DIR/junit}" --dry-run
and
./openshift-tests run "openshift/machine-config-operator/disruptive" -o "${ARTIFACT_DIR}/e2e.log" --junit-dir "${ARTIFACT_DIR/junit}" --dry-run
to see what tests would run. You can skip the --dry-run as well and actually run the tests if you like. Unfortunately you do need a cluster even for the --dry-run
There was a problem hiding this comment.
Also I'm wondering if you need both [Suite:openshift/conformance/serial] and [Serial] or [Serial] alone will do it.
I've tried with the existing tests via e2e-aws-ovn-serial-1of2/e2e-aws-ovn-serial-2of2 below, and [Serial] alone does not make the tests show up, whereas adding the full suite does (this is with the bootimage tests we have already, all the other tests are techpreview only so far)
And yes, let me remove the serial suite tag from the slow test.
|
/payload-job periodic-ci-openshift-release-master-ci-4.20-e2e-aws-ovn-techpreview-serial |
|
@isabella-janssen: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/7ab6ea20-35a1-11f0-8eda-2866fda3ec57-0 |
yuqi-zhang
force-pushed
the
add-mco-disruptive-suite
branch
from
b625c64 to
e10d904
Compare
yuqi-zhang
force-pushed
the
add-mco-disruptive-suite
branch
from
e10d904 to
c2bcd59
Compare
|
Job Failure Risk Analysis for sha: c2bcd59
Risk analysis has seen new tests most likely introduced by this PR. New Test Risks for sha: c2bcd59
New tests seen in this PR at sha: c2bcd59
|
|
/hold cancel Let's try to add these tests for the MCO suite, it should be fine since they will still run via serial/parallel so we shouldn't lose any signal. |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
/payload-job periodic-ci-openshift-release-master-ci-4.20-e2e-aws-ovn-techpreview-serial periodic-ci-openshift-release-master-ci-4.20-e2e-aws-ovn-techpreview Running these payloads to confirm that the same MCO tests continue running in the same test suites as before. |
|
@isabella-janssen: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/323a05b0-4222-11f0-8517-40a4de105581-0 |
|
Looks like the list of tests is correct although the run didn't pass |
I am only seeing one of the MCN parallel tests in the payload rehearsal (only Otherwise, the test failure does not look related to this work, so hopefully that was just a bad run. |
Many MCO tests require node disruption, so it was determined that these would best live as a separate suite. Create the MCO suite and move existing MCO tests in origin to it. The next goal is to add On Cluster Layering tests to this as well, potentially via OTE.
yuqi-zhang
force-pushed
the
add-mco-disruptive-suite
branch
from
c2bcd59 to
c4a38fc
Compare
|
Ack, sorry, missed those for some reason, should be fixed now, thanks! |
Eases transition
yuqi-zhang
force-pushed
the
add-mco-disruptive-suite
branch
from
c4a38fc to
5c9e36d
Compare
|
/payload-job periodic-ci-openshift-release-master-ci-4.20-e2e-aws-ovn-techpreview-serial periodic-ci-openshift-release-master-ci-4.20-e2e-aws-ovn-techpreview Running a rehearsal with the MCN parallel tests tagged. |
|
@isabella-janssen: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f74bf430-42d6-11f0-8593-bfa7613291a3-0 |
|
/lgtm Looks good to me from the MCO side of things! All tests seem to be running in the same suites as previously. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: isabella-janssen, neisw, yuqi-zhang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
@yuqi-zhang: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Job Failure Risk Analysis for sha: 5c9e36d
Risk analysis has seen new tests most likely introduced by this PR. New Test Risks for sha: 5c9e36d
New tests seen in this PR at sha: 5c9e36d
|
openshift-merge-bot
Bot
merged commit 1cef4af
into
openshift:main
|
[ART PR BUILD NOTIFIER] Distgit: openshift-enterprise-tests |
|
/cherry-pick release-4.19 |
|
@yuqi-zhang: #29776 failed to apply on top of branch "release-4.19": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
7 participants
Many MCO tests require node disruption, so it was determined that these would best live as a separate suite.
Create the MCO suite and move existing MCO tests in origin to it. The next goal is to add On Cluster Layering tests to this as well, potentially via OTE.