[release-4.19] MCO-1720: Promote MachineConfigNode feature gate to default #2376
Conversation
|
@isabella-janssen: This pull request references MCO-1720 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.19.0" version, but no target version was set. In 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
|
Hello @isabella-janssen! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
size/XXL
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: isabella-janssen The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest-required |
|
@isabella-janssen We don't generally backport feature promotions post GA unless there is a very very good reason to do so, what's the motivation here? |
|
@JoelSpeed Hi! Due to it finalizing in the last minute, the aggreement was to push it via 4.19 z-stream.
Overall, the api does exist for a while, and is a mature one. I hope the above clarifies things, but when we saw that we missed 4.19 the agreed plan was to backport. |
|
/retest-required |
|
Do we have the associated SBAR for this feature backport that we can link here? |
|
/hold Holding while we create an SBAR for this feature & PinnedImage sets. |
openshift-ci
bot
added
the
do-not-merge/hold
|
/retest-required |
3 similar comments
|
/retest-required |
|
/retest-required |
|
/retest-required |
|
/test verify-feature-promotion |
|
/retest-required |
2 similar comments
|
/retest-required |
|
/retest-required |
|
/test verify-feature-promotion |
|
/retest-required |
|
As far as I can tell, some tests have been renamed recetly and those haven't timed out yet. The only test I can see which doesn't pass enough is If we can get a few more runs there to see if it comes above 95% I think we are good to promote this |
|
/retest-required |
That's correct, they were updated June 26th (openshift/origin#29918).
I have some manually triggered runs for that running now, hopefully we can get the passing rate up soon. Related SBAR can be found here. Thanks for the continued patience and help throughout this effort @JoelSpeed! |
|
/test verify-feature-promotion |
1 similar comment
|
/test verify-feature-promotion |
|
/test verify-feature-promotion |
1 similar comment
|
/test verify-feature-promotion |
|
/unhold @JoelSpeed It looks like the Two tests, |
openshift-ci
bot
removed
the
do-not-merge/hold
How did we come to this conclusion? I would have expected all tests to contribute, whether they are new or not? Or are these tests specifically marked up in a way that they always pass? We have a method for marking tests as flakes so that they do not impact the initial signal, are you doing that? |
When originally working to GA MachineConfigNodes & PinnedImageSets in 4.19, we decided to use 11 tests to prove component readiness for the features, 6 specific to MCN & 5 for PIS, and the tests being used for MCN's component readiness were documented in Jira. We used those 11 tests to prove component readiness in lifting the feature gates in 4.20 and planned to use the same tests to support the 4.19 backport. Those tests now seem to have reached the required number of runs and pass rates. The
No, the tests pass / fail based on whether they perform as expected or not.
We are not currently doing this and I am not aware of any plans to do this. |
Perhaps a better way to frame it is that these tests were introduced after the fact as additional checks for the features, since no test suite exercised them until recently, so we would like to not consider them as part of the requirement yet (the original 5-tests-per-FG is still being met, with these tests we would have 8 for MCN FG) Put another way, if we were to for some reason add a new test for a feature every week as we expand on testing, does that mean that we'd never satisfy the requirements as there would always be a test not reaching enough # of runs to meet criteria? As for the test pass rate, they also both have 100% pass rate on 4.20, just around 10 runs each (as opposed to a few hundred for all the other tests), and in 4.19 has only 4 and 7 runs, and thus isn't matching graduation criteria by themselves, so we will use them to track component readiness, but not use them to lift the FG if possible. |
|
@JoelSpeed do you have any followup questions or concerns based on the justification from myself or @yuqi-zhang? |
|
/test verify-feature-promotion |
|
@isabella-janssen: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
I'll be honest, this is possibly the first time this has come up. Generally by the time feature promotion happens, the feature is complete, and the testing is complete (or mostly complete) at this time. Given that these features were promoted through the SBAR process, this is a bit of a wrinkle that we haven't crossed before. Looking at the results as of today, the new tests do have some failures, promotion the feature with tests failing will contribute to red component readiness, which will trigger TRT to act. Have we looked at the failures on the newer tests to see what those are? (Since we are currently at 80%, I think it's at least looking into why those runs failed before we assess an override) |
This promotes the
MachineConfigNodefeature gate toDefaultin the 4.19 brach.