Skip to content

Commit

Permalink
Fix potential panics when retrieving VSphereCluster from Cluster.spec…
Browse files Browse the repository at this point in the history
….infrastructureRef

Signed-off-by: Stefan Büringer [email protected]
  • Loading branch information
sbueringer committed Nov 14, 2023
1 parent ea2041d commit e05f383
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 14 deletions.
10 changes: 9 additions & 1 deletion controllers/vspherecluster_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ func (r *clusterReconciler) controlPlaneMachineToCluster(ctx context.Context, o
log.Error(err, "VSphereMachine is missing cluster label or cluster does not exist")
return nil
}
log = log.WithValues("Cluster", klog.KObj(cluster), "VSphereCluster", klog.KRef(cluster.Namespace, cluster.Spec.InfrastructureRef.Name))
log = log.WithValues("Cluster", klog.KObj(cluster))

Check warning on line 529 in controllers/vspherecluster_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/vspherecluster_reconciler.go#L529

Added line #L529 was not covered by tests
ctx = ctrl.LoggerInto(ctx, log)

if conditions.IsTrue(cluster, clusterv1.ControlPlaneInitializedCondition) {
Expand All @@ -537,6 +537,14 @@ func (r *clusterReconciler) controlPlaneMachineToCluster(ctx context.Context, o
return nil
}

if cluster.Spec.InfrastructureRef == nil {
log.Error(err, "Failed to get VSphereCluster: Cluster.spec.infrastructureRef is not yet set")
return nil
}

Check warning on line 543 in controllers/vspherecluster_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/vspherecluster_reconciler.go#L540-L543

Added lines #L540 - L543 were not covered by tests

log = log.WithValues("VSphereCluster", klog.KRef(cluster.Namespace, cluster.Spec.InfrastructureRef.Name))
ctx = ctrl.LoggerInto(ctx, log)

Check warning on line 547 in controllers/vspherecluster_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/vspherecluster_reconciler.go#L545-L547

Added lines #L545 - L547 were not covered by tests
// Fetch the VSphereCluster
vsphereCluster := &infrav1.VSphereCluster{}
vsphereClusterKey := client.ObjectKey{
Expand Down
28 changes: 16 additions & 12 deletions controllers/vspheremachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,25 +239,29 @@ func (r *machineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_

// Checking whether cluster is nil here as we still want to allow delete even if cluster is not found.
if cluster == nil {
log.Info("Cluster could not be fetched")
return reconcile.Result{}, nil
}

if cluster.Spec.InfrastructureRef == nil {
log.Info("Cluster.spec.infrastructureRef is not yet set")
return reconcile.Result{RequeueAfter: 10 * time.Second}, nil
}

Check warning on line 249 in controllers/vspheremachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/vspheremachine_controller.go#L247-L249

Added lines #L247 - L249 were not covered by tests

// If the VSphereCluster has been previously set as an ownerReference remove it. This may have been set in an older
// version of CAPV to prevent VSphereMachines from being orphaned, but is no longer needed.
// For more info see: https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/issues/2054
// TODO: This should be removed in a future release of CAPV.
if cluster.Spec.InfrastructureRef != nil {
machineContext.GetVSphereMachine().SetOwnerReferences(
clusterutilv1.RemoveOwnerRef(
machineContext.GetVSphereMachine().GetOwnerReferences(),
metav1.OwnerReference{
Name: cluster.Spec.InfrastructureRef.Name,
APIVersion: cluster.Spec.InfrastructureRef.APIVersion,
Kind: cluster.Spec.InfrastructureRef.Kind,
},
),
)
}
machineContext.GetVSphereMachine().SetOwnerReferences(
clusterutilv1.RemoveOwnerRef(
machineContext.GetVSphereMachine().GetOwnerReferences(),
metav1.OwnerReference{
Name: cluster.Spec.InfrastructureRef.Name,
APIVersion: cluster.Spec.InfrastructureRef.APIVersion,
Kind: cluster.Spec.InfrastructureRef.Kind,
},
),
)

// Fetch the VSphereCluster and update the machine context
machineContext, err = r.VMService.FetchVSphereCluster(ctx, cluster, machineContext)
Expand Down
2 changes: 1 addition & 1 deletion controllers/vspherevm_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ func (r vmReconciler) retrieveVcenterSession(ctx context.Context, vsphereVM *inf
}

if cluster.Spec.InfrastructureRef == nil {
return nil, errors.Errorf("cannot retrieve vCenter session for cluster %s: .spec.infrastructureRef is nil", klog.KObj(cluster))
return nil, errors.Errorf("cannot retrieve vCenter session for cluster %s: Cluster.spec.infrastructureRef is nil", klog.KObj(cluster))
}
key := ctrlclient.ObjectKey{
Namespace: cluster.Namespace,
Expand Down
9 changes: 9 additions & 0 deletions pkg/util/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ func GetVSphereClusterFromVMwareMachine(ctx context.Context, c client.Client, ma
return nil, err
}

if cluster.Spec.InfrastructureRef == nil {
return nil, errors.Errorf("error getting VSphereCluster name from VSphereMachine %s/%s: Cluster.spec.infrastructureRef not yet set",
machine.Namespace, machine.Name)
}

Check warning on line 51 in pkg/util/cluster.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/cluster.go#L49-L51

Added lines #L49 - L51 were not covered by tests

vsphereClusterKey := apitypes.NamespacedName{
Namespace: machine.Namespace,
Name: cluster.Spec.InfrastructureRef.Name,
Expand All @@ -70,6 +75,10 @@ func GetVSphereClusterFromVSphereMachine(ctx context.Context, c client.Client, m
return nil, err
}

if cluster.Spec.InfrastructureRef == nil {
return nil, errors.Errorf("error getting VSphereCluster name from VSphereMachine %s/%s: Cluster.spec.infrastructureRef not yet set",
machine.Namespace, machine.Name)
}

Check warning on line 81 in pkg/util/cluster.go

View check run for this annotation

Codecov / codecov/patch

pkg/util/cluster.go#L79-L81

Added lines #L79 - L81 were not covered by tests
vsphereClusterKey := apitypes.NamespacedName{
Namespace: machine.Namespace,
Name: cluster.Spec.InfrastructureRef.Name,
Expand Down

0 comments on commit e05f383

Please sign in to comment.