-
Notifications
You must be signed in to change notification settings - Fork 2.1k
OTA-1007: Enable pre-merge test for cincinnati #42053
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
OTA-1007: Enable pre-merge test for cincinnati #42053
Conversation
|
@shellyyang1989, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
Please run |
ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-master.yaml
Outdated
Show resolved
Hide resolved
ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-master.yaml
Outdated
Show resolved
Hide resolved
ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-master.yaml
Outdated
Show resolved
Hide resolved
ci-operator/config/openshift/cincinnati/openshift-cincinnati-master.yaml
Outdated
Show resolved
Hide resolved
|
Doing some experiments in #42113 and https://github.com/openshift/cincinnati-operator/tree/bundle-dockerfile Maybe we'll need |
Thanks! I guess we can promote it to the |
No, rather something ours, like EDIT: I see there's already |
a360f3c to
87b6168
Compare
|
@shellyyang1989, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-master.yaml
Outdated
Show resolved
Hide resolved
ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-master.yaml
Outdated
Show resolved
Hide resolved
8f25e10 to
348e4c4
Compare
petr-muller
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.
I have filed openshift/cincinnati-operator#173 to create the necessary bundle files in the operator repo and suggested the matching changes in this repository.
@shellyyang1989 can you add (temporarily) an item to tests that would install the bundle? That would give us not only a rehearsal that builds the image but also a rehearsal that tries to install it. I have written the test stanza here:
Lines 51 to 75 in aabf16f
| - as: install-bundle | |
| cluster_claim: | |
| architecture: amd64 | |
| cloud: aws | |
| owner: openshift-ci | |
| product: ocp | |
| timeout: 1h0m0s | |
| version: "4.13" | |
| steps: | |
| test: | |
| - as: install | |
| cli: latest | |
| commands: | | |
| oc create namespace install-osus-here | |
| operator-sdk run bundle -n install-osus-here "$OO_BUNDLE" | |
| oc wait --for condition=Available -n install-osus-here deployment updateservice-operator | |
| dependencies: | |
| - env: OO_BUNDLE | |
| name: cincinnati-operator-bundle | |
| from: operator-sdk | |
| resources: | |
| requests: | |
| cpu: 500m | |
| memory: 1000Mi | |
| workflow: generic-claim |
You'd also need to include a sdk image (
Lines 6 to 9 in aabf16f
| operator-sdk: | |
| name: "4.13" | |
| namespace: origin | |
| tag: operator-sdk |
cincinnati-bundle here but I called it differently in Line 69 in aabf16f
| name: cincinnati-operator-bundle |
ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-master.yaml
Outdated
Show resolved
Hide resolved
ci-operator/config/openshift/cincinnati-operator/openshift-cincinnati-operator-master.yaml
Outdated
Show resolved
Hide resolved
348e4c4 to
b9cc5a1
Compare
|
@shellyyang1989, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
b9cc5a1 to
9a47061
Compare
|
/pj-rehearse pull-ci-openshift-cincinnati-operator-master-ci-bundle-cincinnati-bundle pull-ci-openshift-cincinnati-operator-master-install-bundle |
|
bundle.Dockerfile is being enabled here openshift/cincinnati-operator#173 |
|
/pj-rehearse |
|
install-bundle passed 🎉 |
|
/retitle OTA-1007: Enable pre-merge test for cincinnati |
|
@shellyyang1989: This pull request references OTA-1007 which is a valid jira issue. DetailsIn 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. |
|
/lgtm cancel Let me verify one more thing, maybe we'll need to do some renames in our configs (see my JIRA comment) Specifically, maybe we'll need to somehow rename our operand image in the operator config so that cluster-bot is able to find it: |
I spoke to @jianzhangbjz about it and seems like any image name is fine. |
|
This looks promising... Right, so the match between the names needs to be between the My only concern is that our variant config will confuse cluster bot a bit. We promote the but we also build the same image in the main file: It does not seem cluster-bot is taking variants into account, so maybe the the resulting bundle will inject the "standard" Which is fine, I guess? The point of pre-merge testing is more to test the code, not the productized artifact (which will only be built after merge anyway). /lgtm So to me this looks good, feel free to unhold or modify at will (see my previous comment about the hold) |
9a47061 to
bc3d06e
Compare
|
@shellyyang1989, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
bc3d06e to
742a90e
Compare
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
The install-bundle job will not bother us right? I'm fine to leave it as it is. Thank you Petr for your help and support. /unhold |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: petr-muller, shellyyang1989 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 |
|
/pj-rehearse |
/pj-rehearse ack |
|
/refresh |
|
@shellyyang1989: The following test 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. |
Per https://redhat-internal.slack.com/archives/CH76YSYSC/p1691119535303039, adding
operatorstanza to enable PR pre-merge test for cincinnati.