Skip to content
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

Add SME feedback to original PR#25709 #26442

Merged
merged 1 commit into from
Oct 15, 2020
Merged

Add SME feedback to original PR#25709 #26442

merged 1 commit into from
Oct 15, 2020

Conversation

bobfuru
Copy link
Contributor

@bobfuru bobfuru commented Oct 14, 2020 •
edited
Loading

Relates to #25709 - in that PR, many of the SME comments were not addressed. The purpose of this PR is to:

PREVIEW LINKS:

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 14, 2020
@bobfuru bobfuru added this to the Future Release milestone Oct 14, 2020
@bobfuru bobfuru added the peer-review-needed Signifies that the peer review team needs to review this PR label Oct 14, 2020
@openshift-docs-preview-bot
Copy link

The preview will be available shortly at:

@bobfuru
Copy link
Contributor Author

bobfuru commented Oct 14, 2020 •
edited
Loading

@bgilbert Thank you for helping to get this feedback included. Could you PTAL, with special attention to the coreos-installer image mirror instructions? Thank you!

Cc: @miabbott

@bobfuru
Copy link
Contributor Author

bobfuru commented Oct 14, 2020

@openshift/team-documentation PTAL

@bobfuru bobfuru force-pushed the rhcos-sme-feedback-squash branch from b47dbc7 to ac15502 Compare October 14, 2020 01:31
This was referenced Oct 14, 2020
Merged
Merged
bgilbert
bgilbert reviewed Oct 14, 2020
View reviewed changes
Copy link
Contributor

@bgilbert bgilbert left a comment

Choose a reason for hiding this comment

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

The rendered files still mention rhcos-<version>-installer.<architecture>.iso. instead of -live.

@miabbott
Copy link
Member

Thanks @bgilbert for the thorough review! 🙏

@bobfuru bobfuru force-pushed the rhcos-sme-feedback-squash branch 3 times, most recently from 870cb28 to f59c249 Compare October 14, 2020 22:12
@bobfuru
Copy link
Contributor Author

bobfuru commented Oct 14, 2020

@bgilbert and @miabbott. I've applied and resolved most all of the feedback, with a few open questions left, notably the "Embedding an Ignition config in the RHCOS ISO" procedure 😝 .

I need to get this PR merged tomorrow, so let me know how it's looking. Thanks!!

@bobfuru bobfuru force-pushed the rhcos-sme-feedback-squash branch 2 times, most recently from 84a4c43 to dfb75d2 Compare October 14, 2020 23:18
sfortner-RH
sfortner-RH suggested changes Oct 15, 2020
View reviewed changes
Copy link
Contributor

@sfortner-RH sfortner-RH left a comment

Choose a reason for hiding this comment

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

Minor edits, otherwise LGTM.

bgilbert
bgilbert reviewed Oct 15, 2020
View reviewed changes
@miabbott
Copy link
Member

Benjamin has this covered; I would only stress that we should remove any references to using the coreos-installer container (here and elsewhere) since that will not be supported by CEE

@bobfuru bobfuru 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 Oct 15, 2020
@bobfuru
Copy link
Contributor Author

bobfuru commented Oct 15, 2020

Benjamin has this covered; I would only stress that we should remove any references to using the coreos-installer container (here and elsewhere) since that will not be supported by CEE

Roger that, @miabbott. With Benjamin's command updates, there are no longer references to using the coreos-installer container. Thanks!!

@bobfuru bobfuru force-pushed the rhcos-sme-feedback-squash branch 2 times, most recently from cba0059 to a804c75 Compare October 15, 2020 18:09
@bobfuru
Copy link
Contributor Author

bobfuru commented Oct 15, 2020

Fantastic help, @bgilbert! I believe I've addressed all your comments. Could you please approve? Thanks.

@bgilbert
Copy link
Contributor

There's one missed change; LGTM otherwise.

@bobfuru
Copy link
Contributor Author

bobfuru commented Oct 15, 2020

The rendered files still mention rhcos-<version>-installer.<architecture>.iso. instead of -live.
Missed this comment before but it was addressed by your other edits, so no more mention of -installer.

@bobfuru bobfuru force-pushed the rhcos-sme-feedback-squash branch from 56d23ee to 17f5951 Compare October 15, 2020 20:48
@bobfuru bobfuru force-pushed the rhcos-sme-feedback-squash branch from 17f5951 to 47389b4 Compare October 15, 2020 20:50
@bobfuru bobfuru merged commit bb0a66d into openshift:master Oct 15, 2020
@bgilbert
Copy link
Contributor

🎉

@bobfuru
Copy link
Contributor Author

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

/cherrypick enterprise-4.6

@openshift-cherrypick-robot
Copy link

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

@bobfuru: #26442 failed to apply on top of branch "enterprise-4.6":

Applying: Add SME feedback to original PR#25709
Using index info to reconstruct a base tree...
M	installing/installing_bare_metal/installing-bare-metal-network-customizations.adoc
M	installing/installing_bare_metal/installing-bare-metal.adoc
M	installing/installing_bare_metal/installing-restricted-networks-bare-metal.adoc
M	modules/installation-user-infra-machines-iso.adoc
M	modules/installation-user-infra-machines-static-network.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/installation-user-infra-machines-static-network.adoc
CONFLICT (content): Merge conflict in modules/installation-user-infra-machines-static-network.adoc
Auto-merging modules/installation-user-infra-machines-iso.adoc
Auto-merging installing/installing_bare_metal/installing-restricted-networks-bare-metal.adoc
Auto-merging installing/installing_bare_metal/installing-bare-metal.adoc
CONFLICT (content): Merge conflict in installing/installing_bare_metal/installing-bare-metal.adoc
Auto-merging installing/installing_bare_metal/installing-bare-metal-network-customizations.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 Add SME feedback to original PR#25709
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".

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 pushed a commit to bobfuru/openshift-docs that referenced this pull request Oct 15, 2020
Add SME feedback to original PR#25709 (openshift#26442)
c8eec48
bobfuru pushed a commit that referenced this pull request Oct 15, 2020
Add SME feedback to original PR#25709 (#26442) (#26521)
3ad2d57
@bobfuru bobfuru deleted the rhcos-sme-feedback-squash branch October 15, 2020 21:28
@yuvalk yuvalk mentioned this pull request Mar 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bgilbert bgilbert bgilbert left review comments

@miabbott miabbott miabbott left review comments

@sfortner-RH sfortner-RH sfortner-RH requested changes

Assignees
No one assigned
Labels
branch/enterprise-4.6 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
Milestone
OCP 4.6 GA
Development

Successfully merging this pull request may close these issues.
8 participants
@bobfuru @openshift-docs-preview-bot @miabbott @bgilbert @openshift-cherrypick-robot @sfortner-RH @vikram-redhat @openshift-ci-robot