-
Notifications
You must be signed in to change notification settings - Fork 319
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
Pass imagerefs to mco #1773
Pass imagerefs to mco #1773
Conversation
In 4.12, the way the MCO is bootstrapped changed to take a single image-references file instead of individual image flags. This commit modifies the ignition-server to follow the same pattern when invoking the MCO in bootstrap mode.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: csrwng 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 |
Looks great to me! I am not sure how I somehow went off the rails on #1768 |
p := &controllers.LocalIgnitionProvider{ | ||
Client: cl, | ||
ReleaseProvider: &releaseinfo.RegistryClientProvider{}, | ||
CloudProvider: "", | ||
Namespace: o.Namespace, | ||
WorkDir: o.WorkDir, | ||
PreserveOutput: true, | ||
ImageFileCache: imageFileCache, |
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 don't see where this is used
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.
This is for local debugging/testing
@@ -3,6 +3,7 @@ package cmd | |||
import ( |
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.
any chance to reference the mco PR that force us to do this change?
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.
added to the PR desc
/lgtm |
Only Upgrade test failed in e2e-aws which is expected. Previous versions of the control plane operator will not come up without the changes in this PR. |
@csrwng: Overrode contexts on behalf of csrwng: ci/prow/e2e-aws In 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. |
@csrwng: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
What this PR does / why we need it:
In 4.12, the way the MCO is bootstrapped changed to take a single
image-references file instead of individual image flags. This commit
modifies the ignition-server to follow the same pattern when invoking
the MCO in bootstrap mode.
Includes the changes in #1764 so we can pass e2e
The MCO changes were introduced here: openshift/machine-config-operator#3286
and corresponding installer change here: openshift/installer#6234
Checklist