-
Notifications
You must be signed in to change notification settings - Fork 392
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
operator: Add bootstrap arg --machine-config-oscontent-image #355
operator: Add bootstrap arg --machine-config-oscontent-image #355
Conversation
Installer PR using this: openshift/installer#1149 |
Only compile tested as of yet...but this is quite simple and should work. |
Code itself is clearly correct. General approach looks sane to me (this is similar to #343) though will let installer folks comment as well. /approve |
588ed7c
to
edecd96
Compare
/retest Agreed, the change looks good and is pretty simple. |
/retest |
OK, it's turning out to be a bit annoying to get a release payload with this PR so I can test changes to the installer. Since IMO this PR is:
I vote we land this and then actually test it in the installer PR. If we need further changes here we circle back and do it. |
Once rebased 👍 to merging |
edecd96
to
07cc142
Compare
OK rebased 🏄♂️ - did a bit more changes to blend in with using the Images structure. |
@@ -51,6 +52,10 @@ func init() { | |||
bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.mcdImage, "machine-config-daemon-image", "", "Image for Machine Config Daemon. (this overrides the image from --images-json-configmap)") | |||
bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.etcdImage, "etcd-image", "", "Image for Etcd. (this overrides the image from --images-json-configmap)") | |||
bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.setupEtcdEnvImage, "setup-etcd-env-image", "", "Image for Setup Etcd Environment. (this overrides the image from --images-json-configmap)") | |||
bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.mccImage, "machine-config-controller-image", "", "Image for Machine Config Controller. (this cannot be set if --images-json-configmap is set)") | |||
bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.mcsImage, "machine-config-server-image", "", "Image for Machine Config Server. (this cannot be set if --images-json-configmap is set)") | |||
bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.mcdImage, "machine-config-daemon-image", "", "Image for Machine Config Daemon. (this cannot be set if --images-json-configmap is set)") |
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.
Hmm, that looks wrong. One sec.
This will allow the installer to render the payload's osImageURL ConfigMap in the initial MachineConfig as well. Closes: openshift#334
07cc142
to
77ed063
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashcrow, cgwalters, jlebon, yuqi-zhang 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 |
/retest |
…penshift-4.8-ose-cluster-samples-operator Updating ose-cluster-samples-operator builder & base images to be consistent with ART
This will allow the installer to render the payload's osImageURL
ConfigMap in the initial MachineConfig as well.
Closes: #334