Skip to content

Commit 1ad6def

Browse files
committed
Disallow attach with the old CnsNodeVMAttachment CRD after upgrade to 9.1
1 parent 950117d commit 1ad6def

File tree

2 files changed

+92
-0
lines changed

2 files changed

+92
-0
lines changed

pkg/syncer/cnsoperator/controller/cnsnodevmattachment/cnsnodevmattachment_controller.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ import (
5050
csifault "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/fault"
5151
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/prometheus"
5252
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/utils"
53+
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common"
54+
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common/commonco"
5355
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/logger"
5456
k8s "sigs.k8s.io/vsphere-csi-driver/v3/pkg/kubernetes"
5557
cnsoptypes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/syncer/cnsoperator/types"
@@ -69,6 +71,7 @@ const (
6971
var (
7072
backOffDuration map[k8stypes.NamespacedName]time.Duration
7173
backOffDurationMapMutex = sync.Mutex{}
74+
isSharedDiskEnabled bool
7275
)
7376

7477
// Mockable function variables for testing
@@ -135,6 +138,9 @@ func Add(mgr manager.Manager, clusterFlavor cnstypes.CnsClusterFlavor,
135138
return err
136139
}
137140

141+
// If the capability gets enabled at a later point, then container will be restarted and this value will be reinitialized.
142+
isSharedDiskEnabled = commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.SharedDiskFss)
143+
138144
recorder := eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: cnsoperatorapis.GroupName})
139145
return add(mgr, newReconciler(mgr, configInfo, volumeManager, vmOperatorClient, recorder))
140146
}
@@ -347,6 +353,17 @@ func (r *ReconcileCnsNodeVMAttachment) Reconcile(ctx context.Context,
347353
}
348354
nodeUUID := instance.Spec.NodeUUID
349355
if !instance.Status.Attached && instance.DeletionTimestamp == nil {
356+
357+
if isSharedDiskEnabled {
358+
// If isSharedDiskEnabled is enabled, then all new attach operations should happen via the
359+
// new CnsNodeVMBatchAttachment CRD.
360+
err := r.updateErrorOnInstanceToDisallowAttach(ctx, instance)
361+
if err != nil {
362+
return reconcile.Result{RequeueAfter: timeout}, csifault.CSIInternalFault, nil
363+
}
364+
return reconcile.Result{}, "", nil
365+
}
366+
350367
nodeVM, err := getVMByUUIDFromVCenter(internalCtx, dc, nodeUUID)
351368
if err != nil {
352369
msg := fmt.Sprintf("failed to find the VM with UUID: %q for CnsNodeVmAttachment "+
@@ -817,6 +834,24 @@ func isVmCrPresent(ctx context.Context, vmOperatorClient client.Client,
817834
return nil, nil
818835
}
819836

837+
// updateErrorOnInstanceToDisallowAttach sets error on the CnsNodeVMAttachment CR so that devops user can detach this volume
838+
// from the VM and re-attach it so that the new Batch Attach flow is triggered.
839+
func (r *ReconcileCnsNodeVMAttachment) updateErrorOnInstanceToDisallowAttach(ctx context.Context,
840+
instance *v1a1.CnsNodeVmAttachment) error {
841+
log := logger.GetLogger(ctx)
842+
843+
log.Infof("Attach should happen via the new CnsNodeVMBatchAttachment CRD. Skipping attach.")
844+
msg := "CnsNodeVMAttachment CR is deprecated. Please detach this PVC from the VM and then reattach it."
845+
instance.Status.Error = msg
846+
err := k8s.UpdateStatus(ctx, r.client, instance)
847+
if err != nil {
848+
log.Errorf("updateCnsNodeVMAttachment failed. err: %v", err)
849+
return err
850+
}
851+
recordEvent(ctx, r, instance, v1.EventTypeWarning, msg)
852+
return nil
853+
}
854+
820855
// getVCDatacenterFromConfig returns datacenter registered for each vCenter
821856
func getVCDatacentersFromConfig(cfg *config.Config) (map[string][]string, error) {
822857
var err error

pkg/syncer/cnsoperator/controller/cnsnodevmattachment/cnsnodevmattachment_controller_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,3 +329,60 @@ func TestReconcileDetachWithVolumeIDFallbackFailure(t *testing.T) {
329329

330330
t.Logf("✓ Reconcile correctly handled getVolumeID failure and requeued for retry")
331331
}
332+
333+
func TestUpdateErrorOnInstanceToDisallowAttach(t *testing.T) {
334+
ctx := context.Background()
335+
336+
// Set up the scheme
337+
SchemeGroupVersion := schema.GroupVersion{
338+
Group: "cns.vmware.com",
339+
Version: "v1alpha1",
340+
}
341+
s := scheme.Scheme
342+
s.AddKnownTypes(SchemeGroupVersion, &v1a1.CnsNodeVmAttachment{})
343+
metav1.AddToGroupVersion(s, SchemeGroupVersion)
344+
345+
// Create CnsNodeVmAttachment instance
346+
instance := &v1a1.CnsNodeVmAttachment{
347+
ObjectMeta: metav1.ObjectMeta{
348+
Name: "test-attachment",
349+
Namespace: "test-ns",
350+
},
351+
}
352+
353+
// Create fake client with status subresource
354+
fakeClient := fake.NewClientBuilder().
355+
WithScheme(s).
356+
WithStatusSubresource(instance).
357+
WithRuntimeObjects(instance).
358+
Build()
359+
360+
// Create reconciler
361+
r := &ReconcileCnsNodeVMAttachment{
362+
client: fakeClient,
363+
recorder: record.NewFakeRecorder(10),
364+
scheme: s,
365+
}
366+
367+
backOffDuration = make(map[k8stypes.NamespacedName]time.Duration)
368+
369+
// Call the function
370+
err := r.updateErrorOnInstanceToDisallowAttach(ctx, instance)
371+
372+
// Assertions
373+
assert.NoError(t, err, "Function should not return an error")
374+
assert.Equal(t,
375+
"CnsNodeVMAttachment CR is deprecated. Please detach this PVC from the VM and then reattach it.",
376+
instance.Status.Error,
377+
"Status.Error should be updated with the deprecation message",
378+
)
379+
380+
// Verify that the status was persisted by fetching the object from the fake client
381+
fetched := &v1a1.CnsNodeVmAttachment{}
382+
err = fakeClient.Get(ctx, k8stypes.NamespacedName{
383+
Name: "test-attachment",
384+
Namespace: "test-ns",
385+
}, fetched)
386+
assert.NoError(t, err, "Should be able to fetch the instance from fake client")
387+
assert.Equal(t, instance.Status.Error, fetched.Status.Error, "Status should be persisted in fake client")
388+
}

0 commit comments

Comments
 (0)