Skip to content

✨ Make allowedAddressPairs on OpenStackMachine ports mutable#3056

Open
jfpucheu wants to merge 4 commits intokubernetes-sigs:mainfrom
jfpucheu:feat/allowedaddresspairs-mutable
Open

✨ Make allowedAddressPairs on OpenStackMachine ports mutable#3056
jfpucheu wants to merge 4 commits intokubernetes-sigs:mainfrom
jfpucheu:feat/allowedaddresspairs-mutable

Conversation

@jfpucheu
Copy link
Copy Markdown

Summary

  • OpenStackMachineTemplate webhook: excludes allowedAddressPairs from the immutability check so the field can be updated in-place without triggering a node rolling replacement.
  • OpenStackMachine webhook: same exclusion to allow the OSMT controller to annotate OSMs without being rejected by the spec-immutability webhook.
  • OpenStackMachine controller: deep-copies ports before mutation to avoid corrupting the original spec through shared slice backing arrays (which would incorrectly trigger the spec-immutability webhook).
  • Networking service: adds UpdateAllowedAddressPairs() to call ports.Update on the Neutron API.
  • OpenStackMachineTemplate controller: adds reconcileAllowedAddressPairs which walks MachineSets → Machines → OpenStackMachines → OpenStackServers and calls UpdateAllowedAddressPairs() for each provisioned port. Port IDs are read from OpenStackServer.Status.Resources.Ports. Idempotency is tracked via an annotation on each OpenStackMachine (infrastructure.cluster.x-k8s.io/osmt-allowed-address-pairs) so only a metadata-only patch is needed, avoiding the spec-immutability webhook entirely.
  • RBAC: adds machinesets, machines, and openstackmachines get/list/watch/patch permissions to the OSMT controller.
  • Unit tests for reconcileAllowedAddressPairs.
  • E2E test: creates a cluster with 1 CP and 1 worker (via a dedicated MachineDeployment with an explicit port), then patches the OSMT three times to accumulate allowedAddressPairs and verifies via the Neutron API after each patch that the port is updated without server restart.

Motivation

Neutron supports updating allowedAddressPairs on a port without recreating the server. Previously, any change to the OSMT spec (including allowedAddressPairs) was rejected by the immutability webhook, forcing operators to recreate nodes to apply network policy changes. This PR enables live updates of allowedAddressPairs on existing machines.

This feature is particularly useful in OpenStack environments without a LoadBalancer (e.g. FlavorWithoutLB), where solutions like MetalLB or kube-vip are used to expose services via Virtual IPs (VIPs). These tools require adding VIP addresses to the allowedAddressPairs of the worker node ports so that OpenStack allows the traffic to pass through. Without this feature, operators had to recreate nodes every time a new VIP was added — defeating the purpose of in-cluster load balancing.

Test plan

  • Unit tests pass: go test ./controllers/... ./pkg/webhooks/... ./pkg/cloud/services/networking/...
  • E2E test passes: E2E_GINKGO_FOCUS="allowedAddressPairs" make test-e2e
  • Verify Neutron port allowedAddressPairs are updated without Nova server restart

I am not a developer — this contribution was implemented with the assistance of Claude Code because this feature was blocking our production use of MetalLB on OpenStack without LoadBalancer support.

@netlify
Copy link
Copy Markdown

netlify bot commented Mar 15, 2026

Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name Link
🔨 Latest commit ad24391
🔍 Latest deploy log https://app.netlify.com/projects/kubernetes-sigs-cluster-api-openstack/deploys/69ca93f32d44b300087d7765
😎 Deploy Preview https://deploy-preview-3056--kubernetes-sigs-cluster-api-openstack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign fabriziopandini for approval. For more information see the Code Review Process.

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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Welcome @jfpucheu!

It looks like this is your first PR to kubernetes-sigs/cluster-api-provider-openstack 🎉. 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/cluster-api-provider-openstack 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. 😃

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Mar 15, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 15, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Hi @jfpucheu. 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.

Details

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.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 15, 2026
@jfpucheu
Copy link
Copy Markdown
Author

This is my first PR. I have not been able to test on a multi-AZ environment as I only have a single-AZ DevStack setup available."

@jfpucheu jfpucheu force-pushed the feat/allowedaddresspairs-mutable branch from 456b330 to 5880143 Compare March 16, 2026 09:00
@jfpucheu
Copy link
Copy Markdown
Author

/easycla

@jfpucheu jfpucheu force-pushed the feat/allowedaddresspairs-mutable branch from 5880143 to 04296a3 Compare March 16, 2026 09:10
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 16, 2026
@nikParasyr
Copy link
Copy Markdown
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 17, 2026
@lentzi90
Copy link
Copy Markdown
Contributor

/retitle ✨ Make allowedAddressPairs on OpenStackMachine ports mutable

@k8s-ci-robot k8s-ci-robot changed the title feat: make allowedAddressPairs on OpenStackMachine ports mutable ✨ Make allowedAddressPairs on OpenStackMachine ports mutable Mar 17, 2026
@jfpucheu
Copy link
Copy Markdown
Author

The CI failures are not related to this PR. The flatcar test timed out waiting for the control plane to become ready (infrastructure issue), which cascaded into the other failures. Our changes only affect allowedAddressPairs handling and have no impact on the flatcar bootstrap path.
/retest

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 27, 2026
@jfpucheu jfpucheu force-pushed the feat/allowedaddresspairs-mutable branch 2 times, most recently from 587bae3 to 327728a Compare March 30, 2026 15:05
Neutron ports support updating allowedAddressPairs without recreating
the server. This change allows operators to update allowedAddressPairs
on existing machines by patching the OpenStackMachineTemplate, without
triggering a rolling replacement of the nodes.

Changes:
- OpenStackMachineTemplate webhook: exclude allowedAddressPairs from
  the immutability check so the field can be updated in-place.
- OpenStackMachine webhook: same exclusion to allow the controller to
  annotate OSMs without being rejected.
- OpenStackMachine controller: deep-copy ports before mutation to avoid
  corrupting the original spec through shared slice backing arrays,
  which would trigger the spec-immutability webhook.
- Networking service: add UpdateAllowedAddressPairs() to call
  ports.Update on the Neutron API.
- OpenStackMachineTemplate controller: add reconcileAllowedAddressPairs
  which walks MachineSets → Machines → OpenStackMachines → OpenStackServers
  and calls UpdateAllowedAddressPairs() for each provisioned port.
  Idempotency is tracked via an annotation on each OpenStackMachine so
  only a metadata-only patch is needed, avoiding the spec-immutability
  webhook entirely.
- RBAC: add machinesets/machines/openstackmachines get/list/watch/patch
  permissions to the OSMT controller.
- Unit tests for reconcileAllowedAddressPairs.
- E2E test: creates a cluster with 1 CP and 1 worker (via a dedicated
  MachineDeployment with an explicit port), then patches the OSMT three
  times to accumulate allowedAddressPairs and verifies via the Neutron
  API after each patch that the port is updated without server restart.
- Move reconcileAllowedAddressPairs before imageID nil check so it is
  never skipped by the early return in reconcileNormal
- Add missing RBAC marker for openstackservers (get;list;watch)
- Add webhook test cases for allowedAddressPairs mutability:
  - changing only AAP is allowed
  - changing AAP + another field is rejected
  - adding a port is rejected
  - removing a port is rejected
@jfpucheu jfpucheu force-pushed the feat/allowedaddresspairs-mutable branch from 327728a to ad24391 Compare March 30, 2026 15:17
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2026
@bnallapeta
Copy link
Copy Markdown
Contributor

will take a look today.

Copy link
Copy Markdown
Contributor

@bnallapeta bnallapeta left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 31, 2026
@bnallapeta
Copy link
Copy Markdown
Contributor

@lentzi90 PTAL when you can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

Status: Inbox

Development

Successfully merging this pull request may close these issues.

5 participants