OCPBUGS-14425: Alibaba platforms should not be upgreadable #257
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@JoelSpeed: This pull request references Jira Issue OCPBUGS-14425, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. 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/test-infra repository. |
openshift-ci-robot
added
the
jira/valid-bug
|
/cherry-pick release 4.13 |
|
@JoelSpeed: once the present PR merges, I will cherry-pick it on top of release 4.13 in a new PR and assign it to you. 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/test-infra repository. |
|
/test ? |
|
@JoelSpeed: The following commands are available to trigger required jobs:
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/test-infra repository. |
|
It looks like it would work. Perhaps it would be easier to handle with a separate control loop setting that one condition? https://github.com/openshift/cluster-kube-apiserver-operator/blob/master/pkg/operator/featureupgradablecontroller/feature_upgradeable_controller.go#L27-L73 Just a thought, not required if the team accepts it until 4.14 (needed in 4.13 to prevent upgrades). |
IMO it would be good if the team could sit down and review how we are setting conditions in this operator, there's 3 controllers at the moment and the code is a mess, we don't really have time to do that immediately IMO but we do have a card on the backlog that @elmiko created to review how these work and improve them in the long run |
|
/payload-job periodic-ci-openshift-release-master-nightly-4.14-e2e-alibaba-ovn |
|
@JoelSpeed: trigger 1 job(s) for the /payload-(job|aggregate) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/befca4b0-0086-11ee-95cc-ad5c15bf7319-0 |
|
@JoelSpeed: The following tests 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/test-infra repository. I understand the commands that are listed here. |
|
Can see in the Alibaba payload that it added the condition correctly, however, the test suite is failing because the upgradable condition is set to false, looks like we will need to add an exception to origin to allow for this |
+1, i have thoughts about this. basically, i think we just need to have separate conditions for each controller and then let the primary controller set the canonical operator conditions that are used by the wider openshift mechanisms. at the least, that will allow us to untangle the interwoven nature of what is happening currently. |
|
/payload-job periodic-ci-openshift-release-master-nightly-4.14-e2e-alibaba-ovn The test should be fixed now |
|
@JoelSpeed: trigger 1 job(s) for the /payload-(job|aggregate) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/98454550-1a4b-11ee-8dbb-678890f018a3-0 |
|
Start all core operators failure is fixed, other issues in the failure seem to be unrelated |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: elmiko 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 |
openshift-ci
bot
added
the
approved
openshift-merge-robot
merged commit ceabea8
into
openshift:master
|
@JoelSpeed: Jira Issue OCPBUGS-14425: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-14425 has been moved to the MODIFIED state. 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/test-infra repository. |
|
@JoelSpeed: cannot checkout 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/test-infra repository. |
|
/cherry-pick release-4.13 |
|
@JoelSpeed: #257 failed to apply on top of branch "release-4.13": 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/test-infra repository. |
Alibaba clusters are TechPreviewNoUpgrade and should not be upgradeable.
This introduces an upgrade blocker via CCMO to prevent alibaba clusters upgrading.
The condition management in this operator sucks and needs refactoring, for now, I've made an overrides concept so that the top level operator can pass these overrides down. We should come up with a better way of doing this.