Skip to content

OSDOCS-1768: document mutating .spec.endpointPublishingStrategy (new) #39451

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
bmcelvee merged 1 commit into from
Feb 14, 2022
Merged

OSDOCS-1768: document mutating .spec.endpointPublishingStrategy (new) #39451

bmcelvee merged 1 commit into from
Feb 14, 2022

Conversation

@bmcelvee
Copy link
Contributor

@bmcelvee bmcelvee commented Dec 1, 2021 •
edited
Loading

@bmcelvee bmcelvee added this to the Future Release milestone Dec 1, 2021
@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 1, 2021
@bmcelvee bmcelvee changed the title OSDOCS-1768: document mutating .spec.endpointPublishingStrategy OSDOCS-1768: document mutating .spec.endpointPublishingStrategy (new) Dec 1, 2021
@bmcelvee bmcelvee changed the title OSDOCS-1768: document mutating .spec.endpointPublishingStrategy (new) [WIP] OSDOCS-1768: document mutating .spec.endpointPublishingStrategy (new) Dec 1, 2021
@netlify
Copy link

netlify bot commented Dec 1, 2021 •
edited
Loading

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 62bae6b

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/620ab25e55e60200083d3ddc

😎 Browse the preview: https://deploy-preview-39451--osdocs.netlify.app

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 1, 2021
@bmcelvee bmcelvee changed the title [WIP] OSDOCS-1768: document mutating .spec.endpointPublishingStrategy (new) OSDOCS-1768: document mutating .spec.endpointPublishingStrategy (new) Jan 17, 2022
@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 Jan 17, 2022
@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 20, 2022
@bmcelvee bmcelvee force-pushed the OSDOCS-1768-new branch 2 times, most recently from 57074ff to 99df53c Compare January 21, 2022 16:12
@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 21, 2022
@bmcelvee
Copy link
Contributor Author

bmcelvee commented Feb 2, 2022

Hi, @Miciah! I reviewed the enhancement proposal and PRs for the endpoint publishing strategy and took a first pass at documenting the procedures. Would you mind giving it a review, please?

I also wanted to note that I added it as a new assembly to help with breaking down the giant Ingress Operator assembly and make it easier to users to find the content.

@bmcelvee bmcelvee mentioned this pull request Feb 2, 2022
Merged
@bmcelvee
Copy link
Contributor Author

bmcelvee commented Feb 7, 2022

Hi, @Miciah! I know you're busy, so here's just a little bump on this for review. Thanks!

Miciah
Miciah reviewed Feb 11, 2022
View reviewed changes
modules/nw-ingress-controller-configuration-parameters.adoc Outdated
* Bare metal: `NodePortService`
* Other: `HostNetwork`

For most platforms, the `endpointPublishingStrategy` value cannot be updated. However, on GCP, you can configure the `loadbalancer.providerParameters.gcp.clientAccess` subfield.
For most platforms, the `endpointPublishingStrategy` value can be updated. However, on GCP, you can configure the `loadbalancer.providerParameters.gcp.clientAccess` subfield.
Copy link
Contributor

@Miciah Miciah Feb 10, 2022

Choose a reason for hiding this comment

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

The sentence flow seems off now with the "However". The text should convey that endpointPublishingStrategy has some fields that can be updated, and these fields now include load-balancer scope. I checked the source code, and the following fields under spec.endpointPublishingStrategy are mutable:

  • spec.endpointPublishingStrategy.loadBalancer.scope (mutable as of OpenShift 4.10).
  • spec.endpointPublishingStrategy.loadbalancer.providerParameters.gcp.clientAccess (added in OpenShift 4.8).
  • spec.endpointPublishingStrategy.hostNetwork.protocol (added in OpenShift 4.8, but BZ#1997226 may cause problems if users try to mutate it on a cluster upgraded to 4.8 or 4.9).
  • spec.endpointPublishingStrategy.nodePort.protocol (ditto).

Copy link
Contributor Author

@bmcelvee bmcelvee Feb 11, 2022

Choose a reason for hiding this comment

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

Updating this table entry. Should we list hostNetwork.protocol
and nodePort.protocol as mutable if they can cause problems?

Copy link
Contributor

@Miciah Miciah Mar 4, 2022

Choose a reason for hiding this comment

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

Updating this table entry. Should we list hostNetwork.protocol and nodePort.protocol as mutable if they can cause problems?

Mutating them shouldn't cause problems on OpenShift 4.10, so listing them as mutable here is fine.

@bmcelvee
Copy link
Contributor Author

bmcelvee commented Feb 11, 2022

@lihongan would you mind giving this PR a review, please? Thanks!

@lihongan
Copy link

lihongan commented Feb 14, 2022

LGTM
Thank you all!

ahardin-rh
ahardin-rh reviewed Feb 14, 2022
View reviewed changes
ahardin-rh
ahardin-rh reviewed Feb 14, 2022
View reviewed changes
ahardin-rh
ahardin-rh reviewed Feb 14, 2022
View reviewed changes
ahardin-rh
ahardin-rh reviewed Feb 14, 2022
View reviewed changes
@ahardin-rh ahardin-rh added the peer-review-done Signifies that the peer review team has reviewed this PR label Feb 14, 2022
@ahardin-rh
Copy link
Contributor

ahardin-rh commented Feb 14, 2022

@bmcelvee LGTM! Just one comment that applies to a few instances.

@bmcelvee bmcelvee merged commit ec0486a into openshift:main Feb 14, 2022
@bmcelvee
Copy link
Contributor Author

bmcelvee commented Feb 14, 2022

/cherrypick enterprise-4.10

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Feb 14, 2022
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Feb 14, 2022

@bmcelvee: new pull request created: #41884

Details

In response to this:

/cherrypick enterprise-4.10

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 modified the milestones: Future Release, OCP 4.10 GA Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahardin-rh ahardin-rh ahardin-rh left review comments

+1 more reviewer

@Miciah Miciah Miciah left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

branch/enterprise-4.10 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.10 GA

Development

Successfully merging this pull request may close these issues.

6 participants

@bmcelvee @lihongan @ahardin-rh @openshift-cherrypick-robot @Miciah @bobfuru