Repository: kubernetes-sigs/cluster-api-provider-vsphere
Pull request #2413 (2023-10-04 08:55:12)

🌱 Removal of if block usage wrt logging in vsphere machine controller
**What this PR does / why we need it**:
Resolving open comments from #2395

**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 #


k8s-ci-robot: [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 [vincepri](https://github.com/vincepri) for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fcluster-api-provider-vsphere).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["vincepri"]} -->
Ankitasw: cc @killianmuldoon 
k8s-ci-robot: @Ankitasw: 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-vsphere-test-main | 186b0bf454836e805445218ee4073890d51e96d7 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cluster-api-provider-vsphere/2413/pull-cluster-api-provider-vsphere-test-main/1709492360339001344) | true | `/test pull-cluster-api-provider-vsphere-test-main`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=cluster-api-provider-vsphere&pr=2413). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3AAnkitasw). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

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/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
chrischdi: /retest

(flake at https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cluster-api-provider-vsphere/2413/pull-cluster-api-provider-vsphere-test-main/1709492360339001344 , will take a look at this separately)
Ankitasw: @chrischdi I think it's failing because of the changes in this PR, I will take a look locally.
k8s-ci-robot: @Ankitasw: 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-vsphere-test-main | 186b0bf454836e805445218ee4073890d51e96d7 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cluster-api-provider-vsphere/2413/pull-cluster-api-provider-vsphere-test-main/1709501751603761152) | true | `/test pull-cluster-api-provider-vsphere-test-main`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=cluster-api-provider-vsphere&pr=2413). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3AAnkitasw). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

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/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
chrischdi: > I think for simplicity we should probably keep the defer for the `POST` log. There's suggestions for what that should look like below.
> 
> One issue: I don't like the way we're not handling the error from `getVirtualMachinesInCluster` explicitly, but I'm not very comfortable changing this code as it's supervisor so maybe we should keep it as is? @chrischdi WDYT?
> 
> One safe change would be to have a different log if the result is nil, but that's more complexity for little enough gain IMO.

I think I agree, would be maybe a good thing to get input e.g. from @srm09 ?

Removing the `if` results in two additional `List` API calls to the vmoperator cluster. But I think we can consider them cheap, **only if these are cached objects**?! However, if the called function changes, we might not take that into account.
killianmuldoon: > only if these are cached objects?!

I hadn't considered that - I assume these objects are cached, but we should confirm
k8s-ci-robot: @Ankitasw: 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-vsphere-test-main | 0816e23e8fe4a5eeb854801f6c1670e09d485e0d | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cluster-api-provider-vsphere/2413/pull-cluster-api-provider-vsphere-test-main/1709585501595373568) | true | `/test pull-cluster-api-provider-vsphere-test-main`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=cluster-api-provider-vsphere&pr=2413). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3AAnkitasw). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

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/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
chrischdi: Oh wow, I took a look and we actually found a real nil pointer case :-) So the tests are okay, `getVirtualMachinesInCluster` is not okay when `supervisorMachineCtx.Cluster` is nil. The test case explicitly says:

```
Create and Delete a VSphereMachine with a Machine but without a Cluster
```

https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/0816e23e8fe4a5eeb854801f6c1670e09d485e0d/controllers/vmware/test/controllers_test.go#L477 

And nil for the `cluster` inside the context is an valid case for a machine which gets deleted.

I think we should add at the very top of `getVirtualMachinesInCluster`: (+ improving the error text)

```go
if supervisorMachineCtx.Cluster == nil {
  return 0, errors.New("No cluster set for machine")
}
```
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs) Report
Attention: `37 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`43bf20b`)](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/commit/43bf20be0df2a2fd626518da4561dc1e5d652a08?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs) 61.07% compared to head [(`f9027f6`)](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs) 61.36%.
> Report is 32 commits behind head on main.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #2413      +/-   ##
==========================================
+ Coverage   61.07%   61.36%   +0.29%     
==========================================
  Files         163      163              
  Lines        9435     9436       +1     
==========================================
+ Hits         5762     5790      +28     
+ Misses       3261     3237      -24     
+ Partials      412      409       -3     
```


| [Files](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs) | Coverage Δ | |
|---|---|---|
| [controllers/vmware/vspherecluster\_reconciler.go](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs#diff-Y29udHJvbGxlcnMvdm13YXJlL3ZzcGhlcmVjbHVzdGVyX3JlY29uY2lsZXIuZ28=) | `72.93% <100.00%> (ø)` | |
| [controllers/vspherecluster\_reconciler.go](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs#diff-Y29udHJvbGxlcnMvdnNwaGVyZWNsdXN0ZXJfcmVjb25jaWxlci5nbw==) | `61.77% <ø> (-0.42%)` | :arrow_down: |
| [pkg/context/fake/fake\_machine\_context.go](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs#diff-cGtnL2NvbnRleHQvZmFrZS9mYWtlX21hY2hpbmVfY29udGV4dC5nbw==) | `94.11% <ø> (ø)` | |
| [pkg/context/machine\_context.go](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs#diff-cGtnL2NvbnRleHQvbWFjaGluZV9jb250ZXh0Lmdv) | `85.71% <100.00%> (ø)` | |
| [pkg/context/vmware/machine\_context.go](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs#diff-cGtnL2NvbnRleHQvdm13YXJlL21hY2hpbmVfY29udGV4dC5nbw==) | `100.00% <100.00%> (ø)` | |
| [pkg/context/vspheredeploymentzone\_context.go](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs#diff-cGtnL2NvbnRleHQvdnNwaGVyZWRlcGxveW1lbnR6b25lX2NvbnRleHQuZ28=) | `84.61% <ø> (ø)` | |
| [pkg/services/govmomi/service.go](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs#diff-cGtnL3NlcnZpY2VzL2dvdm1vbWkvc2VydmljZS5nbw==) | `1.23% <ø> (ø)` | |
| [pkg/services/govmomi/util.go](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs#diff-cGtnL3NlcnZpY2VzL2dvdm1vbWkvdXRpbC5nbw==) | `9.24% <ø> (ø)` | |
| [pkg/services/vmoperator/resource\_policy.go](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs#diff-cGtnL3NlcnZpY2VzL3Ztb3BlcmF0b3IvcmVzb3VyY2VfcG9saWN5Lmdv) | `80.30% <100.00%> (+9.09%)` | :arrow_up: |
| [pkg/taggable/get.go](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs#diff-cGtnL3RhZ2dhYmxlL2dldC5nbw==) | `64.70% <100.00%> (ø)` | |
| ... and [9 more](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs) | |

... and [3 files with indirect coverage changes](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/kubernetes-sigs/cluster-api-provider-vsphere/pull/2413?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kubernetes-sigs).


Patch:
diff --git a/OWNERS_ALIASES b/OWNERS_ALIASES
index 190ae44c..0f4ed22c 100644
--- a/OWNERS_ALIASES
+++ b/OWNERS_ALIASES
@@ -19,9 +19,6 @@ aliases:
     - randomvariable
     - srm09
     - yastij
-    - fabriziopandini
-    - killianmuldoon
-    - sbueringer
     - gab-satchi
     - chrischdi
   cluster-api-vsphere-reviewers:
diff --git a/hack/build-dev.sh b/hack/build-dev.sh
new file mode 100755
index 00000000..3aaa85eb
--- /dev/null
+++ b/hack/build-dev.sh
@@ -0,0 +1,79 @@
+#!/bin/bash
+
+# Copyright 2019 The Kubernetes Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+#
+# TODO(akutz) This script can probably be removed once v1alpha2 is released.
+#
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+# Change directories to the parent directory of the one in which this
+# script is located.
+cd "$(dirname "${BASH_SOURCE[0]}")/.."
+
+export CGO_ENABLED=0
+export GOFLAGS="-ldflags=-extldflags=-static"
+export GOPROXY="${GOPROXY:-https://proxy.golang.org}"
+
+CLUSTERCTL_OUT="${CLUSTERCTL_OUT:-$(pwd)/clusterctl}"
+CAPV_MANAGER_IMAGE="${CAPV_MANAGER_IMAGE:-gcr.io/cluster-api-provider-vsphere/dev/v1alpha3/capv-manager:latest}"
+CAPI_MANAGER_IMAGE="${CAPI_MANAGER_IMAGE:-gcr.io/cluster-api-provider-vsphere/dev/v1alpha3/capi-manager:latest}"
+CABPK_MANAGER_IMAGE="${CABPK_MANAGER_IMAGE:-gcr.io/cluster-api-provider-vsphere/dev/v1alpha3/cabpk-manager:latest}"
+
+# Build the CAPV manager image.
+docker build -t "${CAPV_MANAGER_IMAGE}" .
+docker push "${CAPV_MANAGER_IMAGE}"
+
+# Create a temporary directory into which the CAPI and CABPK repos can
+# be cloned.
+cd "$(mktemp -d)"
+
+# Clone the CAPI and CABPK repositories.
+git clone "${CAPI_REPO:-https://github.com/kubernetes-sigs/cluster-api.git}" capi
+git clone "${CABPK_REPO:-https://github.com/kubernetes-sigs/cluster-api-bootstrap-provider-kubeadm}" cabpk
+
+# Switch to the CAPI repo.
+pushd capi
+
+# Checkout the CAPI ref if one is set.
+[ -n "${CAPI_REF-}" ] && git checkout -b "${CAPI_REF}" "${CAPI_REF}"
+
+# Build clusterctl
+go build -o "${CLUSTERCTL_OUT}" ./cmd/clusterctl
+
+# Build the CAPI manager image.
+docker build -t "${CAPI_MANAGER_IMAGE}" .
+docker push "${CAPI_MANAGER_IMAGE}"
+
+# Switch to the CABPK repo.
+popd && pushd cabpk
+
+# Checkout the CABPK ref if one is set.
+[ -n "${CABPK_REF-}" ] && git checkout -b "${CABPK_REF}" "${CABPK_REF}"
+
+# Build the CABPK manager image.
+docker build -t "${CABPK_MANAGER_IMAGE}" .
+docker push "${CABPK_MANAGER_IMAGE}"
+
+cat <<EOF
+
+clusterctl     ${CLUSTERCTL_OUT}
+capi_manager   ${CAPI_MANAGER_IMAGE}
+cabpk_manager  ${CABPK_MANAGER_IMAGE}
+capv_manager   ${CAPV_MANAGER_IMAGE}
+EOF
diff --git a/pkg/services/govmomi/ipam/parse.go b/pkg/services/govmomi/ipam/parse.go
index bef844c9..74d01f70 100644
--- a/pkg/services/govmomi/ipam/parse.go
+++ b/pkg/services/govmomi/ipam/parse.go
@@ -51,10 +51,10 @@ func parseAddressWithPrefix(ipamAddress *ipamv1.IPAddress) (netip.Prefix, error)
 // does not conflict with the gateway addresses parsed from other
 // ipamv1.IPAddresses on the current device. Gateway addresses must be the same
 // family as the address on the ipamv1.IPAddress. Gateway addresses of one
-// family must match the other addresses of the same family. A gateway address
-// is optional. If it is not set this function returns `nil, nil`.
+// family must match the other addresses of the same family. IPv4 Gateways are
+// required, but IPv6 gateways are not.
 func parseGateway(ipamAddress *ipamv1.IPAddress, addressWithPrefix netip.Prefix, ipamDeviceConfig ipamDeviceConfig) (*netip.Addr, error) {
-	if ipamAddress.Spec.Gateway == "" {
+	if ipamAddress.Spec.Gateway == "" && addressWithPrefix.Addr().Is6() {
 		return nil, nil
 	}
 
diff --git a/pkg/services/vmoperator/vmopmachine.go b/pkg/services/vmoperator/vmopmachine.go
index 293ae142..0fe424cf 100644
--- a/pkg/services/vmoperator/vmopmachine.go
+++ b/pkg/services/vmoperator/vmopmachine.go
@@ -80,19 +80,17 @@ func (v *VmopMachineService) ReconcileDelete(ctx context.Context, machineCtx cap
 	}
 	log.V(2).Info("Destroying VM")
 
-	// If debug logging is enabled, report the number of vms in the cluster before and after the reconcile
-	if log.V(5).Enabled() {
+	vms, err := v.getVirtualMachinesInCluster(ctx, supervisorMachineCtx)
+	log.V(5).Info("Trace Destroy PRE: VirtualMachines", "vmcount", len(vms), "error", err)
+	defer func() {
 		vms, err := v.getVirtualMachinesInCluster(ctx, supervisorMachineCtx)
-		log.Info("Trace Destroy PRE: VirtualMachines", "vmcount", len(vms), "error", err)
-		defer func() {
-			vms, err := v.getVirtualMachinesInCluster(ctx, supervisorMachineCtx)
-			log.Info("Trace Destroy POST: VirtualMachines", "vmcount", len(vms), "error", err)
-		}()
-	}
+		log.V(5).Info("Trace Destroy POST: VirtualMachines", "vmcount", len(vms), "error", err)
+	}()
 
 	// First, check to see if it's already deleted
 	vmopVM := vmoprv1.VirtualMachine{}
 	if err := v.Client.Get(ctx, types.NamespacedName{Namespace: supervisorMachineCtx.Machine.Namespace, Name: supervisorMachineCtx.Machine.Name}, &vmopVM); err != nil {
+		// If debug logging is enabled, report the number of vms in the cluster before and after the reconcile
 		if apierrors.IsNotFound(err) {
 			supervisorMachineCtx.VSphereMachine.Status.VMStatus = vmwarev1.VirtualMachineStateNotFound
 			return err
@@ -116,7 +114,6 @@ func (v *VmopMachineService) ReconcileDelete(ctx context.Context, machineCtx cap
 		supervisorMachineCtx.VSphereMachine.Status.VMStatus = vmwarev1.VirtualMachineStateError
 		return err
 	}
-
 	supervisorMachineCtx.VSphereMachine.Status.VMStatus = vmwarev1.VirtualMachineStateDeleting
 	return nil
 }
@@ -143,14 +140,12 @@ func (v *VmopMachineService) ReconcileNormal(ctx context.Context, machineCtx cap
 	log.V(2).Info("Reconciling VM")
 
 	// If debug logging is enabled, report the number of vms in the cluster before and after the reconcile
-	if log.V(5).Enabled() {
-		vms, err := v.getVirtualMachinesInCluster(ctx, supervisorMachineCtx)
-		log.Info("Trace ReconcileVM PRE: VirtualMachines", "vmcount", len(vms), "error", err)
-		defer func() {
-			vms, err := v.getVirtualMachinesInCluster(ctx, supervisorMachineCtx)
-			log.Info("Trace ReconcileVM POST: VirtualMachines", "vmcount", len(vms), "error", err)
-		}()
-	}
+	vms, err := v.getVirtualMachinesInCluster(ctx, supervisorMachineCtx)
+	log.V(5).Info("Trace ReconcileVM PRE: VirtualMachines", "vmcount", len(vms), "error", err)
+	defer func() {
+		vms, err = v.getVirtualMachinesInCluster(ctx, supervisorMachineCtx)
+		log.V(5).Info("Trace ReconcileVM POST: VirtualMachines", "vmcount", len(vms), "error", err)
+	}()
 
 	// Set the VM state. Will get reset throughout the reconcile
 	supervisorMachineCtx.VSphereMachine.Status.VMStatus = vmwarev1.VirtualMachineStatePending
@@ -373,6 +368,9 @@ func (v *VmopMachineService) reconcileProviderID(ctx context.Context, supervisor
 // getVirtualMachinesInCluster returns all VMOperator VirtualMachine objects in the current cluster.
 // First filter by clusterSelectorKey. If the result is empty, they fall back to legacyClusterSelectorKey.
 func (v *VmopMachineService) getVirtualMachinesInCluster(ctx context.Context, supervisorMachineCtx *vmware.SupervisorMachineContext) ([]*vmoprv1.VirtualMachine, error) {
+	if supervisorMachineCtx.Cluster == nil {
+		return []*vmoprv1.VirtualMachine{}, errors.Errorf("No cluster is set for machine %s in namespace %s", supervisorMachineCtx.GetVSphereMachine().GetName(), supervisorMachineCtx.GetVSphereMachine().GetNamespace())
+	}
 	labels := map[string]string{clusterSelectorKey: supervisorMachineCtx.Cluster.Name}
 	vmList := &vmoprv1.VirtualMachineList{}
 


Test patch:
diff --git a/pkg/services/govmomi/ipam/status_test.go b/pkg/services/govmomi/ipam/status_test.go
index ebac0483..a37d4c37 100644
--- a/pkg/services/govmomi/ipam/status_test.go
+++ b/pkg/services/govmomi/ipam/status_test.go
@@ -723,7 +723,8 @@ func Test_BuildState(t *testing.T) {
 			g.Expect(ctx.Client.Update(ctx, address1)).NotTo(gomega.HaveOccurred())
 
 			_, err := BuildState(ctx, networkStatus)
-			g.Expect(err).NotTo(gomega.HaveOccurred())
+			g.Expect(err).To(gomega.HaveOccurred())
+			g.Expect(err).To(gomega.MatchError("IPAddress my-namespace/vsphereVM1-0-0 has invalid gateway: \"\""))
 		})
 
 		t.Run("when a provider assigns an IPv6 IPAddress without a Gateway field", func(_ *testing.T) {