Add tech preview annotation to manifests #20
Conversation
JoelSpeed
left a comment
JoelSpeed
left a comment
There was a problem hiding this comment.
I expect there will be other things we can use this feature for that we are currently using the operator for, is the plan to extract those in a separate PR?
manifests/image-references
Outdated
| - name: cluster-api | ||
| from: | ||
| kind: DockerImage | ||
| name: registry.ci.openshift.org/openshift:cluster-api |
There was a problem hiding this comment.
This image is definitely in payload right?
There was a problem hiding this comment.
Looks like no, I thought it is at this moment
There was a problem hiding this comment.
Now I remember, Clayton asked to call it cluster-capi-controllers. If images task passes in CI then it's fine.
alexander-demicev
force-pushed
the
annotations
branch
2 times, most recently
from
60c4c70 to
710c8ea
Compare
|
@JoelSpeed yes, I will do it in a separate PR, after CVO PR merges |
|
/retest |
|
/lgtm |
manifests/image-references
Outdated
| from: | ||
| kind: DockerImage | ||
| name: registry.ci.openshift.org/openshift:cluster-capi-operator | ||
| - name: cluster-capi-controllers |
There was a problem hiding this comment.
I don't see cluster-capi-controllers within the OCP build data, I would expect it needs to be here before we can merge an image reference: https://github.com/openshift/ocp-build-data/tree/openshift-4.10/images
While this may work for CI, merging without proper build data could break the nightly build process
There was a problem hiding this comment.
https://github.com/openshift/ocp-build-data/blob/openshift-4.10/images/ose-cluster-api.yml, there is a config for capi
There was a problem hiding this comment.
I removed new image references from this PR, need to figure out what is wrong.
alexander-demicev
force-pushed
the
annotations
branch
from
710c8ea to
0f3ec9c
Compare
alexander-demicev
force-pushed
the
annotations
branch
3 times, most recently
from
34c00c1 to
6b06da8
Compare
|
/approve |
openshift-ci
bot
added
the
approved
|
/lgtm |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
alexander-demicev
force-pushed
the
annotations
branch
from
6b06da8 to
8c6a17b
Compare
alexander-demicev
force-pushed
the
annotations
branch
from
8c6a17b to
154f61e
Compare
|
/retest |
|
Seems like you're missing an annotation on at least one manifest, based on this run: $ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_cluster-capi-operator/20/pull-ci-openshift-cluster-capi-operator-main-e2e-aws-capi-techpreview/1467934131437441024/artifacts/e2e-aws-capi-techpreview/gather-extra/artifacts/pods/openshift-cluster-version_cluster-version-operator-b797587c5-7wcwb_cluster-version-operator.log | grep -o 'error running apply.*' | sort | uniq
error running apply for role "openshift-cluster-api/capg-leader-election-role" (187 of 823): namespaces "openshift-cluster-api" not found
error running apply for serviceaccount "openshift-cluster-api/capz-manager" (177 of 823): namespaces "openshift-cluster-api" not foundOr maybe you need to keep the namespace regardless of tech-preview-ness to support those resources for some non-tech-preview component? |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
5 similar comments
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/hold |
openshift-ci
bot
added
do-not-merge/hold
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
alexander-demicev
force-pushed
the
annotations
branch
2 times, most recently
from
968691d to
67a4f70
Compare
alexander-demicev
force-pushed
the
annotations
branch
from
67a4f70 to
e02fb3a
Compare
|
/retest |
4 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Fedosin, JoelSpeed 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 |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
@alexander-demichev: all tests passed! 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. |
8 participants
Add tech preview annotation to manifests, after this PR https://github.com/openshift/cluster-version-operator/pull/694/files#diff-7a023c513e7d3dd1938909a860915f4541530097d5d25a4d743f3eabaad6fc95R227is merged, CVO will be responsible for creating manifests on tech preview clusters.