add unit test for attach disk

Author: huww98

pkg/disk/batcher/passthrough.go

@@ -0,0 +1,25 @@
+package batcher
+
+import (
+	"context"
+
+	"github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/disk/desc"
+)
+
+type Passthrough[T any] struct {
+	client desc.Client[T]
+}
+
+func NewPassthrough[T any](client desc.Client[T]) Passthrough[T] {
+	return Passthrough[T]{
+		client: client,
+	}
+}
+
+func (w Passthrough[T]) Describe(ctx context.Context, id string) (*T, error) {
+	resp, err := w.client.Describe([]string{id})
+	if err != nil {
+		return nil, err
+	}
+	return &resp.Resources[0], nil
+}

pkg/disk/cloud_test.go

@@ -13,7 +13,9 @@ import (
 	"github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/cloud"
 	"github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/disk/batcher"
 	"github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/disk/desc"
+	"github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/disk/waitstatus"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog/v2/ktesting"
 	"k8s.io/utils/clock"
@@ -598,3 +600,149 @@ func Test_getDiskDescribeRequest(t *testing.T) {
 		})
 	}
 }
+
+func testAttachDetach(t *testing.T) (context.Context, *cloud.MockECSInterface, *DiskAttachDetach) {
+	ctrl := gomock.NewController(t)
+	ecs := cloud.NewMockECSInterface(ctrl)
+	_, ctx := ktesting.NewTestContext(t)
+
+	client := desc.Disk(ecs)
+	b := batcher.NewPassthrough(client)
+	return ctx, ecs, &DiskAttachDetach{
+		slots:           NewSlots(0, 0),
+		ecs:             ecs,
+		waiter:          waitstatus.NewSimple(client, clock.RealClock{}),
+		batcher:         b,
+		attachThrottler: defaultThrottler(),
+		detachThrottler: defaultThrottler(),
+		dev:             DefaultDeviceManager,
+	}
+}
+
+func disk(status string, node string) ecs.Disk {
+	disk := ecs.Disk{
+		Status:       status,
+		Category:     "cloud_regional_disk_auto", // only one support forceAttach
+		MultiAttach:  "Disabled",
+		DiskId:       "d-testdiskid",
+		InstanceId:   node,
+		SerialNumber: "fake-serial-number",
+	}
+	if node != "" {
+		disk.Attachments.Attachment = []ecs.Attachment{
+			{InstanceId: node},
+		}
+	}
+	return disk
+}
+
+func diskResp(disk ecs.Disk) *ecs.DescribeDisksResponse {
+	return &ecs.DescribeDisksResponse{
+		Disks: ecs.DisksInDescribeDisks{
+			Disk: []ecs.Disk{disk},
+		},
+	}
+}
+
+func TestAttachDisk(t *testing.T) {
+	GlobalConfigVar.DetachBeforeAttach = true // This is the default
+	cases := []struct {
+		name          string
+		before, after ecs.Disk
+		detaching     bool
+		detach        bool
+		detached      ecs.Disk
+		noAttach      bool
+		forceAttach   bool
+		expectErr     bool
+	}{
+		{
+			name:     "already attached",
+			before:   disk("In_use", "i-testinstanceid"),
+			noAttach: true,
+		},
+		{
+			name:        "attached to other",
+			before:      disk("In_use", "i-anotherinstance"),
+			detaching:   true,
+			forceAttach: true,
+			after:       disk("In_use", "i-testinstanceid"),
+		},
+		{
+			name:     "attached to other (no force attach)",
+			before:   disk("In_use", "i-anotherinstance"),
+			detach:   true,
+			detached: disk("Available", ""),
+			after:    disk("In_use", "i-testinstanceid"),
+		},
+		{
+			name:   "normal",
+			before: disk("Available", ""),
+			after:  disk("In_use", "i-testinstanceid"),
+		},
+		{
+			name:      "attaching",
+			before:    disk("Attaching", ""),
+			noAttach:  true,
+			expectErr: true,
+		},
+		{
+			name:   "detaching from self",
+			before: disk("Detaching", "i-testinstanceid"),
+			after:  disk("In_use", "i-testinstanceid"), // FIXME
+		},
+		{
+			// This not supported by ECS, but desired by us to speed up failover. Hopes they will support it someday.
+			name:        "detaching from other",
+			before:      disk("Detaching", "i-anotherinstance"),
+			detaching:   true,
+			forceAttach: true,
+			after:       disk("In_use", "i-testinstanceid"),
+		},
+		{
+			// This is likely to fail in real env. But we try it anyway, in case the detach just finished after we checked
+			name:        "detaching from other (no force attach)",
+			before:      disk("Detaching", "i-anotherinstance"),
+			forceAttach: false,
+			after:       disk("In_use", "i-testinstanceid"),
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx, c, ad := testAttachDetach(t)
+
+			if tc.detaching {
+				ad.detaching.Store("d-testdiskid", "i-anotherinstance")
+			}
+
+			c.EXPECT().DescribeDisks(gomock.Any()).Return(diskResp(tc.before), nil)
+			if tc.detach {
+				detachCall := c.EXPECT().DetachDisk(gomock.Any()).Return(&ecs.DetachDiskResponse{}, nil)
+				c.EXPECT().DescribeDisks(gomock.Any()).Return(diskResp(tc.detached), nil).After(detachCall)
+			}
+			force := false
+			if !tc.noAttach {
+				attachCall := c.EXPECT().AttachDisk(gomock.Any()).Return(&ecs.AttachDiskResponse{}, nil).
+					Do(func(request *ecs.AttachDiskRequest) {
+						if request.Force.HasValue() {
+							var err error
+							force, err = request.Force.GetValue()
+							if err != nil {
+								t.Fatalf("unexpected error: %v", err)
+							}
+						}
+					})
+				c.EXPECT().DescribeDisks(gomock.Any()).Return(diskResp(tc.after), nil).After(attachCall)
+			}
+			serial, err := ad.attachDisk(ctx, "d-testdiskid", "i-testinstanceid", false)
+
+			assert.Equal(t, tc.forceAttach, force)
+			if tc.expectErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, "fake-serial-number", serial)
+			}
+		})
+	}
+}