:bug: skip reconcile for ipaddress and ipaddressclaim when cluster is paused by archerwu9425 · Pull Request #3037 · kubernetes-sigs/cluster-api-provider-openstack

archerwu9425 · 2026-03-03T05:59:15Z

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #3029

Special notes for your reviewer:

This PR is to fix ipaddressclaim and ipaddress keep reconciling while cluster is paused.
Following the cluster-api ipam provider contract:
https://cluster-api.sigs.k8s.io/developer/providers/contracts/ipam#ipam-provider

Changes include:

If the related Cluster is paused, abort reconciliation
Add finalizer openstackfloatingippool.infrastructure.cluster.x-k8s.io to ipaddressclaim
For ipaddress, if the ipaddressclaim defined in spec.ClaimRef or the related cluster is paused, abort reconciliation
If ipaddress is deleted, update the status of the ipaddressclaim defined in spec.ClaimRef

TODOs:

squashed commits
if necessary:
- includes documentation
- adds unit tests

/hold

netlify · 2026-03-03T05:59:21Z

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

Name	Link
🔨 Latest commit	`b7566a1`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-cluster-api-openstack/deploys/69c33a46025701000836ea19
😎 Deploy Preview	https://deploy-preview-3037--kubernetes-sigs-cluster-api-openstack.netlify.app
📱 Preview on mobile	Toggle 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 · 2026-03-03T05:59:21Z

[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 emilienm 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:

OWNERS

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 · 2026-03-03T05:59:24Z

Welcome @archerwu9425!

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. 😃

k8s-ci-robot · 2026-03-03T05:59:25Z

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

nikParasyr · 2026-03-03T06:10:21Z

/ok-to-test

archerwu9425 · 2026-03-03T09:14:07Z

/test pull-cluster-api-provider-openstack-e2e-test

archerwu9425 · 2026-03-04T06:09:50Z

@lentzi90 @nikParasyr Could you please help review? Thanks

lentzi90

Thanks for the PR!
I agree that we have a bug in that we reconcile the IPAddresses and claims even when the cluster is paused. This PR seems to do more than just fix this bug though.
It also adds no tests. I think we do need some tests to verify the behavior and I have a few concerns (see below).

…ss before remove claim finalzier

archerwu9425 · 2026-03-05T12:54:38Z

@lentzi90 changes made based on comment and tests added. Could you please help review again? Thanks

archerwu9425 · 2026-03-06T10:17:30Z

@lentzi90 Could you please help review again? Thanks

archerwu9425 · 2026-03-09T02:28:32Z

@lentzi90 @nikParasyr Could you please help review? Thanks

archerwu9425 · 2026-03-17T00:53:42Z

@lentzi90 @nikParasyr Could you please help review? Thanks

nikParasyr · 2026-03-17T06:30:55Z

@archerwu9425 i have it on my list, probably end of the week as it is a bit hectic now.
@bnallapeta if you have time in the meantime, would be nice

bnallapeta · 2026-03-17T07:54:06Z

@archerwu9425 i have it on my list, probably end of the week as it is a bit hectic now. @bnallapeta if you have time in the meantime, would be nice

Will put it on my list. But unfortunately, cannot get it to soon.

bnallapeta · 2026-03-23T07:23:24Z

+		cluster, err := util.GetClusterFromMetadata(ctx, r.Client, claim.ObjectMeta)
+		if err != nil {
+			log.Error(err, "Failed to get owning cluster, skipping claim", "claim", claim.Name)
+			continue


Consider a scenario where there's no cluster label no the claim. In that case, we would get ErrNoCluster and since you have a continue here, it would just skip the claim completely.

This can cause a problem since any IPAddressClaim without a cluster label will never be processed and never be deleted (since finalizer won't be removed).

I think we should process it normally when label is not found as well as this will avoid the regression. Currently, this is what we do.

bnallapeta · 2026-03-23T07:26:40Z

+				}
+				if cluster != nil && annotations.IsPaused(cluster, claim) {
+					scope.Logger().V(4).Info("IPAddress owner IPAddressClaim or linked Cluster is paused, skipping deletion", "ipAddress", ipAddress.Name, "claim", claim.Name)
+					continue


This means the deletion will be skipped. So, the floating IP exists in OpenStack. And the following code at line 367 will mark this as available. This might cause the IP to be allocated to two different claims.

Fix: add this to ClaimedIPs. That way, this IP won't be utilized by any other resource.

bnallapeta · 2026-03-23T07:30:21Z

+	cluster, err := util.GetClusterFromMetadata(ctx, r.Client, claim.ObjectMeta)
+	if err != nil {
+		log.Error(err, "Failed to get owning cluster, skipping mapping", "claim", claim.Name, "namespace", claim.Namespace)
+		return nil


Same issue as above. If the claim doesn't have a cluster label, GetClusterFromMetadata returns ErrNoCluster, and this returns nil. Meaning, changes to this claim will never trigger a pool reconciliation. The controller won't even notice the claim was created, updated, or deleted.

fix: If there's no cluster, we should still map the event to the pool so reconciliation proceeds normally.

k8s-ci-robot · 2026-03-25T01:38:37Z

@archerwu9425: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-provider-openstack-test	`b7566a1`	link	true	`/test pull-cluster-api-provider-openstack-test`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

k8s-ci-robot · 2026-03-28T01:58:18Z

PR needs rebase.

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.

skip reconcil for ipaddress and ipaddressclaim when cluster is paused

274b180

github-project-automation bot added this to CAPO Roadmap Mar 3, 2026

github-project-automation bot moved this to Inbox in CAPO Roadmap Mar 3, 2026

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 3, 2026

k8s-ci-robot requested review from lentzi90 and nikParasyr March 3, 2026 05:59

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 3, 2026

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 3, 2026

archerwu9425 changed the title ~~skip reconcile for ipaddress and ipaddressclaim when cluster is paused~~ (:bug:)skip reconcile for ipaddress and ipaddressclaim when cluster is paused Mar 3, 2026

archerwu9425 changed the title ~~(:bug:)skip reconcile for ipaddress and ipaddressclaim when cluster is paused~~ [:bug:] skip reconcile for ipaddress and ipaddressclaim when cluster is paused Mar 3, 2026

archerwu9425 changed the title ~~[:bug:] skip reconcile for ipaddress and ipaddressclaim when cluster is paused~~ 🐛 skip reconcile for ipaddress and ipaddressclaim when cluster is paused Mar 3, 2026

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 3, 2026

archerwu9425 added 2 commits March 3, 2026 16:17

fix static test failures

83f177d

fix lint error and enhance claim reconcile process

51e048a

archerwu9425 and others added 2 commits March 3, 2026 18:14

Merge branch 'kubernetes-sigs:main' into main

b6fbb6d

enhance ipaddressclaim reconcile logic

adb49db

lentzi90 reviewed Mar 4, 2026

View reviewed changes

Comment thread controllers/openstackfloatingippool_controller.go Outdated

Comment thread controllers/openstackfloatingippool_controller.go

Comment thread controllers/openstackfloatingippool_controller.go Outdated

Comment thread controllers/openstackfloatingippool_controller.go

archerwu9425 added 2 commits March 5, 2026 15:59

fix IPAddress skip delete when empty claimRef issue and deleteIPAddre…

4baebbd

…ss before remove claim finalzier

add test for openstackfloatingippool_controller

0b5f0d7

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 5, 2026

archerwu9425 requested a review from lentzi90 March 5, 2026 09:58

bnallapeta reviewed Mar 23, 2026

View reviewed changes

Merge branch 'kubernetes-sigs:main' into main

b7566a1

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 28, 2026

Conversation

archerwu9425 commented Mar 3, 2026

Uh oh!

netlify bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Uh oh!

k8s-ci-robot commented Mar 3, 2026

Uh oh!

k8s-ci-robot commented Mar 3, 2026

Uh oh!

k8s-ci-robot commented Mar 3, 2026

Uh oh!

nikParasyr commented Mar 3, 2026

Uh oh!

archerwu9425 commented Mar 3, 2026

Uh oh!

archerwu9425 commented Mar 4, 2026

Uh oh!

lentzi90 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

archerwu9425 commented Mar 5, 2026

Uh oh!

archerwu9425 commented Mar 6, 2026

Uh oh!

archerwu9425 commented Mar 9, 2026

Uh oh!

archerwu9425 commented Mar 17, 2026

Uh oh!

nikParasyr commented Mar 17, 2026

Uh oh!

bnallapeta commented Mar 17, 2026

Uh oh!

bnallapeta Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

bnallapeta Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

bnallapeta Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Mar 25, 2026

Uh oh!

k8s-ci-robot commented Mar 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

netlify bot commented Mar 3, 2026 •

edited

Loading