Skip to content

Conversation

@mburke5678
Copy link
Contributor

@mburke5678 mburke5678 commented Mar 20, 2023

Update the list of MC changes that do not trigger an MCO reboot of the nodes.
https://issues.redhat.com/browse/RFE-2883

Converted the list of conditions that do not trigger a node reboot to a snippet. Updated the list of registries.confchanges based on MCO internal docs.

Previews:
Disabling the Machine Config Operator from automatically rebooting

About the Machine Config Operator

QE review:

  • QE has approved this change.

@ocpdocs-previewbot
Copy link

@sinnykumari
Copy link

Thanks Michael for updating this section.
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 22, 2023
@mburke5678
Copy link
Contributor Author

@mike-nguyen You reviewed the addition of these conditions. Are you willing and able to take a look at the slight modifications?

@mike-nguyen
Copy link
Member

@mburke5678 I haven't worked on the MCO for quite some time. @rioliu-rh can you have the appropriate QE contact review this?

@rioliu-rh
Copy link

/cc @xiuwang
can you take a look from image-registry perspective? thanks

@openshift-ci openshift-ci bot requested a review from xiuwang March 23, 2023 03:03
** Changes to the global pull secret or pull secret in the `openshift-config` namespace.
** Automatic rotation of the `/etc/kubernetes/kubelet-ca.crt` certificate authority (CA) by the Kubernetes API Server Operator.

* When the MCO detects changes to the `/etc/containers/registries.conf` file, such as adding or editing an `ImageDigestMirrorSet` or `ImageTagMirrorSet` object, it drains the corresponding nodes, applies the changes, and uncordons the nodes.The node drain does not happen for the following changes:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mburke5678 ImageDigestMirrorSet and ImageTagMirrorSet are new resource to us. we need to verify the behavior mentioned in the doc

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have verified the behavior today:

  • ImageDigestMirrorSet applies the mirrors with pull-from-mirror = "digest-only" and the drain operation is skipped.

  • ImageTagMirrorSet applies the mirrors with pull-from-mirror = "tag-only" and it performs a drain operation as expected.

@sergiordlr
Copy link

It looks good to me.

@mburke5678 mburke5678 added the peer-review-needed Signifies that the peer review team needs to review this PR label Mar 23, 2023
Copy link
Contributor

@snarayan-redhat snarayan-redhat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@snarayan-redhat snarayan-redhat added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Mar 24, 2023
@mburke5678 mburke5678 merged commit 001a41b into openshift:main Mar 24, 2023
@mburke5678
Copy link
Contributor Author

/cherrypick enterprise-4.8

@openshift-cherrypick-robot

@mburke5678: #57440 failed to apply on top of branch "enterprise-4.8":

Applying: Updating imagecontentsourcepolicy should not trigger a node drain
.git/rebase-apply/patch:17: trailing whitespace.
[NOTE] 
.git/rebase-apply/patch:78: new blank line at EOF.
+
warning: 2 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	modules/troubleshooting-disabling-autoreboot-mco.adoc
M	modules/understanding-machine-config-operator.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/understanding-machine-config-operator.adoc
CONFLICT (content): Merge conflict in modules/understanding-machine-config-operator.adoc
Auto-merging modules/troubleshooting-disabling-autoreboot-mco.adoc
CONFLICT (content): Merge conflict in modules/troubleshooting-disabling-autoreboot-mco.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Updating imagecontentsourcepolicy should not trigger a node drain
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick enterprise-4.8

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.

@mburke5678
Copy link
Contributor Author

/cherrypick enterprise-4.13

@openshift-cherrypick-robot

@mburke5678: new pull request created: #57716

Details

In response to this:

/cherrypick enterprise-4.13

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.8 branch/enterprise-4.9 branch/enterprise-4.10 branch/enterprise-4.11 branch/enterprise-4.12 branch/enterprise-4.13 lgtm Indicates that a PR is ready to be merged. peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants