-
Notifications
You must be signed in to change notification settings - Fork 20
update to release.openshift.io/feature-set to match OCP 4.12 #45
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
Conversation
exdx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
Hi @deads2k, the verify check is failing because the new annotation is not being applied to the PO CRD. We would need to update it here Line 82 in b205d77
|
Looks like the CRD manifest is copied from the vendor folder (this is fine). Not sure what the additional annotation is about, but I have fixed openshift/api and revendored to see if that fixes it |
|
/retest |
|
@deads2k: 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/test-infra repository. I understand the commands that are listed here. |
| release.openshift.io/feature-gate: TechPreviewNoUpgrade | ||
| release.openshift.io/feature-set: TechPreviewNoUpgrade | ||
| name: platform-operators-aggregated | ||
| namespace: openshift-platform-operators |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ClusterOperator are cluster-scoped resources, so this line is not necessary.
Looking at CVO logs, I'm unclear about why. The CVO knows it's $ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_platform-operators/45/pull-ci-openshift-platform-operators-main-e2e-techpreview/1567966053030432768/artifacts/e2e-techpreview/gather-extra/artifacts/pods/openshift-cluster-version_cluster-version-operator-5cb8b76556-jv6mw_cluster-version-operator.log | grep 'featurechangestopper\|platform-operators-aggregated'
I0908 20:32:24.570421 1 featurechangestopper.go:100] Starting stop-on-featureset-change controller with "TechPreviewNoUpgrade".Or, I guess, anything in |
|
@wking Yeah, the io.openshift.release.operator Dockerfile label still needs to be toggled on to true in this repository. That's being done in #23 but in the meantime, there won't be any e2e feedback. We were seeing e2e runs in that 23 PR fail because the aggregate platform operators CO resource didn't exist, but looking at a recent run it looks like we're back in business. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, exdx, timflannagan 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 |
|
These changes are already present in the #23 which has passed the e2e suite in recent runs. We're still in the process of bootstrapping this repository while we wait for payload introduction, so I'm going to add the requisite no-ff labels until then. /label px-approved |
This was added in
openshift/cluster-version-operator#821 to allow
more featuresets and allow for a future migration to include actual gates
/cc @wking
Looking at this repo, it was not present in 4.11, so there is no migration concern.