Skip to content

Avoid reboots in select cases #28090

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.

Sign up for GitHub

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

Jump to bottom
Merged
mburke5678 merged 1 commit into from
Dec 18, 2020
Merged

Avoid reboots in select cases #28090

mburke5678 merged 1 commit into from
Dec 18, 2020

Conversation

@mburke5678
Copy link
Contributor

@mburke5678 mburke5678 commented Dec 14, 2020

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 14, 2020
@mburke5678
Copy link
Contributor Author

mburke5678 commented Dec 14, 2020

@yuqi-zhang Would you be the best person to review these doc changes?

@mburke5678 mburke5678 added this to the Future Release milestone Dec 14, 2020
yuqi-zhang
yuqi-zhang reviewed Dec 14, 2020
View reviewed changes
Copy link

@yuqi-zhang yuqi-zhang left a comment

Choose a reason for hiding this comment

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

Generally I think we should specify that these are explicitly 4.7 specifically (or maybe implied is good enough?) and that they still disrupt your workloads due to the drain action. There just won't be a physical reboot.

modules/images-configuration-insecure.adoc Outdated
Copy link

@yuqi-zhang yuqi-zhang Dec 14, 2020

Choose a reason for hiding this comment

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

Perhaps we should specify specifically that:

  1. as of 4.7, this change will no longer trigger a reboot (if no other changes were applied at the same time)
  2. it will instead drain the nodes, apply the changes and reload crio.service on the host, and then uncordon
  3. the /host/etc/containers/registries.conf is a bit odd, it should just be /etc/containers/registries.conf on the actual hosts. The only time you would see if in /host... is if you have a container that mounted the system root to /host (e.g. a debug container)

modules/images-update-global-pull-secret.adoc Outdated
Copy link

@yuqi-zhang yuqi-zhang Dec 14, 2020

Choose a reason for hiding this comment

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

Maybe should specify as of 4.7, this will not reboot

modules/troubleshooting-disabling-autoreboot-mco.adoc Outdated
Copy link

@yuqi-zhang yuqi-zhang Dec 14, 2020

Choose a reason for hiding this comment

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

... e.g. ICSP changes

Copy link
Contributor Author

@mburke5678 mburke5678 Dec 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@yuqi-zhang Thanks for the comments.
What is ICSP? ImageContentSourcePolicy?
We don't use that acronym in the docs apparently.

Copy link

@yuqi-zhang yuqi-zhang Dec 14, 2020

Choose a reason for hiding this comment

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

Ah sorry that would be imageContentSourcePolicy, e.g. here: https://docs.openshift.com/container-platform/4.6/updating/updating-restricted-network-cluster.html#images-configuration-registry-mirror_updating-restricted-network-cluster

i.e. if you create/update an imagecontentsourcepolicy object, it gets populated into a machineconfig. This was one of the asks from customers so I thought it might have made sense to explicitly list it here. I defer to your judgement on that though.

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 14, 2020
@mburke5678
Copy link
Contributor Author

mburke5678 commented Dec 15, 2020

Check reference in images-configuration-file.adoc

mburke5678
mburke5678 commented Dec 17, 2020
View reviewed changes
modules/machine-config-overview.adoc Outdated
Copy link
Contributor Author

@mburke5678 mburke5678 Dec 17, 2020

Choose a reason for hiding this comment

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

Fixed typo

mburke5678
mburke5678 commented Dec 17, 2020
View reviewed changes
modules/nodes-pods-secrets-creating.adoc Outdated
Copy link
Contributor Author

@mburke5678 mburke5678 Dec 17, 2020

Choose a reason for hiding this comment

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

Fixed typo

@mike-nguyen
Copy link
Member

mike-nguyen commented Dec 17, 2020

lgtm

mburke5678
mburke5678 commented Dec 17, 2020
View reviewed changes
modules/images-configuration-parameters.adoc Outdated
Copy link
Contributor Author

@mburke5678 mburke5678 Dec 17, 2020

Choose a reason for hiding this comment

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

Fixed typo

@openshift-docs-preview-bot
Copy link

openshift-docs-preview-bot commented Dec 18, 2020

The preview will be available shortly at:

bmcelvee
bmcelvee reviewed Dec 18, 2020
View reviewed changes
modules/troubleshooting-disabling-autoreboot-mco.adoc Outdated
Copy link
Contributor

@bmcelvee bmcelvee Dec 18, 2020

Choose a reason for hiding this comment

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

Suggestions since this content was already in place :)

s/Machine Config Operator/Machine Config Operator (MCO)
s/CA/certificate authority (CA)

@bmcelvee
Copy link
Contributor

bmcelvee commented Dec 18, 2020

I left a couple of small suggestions, otherwise LGTM!

@bmcelvee bmcelvee added the peer-review-done Signifies that the peer review team has reviewed this PR label Dec 18, 2020
@mburke5678 mburke5678 merged commit a8e4c4e into openshift:master Dec 18, 2020
@mburke5678
Copy link
Contributor Author

mburke5678 commented Dec 18, 2020 •
edited by openshift-ci-robot
Loading

/cherrypick enterprise-4.7

@mburke5678 mburke5678 deleted the mco-avoid-reboots branch December 18, 2020 22:04
@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Dec 18, 2020
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Dec 18, 2020 •
edited by openshift-ci-robot
Loading

@mburke5678: new pull request created: #28251

Details

In response to this:

/cherrypick enterprise-4.7

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 mburke5678 mentioned this pull request Dec 23, 2020
Merged
@mburke5678 mburke5678 mentioned this pull request Jan 6, 2021
Merged
mburke5678 added a commit to mburke5678/openshift-docs that referenced this pull request Jan 6, 2021
@mburke5678
Adding MCO behavior changes from openshift#28090
d7fdd3d
mburke5678 added a commit that referenced this pull request Jan 6, 2021
@mburke5678
Merge pull request #28388 from mburke5678/mco-avoid-reboots-follow-up
ca4dac6 
Adding MCO behavior changes from #28090
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/openshift-docs that referenced this pull request Jan 6, 2021
@mburke5678
Adding MCO behavior changes from openshift#28090
0de0d01
mburke5678 added a commit that referenced this pull request Jan 6, 2021
@mburke5678
Merge pull request #28390 from openshift-cherrypick-robot/cherry-pick…
67f07e0 
…-28388-to-enterprise-4.7

[enterprise-4.7] Adding MCO behavior changes from #28090
@yuvalk yuvalk mentioned this pull request Mar 18, 2021
Closed
@mburke5678 mburke5678 mentioned this pull request Mar 22, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bmcelvee bmcelvee bmcelvee left review comments

+1 more reviewer

@yuqi-zhang yuqi-zhang yuqi-zhang left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

branch/enterprise-4.7 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

Milestone

OCP 4.7 GA

Development

Successfully merging this pull request may close these issues.

8 participants

@mburke5678 @mike-nguyen @openshift-docs-preview-bot @bmcelvee @openshift-cherrypick-robot @yuqi-zhang @vikram-redhat @openshift-ci-robot