Partial Cilium 1.16+ Support & Add vars for configuring cilium IP load balancer pools and bgp v1 & v2 apis

logicsys commented 1 month ago

What type of PR is this?

/kind feature

What this PR does / why we need it: Allows user to define Cilium IP Load Balancer pools & BGP Peer Policies, the following new defaults have been added to preserve current behavior:

# -- Configure Loadbalancer IP Pools
cilium_loadbalancer_ip_pools: []

# -- Enable BGP Control Plane
cilium_enable_bgp_control_plane: false

# -- Configure BGP Instances (New bgpv2 API v1.16+)
cilium_bgp_cluster_configs: []

# -- Configure BGP Peers (New bgpv2 API v1.16+)
cilium_bgp_peer_configs: []

# -- Configure BGP Advertisements (New bgpv2 API v1.16+)
cilium_bgp_advertisements: []

# -- Configure BGP Node Config Overrides (New bgpv2 API v1.16+)
cilium_bgp_node_config_overrides: []

# -- Configure BGP Peers (Legacy < v1.16)
cilium_bgp_peering_policies: []

Due to CRDs being necessary before these features can be configured, this change first waits for these CRDs to exist before attempting to create, if above vars are configured.

Does this PR introduce a user-facing change?: No user-facing change, current behavior is preserved with defaults.

Partial Support of Cilium v1.16+ - kube-proxy replacement var changes
Add optional support for configuring _BGP Control Plane_, _IP Load Balancer Pools_ , _Legacy BGP Peer Config v1_  and _BGP Config v2_ features in Cilium

linux-foundation-easycla[bot] commented 1 month ago

The committers listed above are authorized under a signed CLA.

:white_check_mark: login: logicsys (f73b7d119c515ed9464dc60e3467225ed1b4fbc2, 11718da1418ae7fdc8600031fc2b2a625d5253ed)

k8s-ci-robot commented 1 month ago

Welcome @logicsys!

It looks like this is your first PR to kubernetes-sigs/kubespray 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubespray has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

k8s-ci-robot commented 1 month ago

Hi @logicsys. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.

logicsys commented 1 month ago

/kind feature

VannTen commented 1 month ago

/ok-to-test

tico88612 commented 1 month ago

@logicsys Is there any special reason to force push many commits? Random pushes drain computing resources and do not pass the CI.

logicsys commented 1 month ago

I was pushing fixes to satisfy the CI tests as they failed, force pushing after a squash, so as to not muddy up the pull request with many commits

logicsys commented 1 month ago

if there was a way for me to run the CI tests locally, then I can do that to save resources

logicsys commented 1 month ago

think thats everything fixed now though, the failing tests appear to be due to some vms not being able to be created

tico88612 commented 1 month ago

if there was a way for me to run the CI tests locally, then I can do that to save resources

You can use Vagrant to test your feature.

think thats everything fixed now though, the failing tests appear to be due to some vms not being able to be created

This is because the frequent creation of machines will trigger Vagrant machine HTTP 429, we're still working on it. Continuous push does not speed up CI passes.

logicsys commented 1 month ago

great, I'll use vagrant next time.

Yes I understand frequent pushes will not speed up CI passes, I was only pushing again, to fix the issues that were raised by the CI tests as they failed.

Apologies for the burned resources

Does the vagrant test route, perform the linting steps that the CI does? Thats where i had issues, rather than with the actual functionality of the changes in this PR, which i had of course tested myself before creating this PR.

If not, I could perhaps look at getting the lint tests to run via vagrant, as my next PR ha

tico88612 commented 1 month ago

/retest-failed

logicsys commented 1 month ago

appears to still be a 429 vm creation error as far as i can tell in the molecule_containerd test :( maybe with one more re-test ?

tico88612 commented 1 month ago

I think Kubespray CI didn't record the molecule_containerd job ID; let me retest for it.

/retest

EDIT: not working with content.

tico88612 commented 1 month ago

/retest

logicsys commented 1 month ago

this has of course grown into adding some v1.16+ cilium support, as the new bgp apis do not exist in our current default cilium v1.15.9, i've not set the cilium default version any higher, ive just overridden for my own tests for now, just a couple more tests to run, but think i've got the new bgpv2 apis working properly when configured via kubespray vars.

Full support of cilium v1.16 will likely require more than this PR alone, but at least the way forward is slightly paved with these changes ha

logicsys commented 1 month ago

sorry was one last typo to fix there, ready for review again now

yankay commented 2 weeks ago

HI @cyclinder Would you please help to review it?

k8s-ci-robot commented 1 week ago

@cyclinder: GitHub didn't allow me to request PR reviews from the following users: the, approval, please, for.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to [this](https://github.com/kubernetes-sigs/kubespray/pull/11620#discussion_r1836035949): >/cc @yankay please for the approval Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.

yankay commented 3 days ago

Thanks @logicsys /approve

k8s-ci-robot commented 3 days ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: logicsys, yankay

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/kubespray/blob/master/OWNERS)~~ [yankay] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

kubernetes-sigs / kubespray

Partial Cilium 1.16+ Support & Add vars for configuring cilium IP load balancer pools and bgp v1 & v2 apis #11620