🐛 Fix panic when OpenStackCluster.Status.Network is nil in HCP scenarios #2635

bnallapeta · 2025-08-03T05:21:58Z

What this PR does / why we need it:
Fixes a nil pointer panic in OpenStackMachineReconciler when OpenStackCluster.Status.Network is nil, which occurs in Hosted Control Plane scenarios. The controller now gracefully handles missing cluster network by checking for nil before access and returning a terminal error instead of panicking. Also adds comprehensive HCP E2E test suite.

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 #2380

TODOs:

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

/hold

netlify · 2025-08-03T05:22:04Z

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

Name	Link
🔨 Latest commit	`c07dc18`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-cluster-api-openstack/deploys/68a8830147009a0008d569b9
😎 Deploy Preview	https://deploy-preview-2635--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.

linux-foundation-easycla · 2025-08-03T05:22:05Z

The committers listed above are authorized under a signed CLA.

✅ login: bnallapeta / name: Bharath Nallapeta (02169f4, 70b4a79, c07dc18)

k8s-ci-robot · 2025-08-03T05:22:08Z

Hi @bnallapeta. 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. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

bnallapeta · 2025-08-03T05:30:14Z

@EmilienM @mdbooth @lentzi90

Marking it as Draft as we are working on the e2e tests. This is the approach being taken for e2e. Let me know your thoughts.

Spawn up a kind cluster -> turn it into a management cluster by installing CAPI/CAPO on it
Deploy a k8s cluster on OpenStack using this kind cluster. Let's call this Cluster A
Turn Cluster A into management cluster by installing CAPI/CAPO on it
Deploy a k8s cluster in the hosted control plane method using kamaji provider using cluster A. Let's call this Cluster B
Run the actual e2e tests to test out the panic case on Cluster B

Right now, we are somewhere in the 3rd/4th step and are facing the below challenges:

First, when we specify our Kamaji as a control plane provider in e2e_conf.yaml and name it as kamaji this error pops up:

[FAILED] The e2e test config file is not valid
Expected success, but got an error:
    <*errors.fundamental | 0xc000c00228>:
    invalid argument: invalid config: control-plane-provider should be named kubeadm

Second, if we try to bypass and name this kamaji as kubeadm, kamaji doesn't get installed as such and later on, we face issues with finding Kamaji CRDs

[FAILED] in [It] - /root/cluster-api-provider-openstack/test/e2e/suites/hcp/hcp_helpers.go:148 @ 07/31/25 11:01:25.32 < Exit [It] should create and manage HCP-capable cluster @ 07/31/25 11:01:25.32 (7m12.465s) << Timeline [FAILED] Timed out after 180.000s. Expected success, but got an error: <*apiutil.ErrResourceDiscoveryFailed | 0xc000123428>: unable to retrieve the complete list of server APIs: [kamaji.clastix.io/v1alpha1](http://kamaji.clastix.io/v1alpha1): no matches for [kamaji.clastix.io/v1alpha1](http://kamaji.clastix.io/v1alpha1), Resource= { { Group: "[kamaji.clastix.io](http://kamaji.clastix.io/)", Version: "v1alpha1", }: <*meta.NoResourceMatchError | 0xc0012143c0>{ PartialResource: { Group: "[kamaji.clastix.io](http://kamaji.clastix.io/)", Version: "v1alpha1", Resource: "", }, }, } In [It] at: /root/cluster-api-provider-openstack/test/e2e/suites/hcp/hcp_helpers.go:148 @ 07/31/25 11:01:25.32

Would really appreciate your help on this to move forward.

cc @orkhan-os

mdbooth

Appreciate this is just a draft, but I had a quick look over it anyway.

controllers/openstackmachine_controller.go

controllers/openstackmachine_controller_test.go

docs/book/src/development/development.md

pkg/cloud/services/networking/securitygroups.go

lentzi90 · 2025-08-18T05:18:32Z

/ok-to-test

mdbooth

Code change lgtm. Just some weirdness in the tests which needs clearing up.

controllers/openstackmachine_controller_test.go

lentzi90

Am I understanding correctly that this changes the behavior for security groups on OpenStackMachines? We cannot sneak in that change as a bug fix (🐛 ). If we want to change it (and I am not sure we do), the PR has to be marked as a breaking change (⚠️ ).

controllers/openstackmachine_controller_test.go

controllers/openstackmachine_controller.go

lentzi90

The code looks good to me know. Added some comments for the documentation

docs/book/src/topics/hosted-control-plane.md

bnallapeta · 2025-08-22T07:10:50Z

@lentzi90 addressed all the comments on docs. Just one conversation yet to be resolved.

Also, with this commit, I updated all references of tenant cluster to workload cluster to stay in touch with CAPI/CAPO nomenclature.

Signed-off-by: Bharath Nallapeta <[email protected]> addressed PR comments on secGroups Signed-off-by: Bharath Nallapeta <[email protected]>

Signed-off-by: Bharath Nallapeta <[email protected]>

Signed-off-by: Bharath Nallapeta <[email protected]> Signed-off-by: Bharath Nallapeta <[email protected]> Signed-off-by: Bharath Nallapeta <[email protected]>

lentzi90

I'm ok with this now. @mdbooth do you have time to take another look also?
/approve

k8s-ci-robot · 2025-08-27T06:35:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lentzi90

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~~ [lentzi90]

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

github-project-automation bot added this to CAPO Roadmap Aug 3, 2025

github-project-automation bot moved this to Inbox in CAPO Roadmap Aug 3, 2025

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

k8s-ci-robot requested a review from lentzi90 August 3, 2025 05:22

k8s-ci-robot requested a review from mdbooth August 3, 2025 05:22

bnallapeta marked this pull request as draft August 3, 2025 05:22

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 3, 2025

mdbooth reviewed Aug 4, 2025

View reviewed changes

bnallapeta force-pushed the hcp-2380 branch 2 times, most recently from 3321655 to 0ab4e3c Compare August 4, 2025 14:45

bnallapeta force-pushed the hcp-2380 branch from 0ab4e3c to 6d2c113 Compare August 14, 2025 06:19

bnallapeta force-pushed the hcp-2380 branch from 6d2c113 to 68f2229 Compare August 14, 2025 06:20

bnallapeta marked this pull request as ready for review August 14, 2025 06:23

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 14, 2025

k8s-ci-robot requested a review from EmilienM August 14, 2025 06:23

bnallapeta mentioned this pull request Aug 17, 2025

Panic in OpenStackMachineReconciler if OpenStackCluster.Status.Network is nil (Hosted Control Plane scenario) #2380

Open

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 Aug 18, 2025

mdbooth reviewed Aug 18, 2025

View reviewed changes

controllers/openstackmachine_controller_test.go Outdated Show resolved Hide resolved

controllers/openstackmachine_controller_test.go Show resolved Hide resolved

controllers/openstackmachine_controller_test.go Show resolved Hide resolved

bnallapeta force-pushed the hcp-2380 branch from 8ce68fd to a1274fa Compare August 19, 2025 05:24

lentzi90 reviewed Aug 21, 2025

View reviewed changes

controllers/openstackmachine_controller_test.go Outdated Show resolved Hide resolved

controllers/openstackmachine_controller.go Outdated Show resolved Hide resolved

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 Aug 21, 2025

lentzi90 reviewed Aug 22, 2025

View reviewed changes

bnallapeta added 2 commits August 22, 2025 17:57

handle nil cluster network & SG precedence, add TerminalError

02169f4

Signed-off-by: Bharath Nallapeta <[email protected]> addressed PR comments on secGroups Signed-off-by: Bharath Nallapeta <[email protected]>

unit tests: add cases for nil network and SG override

70b4a79

Signed-off-by: Bharath Nallapeta <[email protected]>

bnallapeta force-pushed the hcp-2380 branch 2 times, most recently from bd01438 to cefcbae Compare August 22, 2025 12:31

added guide to deploy a openstack cluster - hosted cp

c07dc18

Signed-off-by: Bharath Nallapeta <[email protected]> Signed-off-by: Bharath Nallapeta <[email protected]> Signed-off-by: Bharath Nallapeta <[email protected]>

bnallapeta force-pushed the hcp-2380 branch from cefcbae to c07dc18 Compare August 22, 2025 14:47

lentzi90 reviewed Aug 27, 2025

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 27, 2025

🐛 Fix panic when OpenStackCluster.Status.Network is nil in HCP scenarios #2635

Are you sure you want to change the base?

🐛 Fix panic when OpenStackCluster.Status.Network is nil in HCP scenarios #2635

Conversation

bnallapeta commented Aug 3, 2025

Uh oh!

netlify bot commented Aug 3, 2025 • 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!

linux-foundation-easycla bot commented Aug 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Aug 3, 2025

Uh oh!

bnallapeta commented Aug 3, 2025

Uh oh!

mdbooth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lentzi90 commented Aug 18, 2025

Uh oh!

mdbooth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lentzi90 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lentzi90 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bnallapeta commented Aug 22, 2025

Uh oh!

lentzi90 left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Aug 27, 2025

Uh oh!

Uh oh!

netlify bot commented Aug 3, 2025 •

edited

Loading

linux-foundation-easycla bot commented Aug 3, 2025 •

edited

Loading