Skip to content

machine_management: add note about using original boot media #26215

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
bobfuru merged 5 commits into from
Oct 12, 2020
Merged

machine_management: add note about using original boot media #26215

bobfuru merged 5 commits into from
Oct 12, 2020

Conversation

@miabbott
Copy link
Member

@miabbott miabbott commented Oct 7, 2020 •
edited
Loading

We don't support using newer boot media to add nodes to a clusters.

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 7, 2020
@miabbott
Copy link
Member Author

miabbott commented Oct 7, 2020

cc: @ashcrow @bobfuru @yuqi-zhang @kikisdeliveryservice

@miabbott
Copy link
Member Author

miabbott commented Oct 7, 2020

This should be added to the 4.6 docs, too

yuqi-zhang
yuqi-zhang approved these changes Oct 7, 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.

Note that this isn't just 4.6, we've technically never "supported" updating boot media directly in any way. It just so happens that the 4.6 ignition spec upgrade is the first time we've updated the pointer config

@kikisdeliveryservice
Copy link

kikisdeliveryservice commented Oct 7, 2020

Note that this isn't just 4.6, we've technically never "supported" updating boot media directly in any way. It just so happens that the 4.6 ignition spec upgrade is the first time we've updated the pointer config

Maybe worth being extra clear then and just saying to use the original boot media wiht no 4.6 caveat

@miabbott miabbott force-pushed the updating_boot_media branch from 3d20207 to eea640b Compare October 8, 2020 13:35
@miabbott
Copy link
Member Author

miabbott commented Oct 8, 2020

Maybe worth being extra clear then and just saying to use the original boot media wiht no 4.6 caveat

Pushed an update that was more clear

kalexand-rh
kalexand-rh reviewed Oct 8, 2020
View reviewed changes
Copy link
Contributor

@kalexand-rh kalexand-rh left a comment

Choose a reason for hiding this comment

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

I have a few requests, but this is looking good.

machine_management/user_infra/adding-bare-metal-compute-user-infra.adoc Outdated
Copy link
Contributor

@kalexand-rh kalexand-rh Oct 8, 2020

Choose a reason for hiding this comment

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

Suggested change
When adding additional compute machines to your {product-title} cluster, you should use the
When adding additional compute machines to your {product-title} cluster, use the

machine_management/user_infra/adding-bare-metal-compute-user-infra.adoc Outdated
Copy link
Contributor

@kalexand-rh kalexand-rh Oct 8, 2020

Choose a reason for hiding this comment

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

Suggested change
{op-system} boot media that matches the same minor version that was used to originally
{op-system} boot media that matches the same minor version that was used to

machine_management/user_infra/adding-bare-metal-compute-user-infra.adoc Outdated
Copy link
Contributor

@kalexand-rh kalexand-rh Oct 8, 2020

Choose a reason for hiding this comment

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

Suggested change
install the {product-title} cluster. For example, if you originally installed
install the {product-title} cluster. For example, if you installed

machine_management/user_infra/adding-bare-metal-compute-user-infra.adoc Outdated
Copy link
Contributor

@kalexand-rh kalexand-rh Oct 8, 2020

Choose a reason for hiding this comment

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

Suggested change
{product-title} 4.4, you should add additional compute machines using {op-system} 4.4
{product-title} 4.4, you must add additional compute machines using {op-system} 4.4

machine_management/user_infra/adding-bare-metal-compute-user-infra.adoc Outdated
Copy link
Contributor

@kalexand-rh kalexand-rh Oct 8, 2020

Choose a reason for hiding this comment

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

Please add a sentence about upgrading the machines after you add them.

@kalexand-rh kalexand-rh added this to the Next Release milestone Oct 8, 2020
bobfuru
bobfuru suggested changes Oct 8, 2020
View reviewed changes
Copy link
Contributor

@bobfuru bobfuru left a comment

Choose a reason for hiding this comment

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

Added a suggestion to Kathryn's. Thanks!

machine_management/user_infra/adding-bare-metal-compute-user-infra.adoc Outdated
Copy link
Contributor

@bobfuru bobfuru Oct 8, 2020

Choose a reason for hiding this comment

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

Adding additional compute machines to your cluster using newer {op-system} boot media is not supported.

Suggested change
It is not supported to add additional compute machines to your cluster using newer
It is not supported to add additional compute machines to your cluster using newer

@miabbott
machine_management: add note about using original boot media
d39a556 
We don't support using newer boot media to add nodes to a clusters.
@miabbott miabbott force-pushed the updating_boot_media branch from eea640b to d39a556 Compare October 8, 2020 18:04
@miabbott
Copy link
Member Author

miabbott commented Oct 8, 2020

Thanks @kalexand-rh @bobfuru; pushed update with your feedback

bobfuru
bobfuru reviewed Oct 8, 2020
View reviewed changes
Copy link
Contributor

@bobfuru bobfuru left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi @miabbott - a minor nit that I missed before, use a single space between sentences. I'll go ahead and make the change. Otherwise, LGTM.

Let me know if you feel this is ready for me to squash and merge. Thanks!

@miabbott
Copy link
Member Author

miabbott commented Oct 12, 2020

@bobfuru The content looks good to me; ready for squash and merge

@bobfuru bobfuru merged commit 1a00202 into openshift:master Oct 12, 2020
@bobfuru
Copy link
Contributor

bobfuru commented Oct 12, 2020

@bobfuru The content looks good to me; ready for squash and merge

Well, the button said "Squash and merge" but I guess it kept it as 5 commits ¯_(ツ)_/¯ but at least it merged. Thanks, @miabbott !

@bobfuru
Copy link
Contributor

bobfuru commented Oct 12, 2020 •
edited by openshift-ci-robot
Loading

/cherrypick enterprise-4.6

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Oct 12, 2020
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Oct 12, 2020 •
edited by openshift-ci-robot
Loading

@bobfuru: new pull request created: #26359

Details

In response to this:

/cherrypick enterprise-4.6

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.

@bobfuru
Copy link
Contributor

bobfuru commented Oct 12, 2020 •
edited by openshift-ci-robot
Loading

/cherrypick enterprise-4.5

@bobfuru
Copy link
Contributor

bobfuru commented Oct 12, 2020 •
edited by openshift-ci-robot
Loading

/cherrypick enterprise-4.4

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Oct 12, 2020
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Oct 12, 2020 •
edited by openshift-ci-robot
Loading

@bobfuru: new pull request created: #26361

Details

In response to this:

/cherrypick enterprise-4.5

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.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Oct 12, 2020
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Oct 12, 2020 •
edited by openshift-ci-robot
Loading

@bobfuru: new pull request created: #26362

Details

In response to this:

/cherrypick enterprise-4.4

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.

@bobfuru
Copy link
Contributor

bobfuru commented Oct 12, 2020 •
edited by openshift-ci-robot
Loading

/cherrypick enterprise-4.3

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Oct 12, 2020
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Oct 12, 2020 •
edited by openshift-ci-robot
Loading

@bobfuru: new pull request created: #26363

Details

In response to this:

/cherrypick enterprise-4.3

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.

@bobfuru bobfuru mentioned this pull request Oct 13, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kalexand-rh kalexand-rh kalexand-rh left review comments

+3 more reviewers

@bobfuru bobfuru bobfuru approved these changes

@ashcrow ashcrow ashcrow approved these changes

@yuqi-zhang yuqi-zhang yuqi-zhang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

branch/enterprise-4.3 branch/enterprise-4.4 branch/enterprise-4.5 branch/enterprise-4.6 size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

Next Release

Development

Successfully merging this pull request may close these issues.

8 participants

@miabbott @kikisdeliveryservice @bobfuru @openshift-cherrypick-robot @ashcrow @yuqi-zhang @kalexand-rh @openshift-ci-robot