feat: fetch VMs to recover AzVolumeAttachments with replica role and …

…better recover AzVolumeAttachment.Status

pkg/azureconstants/azure_constants.go

@@ -131,7 +131,7 @@ const (
 	PodNameKey                         = "disk.csi/azure.com/pod-name"
 	PreProvisionedVolumeAnnotation     = "disk.csi.azure.com/pre-provisioned"
 	RequestIDKey                       = "disk.csi.azure.com/request-id"
-	RequestStartimeKey                 = "disk.csi.azure.com/request-starttime"
+	RequestStartTimeKey                = "disk.csi.azure.com/request-starttime"
 	RequestTimeFormat                  = time.RFC3339Nano
 	RequesterKey                       = "disk.csi.azure.com/requester-name"
 	WorkflowKey                        = "disk.csi.azure.com/requester-name"

pkg/controller/attach_detach_test.go

@@ -21,17 +21,21 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-03-01/compute"
 	"github.com/golang/mock/gomock"
 	"github.com/pborman/uuid"
 	"github.com/stretchr/testify/require"
 	apiErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 	fakev1 "k8s.io/client-go/kubernetes/fake"
 	"k8s.io/klog/v2/klogr"
+	"sigs.k8s.io/azuredisk-csi-driver/pkg/apis/azuredisk/v1beta2"
 	azdiskv1beta2 "sigs.k8s.io/azuredisk-csi-driver/pkg/apis/azuredisk/v1beta2"
 	azdiskfakes "sigs.k8s.io/azuredisk-csi-driver/pkg/apis/client/clientset/versioned/fake"
+	"sigs.k8s.io/azuredisk-csi-driver/pkg/azureconstants"
 	consts "sigs.k8s.io/azuredisk-csi-driver/pkg/azureconstants"
 	"sigs.k8s.io/azuredisk-csi-driver/pkg/azureutils"
 	"sigs.k8s.io/azuredisk-csi-driver/pkg/azureutils/mockclient"
@@ -50,6 +54,7 @@ func NewTestAttachDetachController(controller *gomock.Controller, namespace stri
 		retryInfo:         newRetryInfo(),
 		SharedState:       controllerSharedState,
 		logger:            klogr.New(),
+		volumeInfos:       make(map[string]*volumeInfo),
 	}
 }
 
@@ -66,12 +71,42 @@ func mockClientsAndAttachmentProvisioner(controller *ReconcileAttachDetach) {
 			return attachResult
 		}).
 		MaxTimes(1)
+
 	controller.cloudDiskAttacher.(*mockattachmentprovisioner.MockCloudDiskAttachDetacher).EXPECT().
 		UnpublishVolume(gomock.Any(), testManagedDiskURI0, gomock.Any()).
 		DoAndReturn(func(ctx context.Context, volumeID, nodeID string) error {
 			return nil
 		}).
 		MaxTimes(1)
+
+	controller.cloudDiskAttacher.(*mockattachmentprovisioner.MockCloudDiskAttachDetacher).EXPECT().
+		GetNodeDataDisks(gomock.Any()).
+		DoAndReturn(func(nodeName types.NodeName) ([]compute.DataDisk, *string, error) {
+			var disk0 = compute.DataDisk{Name: &testPersistentVolume0.Name}
+			var disks = []compute.DataDisk{0: disk0}
+			return disks, nil, nil
+		}).AnyTimes()
+
+	controller.cloudDiskAttacher.(*mockattachmentprovisioner.MockCloudDiskAttachDetacher).EXPECT().
+		GetDiskLun(gomock.Any(), gomock.Any(), gomock.Any()).
+		DoAndReturn(func(diskName, diskURI string, nodeName types.NodeName) (int32, *string, error) {
+			return int32(1), nil, nil
+		}).AnyTimes()
+
+	controller.cloudDiskAttacher.(*mockattachmentprovisioner.MockCloudDiskAttachDetacher).EXPECT().
+		CheckDiskExists(gomock.Any(), getTestDiskURI(testPersistentVolume2Name)).
+		DoAndReturn(func(ctx context.Context, diskURI string) (*compute.Disk, error) {
+			return nil, nil
+		}).MaxTimes(1)
+
+	controller.cloudDiskAttacher.(*mockattachmentprovisioner.MockCloudDiskAttachDetacher).EXPECT().
+		CheckDiskExists(gomock.Any(), gomock.Any()).
+		DoAndReturn(func(ctx context.Context, diskURI string) (*compute.Disk, error) {
+			return &compute.Disk{
+				ID:                &testPersistentVolume0Name,
+				ManagedByExtended: &[]string{testNode0Name, testNode1Name},
+			}, nil
+		}).AnyTimes()
 }
 
 func TestAttachDetachReconcile(t *testing.T) {
@@ -329,6 +364,7 @@ func TestAttachDetachRecover(t *testing.T) {
 					mockCtl,
 					testNamespace,
 					&testVolumeAttachment,
+					&testNode1,
 					&testPersistentVolume0)
 
 				mockClientsAndAttachmentProvisioner(controller)
@@ -340,7 +376,48 @@ func TestAttachDetachRecover(t *testing.T) {
 
 				azVolumeAttachments, localErr := controller.azClient.DiskV1beta2().AzVolumeAttachments(testNamespace).List(context.TODO(), metav1.ListOptions{})
 				require.NoError(t, localErr)
-				require.Len(t, azVolumeAttachments.Items, 1)
+				require.Len(t, azVolumeAttachments.Items, 2)
+
+				azVolumeAttachment := azVolumeAttachments.Items[1]
+				require.NotNil(t, azVolumeAttachment.Status.Detail)
+				require.NotNil(t, azVolumeAttachment.Status.Detail.PublishContext)
+				require.Equal(t, azVolumeAttachment.Status.Detail.PublishContext[azureconstants.LUN], "1")
+				require.NotNil(t, azVolumeAttachment.Status.Detail.Role)
+				require.Equal(t, azVolumeAttachment.Status.Detail.Role, v1beta2.Role("Replica"))
+				require.NotNil(t, azVolumeAttachment.Status.State)
+				require.Equal(t, azVolumeAttachment.Status.State, v1beta2.AzVolumeAttachmentAttachmentState("Attached"))
+			},
+		},
+		{
+			description: "[Success] Should create AzVolumeAttachment instances with Replica role using Azure Disk CSI Driver.",
+			setupFunc: func(t *testing.T, mockCtl *gomock.Controller) *ReconcileAttachDetach {
+
+				controller := NewTestAttachDetachController(
+					mockCtl,
+					testNamespace,
+					&testVolumeAttachment,
+					&testNode1,
+					&testPersistentVolume0)
+
+				mockClientsAndAttachmentProvisioner(controller)
+
+				return controller
+			},
+			verifyFunc: func(t *testing.T, controller *ReconcileAttachDetach, err error) {
+				require.NoError(t, err)
+
+				azVolumeAttachments, localErr := controller.azClient.DiskV1beta2().AzVolumeAttachments(testNamespace).List(context.TODO(), metav1.ListOptions{})
+				require.NoError(t, localErr)
+				require.Len(t, azVolumeAttachments.Items, 2)
+
+				azVolumeAttachment := azVolumeAttachments.Items[1]
+				require.NotNil(t, azVolumeAttachment.Status.Detail)
+				require.NotNil(t, azVolumeAttachment.Status.Detail.PublishContext)
+				require.Equal(t, azVolumeAttachment.Status.Detail.PublishContext[azureconstants.LUN], "1")
+				require.NotNil(t, azVolumeAttachment.Status.Detail.Role)
+				require.Equal(t, azVolumeAttachment.Status.Detail.Role, v1beta2.Role("Replica"))
+				require.NotNil(t, azVolumeAttachment.Status.State)
+				require.Equal(t, azVolumeAttachment.Status.State, v1beta2.AzVolumeAttachmentAttachmentState("Attached"))
 			},
 		},
 		{
@@ -386,24 +463,24 @@ func TestAttachDetachRecover(t *testing.T) {
 
 				require.NotNil(t, azVolumeAttachment.Status.Detail)
 				require.NotNil(t, azVolumeAttachment.Status.Detail.PublishContext)
-				require.Equal(t, azVolumeAttachment.Status.Detail.PublishContext["testKey"], "testValue")
+				require.Equal(t, azVolumeAttachment.Status.Detail.PublishContext[azureconstants.LUN], "1")
 			},
 		},
 		{
 			description: "[Success] Should update AzVolumeAttachment CRIs to right state",
 			setupFunc: func(t *testing.T, mockCtl *gomock.Controller) *ReconcileAttachDetach {
-				newAzVolumeAttachment0 := testPrimaryAzVolumeAttachment0.DeepCopy()
-				newAzVolumeAttachment0.Status.State = azdiskv1beta2.Attaching
-				newAzVolumeAttachment0.Status.Detail = &azdiskv1beta2.AzVolumeAttachmentStatusDetail{}
-				newAzVolumeAttachment0.Status.Error = &azdiskv1beta2.AzError{}
+				newAzVolumeAttachment2 := testPrimaryAzVolumeAttachment2.DeepCopy()
+				newAzVolumeAttachment2.Status.State = azdiskv1beta2.Attaching
+				newAzVolumeAttachment2.Status.Detail = &azdiskv1beta2.AzVolumeAttachmentStatusDetail{}
+				newAzVolumeAttachment2.Status.Error = &azdiskv1beta2.AzError{}
 
 				newAzVolumeAttachment1 := testPrimaryAzVolumeAttachment1.DeepCopy()
 				newAzVolumeAttachment1.Status.State = azdiskv1beta2.Detaching
 
 				controller := NewTestAttachDetachController(
 					mockCtl,
 					testNamespace,
-					newAzVolumeAttachment0,
+					newAzVolumeAttachment2,
 					newAzVolumeAttachment1)
 
 				mockClientsAndAttachmentProvisioner(controller)
@@ -413,7 +490,7 @@ func TestAttachDetachRecover(t *testing.T) {
 			verifyFunc: func(t *testing.T, controller *ReconcileAttachDetach, err error) {
 				require.NoError(t, err)
 
-				azVolumeAttachment, localErr := controller.azClient.DiskV1beta2().AzVolumeAttachments(testNamespace).Get(context.TODO(), testPrimaryAzVolumeAttachment0Name, metav1.GetOptions{})
+				azVolumeAttachment, localErr := controller.azClient.DiskV1beta2().AzVolumeAttachments(testNamespace).Get(context.TODO(), testPrimaryAzVolumeAttachment2Name, metav1.GetOptions{})
 				require.NoError(t, localErr)
 				require.Equal(t, azVolumeAttachment.Status.State, azdiskv1beta2.AttachmentPending)
 				require.Nil(t, azVolumeAttachment.Status.Detail)

pkg/controller/common.go

@@ -36,6 +36,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/component-helpers/scheduling/corev1/nodeaffinity"
 	azdiskv1beta2 "sigs.k8s.io/azuredisk-csi-driver/pkg/apis/azuredisk/v1beta2"
@@ -88,6 +89,7 @@ const (
 	replica          operationRequester = "replica-controller"
 	nodeavailability operationRequester = "nodeavailability-controller"
 	pod              operationRequester = "pod-controller"
+	attachdetach     operationRequester = "attachdetach-controller"
 )
 
 type attachmentCleanUpMode int
@@ -134,6 +136,8 @@ type CloudProvisioner interface {
 	DeleteVolume(ctx context.Context, volumeID string, secrets map[string]string) error
 	PublishVolume(ctx context.Context, volumeID string, nodeID string, volumeContext map[string]string) provisioner.CloudAttachResult
 	UnpublishVolume(ctx context.Context, volumeID string, nodeID string) error
+	GetNodeDataDisks(nodeName types.NodeName) ([]compute.DataDisk, *string, error)
+	GetDiskLun(diskName, diskURI string, nodeName types.NodeName) (int32, *string, error)
 	ExpandVolume(ctx context.Context, volumeID string, capacityRange *azdiskv1beta2.CapacityRange, secrets map[string]string) (*azdiskv1beta2.AzVolumeStatusDetail, error)
 	ListVolumes(ctx context.Context, maxEntries int32, startingToken string) (*azdiskv1beta2.ListVolumesResult, error)
 	CreateSnapshot(ctx context.Context, sourceVolumeID string, snapshotName string, secrets map[string]string, parameters map[string]string) (*azdiskv1beta2.Snapshot, error)

pkg/controller/common_test.go

@@ -167,6 +167,7 @@ var (
 
 	testPrimaryAzVolumeAttachment0Name = azureutils.GetAzVolumeAttachmentName(testPersistentVolume0Name, testNode0Name)
 	testPrimaryAzVolumeAttachment1Name = azureutils.GetAzVolumeAttachmentName(testPersistentVolume1Name, testNode0Name)
+	testPrimaryAzVolumeAttachment2Name = azureutils.GetAzVolumeAttachmentName(testPersistentVolume2Name, testNode2Name)
 
 	testPrimaryAzVolumeAttachment0 = createTestAzVolumeAttachment(testPersistentVolume0Name, testNode0Name, azdiskv1beta2.PrimaryRole)
 
@@ -176,6 +177,8 @@ var (
 
 	testPrimaryAzVolumeAttachment1Request = createReconcileRequest(testNamespace, testPrimaryAzVolumeAttachment1Name)
 
+	testPrimaryAzVolumeAttachment2 = createTestAzVolumeAttachment(testPersistentVolume2Name, testNode2Name, azdiskv1beta2.PrimaryRole)
+
 	testReplicaAzVolumeAttachmentName = azureutils.GetAzVolumeAttachmentName(testPersistentVolume0Name, testNode1Name)
 
 	testReplicaAzVolumeAttachment = createTestAzVolumeAttachment(testPersistentVolume0Name, testNode1Name, azdiskv1beta2.ReplicaRole)

pkg/controller/pod.go

@@ -164,6 +164,7 @@ func (r *ReconcilePod) Recover(ctx context.Context) error {
 			w.Logger().V(5).Infof("failed to add necessary components for pod (%s)", podKey)
 			continue
 		}
+		// For cases like: upgrading driver V1 to V2, createReplicas() is needed here since V1 doesn't have any replica attachment.
 		if err := r.createReplicas(ctx, podKey); err != nil {
 			w.Logger().V(5).Infof("failed to create replica AzVolumeAttachments for pod (%s)", podKey)
 		}