-
Notifications
You must be signed in to change notification settings - Fork 425
[DO NOT MERGE] - expands skip-ignition-apply work in MCD #4961
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
base: main
Are you sure you want to change the base?
Conversation
Skipping CI for Draft Pull Request. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cheesesashimi 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 |
PR needs rebase. 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. |
@cheesesashimi: The following test 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-sigs/prow repository. I understand the commands that are listed here. |
This is meant to expand on @dkhater-redhat 's work around skipping the ignition apply path in the MCD in this PR: https://github.com/openshift/machine-config-operator/pull/4871/files . My work is intended to be cherry-picked into Dalia's PR and expanded upon.
Here's how this works:
triggerUpdate()
is called. This is where we effectively "merge" the OCL and non-OCL update paths.triggerUpdate()
, we embed the OCL image pullspecs into the MachineConfig, overriding the OSImageURL value. The reason we override this value is because many of the update functions use OSImageURL as their source of truth.applyOSImageUpdates()
function (which was already done in a separate PR), only minor code changes were necessary.A few notes about the implementation:
triggerUpdate()
andtriggerUpdateWithMachineConfig()
methods were consolidated into a singular code path for both OCL and non-OCL paths.canonicalizeMachineConfigImage()
function was renamed toembedOCLImageIntoMachineConfig()
and refactored to set a few annotations in the case that it embeds an image into the MachineConfig by overriding the OSImageURL value. A companionextractOCLImageFromMachineConfig()
was also implemented that will revert the OSImageURL back its original value on-demand. Both functions will perform a DeepCopy of the MachineConfig so that the mutations we make are not reflected in the lister cache.newMachineConfigDiff()
function where it extracts these values to determine if OCL is enabled. 2) BeforestoreCurrentConfigOnDisk()
is called.