-
Notifications
You must be signed in to change notification settings - Fork 1.8k
OSDOCS-10810: Add BGP routing #88826
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.
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
base: main
Are you sure you want to change the base?
Conversation
- https://issues.redhat.com/browse/OSDOCS-10810 Includes BGP only.
@jab-rh: This pull request references OSDOCS-10810 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
1 similar comment
@jab-rh: This pull request references OSDOCS-10810 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
🤖 Thu May 08 15:39:10 - Prow CI generated the docs preview: |
@jab-rh: This pull request references OSDOCS-10810 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
LGTM |
The This is because your PR targets the If the update in your PR does NOT apply to version 4.19 onward, please re-target this PR to go directly into the appropriate version branch or branches (enterprise-4.x) instead of main. |
/lgtm |
@jab-rh: This pull request references OSDOCS-10810 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
Hi @jechen0648 Can you verify this documentation PR when you have a moment? It includes just the BGP parts. The additional routing parts were split into a separate PR by request. Thanks! |
+ | ||
[source,terminal] | ||
---- | ||
$ oc create namespace openshift-frr-k8s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to create the namespace here? I thought the namespace is created automatically when metalLB operator is upgraded to 4.19, @asood-rh correct me if it's not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zhaozhanqi We are copying over custom FRR configurations before upgrading that is why we need to manually create the namespace. If there were no custom FRR configuration just plain upgrade of metallb operator, the namespace will be created by metallb install.
Sorry I missed your question.
@jab-rh: This pull request references OSDOCS-10810 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
@jab-rh: This pull request references OSDOCS-10810 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
/lgtm |
/remove-label peer-review-needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/remove-label peer-review-in-progress
/label peer-review-done
File: about-bgp-routing | ||
- Name: Configuring BGP routing | ||
File: configuring-bgp-routing | ||
- Name: Migrating FRR-K8s custom resources |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the project name has a lowercase "k": FRR-k8s
We should refer to it consistently and accurately wherever it appears. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kowen-rh, wasn't really familiar with FRR-K8s, but the original documentation work in this area is: modules/nw-metallb-frr-k8s-merge-multiple-configurations.adoc
and that does capitalize K, so I've been maintaining consistency with this original work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense to me as long as it's consistent. 🙂
modules/nw-bgp-about.adoc
Outdated
- {vmw-full} on-premise | ||
|
||
[id="prerequisites_{context}"] | ||
== Prerequisites |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a procedure, so we don't need a section for prerequisites. I'd remove these two lines so that this is at the end of the "Supported platforms" section.
The following custom resources are used to configure BGP routing: | ||
|
||
`FRRConfiguration`:: | ||
This custom resource defines the FRR configuration for the BGP routing. This CR is namespaced. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a comment, but it seems like this module serves as a "miscellaneous information" page. Be careful with those, as they can become unwieldy as time progresses and more is added. It might be worth rethinking how this information is organized so that related concepts are grouped together.
New changes are detected. LGTM label has been removed. |
@jab-rh: all tests passed! Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
@kowen-rh, I've applied the suggested changes, thanks! |
Includes BGP only.
Version(s): 4.19.z; 4.18.z
Issue:
Link to docs preview:
QE review:
Additional information: