Skip to content

OSDOCS-3691: Ported Network Polices to OSD/ROSA #48324

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 2, 2022
Merged

OSDOCS-3691: Ported Network Polices to OSD/ROSA #48324

merged 1 commit into from
Sep 2, 2022

Conversation

EricPonvelle
Copy link
Contributor

@EricPonvelle EricPonvelle commented Jul 26, 2022 •
edited
Loading

Version(s):
4.11+

Issue:
https://issues.redhat.com/browse/OSDOCS-3868

Link to docs preview:

NOTE: the striked items are present in OCP but not in either of the ROSA/OSD doc set.

Additional information:
Ported the Network Policies book to OSD/ROSA

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 26, 2022
@EricPonvelle EricPonvelle force-pushed the OSDOCS-3691_NetworkingPort branch 2 times, most recently from 30b6ab4 to d29b17c Compare July 26, 2022 20:57
@EricPonvelle EricPonvelle force-pushed the OSDOCS-3691_NetworkingPort branch 4 times, most recently from d3c0c3c to 9d2dd01 Compare August 4, 2022 14:17
@bergerhoffer
Copy link
Contributor

The enterprise-4.12 label has been added to this PR.

This is because your PR targets the main branch and is labeled for enterprise-4.11. And any PR going into main must also target the latest version branch (enterprise-4.12).

If the update in your PR does NOT apply to version 4.12 onward, please re-target this PR to go directly into the appropriate version branch or branches (enterprise-4.x) instead of main.

@EricPonvelle EricPonvelle force-pushed the OSDOCS-3691_NetworkingPort branch 2 times, most recently from 2cbf4b0 to 351c946 Compare August 11, 2022 20:42
@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 Aug 11, 2022
@EricPonvelle EricPonvelle force-pushed the OSDOCS-3691_NetworkingPort branch 14 times, most recently from 36555df to 1e6b486 Compare August 16, 2022 18:26
@dustman9000
Copy link
Member

dustman9000 commented Aug 24, 2022 •
edited
Loading

This looks good. My only comment would be to add a note about avoiding deleting/modifying SRE managed networkpolicies:

NAMESPACE                      NAME                                POD-SELECTOR                                                                              AGE
openshift-monitoring           token-refresher                     app.kubernetes.io/component=authentication-proxy,app.kubernetes.io/name=token-refresher   183d
openshift-ocm-agent-operator   ocm-agent-allow-only-alertmanager   app=ocm-agent                                                                             149d

@EricPonvelle
Copy link
Contributor Author

@xueli181114 https://issues.redhat.com/browse/OSDOCS-3868 is the correct issue. The 3601 referred to a larger porting effort that has changed.

@EricPonvelle
Copy link
Contributor Author

This looks good. My only comment would be to add a note about avoiding deleting/modifying SRE managed networkpolicies:

NAMESPACE                      NAME                                POD-SELECTOR                                                                              AGE
openshift-monitoring           token-refresher                     app.kubernetes.io/component=authentication-proxy,app.kubernetes.io/name=token-refresher   183d
openshift-ocm-agent-operator   ocm-agent-allow-only-alertmanager   app=ocm-agent                                                                             149d

@dustman9000 So where do I need to add this note? In the About Network Policies topic? I could not find that Namespace table anywhere in the OSD/ROSA documentation.

IMPORTANT

You should not delete or edit existing networkpolicies that are managed by Red Hat SRE.

jboxman-rh
jboxman-rh reviewed Aug 26, 2022
View reviewed changes
Copy link

@jboxman-rh jboxman-rh left a comment

Choose a reason for hiding this comment

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

@EricPonvelle, I airdropped a few comments. Thanks!

opayne1
opayne1 reviewed Aug 26, 2022
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.

Looks great! Just a few things to consider. I think the biggest thing is making the UI callouts consistent. I think most of the ones in quotations should be bolded. Happy to answer any questions you might have about the review. It also may be good for someone closer to this content to rereview this after you add in the suggestions just to make sure I didn't miss anything. :)

@EricPonvelle EricPonvelle force-pushed the OSDOCS-3691_NetworkingPort branch from 1e6b486 to 21e8d33 Compare August 31, 2022 20:34
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 31, 2022
@openshift-ci
Copy link

openshift-ci bot commented Aug 31, 2022

New changes are detected. LGTM label has been removed.

@pneedle-rh pneedle-rh self-requested a review September 1, 2022 08:25
@pneedle-rh
Copy link
Contributor

pneedle-rh commented Sep 1, 2022 •
edited
Loading

@EricPonvelle hi! Is there a specific OCP requirement in the PR for enterprise-4.10? For the OSD and ROSA docs, only enterprise-4.11 and enterprise-4.12 are required now, given that those libraries are now published from enterprise-4.11.

If there is no requirement for 4.10, I'll remove the label. Thanks!

pneedle-rh
pneedle-rh reviewed Sep 1, 2022
View reviewed changes
Copy link
Contributor

@pneedle-rh pneedle-rh left a comment

Choose a reason for hiding this comment

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

@EricPonvelle I have completed my review of this PR. Please let me know if you have any questions about my suggestions. Thanks!

@pneedle-rh pneedle-rh added peer-review-needed Signifies that the peer review team needs to review this PR 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 Sep 1, 2022
@EricPonvelle EricPonvelle force-pushed the OSDOCS-3691_NetworkingPort branch from 21e8d33 to 3ee223d Compare September 1, 2022 19:55
@yuwang-RH
Copy link
Member

LGTM~

pneedle-rh
pneedle-rh reviewed Sep 2, 2022
View reviewed changes
Copy link
Contributor

@pneedle-rh pneedle-rh left a comment

Choose a reason for hiding this comment

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

@EricPonvelle thank you for reviewing my suggestions. I have added a few additional comments, following the updates. Other than those, this PR looks good to me! Thanks.

@EricPonvelle EricPonvelle force-pushed the OSDOCS-3691_NetworkingPort branch from 3ee223d to e1d200f Compare September 2, 2022 15:31
@EricPonvelle EricPonvelle merged commit 6398cea into openshift:main Sep 2, 2022
@EricPonvelle
Copy link
Contributor Author

/cherrypick enterprise-4.11

@EricPonvelle
Copy link
Contributor Author

/cherrypick enterprise-4.12

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Sep 2, 2022
Merged
@openshift-cherrypick-robot
Copy link

@EricPonvelle: new pull request created: #50014

In response to this:

/cherrypick enterprise-4.11

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 2, 2022
Merged
@openshift-cherrypick-robot
Copy link

@EricPonvelle: new pull request created: #50015

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.

@EricPonvelle EricPonvelle deleted the OSDOCS-3691_NetworkingPort branch September 2, 2022 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@opayne1 opayne1 opayne1 left review comments

+2 more reviewers

@pneedle-rh pneedle-rh pneedle-rh left review comments

@jboxman-rh jboxman-rh jboxman-rh left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@xueli181114 xueli181114

Labels
branch/enterprise-4.11 branch/enterprise-4.12 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
Continuous Release
Development

Successfully merging this pull request may close these issues.
9 participants
@EricPonvelle @bergerhoffer @dustman9000 @xueli181114 @pneedle-rh @yuwang-RH @openshift-cherrypick-robot @opayne1 @jboxman-rh