-
Notifications
You must be signed in to change notification settings - Fork 1.5k
OCPBUGS-67310: azure: allow hive to pass empty rhcos image string #10159
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
OCPBUGS-67310: azure: allow hive to pass empty rhcos image string #10159
Conversation
|
/hold floating this to see if it helps hive, but I think hive prefers to pull the image off the worker machineset and then feed that into the function. why not both may apply here |
|
Linter error but otherwise lgtm |
hive does not know the rhcos image value and will pass an empty string for azure machines. In this case, we can leave the image empty so that it will use the default from MAPI.
402caa4 to
ec85cea
Compare
This also handles confidential VMs for hive, which would still use image galleries.
|
@patrickdillon: This pull request references Jira Issue OCPBUGS-67310, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/label tide/merge-method-squash |
|
/jira refresh |
|
@patrickdillon: This pull request references Jira Issue OCPBUGS-67310, 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: 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@patrickdillon: 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-sigs/prow repository. I understand the commands that are listed here. |
|
/hold cancel Ok we do need this for hive /cherry-pick release-4.21 |
|
@patrickdillon: once the present PR merges, I will cherry-pick it on top of 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-sigs/prow repository. |
|
/label acknowledge-critical-fixes-only |
|
/lgtm |
|
/payload-job periodic-ci-openshift-openshift-tests-private-release-4.21-amd64-nightly-azure-ipi-confidential-trustedlaunch-mini-perm-f7 |
|
@tthvo: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/9b72d4f0-d7b2-11f0-8a85-53e3601e5767-0 |
|
/payload-job periodic-ci-openshift-openshift-tests-private-release-4.21-amd64-nightly-azure-ipi-confidentialvm-vmgueststateonly-f28-destructive |
|
@tthvo: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/a28c0040-d7b2-11f0-8210-22c3d8daf9d5-0 |
| case rhcosImg == "" && !confidentialVM: | ||
| // hive calls the machines function, but may pass an empty | ||
| // string for rhcos. In which case, allow MAO to choose default. | ||
| return nil | ||
| default: // Installer-created image gallery, for OKD && confidential VMs. |
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.
❓ Would this break OKD install since OKD uses image gallery? 👇
installer/pkg/infrastructure/azure/azure.go
Lines 257 to 259 in 7169d1c
| // Create a managed image, which is used for OKD or confidential VMs on OCP. | |
| hasConfidentialVM := getMachinePoolSecurityType(installConfig) != "" | |
| if (hasConfidentialVM || installConfig.IsOKD()) && platform.CloudName != aztypes.StackCloud { |
I guess I am unsure whether rhcosImg is empty in the case of OKD...
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 guess I am unsure whether rhcosImg is empty in the case of OKD...
It's not empty (rhcosImg does not seem like a great variable name in retrospect).
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.
Oh thanks! That clears up my questions :D
|
@tthvo Thanks for the help to trigger the two confidential VM testing jobs, I see they both passed the installation step. Although ASH's code path is unchanged, just to double confirm to provide more confidence. ASH installation is still relying on two more PRs to be merged into nightly. /payload-job-with-prs periodic-ci-openshift-openshift-tests-private-release-4.21-amd64-nightly-azure-stack-ipi-f28 openshift/cloud-provider-azure#152 openshift/azure-disk-csi-driver#116 |
|
@gpei: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/5a43cbf0-d7f4-11f0-998d-1518920a4393-0 |
|
/payload-job periodic-ci-openshift-hive-master-periodic-e2e-azure-weekly |
|
@gpei: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/b6457c20-d955-11f0-8aa1-9212e2b235b8-0 |
|
Seems the hive periodic job can't be started in this way. |
|
@gpei: This PR has been marked as verified by 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: patrickdillon 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 |
bbc0f9f
into
openshift:main
|
@patrickdillon: Jira Issue Verification Checks: Jira Issue OCPBUGS-67310 Jira Issue OCPBUGS-67310 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@patrickdillon: new pull request created: #10163 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-sigs/prow repository. |
hive does not know the rhcos image value and will pass an empty string for azure machines. In this case, we can leave the image empty so that it will use the default from MAPI.