Skip to content

Conversation

@mburke5678
Copy link
Contributor

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 15, 2020
@mburke5678 mburke5678 force-pushed the node-unqualifiedsearchregistries branch from 25292cc to c1e5bc5 Compare December 21, 2020 16:10
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 21, 2020
@openshift-docs-preview-bot

The preview will be available shortly at:

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 21, 2020
@mburke5678 mburke5678 force-pushed the node-unqualifiedsearchregistries branch from 381597c to 651ab26 Compare December 24, 2020 02:28
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an editorial change for consistency only. No need for QE review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an editorial change for 4.7 MCO change previously reviewed. No need for QE review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an editorial change for consistency only. No need for QE review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an editorial change for consistency only. No need for QE review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an editorial change for consistency only. No need for QE review.

Copy link
Contributor Author

@mburke5678 mburke5678 Dec 24, 2020

Choose a reason for hiding this comment

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

QE, please review this new text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

QE, please review this new text.

@mburke5678
Copy link
Contributor Author

@sunilcio PTAL. I made several editorial changes to ensure consistency throughout the other modules in the assembly. Please feel free to not review these changes.

@mburke5678
Copy link
Contributor Author

@sunilcio PTAL

@sunilcio
Copy link

sunilcio commented Jan 6, 2021

@mburke5678 thanks, lgtm!

@mburke5678 mburke5678 force-pushed the node-unqualifiedsearchregistries branch 3 times, most recently from 5fe0c9a to 578c1e4 Compare January 6, 2021 18:55
@codyhoag codyhoag self-requested a review January 6, 2021 19:02
Copy link
Contributor

@codyhoag codyhoag left a comment

Choose a reason for hiding this comment

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

Just a few suggestions; looks good otherwise! Just a reminder to add the labels before merging 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The Machine Config Operator (MCO) watches the `image.config.openshift.io/cluster` resource for any changes to the registries. When the MCO detects a change, it drains the nodes, applies the change, and uncordons the nodes. After the nodes return to the `Ready` state, if the `containerRuntimeSearchRegistries` parameter is added, the MCO and creates a file in the `/etc/containers/registries.conf.d` directory on each node with the listed registries. The file overrides the default list of unqualified search registries in the `/host/etc/containers/registries.conf` file. There is no way to fall back to the default list of unqualified search registries.
The Machine Config Operator (MCO) watches the `image.config.openshift.io/cluster` resource for any changes to the registries. When the MCO detects a change, it drains the nodes, applies the change, and uncordons the nodes. After the nodes return to the `Ready` state, if the `containerRuntimeSearchRegistries` parameter is added, the MCO creates a file in the `/etc/containers/registries.conf.d` directory on each node with the listed registries. The file overrides the default list of unqualified search registries in the `/host/etc/containers/registries.conf` file. There is no way to fall back to the default list of unqualified search registries.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The `containerRuntimeSearchRegistries` parameter works only with the Podman and CRI-O container engines and the registries in the list can be used only in pod specs, not in builds and image streams.
The `containerRuntimeSearchRegistries` parameter works only with the Podman and CRI-O container engines. The registries in the list can be used only in pod specs, not in builds and image streams.

Copy link
Contributor

Choose a reason for hiding this comment

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

The + markers are currently rendering (line 83 and 88). I'd suggest making this verification step a numbered step, which would fix the rendering issue.

. To check that the registries have been added, use the following command on a node:

@codyhoag codyhoag added the peer-review-done Signifies that the peer review team has reviewed this PR label Jan 6, 2021
@mburke5678 mburke5678 force-pushed the node-unqualifiedsearchregistries branch from 578c1e4 to c62f761 Compare January 6, 2021 20:12
@mburke5678 mburke5678 added this to the Future Release milestone Jan 6, 2021
@mburke5678 mburke5678 merged commit b8c436f into openshift:master Jan 6, 2021
@mburke5678 mburke5678 deleted the node-unqualifiedsearchregistries branch January 6, 2021 20:21
@mburke5678
Copy link
Contributor Author

mburke5678 commented Jan 6, 2021

/cherrypick enterprise-4.7

@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Jan 6, 2021

@mburke5678: new pull request created: #28416

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.

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

Labels

branch/enterprise-4.7 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants