🐛 controllers/openstackcluster_controller.go fix nil pointer and dump openstack ports #1217

chrischdi · 2022-04-25T12:52:29Z

What this PR does / why we need it:

fixes nil pointer when reconcileDelete gets executed a second time and security groups already got deleted

  I0425 01:06:29.568971       1 openstackcluster_controller.go:132] controller/openstackcluster "msg"="Reconciling Cluster delete" "cluster"="cluster-conformance-tn6axt" "name"="cluster-conformance-tn6axt" "namespace"="conformance-tn6axt" "reconciler group"="infrastructure.cluster.x-k8s.io" "reconciler kind"="OpenStackCluster" 
  panic: runtime error: invalid memory address or nil pointer dereference
  [signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x14e6d9a]

  goroutine 419 [running]:
  sigs.k8s.io/cluster-api-provider-openstack/controllers.bastionToInstanceSpec(0xc000069000, {0xc0006300c0, 0xc000069000})
  	/workspace/controllers/openstackcluster_controller.go:376 +0x25a
  sigs.k8s.io/cluster-api-provider-openstack/controllers.deleteBastion(0x1a8d7a0, 0xc0009e2000, 0xc000069000)
  	/workspace/controllers/openstackcluster_controller.go:226 +0x18b
  sigs.k8s.io/cluster-api-provider-openstack/controllers.reconcileDelete({0x1a6fe18, 0xc00083c030}, 0xc000dd0e80, 0xc00059dae0, 0xc0009e2000, 0xc000069000)
  	/workspace/controllers/openstackcluster_controller.go:134 +0xa5
  sigs.k8s.io/cluster-api-provider-openstack/controllers.(*OpenStackClusterReconciler).Reconcile(0xc0007f67b0, {0x1a6fe18, 0xc00083c030}, {{{0xc0007bc3c0, 0x1769dc0}, {0xc00093c3e0, 0x30}}})
  	/workspace/controllers/openstackcluster_controller.go:124 +0x72a
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile(0xc000690bb0, {0x1a6fe18, 0xc00083c000}, {{{0xc0007bc3c0, 0x1769dc0}, {0xc00093c3e0, 0x413c34}}})
  	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:114 +0x26f
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc000690bb0, {0x1a6fd70, 0xc0007fa080}, {0x16a7c00, 0xc00007e000})
  	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:311 +0x33e
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc000690bb0, {0x1a6fd70, 0xc0007fa080})
  	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266 +0x205
  sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2()
  	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227 +0x85
  created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2
  	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:223 +0x357

[0]

adds dump of existing ports at e2e tests so we could debug further the following if it happens again:

  failed to delete network: Expected HTTP response code [] when accessing [DELETE http://10.0.3.15:9696/v2.0/networks/d5988cfd-6b2b-456f-a174-7c7c42040c72], but got 409 instead
  {"NeutronError": {"type": "NetworkInUse", "message": "Unable to complete operation on network d5988cfd-6b2b-456f-a174-7c7c42040c72. There are one or more ports still in use on the network.", "detail": ""}}

[1]

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

Maybe fixes the at cleanup still broken e2e conformance periodics test: https://testgrid.k8s.io/sig-cluster-lifecycle-cluster-api-provider-openstack#periodic-conformance-test-main&show-stale-tests=

At fixes a potential nil pointer when using bastions and at least provides more information for debugging as I was not able to reproduce the issue locally.

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

[ x] squashed commits
if necessary:
- includes documentation
- adds unit tests

/hold

chrischdi · 2022-04-25T12:52:36Z

/test help

k8s-ci-robot · 2022-04-25T12:52:37Z

@chrischdi: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

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

The following commands are available to trigger optional jobs:

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

Use /test all to run the following jobs that were automatically triggered:

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

In response to this:

/test help

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/test-infra repository.

netlify · 2022-04-25T12:52:45Z

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

Name	Link
🔨 Latest commit	`b32a5da`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/62669d781221ba0009f30366
😎 Deploy Preview	https://deploy-preview-1217--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 site settings.

chrischdi · 2022-04-25T12:52:48Z

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

jichenjc · 2022-04-25T12:54:44Z

/lgtm

Thanks for catch this

test/e2e/shared/openstack.go

chrischdi · 2022-04-25T13:00:33Z

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

chrischdi · 2022-04-25T13:06:43Z

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

…ore openstack data * fixes nil pointer when reconcileDelete gets executed a second time and security groups already got deleted * adds dump of existing ports at e2e tests

chrischdi · 2022-04-25T13:09:25Z

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

apricote · 2022-04-25T13:13:47Z

/lgtm

tobiasgiese · 2022-04-25T13:17:22Z

Thanks!
/lgtm

k8s-ci-robot · 2022-04-25T13:26:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrischdi, mdbooth

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~~ [chrischdi,mdbooth]

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

mdbooth · 2022-04-25T13:26:57Z

/lgtm

chrischdi · 2022-04-25T14:43:13Z

/hold cancel

chrischdi · 2022-04-25T14:43:23Z

/hold

Want to check sth

chrischdi · 2022-04-25T14:44:20Z

Works

/hold cancel

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 25, 2022

k8s-ci-robot requested review from jichenjc and tobiasgiese April 25, 2022 12:52

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 25, 2022

k8s-ci-robot assigned jichenjc Apr 25, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2022

apricote reviewed Apr 25, 2022

View reviewed changes

test/e2e/shared/openstack.go Outdated Show resolved Hide resolved

chrischdi force-pushed the pr-fix-e2e-conformance-bastion-sec-nil branch from 3c82755 to c971815 Compare April 25, 2022 12:59

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 25, 2022

chrischdi force-pushed the pr-fix-e2e-conformance-bastion-sec-nil branch from c971815 to ac2d4e0 Compare April 25, 2022 13:03

controllers/openstackcluster_controller.go fix nil pointer and dump m…

b32a5da

…ore openstack data * fixes nil pointer when reconcileDelete gets executed a second time and security groups already got deleted * adds dump of existing ports at e2e tests

chrischdi force-pushed the pr-fix-e2e-conformance-bastion-sec-nil branch from ac2d4e0 to b32a5da Compare April 25, 2022 13:09

k8s-ci-robot assigned apricote Apr 25, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2022

k8s-ci-robot assigned tobiasgiese Apr 25, 2022

mdbooth approved these changes Apr 25, 2022

View reviewed changes

k8s-ci-robot assigned mdbooth Apr 25, 2022

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 25, 2022

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

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 25, 2022

k8s-ci-robot merged commit 06cb0c7 into kubernetes-sigs:main Apr 25, 2022

apricote mentioned this pull request Apr 26, 2022

🐛 always wait for active Loadbalancer after getOrCreate #1200

Merged

🐛 controllers/openstackcluster_controller.go fix nil pointer and dump openstack ports #1217

🐛 controllers/openstackcluster_controller.go fix nil pointer and dump openstack ports #1217

Uh oh!

Conversation

chrischdi commented Apr 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrischdi commented Apr 25, 2022

Uh oh!

k8s-ci-robot commented Apr 25, 2022

Uh oh!

netlify bot commented Apr 25, 2022 • 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!

chrischdi commented Apr 25, 2022

Uh oh!

jichenjc commented Apr 25, 2022

Uh oh!

Uh oh!

chrischdi commented Apr 25, 2022

Uh oh!

chrischdi commented Apr 25, 2022

Uh oh!

chrischdi commented Apr 25, 2022

Uh oh!

apricote commented Apr 25, 2022

Uh oh!

tobiasgiese commented Apr 25, 2022

Uh oh!

k8s-ci-robot commented Apr 25, 2022

Uh oh!

mdbooth commented Apr 25, 2022

Uh oh!

chrischdi commented Apr 25, 2022

Uh oh!

chrischdi commented Apr 25, 2022

Uh oh!

chrischdi commented Apr 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

chrischdi commented Apr 25, 2022 •

edited

Loading

netlify bot commented Apr 25, 2022 •

edited

Loading