Skip to content

Commit bbb9c56

Browse files
anshuman-agarwalaanshuman-agarwala
committed
Added support for additionalVolumes in VPCMachines
1 parent 7640740 commit bbb9c56

16 files changed

+461
-22
lines changed

‎api/v1beta1/zz_generated.conversion.go‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/v1beta2/common.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package v1beta2
1818

1919
import (
20+
"fmt"
2021
"strconv"
2122

2223
"k8s.io/apimachinery/pkg/util/intstr"
@@ -80,9 +81,23 @@ func defaultIBMVPCMachineSpec(spec *IBMVPCMachineSpec) {
8081
}
8182
}
8283

83-
func validateBootVolume(spec IBMVPCMachineSpec) field.ErrorList {
84+
func validateVolumes(spec IBMVPCMachineSpec) field.ErrorList {
8485
var allErrs field.ErrorList
8586

87+
for i := range spec.AdditionalVolumes {
88+
if spec.AdditionalVolumes[i].Profile == "custom" {
89+
if spec.AdditionalVolumes[i].Iops == 0 {
90+
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` "))
91+
}
92+
if spec.AdditionalVolumes[i].SizeGiB == 0 {
93+
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` "))
94+
}
95+
}
96+
if spec.AdditionalVolumes[i].Iops != 0 && spec.AdditionalVolumes[i].Profile != "custom" {
97+
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`"))
98+
}
99+
}
100+
86101
if spec.BootVolume == nil {
87102
return allErrs
88103
}

‎api/v1beta2/common_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -110,58 +110,58 @@ func TestValidateIBMPowerVSProcessorValues(t *testing.T) {
110110
}
111111
}
112112

113-
func Test_validateBootVolume(t *testing.T) {
113+
func Test_validateVolumes(t *testing.T) {
114114
tests := []struct {
115115
name string
116116
spec IBMVPCMachineSpec
117117
wantError bool
118118
}{
119119
{
120-
name: "Nil bootvolume",
120+
name: "Nil bootvolume for Boot Volume",
121121
spec: IBMVPCMachineSpec{
122122
BootVolume: nil,
123123
},
124124
wantError: false,
125125
},
126126
{
127-
name: "valid sizeGiB",
127+
name: "valid sizeGiB for Boot Volume",
128128
spec: IBMVPCMachineSpec{
129129
BootVolume: &VPCVolume{SizeGiB: 20},
130130
},
131131
wantError: false,
132132
},
133133
{
134-
name: "Invalid sizeGiB",
134+
name: "Invalid sizeGiB for Boot Volume",
135135
spec: IBMVPCMachineSpec{
136136
BootVolume: &VPCVolume{SizeGiB: 1},
137137
},
138138
wantError: true,
139139
},
140140
{
141-
name: "Valid Iops",
141+
name: "Missing Iops for custom profile for Additional Volume",
142142
spec: IBMVPCMachineSpec{
143-
BootVolume: &VPCVolume{Iops: 1000, Profile: "custom"},
143+
AdditionalVolumes: []*VPCVolume{{Profile: "custom", SizeGiB: 20}},
144144
},
145145
wantError: true,
146146
},
147147
{
148-
name: "Invalid Iops",
148+
name: "Missing SizeGiB for custom profile for Additional Volume",
149149
spec: IBMVPCMachineSpec{
150-
BootVolume: &VPCVolume{Iops: 1234, Profile: "general-purpose"},
150+
AdditionalVolumes: []*VPCVolume{{Profile: "custom", Iops: 4000}},
151151
},
152152
wantError: true,
153153
},
154154
{
155-
name: "Missing Iops for custom profile",
155+
name: "Valid iops and sizeGiB for custom profile",
156156
spec: IBMVPCMachineSpec{
157-
BootVolume: &VPCVolume{Profile: "general-purpose"},
157+
AdditionalVolumes: []*VPCVolume{{Profile: "custom", SizeGiB: 25, Iops: 4000}},
158158
},
159-
wantError: true,
159+
wantError: false,
160160
},
161161
}
162162
for _, tt := range tests {
163163
t.Run(tt.name, func(t *testing.T) {
164-
if err := validateBootVolume(tt.spec); (err != nil) != tt.wantError {
164+
if err := validateVolumes(tt.spec); (err != nil) != tt.wantError {
165165
t.Errorf("validateBootVolume() = %v, wantError %v", err, tt.wantError)
166166
}
167167
})

‎api/v1beta2/ibmvpcmachine_types.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ type IBMVPCMachineSpec struct {
7878
// SSHKeys is the SSH pub keys that will be used to access VM.
7979
// ID will take higher precedence over Name if both specified.
8080
SSHKeys []*IBMVPCResourceReference `json:"sshKeys,omitempty"`
81+
82+
// AdditionalVolumes is the list of additional volumes attached to the instance
83+
// There is a hard limit of 12 volume attachments per instance:
84+
// https://cloud.ibm.com/docs/vpc?topic=vpc-attaching-block-storage&interface=api#vol-attach-limits
85+
// +kubebuilder:validation:Optional
86+
// +kubebuilder:validation:MaxItems=12
87+
// +kubebuilder:validation:XValidation:rule="oldSelf.all(x, x in self)",message="Values may only be added"
88+
AdditionalVolumes []*VPCVolume `json:"additionalVolumes,omitempty"`
8189
}
8290

8391
// IBMVPCResourceReference is a reference to a specific VPC resource by ID or Name
@@ -95,7 +103,7 @@ type IBMVPCResourceReference struct {
95103
Name *string `json:"name,omitempty"`
96104
}
97105

98-
// VPCVolume defines the volume information for the instance.
106+
// VPCVolume defines the volume information.
99107
type VPCVolume struct {
100108
// DeleteVolumeOnInstanceDelete If set to true, when deleting the instance the volume will also be deleted.
101109
// Default is set as true
@@ -108,14 +116,15 @@ type VPCVolume struct {
108116
// +optional
109117
Name string `json:"name,omitempty"`
110118

111-
// SizeGiB is the size of the virtual server's boot disk in GiB.
119+
// SizeGiB is the size of the virtual server's disk in GiB.
112120
// Default to the size of the image's `minimum_provisioned_size`.
113121
// +optional
114122
SizeGiB int64 `json:"sizeGiB,omitempty"`
115123

116-
// Profile is the volume profile for the bootdisk, refer https://cloud.ibm.com/docs/vpc?topic=vpc-block-storage-profiles
124+
// Profile is the volume profile for the disk, refer https://cloud.ibm.com/docs/vpc?topic=vpc-block-storage-profiles
117125
// for more information.
118126
// Default to general-purpose
127+
// NOTE: If a profile other than custom is specified, the Iops and SizeGiB fields will be ignored
119128
// +kubebuilder:validation:Enum="general-purpose";"5iops-tier";"10iops-tier";"custom"
120129
// +kubebuilder:default=general-purpose
121130
// +optional

‎api/v1beta2/ibmvpcmachine_webhook.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,5 +73,5 @@ func (r *IBMVPCMachine) ValidateDelete() (admission.Warnings, error) {
7373
}
7474

7575
func (r *IBMVPCMachine) validateIBMVPCMachineBootVolume() field.ErrorList {
76-
return validateBootVolume(r.Spec)
76+
return validateVolumes(r.Spec)
7777
}

‎api/v1beta2/ibmvpcmachinetemplate_webhook.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,5 +72,5 @@ func (r *IBMVPCMachineTemplate) ValidateDelete() (admission.Warnings, error) {
7272
}
7373

7474
func (r *IBMVPCMachineTemplate) validateIBMVPCMachineBootVolume() field.ErrorList {
75-
return validateBootVolume(r.Spec.Template.Spec)
75+
return validateVolumes(r.Spec.Template.Spec)
7676
}

‎api/v1beta2/zz_generated.deepcopy.go‎

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cloud/scope/machine.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,3 +1155,58 @@ func (m *MachineScope) APIServerPort() int32 {
11551155
}
11561156
return infrav1beta2.DefaultAPIServerPort
11571157
}
1158+
1159+
func (m *MachineScope) GetVolumeAttachments() ([]vpcv1.VolumeAttachment, error) {
1160+
options := vpcv1.ListInstanceVolumeAttachmentsOptions{
1161+
InstanceID: &m.IBMVPCMachine.Status.InstanceID,
1162+
}
1163+
result, _, err := m.IBMVPCClient.GetVolumeAttachments(&options)
1164+
if err != nil {
1165+
return nil, err
1166+
}
1167+
return result.VolumeAttachments, nil
1168+
}
1169+
1170+
func (m *MachineScope) CreateAndAttachVolume(vpcVolume *infrav1beta2.VPCVolume) error {
1171+
volumeOptions := vpcv1.CreateVolumeOptions{}
1172+
// TODO: EncryptionKeyCRN is not supported for now, the field is omitted from the manifest
1173+
if vpcVolume.Profile != "custom" {
1174+
volumeOptions.VolumePrototype = &vpcv1.VolumePrototype{
1175+
Profile: &vpcv1.VolumeProfileIdentityByName{
1176+
Name: &vpcVolume.Profile,
1177+
},
1178+
Zone: &vpcv1.ZoneIdentity{
1179+
Name: &m.IBMVPCMachine.Spec.Zone,
1180+
},
1181+
Capacity: &vpcVolume.SizeGiB,
1182+
}
1183+
} else {
1184+
volumeOptions.VolumePrototype = &vpcv1.VolumePrototype{
1185+
Iops: &vpcVolume.Iops,
1186+
Profile: &vpcv1.VolumeProfileIdentityByName{
1187+
Name: &vpcVolume.Profile,
1188+
},
1189+
Zone: &vpcv1.ZoneIdentity{
1190+
Name: &m.IBMVPCMachine.Spec.Zone,
1191+
},
1192+
Capacity: &vpcVolume.SizeGiB,
1193+
}
1194+
}
1195+
1196+
volumeResult, _, err := m.IBMVPCClient.CreateVolume(&volumeOptions)
1197+
if err != nil {
1198+
return err
1199+
}
1200+
1201+
attachmentOptions := vpcv1.CreateInstanceVolumeAttachmentOptions{
1202+
InstanceID: &m.IBMVPCMachine.Status.InstanceID,
1203+
Volume: &vpcv1.VolumeAttachmentPrototypeVolume{
1204+
ID: volumeResult.ID,
1205+
},
1206+
DeleteVolumeOnInstanceDelete: &vpcVolume.DeleteVolumeOnInstanceDelete,
1207+
Name: &vpcVolume.Name,
1208+
}
1209+
1210+
_, _, err = m.IBMVPCClient.AttachVolumeToInstance(&attachmentOptions)
1211+
return err
1212+
}

‎cloud/scope/machine_test.go‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,3 +1096,121 @@ func TestDeleteVPCLoadBalancerPoolMember(t *testing.T) {
10961096
})
10971097
})
10981098
}
1099+
1100+
func TestGetVolumeAttachments(t *testing.T) {
1101+
1102+
setup := func(t *testing.T) (*gomock.Controller, *mock.MockVpc) {
1103+
t.Helper()
1104+
return gomock.NewController(t), mock.NewMockVpc(gomock.NewController(t))
1105+
}
1106+
1107+
vpcMachine := infrav1beta2.IBMVPCMachine{
1108+
Status: infrav1beta2.IBMVPCMachineStatus{
1109+
InstanceID: "foo-instance-id",
1110+
},
1111+
}
1112+
1113+
dummyVolumeAttachments := vpcv1.VolumeAttachmentCollection{
1114+
VolumeAttachments: []vpcv1.VolumeAttachment{{
1115+
Name: new(string),
1116+
},
1117+
{
1118+
Name: new(string),
1119+
}},
1120+
}
1121+
1122+
t.Run("Return List of Volume Attachments for Machine", func(t *testing.T) {
1123+
g := NewWithT(t)
1124+
mockController, mockVPC := setup(t)
1125+
t.Cleanup(mockController.Finish)
1126+
scope := setupMachineScope(clusterName, machineName, mockVPC)
1127+
scope.IBMVPCMachine.Status = vpcMachine.Status
1128+
mockVPC.EXPECT().GetVolumeAttachments(gomock.AssignableToTypeOf(&vpcv1.ListInstanceVolumeAttachmentsOptions{})).Return(&dummyVolumeAttachments, nil, nil)
1129+
attachments, err := scope.GetVolumeAttachments()
1130+
g.Expect(attachments).To(Equal(dummyVolumeAttachments.VolumeAttachments))
1131+
g.Expect(err).Should(Succeed())
1132+
})
1133+
1134+
t.Run("Return Error when GetVolumeAttachments fails", func(t *testing.T) {
1135+
g := NewWithT(t)
1136+
mockController, mockVPC := setup(t)
1137+
t.Cleanup(mockController.Finish)
1138+
scope := setupMachineScope(clusterName, machineName, mockVPC)
1139+
scope.IBMVPCMachine.Status = vpcMachine.Status
1140+
mockVPC.EXPECT().GetVolumeAttachments(gomock.AssignableToTypeOf(&vpcv1.ListInstanceVolumeAttachmentsOptions{})).Return(nil, nil, errors.New("Error when getting volume attachments"))
1141+
attachments, err := scope.GetVolumeAttachments()
1142+
g.Expect(attachments).To(BeNil())
1143+
g.Expect(err).ShouldNot(Succeed())
1144+
})
1145+
}
1146+
1147+
func TestCreateAndAttachVolume(t *testing.T) {
1148+
1149+
setup := func(t *testing.T) (*gomock.Controller, *mock.MockVpc) {
1150+
t.Helper()
1151+
return gomock.NewController(t), mock.NewMockVpc(gomock.NewController(t))
1152+
}
1153+
1154+
volumeName := "foo-volume"
1155+
volumeID := "foo-volume-id"
1156+
1157+
vpcMachine := infrav1beta2.IBMVPCMachine{
1158+
Status: infrav1beta2.IBMVPCMachineStatus{
1159+
InstanceID: "foo-instance-id",
1160+
},
1161+
}
1162+
1163+
infraVolume := infrav1beta2.VPCVolume{
1164+
Name: volumeName,
1165+
}
1166+
1167+
vpcVolume := vpcv1.Volume{
1168+
Name: &volumeName,
1169+
ID: &volumeID,
1170+
}
1171+
1172+
volumeCreationError := errors.New("Error while creating volume")
1173+
1174+
volumeAttachmentError := errors.New("Error while attaching volume")
1175+
1176+
t.Run("Volume creation and attachment is successful", func(t *testing.T) {
1177+
g := NewWithT(t)
1178+
mockController, mockVPC := setup(t)
1179+
t.Cleanup(mockController.Finish)
1180+
scope := setupMachineScope(clusterName, machineName, mockVPC)
1181+
scope.IBMVPCMachine.Status = vpcMachine.Status
1182+
mockVPC.EXPECT().CreateVolume(gomock.AssignableToTypeOf(&vpcv1.CreateVolumeOptions{})).Return(&vpcVolume, nil, nil)
1183+
mockVPC.EXPECT().AttachVolumeToInstance(gomock.AssignableToTypeOf(&vpcv1.CreateInstanceVolumeAttachmentOptions{})).Return(nil, nil, nil)
1184+
1185+
err := scope.CreateAndAttachVolume(&infraVolume)
1186+
g.Expect(err).Should(Succeed())
1187+
})
1188+
1189+
t.Run("Volume Creation Fails", func(t *testing.T) {
1190+
g := NewWithT(t)
1191+
mockController, mockVPC := setup(t)
1192+
t.Cleanup(mockController.Finish)
1193+
scope := setupMachineScope(clusterName, machineName, mockVPC)
1194+
scope.IBMVPCMachine.Status = vpcMachine.Status
1195+
mockVPC.EXPECT().CreateVolume(gomock.AssignableToTypeOf(&vpcv1.CreateVolumeOptions{})).Return(nil, nil, volumeCreationError)
1196+
1197+
err := scope.CreateAndAttachVolume(&infraVolume)
1198+
g.Expect(err).ShouldNot(Succeed())
1199+
g.Expect(errors.Is(err, volumeCreationError)).To(BeTrue())
1200+
})
1201+
1202+
t.Run("Volume Attachment Fails", func(t *testing.T) {
1203+
g := NewWithT(t)
1204+
mockController, mockVPC := setup(t)
1205+
t.Cleanup(mockController.Finish)
1206+
scope := setupMachineScope(clusterName, machineName, mockVPC)
1207+
scope.IBMVPCMachine.Status = vpcMachine.Status
1208+
mockVPC.EXPECT().CreateVolume(gomock.AssignableToTypeOf(&vpcv1.CreateVolumeOptions{})).Return(&vpcVolume, nil, nil)
1209+
mockVPC.EXPECT().AttachVolumeToInstance(gomock.AssignableToTypeOf(&vpcv1.CreateInstanceVolumeAttachmentOptions{})).Return(nil, nil, volumeAttachmentError)
1210+
1211+
err := scope.CreateAndAttachVolume(&infraVolume)
1212+
g.Expect(err).ShouldNot(Succeed())
1213+
g.Expect(errors.Is(err, volumeAttachmentError)).To(BeTrue())
1214+
})
1215+
1216+
}

0 commit comments

Comments
 (0)