Added support for additionalVolumes in VPCMachines

Author: anshuman-agarwala

api/v1beta1/zz_generated.conversion.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1beta2
 
 import (
+	"fmt"
 	"strconv"
 
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -80,8 +81,23 @@ func defaultIBMVPCMachineSpec(spec *IBMVPCMachineSpec) {
 	}
 }
 
-func validateBootVolume(spec IBMVPCMachineSpec) field.ErrorList {
+func validateVolumes(spec IBMVPCMachineSpec) field.ErrorList {
 	var allErrs field.ErrorList
+	const customProfile = "custom"
+
+	for i := range spec.AdditionalVolumes {
+		if spec.AdditionalVolumes[i].Profile == customProfile {
+			if spec.AdditionalVolumes[i].Iops == 0 {
+				allErrs = append(allErrs, field.Invalid(field.NewPath(fmt.Sprintf("spec.AdditionalVolumes[%d]", i)), spec, "iops has to be specified when profile is set to `custom` "))
+			}
+			if spec.AdditionalVolumes[i].SizeGiB == 0 {
+				allErrs = append(allErrs, field.Invalid(field.NewPath(fmt.Sprintf("spec.AdditionalVolumes[%d]", i)), spec, "sizeGiB has to be specified when profile is set to `custom` "))
+			}
+		}
+		if spec.AdditionalVolumes[i].Iops != 0 && spec.AdditionalVolumes[i].Profile != customProfile {
+			allErrs = append(allErrs, field.Invalid(field.NewPath(fmt.Sprintf("spec.AdditionalVolumes[%d]", i)), spec, "iops applicable only to volumes using a profile of type `custom`"))
+		}
+	}
 
 	if spec.BootVolume == nil {
 		return allErrs
@@ -91,7 +107,7 @@ func validateBootVolume(spec IBMVPCMachineSpec) field.ErrorList {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec.bootVolume.sizeGiB"), spec, "valid Boot VPCVolume size is 10 - 250 GB"))
 	}
 
-	if spec.BootVolume.Iops != 0 && spec.BootVolume.Profile != "custom" {
+	if spec.BootVolume.Iops != 0 && spec.BootVolume.Profile != customProfile {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec.bootVolume.iops"), spec, "iops applicable only to volumes using a profile of type `custom`"))
 	}
 

api/v1beta2/common.go

@@ -110,58 +110,58 @@ func TestValidateIBMPowerVSProcessorValues(t *testing.T) {
 	}
 }
 
-func Test_validateBootVolume(t *testing.T) {
+func Test_validateVolumes(t *testing.T) {
 	tests := []struct {
 		name      string
 		spec      IBMVPCMachineSpec
 		wantError bool
 	}{
 		{
-			name: "Nil bootvolume",
+			name: "Nil bootvolume for Boot Volume",
 			spec: IBMVPCMachineSpec{
 				BootVolume: nil,
 			},
 			wantError: false,
 		},
 		{
-			name: "valid sizeGiB",
+			name: "valid sizeGiB for Boot Volume",
 			spec: IBMVPCMachineSpec{
 				BootVolume: &VPCVolume{SizeGiB: 20},
 			},
 			wantError: false,
 		},
 		{
-			name: "Invalid sizeGiB",
+			name: "Invalid sizeGiB for Boot Volume",
 			spec: IBMVPCMachineSpec{
 				BootVolume: &VPCVolume{SizeGiB: 1},
 			},
 			wantError: true,
 		},
 		{
-			name: "Valid Iops",
+			name: "Missing Iops for custom profile for Additional Volume",
 			spec: IBMVPCMachineSpec{
-				BootVolume: &VPCVolume{Iops: 1000, Profile: "custom"},
+				AdditionalVolumes: []*VPCVolume{{Profile: "custom", SizeGiB: 20}},
 			},
 			wantError: true,
 		},
 		{
-			name: "Invalid Iops",
+			name: "Missing SizeGiB for custom profile for Additional Volume",
 			spec: IBMVPCMachineSpec{
-				BootVolume: &VPCVolume{Iops: 1234, Profile: "general-purpose"},
+				AdditionalVolumes: []*VPCVolume{{Profile: "custom", Iops: 4000}},
 			},
 			wantError: true,
 		},
 		{
-			name: "Missing Iops for custom profile",
+			name: "Valid iops and sizeGiB for custom profile",
 			spec: IBMVPCMachineSpec{
-				BootVolume: &VPCVolume{Profile: "general-purpose"},
+				AdditionalVolumes: []*VPCVolume{{Profile: "custom", SizeGiB: 25, Iops: 4000}},
 			},
-			wantError: true,
+			wantError: false,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if err := validateBootVolume(tt.spec); (err != nil) != tt.wantError {
+			if err := validateVolumes(tt.spec); (err != nil) != tt.wantError {
 				t.Errorf("validateBootVolume() = %v, wantError %v", err, tt.wantError)
 			}
 		})

api/v1beta2/common_test.go

@@ -78,6 +78,14 @@ type IBMVPCMachineSpec struct {
 	// SSHKeys is the SSH pub keys that will be used to access VM.
 	// ID will take higher precedence over Name if both specified.
 	SSHKeys []*IBMVPCResourceReference `json:"sshKeys,omitempty"`
+
+	// AdditionalVolumes is the list of additional volumes attached to the instance
+	// There is a hard limit of 12 volume attachments per instance:
+	// https://cloud.ibm.com/docs/vpc?topic=vpc-attaching-block-storage&interface=api#vol-attach-limits
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:validation:MaxItems=12
+	// +kubebuilder:validation:XValidation:rule="oldSelf.all(x, x in self)",message="Values may only be added"
+	AdditionalVolumes []*VPCVolume `json:"additionalVolumes,omitempty"`
 }
 
 // IBMVPCResourceReference is a reference to a specific VPC resource by ID or Name
@@ -95,7 +103,7 @@ type IBMVPCResourceReference struct {
 	Name *string `json:"name,omitempty"`
 }
 
-// VPCVolume defines the volume information for the instance.
+// VPCVolume defines the volume information.
 type VPCVolume struct {
 	// DeleteVolumeOnInstanceDelete If set to true, when deleting the instance the volume will also be deleted.
 	// Default is set as true
@@ -108,14 +116,15 @@ type VPCVolume struct {
 	// +optional
 	Name string `json:"name,omitempty"`
 
-	// SizeGiB is the size of the virtual server's boot disk in GiB.
+	// SizeGiB is the size of the virtual server's disk in GiB.
 	// Default to the size of the image's `minimum_provisioned_size`.
 	// +optional
 	SizeGiB int64 `json:"sizeGiB,omitempty"`
 
-	// Profile is the volume profile for the bootdisk, refer https://cloud.ibm.com/docs/vpc?topic=vpc-block-storage-profiles
+	// Profile is the volume profile for the disk, refer https://cloud.ibm.com/docs/vpc?topic=vpc-block-storage-profiles
 	// for more information.
 	// Default to general-purpose
+	// NOTE: If a profile other than custom is specified, the Iops and SizeGiB fields will be ignored
 	// +kubebuilder:validation:Enum="general-purpose";"5iops-tier";"10iops-tier";"custom"
 	// +kubebuilder:default=general-purpose
 	// +optional

api/v1beta2/ibmvpcmachine_types.go

@@ -73,5 +73,5 @@ func (r *IBMVPCMachine) ValidateDelete() (admission.Warnings, error) {
 }
 
 func (r *IBMVPCMachine) validateIBMVPCMachineBootVolume() field.ErrorList {
-	return validateBootVolume(r.Spec)
+	return validateVolumes(r.Spec)
 }

api/v1beta2/ibmvpcmachine_webhook.go

@@ -72,5 +72,5 @@ func (r *IBMVPCMachineTemplate) ValidateDelete() (admission.Warnings, error) {
 }
 
 func (r *IBMVPCMachineTemplate) validateIBMVPCMachineBootVolume() field.ErrorList {
-	return validateBootVolume(r.Spec.Template.Spec)
+	return validateVolumes(r.Spec.Template.Spec)
 }

api/v1beta2/ibmvpcmachinetemplate_webhook.go

@@ -1155,3 +1155,60 @@ func (m *MachineScope) APIServerPort() int32 {
 	}
 	return infrav1beta2.DefaultAPIServerPort
 }
+
+// GetVolumeAttachments returns the volumeattachments for the instance.
+func (m *MachineScope) GetVolumeAttachments() ([]vpcv1.VolumeAttachment, error) {
+	options := vpcv1.ListInstanceVolumeAttachmentsOptions{
+		InstanceID: &m.IBMVPCMachine.Status.InstanceID,
+	}
+	result, _, err := m.IBMVPCClient.GetVolumeAttachments(&options)
+	if err != nil {
+		return nil, err
+	}
+	return result.VolumeAttachments, nil
+}
+
+// CreateAndAttachVolume creates a new Volume and attaches it to the instance.
+func (m *MachineScope) CreateAndAttachVolume(vpcVolume *infrav1beta2.VPCVolume) error {
+	volumeOptions := vpcv1.CreateVolumeOptions{}
+	// TODO: EncryptionKeyCRN is not supported for now, the field is omitted from the manifest
+	if vpcVolume.Profile != "custom" {
+		volumeOptions.VolumePrototype = &vpcv1.VolumePrototype{
+			Profile: &vpcv1.VolumeProfileIdentityByName{
+				Name: &vpcVolume.Profile,
+			},
+			Zone: &vpcv1.ZoneIdentity{
+				Name: &m.IBMVPCMachine.Spec.Zone,
+			},
+			Capacity: &vpcVolume.SizeGiB,
+		}
+	} else {
+		volumeOptions.VolumePrototype = &vpcv1.VolumePrototype{
+			Iops: &vpcVolume.Iops,
+			Profile: &vpcv1.VolumeProfileIdentityByName{
+				Name: &vpcVolume.Profile,
+			},
+			Zone: &vpcv1.ZoneIdentity{
+				Name: &m.IBMVPCMachine.Spec.Zone,
+			},
+			Capacity: &vpcVolume.SizeGiB,
+		}
+	}
+
+	volumeResult, _, err := m.IBMVPCClient.CreateVolume(&volumeOptions)
+	if err != nil {
+		return err
+	}
+
+	attachmentOptions := vpcv1.CreateInstanceVolumeAttachmentOptions{
+		InstanceID: &m.IBMVPCMachine.Status.InstanceID,
+		Volume: &vpcv1.VolumeAttachmentPrototypeVolume{
+			ID: volumeResult.ID,
+		},
+		DeleteVolumeOnInstanceDelete: &vpcVolume.DeleteVolumeOnInstanceDelete,
+		Name:                         &vpcVolume.Name,
+	}
+
+	_, _, err = m.IBMVPCClient.AttachVolumeToInstance(&attachmentOptions)
+	return err
+}

api/v1beta2/zz_generated.deepcopy.go

@@ -1096,3 +1096,118 @@ func TestDeleteVPCLoadBalancerPoolMember(t *testing.T) {
 		})
 	})
 }
+
+func TestGetVolumeAttachments(t *testing.T) {
+	setup := func(t *testing.T) (*gomock.Controller, *mock.MockVpc) {
+		t.Helper()
+		return gomock.NewController(t), mock.NewMockVpc(gomock.NewController(t))
+	}
+
+	vpcMachine := infrav1beta2.IBMVPCMachine{
+		Status: infrav1beta2.IBMVPCMachineStatus{
+			InstanceID: "foo-instance-id",
+		},
+	}
+
+	dummyVolumeAttachments := vpcv1.VolumeAttachmentCollection{
+		VolumeAttachments: []vpcv1.VolumeAttachment{{
+			Name: new(string),
+		},
+			{
+				Name: new(string),
+			}},
+	}
+
+	t.Run("Return List of Volume Attachments for Machine", func(t *testing.T) {
+		g := NewWithT(t)
+		mockController, mockVPC := setup(t)
+		t.Cleanup(mockController.Finish)
+		scope := setupMachineScope(clusterName, machineName, mockVPC)
+		scope.IBMVPCMachine.Status = vpcMachine.Status
+		mockVPC.EXPECT().GetVolumeAttachments(gomock.AssignableToTypeOf(&vpcv1.ListInstanceVolumeAttachmentsOptions{})).Return(&dummyVolumeAttachments, nil, nil)
+		attachments, err := scope.GetVolumeAttachments()
+		g.Expect(attachments).To(Equal(dummyVolumeAttachments.VolumeAttachments))
+		g.Expect(err).Should(Succeed())
+	})
+
+	t.Run("Return Error when GetVolumeAttachments fails", func(t *testing.T) {
+		g := NewWithT(t)
+		mockController, mockVPC := setup(t)
+		t.Cleanup(mockController.Finish)
+		scope := setupMachineScope(clusterName, machineName, mockVPC)
+		scope.IBMVPCMachine.Status = vpcMachine.Status
+		mockVPC.EXPECT().GetVolumeAttachments(gomock.AssignableToTypeOf(&vpcv1.ListInstanceVolumeAttachmentsOptions{})).Return(nil, nil, errors.New("Error when getting volume attachments"))
+		attachments, err := scope.GetVolumeAttachments()
+		g.Expect(attachments).To(BeNil())
+		g.Expect(err).ShouldNot(Succeed())
+	})
+}
+
+func TestCreateAndAttachVolume(t *testing.T) {
+	setup := func(t *testing.T) (*gomock.Controller, *mock.MockVpc) {
+		t.Helper()
+		return gomock.NewController(t), mock.NewMockVpc(gomock.NewController(t))
+	}
+
+	volumeName := "foo-volume"
+	volumeID := "foo-volume-id"
+
+	vpcMachine := infrav1beta2.IBMVPCMachine{
+		Status: infrav1beta2.IBMVPCMachineStatus{
+			InstanceID: "foo-instance-id",
+		},
+	}
+
+	infraVolume := infrav1beta2.VPCVolume{
+		Name: volumeName,
+	}
+
+	vpcVolume := vpcv1.Volume{
+		Name: &volumeName,
+		ID:   &volumeID,
+	}
+
+	volumeCreationError := errors.New("Error while creating volume")
+
+	volumeAttachmentError := errors.New("Error while attaching volume")
+
+	t.Run("Volume creation and attachment is successful", func(t *testing.T) {
+		g := NewWithT(t)
+		mockController, mockVPC := setup(t)
+		t.Cleanup(mockController.Finish)
+		scope := setupMachineScope(clusterName, machineName, mockVPC)
+		scope.IBMVPCMachine.Status = vpcMachine.Status
+		mockVPC.EXPECT().CreateVolume(gomock.AssignableToTypeOf(&vpcv1.CreateVolumeOptions{})).Return(&vpcVolume, nil, nil)
+		mockVPC.EXPECT().AttachVolumeToInstance(gomock.AssignableToTypeOf(&vpcv1.CreateInstanceVolumeAttachmentOptions{})).Return(nil, nil, nil)
+
+		err := scope.CreateAndAttachVolume(&infraVolume)
+		g.Expect(err).Should(Succeed())
+	})
+
+	t.Run("Volume Creation Fails", func(t *testing.T) {
+		g := NewWithT(t)
+		mockController, mockVPC := setup(t)
+		t.Cleanup(mockController.Finish)
+		scope := setupMachineScope(clusterName, machineName, mockVPC)
+		scope.IBMVPCMachine.Status = vpcMachine.Status
+		mockVPC.EXPECT().CreateVolume(gomock.AssignableToTypeOf(&vpcv1.CreateVolumeOptions{})).Return(nil, nil, volumeCreationError)
+
+		err := scope.CreateAndAttachVolume(&infraVolume)
+		g.Expect(err).ShouldNot(Succeed())
+		g.Expect(errors.Is(err, volumeCreationError)).To(BeTrue())
+	})
+
+	t.Run("Volume Attachment Fails", func(t *testing.T) {
+		g := NewWithT(t)
+		mockController, mockVPC := setup(t)
+		t.Cleanup(mockController.Finish)
+		scope := setupMachineScope(clusterName, machineName, mockVPC)
+		scope.IBMVPCMachine.Status = vpcMachine.Status
+		mockVPC.EXPECT().CreateVolume(gomock.AssignableToTypeOf(&vpcv1.CreateVolumeOptions{})).Return(&vpcVolume, nil, nil)
+		mockVPC.EXPECT().AttachVolumeToInstance(gomock.AssignableToTypeOf(&vpcv1.CreateInstanceVolumeAttachmentOptions{})).Return(nil, nil, volumeAttachmentError)
+
+		err := scope.CreateAndAttachVolume(&infraVolume)
+		g.Expect(err).ShouldNot(Succeed())
+		g.Expect(errors.Is(err, volumeAttachmentError)).To(BeTrue())
+	})
+}