Skip to content

MetalLB updating the pod affinity example OCPBUGS-17959 #63849

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
merged 1 commit into from
Sep 1, 2023
Merged

MetalLB updating the pod affinity example OCPBUGS-17959 #63849

merged 1 commit into from
Sep 1, 2023

Conversation

kquinn1204
Copy link
Contributor

@kquinn1204 kquinn1204 commented Aug 23, 2023 •
edited
Loading

[OCPBUGS-17959]: Updating the pod affinity example

Version(s): 4.14, 4.13, 4.12 and main

Issue:https://issues.redhat.com/browse/OCPBUGS-17959

Link to docs preview:https://63849--docspreview.netlify.app/openshift-enterprise/latest/networking/metallb/metallb-operator-install.html#nw-metallb-operator-setting-pod-priority-affinity_metallb-operator-install

QE review:

  • QE has approved this change.

Additional information:

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 23, 2023
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Aug 23, 2023 •
edited
Loading

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

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

@kquinn1204 kquinn1204 changed the title updating the pod affinity example OCPBUGS-17959 MetalLB updating the pod affinity example OCPBUGS-17959 Aug 24, 2023
@kquinn1204
Copy link
Contributor Author

@asood-rh This is a simple update to correct the pod affintity example for MetalLB hopefully you can QE it and add a LGTM to the ticket.

@asood-rh
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 30, 2023
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 31, 2023
@kquinn1204
Copy link
Contributor Author

/label telco

@openshift-ci openshift-ci bot added the telco Label for all Telco PRs label Aug 31, 2023
@kquinn1204
Copy link
Contributor Author

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Aug 31, 2023
@dfitzmau
Copy link
Contributor

/remove-label peer-review-needed

/label peer-review-in-progress

@openshift-ci openshift-ci bot added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Aug 31, 2023
dfitzmau
dfitzmau reviewed Aug 31, 2023
View reviewed changes
@dfitzmau
Copy link
Contributor

/remove-label peer-review-in-progress

/label peer-review-done

@openshift-ci openshift-ci bot added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Aug 31, 2023
@kquinn1204
Copy link
Contributor Author

Got LGTM from reporter on JIRA

@kquinn1204
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 Sep 1, 2023
@opayne1 opayne1 added merge-review-in-progress Signifies that the merge review team is reviewing this PR branch/enterprise-4.12 branch/enterprise-4.13 branch/enterprise-4.14 and removed merge-review-needed Signifies that the merge review team needs to review this PR labels Sep 1, 2023
@opayne1 opayne1 added this to the Continuous Release milestone Sep 1, 2023
opayne1
opayne1 reviewed Sep 1, 2023
View reviewed changes
Copy link
Contributor

@opayne1 opayne1 left a comment

Choose a reason for hiding this comment

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

I had one spelling nit I noticed and a suggestion while I was at it. Let me know your thoughts and I can merge after. Thanks!

modules/nw-metallb-operator-setting-runtimeclass.adoc Outdated
runtimeClassName: myclass
annotations: <1>
speaker: demo
----
<1> This example uses `annotations` to add metadata such as build release information or GitHub pull request information. You can populate annotations with characters not permitted in labels. However, you cannot use annotations to identify or select objects.
<1> The earlier example uses `annotations` to add metadata such as build release information or GitHub pull request information. You can populate annotations with characters that are no permitted in labels. However, you cannot use annotations to identify or select objects.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supposed to be "not"? Also, I am confused with the use of "The earlier example" does that mean the example from #1 or does it mean the example this callout is in? If it is this example, I would keep the "This"

Suggested change
<1> The earlier example uses `annotations` to add metadata such as build release information or GitHub pull request information. You can populate annotations with characters that are no permitted in labels. However, you cannot use annotations to identify or select objects.
<1> The earlier example uses `annotations` to add metadata such as build release information or GitHub pull request information. You can populate annotations with characters that are not permitted in labels. However, you cannot use annotations to identify or select objects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hi @opayne1 I may have inadvertently touched that I think I was asked to add "previous" example but vale recommended use of "earlier"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest I really think that description is not needed at all but I will revert to original here and I do need to revise this section in a separate PR, there is something else that needs clarification too IMO.

@kquinn1204
Copy link
Contributor Author

@opayne1 thanks for noticing that hopefully good to merge now.

opayne1
opayne1 reviewed Sep 1, 2023
View reviewed changes
Copy link
Contributor

@opayne1 opayne1 left a comment

Choose a reason for hiding this comment

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

Thanks for the quick updates!

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 1, 2023
@opayne1 opayne1 merged commit b1bff3e into openshift:main Sep 1, 2023
@opayne1
Copy link
Contributor

opayne1 commented Sep 1, 2023

/cherrypick enterprise-4.12

@opayne1
Copy link
Contributor

opayne1 commented Sep 1, 2023

/cherrypick enterprise-4.13

@opayne1
Copy link
Contributor

opayne1 commented Sep 1, 2023

/cherrypick enterprise-4.14

@openshift-cherrypick-robot
Copy link

@opayne1: new pull request created: #64234

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.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Sep 1, 2023
Merged
@openshift-cherrypick-robot
Copy link

@opayne1: new pull request created: #64235

In response to this:

/cherrypick enterprise-4.13

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.

This was referenced Sep 1, 2023
Merged
Merged
@openshift-cherrypick-robot
Copy link

@opayne1: new pull request created: #64236

In response to this:

/cherrypick enterprise-4.14

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
Reviewers

@dfitzmau dfitzmau dfitzmau left review comments

@opayne1 opayne1 opayne1 left review comments

Assignees

@asood-rh asood-rh

@opayne1 opayne1

Labels
branch/enterprise-4.12 branch/enterprise-4.13 branch/enterprise-4.14 lgtm Indicates that a PR is ready to be merged. merge-review-in-progress Signifies that the merge review team is reviewing this PR peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files. telco Label for all Telco PRs
Projects
None yet
Milestone
Continuous Release
Development

Successfully merging this pull request may close these issues.
6 participants
@kquinn1204 @ocpdocs-previewbot @asood-rh @dfitzmau @opayne1 @openshift-cherrypick-robot