-
Notifications
You must be signed in to change notification settings - Fork 359
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
Support deploying one FRR container in Kind network #6488
Conversation
b776865
to
3e2d7e4
Compare
3e2d7e4
to
023b415
Compare
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.
How will we ensure that normal Pods used for testing (toolbox, agnhost, nginx, ...) are not scheduled on this extra Node? I guess it will never be Ready and so Pods won't be scheduled there unless they have the appropriate toleration?
Unless I am mistaken, with this change we effectively go from a 3-Node K8s cluster for testing to a 4-Node K8s cluster? This is not an insignificant change as the Github runners have limited resources and an extra Node may put a strain on these resources. Would like @tnqn to review this PR as well.
I think we should only enable this option in the test that enables all features. I am planning to use it to test BGP only (install a host network FRR Pod on the Node without Antrea to mock remote BGP router). |
I have the same concern as @antoninbas. In my understanding the test just needs a BGP router, right? Why it needs a NotReady worker Node to run it, instead of just running the router directly via |
Got that. If so, I will try to enhance the current external server to mock BGP server. I think we need to add some framework code to operate the external server, for example:
Maybe more details should be considered. How do you think about this proposal? @tnqn @antoninbas |
Not sure what you mean by framework code. In my mind you just need an image for 1, customize |
I didn't make it clear. I just meant that I may need to add some fundamental functions because I'm not sure if we have some existing convenient functions to configure the external server you added. @tnqn |
I think there is no problem to have another parameter to deploy a new bgp router (not based on the external server), given they don't have many things in common, and there isn't much code to bring up the external server. |
3c275ca
to
0ad4c1d
Compare
0fb4110
to
9557b49
Compare
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.
LGTM overall
9557b49
to
059eec6
Compare
059eec6
to
7526b58
Compare
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.
LGTM
if $bgp_policy; then | ||
external_frr_cid=$(docker ps -f name="^antrea-external-frr" --format '{{.ID}}') | ||
external_frr_ips=$(docker inspect $external_frr_cid -f '{{.NetworkSettings.Networks.kind.IPAddress}},{{.NetworkSettings.Networks.kind.GlobalIPv6Address}}') | ||
EXTRA_ARGS="$EXTRA_ARGS --external-frr-cid $external_frr_cid --external-frr-ips $external_frr_ips" |
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.
why we need to pass external_frr_cid to test code? I think this would cause the test can only run in kind cluster.
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 thought we could run some commands in the FRR container by passing the cid to test code.
I think this would cause the test can only run in kind cluster.
A question: is the external agnhost container only supported in Kind cluster? I thought that it was the same for the external frr container.
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 thought we could run some commands in the FRR container by passing the cid to test code.
Is there a public API to access the FRR application, like "/clientip" of agnhost container?
A question: is the external agnhost container only supported in Kind cluster? I thought that it was the same for the external frr container.
The test code doesn't make any assumption on how the external agnhost server is deployed so technically you can specify a VM IP or a public IP. It only needs the server's IP.
By passing the container ID to test code, I assume you would then use it to run some validation, then the code can be only ran in a kind cluster, and can't work with real BGP router.
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.
Is there a public API to access the FRR application, like "/clientip" of agnhost container?
I'm afraid we cannot configure it through network. I found that FRR application doesn't provide any API based TCP/IP. It can be configured through Unix Socket API locally.
By passing the container ID to test code, I assume you would then use it to run some validation, then the code can be only ran in a kind cluster, and can't work with real BGP router.
I use the container ID to configure the FRR application using the command like docker exec -it <container ID> <command>
.
12917bb
to
7a7a8bc
Compare
7a7a8bc
to
cbe8956
Compare
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.
LGTM overall, please replace an FRR
to 'a FRR' in your PR title and description.
This commit introduces a new parameter `--deploy-external-frr` in `ci/kind/kind-setup.sh`, enabling to deploy one FRR container in the Kind cluster network. This serves as the foundation for running BGPPolicy e2e tests. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
03f7adf
to
3e83040
Compare
@tnqn @luolanzone Do we have any more comments about this? If not, could we merge this PR? Thanks |
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.
Thanks for the efforts, no more comments from my side, @tnqn could you take another look?
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.
LGTM
/skip-all |
1 similar comment
/skip-all |
This commit introduces a new parameter
--deploy-external-frr
in
ci/kind/kind-setup.sh
, enabling to deploy one FRR container inthe Kind cluster network. This serves as the foundation for running
BGPPolicy e2e tests.