OTA-1531: [6/6] cvo: use early gate checker to avoid delayed initialization #1196
Conversation
Improve comments to clarify some concerns from earlier PR feedback.
|
@petr-muller: This pull request references OTA-1531 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. 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
openshift-ci
bot
added
the
approved
|
@petr-muller: This pull request references OTA-1531 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. 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. |
|
/retest |
Previous changes prepared the code for this switch by adding code to detect enabled CVO gates and featureset early in the CVO execution, right before the controllers are initialized (well before CVO acquires its leader lease and actually starts the controllers). These early detected gate checker was not used anywhere yet. This change makes CVO stop using its late detected gate checker (detected while loading its initial payload) and start using the early one instead. This eliminates the need for the panicking gate checker that was used to guard against checking gates before the actual checker could be established. The change also allows to simplify some code layers. Late initialization on initial payload load is now only necessary for the actual CVO controller, not the featurechangestopper (it is already initialized at that time), so the `Context` layer does not need to coordinate anymore. The starting features set can be initialized in CVO right away, avoiding the need to wire it through call chains; it can simply be an `Operator` member.
petr-muller
force-pushed
the
ota-1531-06-establish-gates-early
branch
from
1ae48cf to
cb32cb8
Compare
petr-muller
changed the title
The meaning about the first of the sentence is not very clear to me without reading more about CVO controller. /lgtm |
|
/test e2e-agnostic-operator-devpreview |
|
/test e2e-agnostic-operator |
|
/test e2e-agnostic-operator-devpreview e2e-agnostic-operator |
|
/hold ci/prow/e2e-agnostic-operator-devpreview is persistent and suspicious |
openshift-ci
bot
added
the
do-not-merge/hold
|
/test ci/prow/e2e-agnostic-operator-techpreview |
|
@petr-muller: The specified target(s) for The following commands are available to trigger optional jobs: Use 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 kubernetes-sigs/prow repository. |
|
/test e2e-agnostic-operator-techpreview |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/retest-required |
openshift-ci
bot
added
do-not-merge/hold
Bad naming got me here. I used the CV-specific informer factory instead of the generic one, resulting in tests always seeing the default featureset because of the `IsNotFound` branch after the GET.
petr-muller
force-pushed
the
ota-1531-06-establish-gates-early
branch
from
14bd699 to
c55bdea
Compare
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hongkailiu, petr-muller The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@petr-muller: all tests passed! 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. |
Previous changes prepared the code for this switch by adding code to detect enabled CVO gates and featureset early in the CVO execution, right before the controllers are initialized (well before CVO acquires its leader lease and actually starts the controllers). These early detected gate checker was not used anywhere yet.
This change makes CVO stop using its late detected gate checker (detected while loading its initial payload) and start using the early one instead. This eliminates the need for the panicking gate checker that was used to guard against checking gates before the actual checker could be established.
The change also allows to simplify some code layers. Late initialization on initial payload load is now only necessary for the actual CVO controller, not the featurechangestopper (it is already initialized at that time), so the
Contextlayer does not need to coordinate anymore. The starting features set can be initialized in CVO right away, avoiding the need to wire it through call chains; it can simply be anOperatormember.