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

OSDOCS-3889 add NLB to AWS networking config #51460

Merged
merged 1 commit into from Oct 27, 2022
Merged

OSDOCS-3889 add NLB to AWS networking config #51460

merged 1 commit into from Oct 27, 2022

Conversation

ShaunaDiaz
Copy link
Contributor

@ShaunaDiaz ShaunaDiaz commented Oct 10, 2022 •
edited

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 10, 2022
@ShaunaDiaz ShaunaDiaz mentioned this pull request Oct 10, 2022
Merged
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Oct 10, 2022 •
edited

🤖 Updated build preview is available at:
https://51460--docspreview.netlify.app

Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/2661

@ShaunaDiaz
Copy link
Contributor Author

@bscott-rh please take a look
@miheer and @lihongan please review and approve or suggest revisions/additions

@ShaunaDiaz ShaunaDiaz force-pushed the OSDOCS-3889 branch 2 times, most recently from 6862937 to d609889 Compare October 17, 2022 20:24
@ShaunaDiaz ShaunaDiaz closed this Oct 18, 2022
@ShaunaDiaz ShaunaDiaz reopened this Oct 18, 2022
miheer
miheer suggested changes Oct 18, 2022
View reviewed changes
modules/installation-aws-config-yaml.adoc Outdated Show resolved Hide resolved
modules/installation-configuration-parameters.adoc Outdated Show resolved Hide resolved
@ShaunaDiaz ShaunaDiaz force-pushed the OSDOCS-3889 branch 3 times, most recently from f221140 to 066bd2c Compare October 18, 2022 15:46
@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 18, 2022
@ShaunaDiaz
Copy link
Contributor Author

@miheer @Miciah PTAL and let me know if this PR looks good now.
Please let me know if there is any additional workflow re: the oc edit ingress.config/cluster -o yaml command having to be run? Want to be sure we don't need additional documentation.

miheer
miheer suggested changes Oct 20, 2022
View reviewed changes
modules/installation-aws-config-yaml.adoc Outdated Show resolved Hide resolved
modules/installation-aws-config-yaml.adoc Outdated Show resolved Hide resolved
modules/installation-configuration-parameters.adoc Outdated Show resolved Hide resolved
@miheer
Copy link

miheer commented Oct 24, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 24, 2022
@ShaunaDiaz
Copy link
Contributor Author

@lihongan please review and let me know if looks good to your (or suggest revisions/additions)

@lihongan
Copy link

Looks good! Thank you @ShaunaDiaz

@ShaunaDiaz ShaunaDiaz changed the title [WIP] [OSDOCS-3889] add NLB to AWS networking config OSDOCS-3889 add NLB to AWS networking config Oct 27, 2022
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 27, 2022
@openshift-ci
Copy link

openshift-ci bot commented Oct 27, 2022

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 27, 2022
@GroceryBoyJr
Copy link
Contributor

/remove-label peer-review-in-progress
/label peer-review-completed

@openshift-ci openshift-ci bot removed the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Oct 27, 2022
@openshift-ci
Copy link

openshift-ci bot commented Oct 27, 2022

@GroceryBoyJr: The label(s) /label peer-review-completed cannot be applied. These labels are supported: platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, downstream-change-needed, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, cnv, dev-tools, distributed-tracing, ims, jira/valid-bug, merge-review-in-progress, merge-review-needed, mtc, multi-arch, oadp, peer-review-done, peer-review-in-progress, peer-review-needed, rhacs, rhv, serverless, service-mesh, staff-eng-approved, telco. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/remove-label peer-review-in-progress
/label peer-review-completed

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.

@GroceryBoyJr
Copy link
Contributor

/label peer-review-done

@ShaunaDiaz
Copy link
Contributor Author

/label merge-review-needed

@openshift-ci openshift-ci bot added peer-review-done Signifies that the peer review team has reviewed this PR merge-review-needed Signifies that the merge review team needs to review this PR labels Oct 27, 2022
@michaelryanpeter
Copy link
Contributor

/label merge-review-in-progress

@openshift-ci openshift-ci bot added the merge-review-in-progress Signifies that the merge review team is reviewing this PR label Oct 27, 2022
michaelryanpeter
michaelryanpeter reviewed Oct 27, 2022
View reviewed changes
Copy link
Contributor

@michaelryanpeter michaelryanpeter left a comment

Choose a reason for hiding this comment

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

I found a few small nits. Please reach out if you have any questions!

modules/installation-aws-config-yaml.adoc Outdated Show resolved Hide resolved
modules/installation-aws-config-yaml.adoc Outdated Show resolved Hide resolved
modules/installation-configuration-parameters.adoc Outdated Show resolved Hide resolved
modules/installation-configuration-parameters.adoc Outdated Show resolved Hide resolved
@michaelryanpeter
Copy link
Contributor

/remove-label merge-review-in-progress
/remove-label merge-review-needed

@openshift-ci openshift-ci bot removed merge-review-in-progress Signifies that the merge review team is reviewing this PR merge-review-needed Signifies that the merge review team needs to review this PR labels Oct 27, 2022
@ShaunaDiaz
Copy link
Contributor Author

/label merge-review-needed

@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label Oct 27, 2022
@bergerhoffer bergerhoffer added the merge-review-in-progress Signifies that the merge review team is reviewing this PR label Oct 27, 2022
bergerhoffer
bergerhoffer reviewed Oct 27, 2022
View reviewed changes
Copy link
Contributor

@bergerhoffer bergerhoffer left a comment

Choose a reason for hiding this comment

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

@ShaunaDiaz just one question for you before this gets merged. If it's possible to use the number 6 callout (and bump all other callout numbers below appropriately) that would be preferred.

But if that's not possible, let us know and we'll merge as is. Thanks!

/remove-label merge-review-in-progress
/remove-label merge-review-needed

modules/installation-aws-config-yaml.adoc Show resolved Hide resolved
@openshift-ci openshift-ci bot removed merge-review-in-progress Signifies that the merge review team is reviewing this PR merge-review-needed Signifies that the merge review team needs to review this PR labels Oct 27, 2022
@ShaunaDiaz
Copy link
Contributor Author

@bergerhoffer Yes, it's the crazy ifdefs. I couldn't change them all.

@bergerhoffer bergerhoffer merged commit db6b750 into openshift:main Oct 27, 2022
@bergerhoffer
Copy link
Contributor

/cherrypick enterprise-4.12

@bergerhoffer bergerhoffer added this to the Planned for 4.12 GA milestone Oct 27, 2022
@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Oct 27, 2022
Merged
@openshift-cherrypick-robot
Copy link

@bergerhoffer: new pull request created: #52252

In response to this:

/cherrypick enterprise-4.12

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.

@ShaunaDiaz ShaunaDiaz mentioned this pull request Oct 31, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miheer miheer miheer requested changes

@bergerhoffer bergerhoffer bergerhoffer left review comments

@GroceryBoyJr GroceryBoyJr GroceryBoyJr left review comments

@michaelryanpeter michaelryanpeter michaelryanpeter left review comments

@bscott-rh bscott-rh bscott-rh left review comments

Assignees

@miheer miheer

Labels
branch/enterprise-4.12 peer-review-done Signifies that the peer review team has reviewed this PR size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
OCP 4.12 GA
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@ShaunaDiaz @ocpdocs-previewbot @miheer @lihongan @GroceryBoyJr @michaelryanpeter @bergerhoffer @openshift-cherrypick-robot @bscott-rh