Skip to content

Readded support for additional disks #2448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 16 additions & 3 deletions api/v1beta2/ibmvpcmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -175,6 +184,10 @@ type IBMVPCMachineStatus struct {
// LoadBalancerPoolMembers is the status of IBM Cloud VPC Load Balancer Backend Pools the machine is a member.
// +optional
LoadBalancerPoolMembers []VPCLoadBalancerBackendPoolMember `json:"loadBalancerPoolMembers,omitempty"`

// AdditionalVolumeIDs is a list of Volume ID as per IBMCloud
// +optional
AdditionalVolumeIDs []string `json:"additionalVolumeIDs,omitempty"`
}

// +kubebuilder:object:root=true
Expand Down
16 changes: 16 additions & 0 deletions api/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

89 changes: 89 additions & 0 deletions cloud/scope/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -1155,3 +1155,92 @@ func (m *MachineScope) APIServerPort() int32 {
}
return infrav1.DefaultAPIServerPort
}

// GetVolumeAttachments returns the volume attachments 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, fmt.Errorf("error while getting volume attachments: %w", err)
}
return result.VolumeAttachments, nil
}

// GetVolumeState returns the volume's state.
func (m *MachineScope) GetVolumeState(volumeID string) (string, error) {
options := vpcv1.GetVolumeOptions{
ID: &volumeID,
}
result, _, err := m.IBMVPCClient.GetVolume(&options)
if err != nil {
return "", fmt.Errorf("could not fetch volume status: %w", err)
}
return *result.Status, err
}

// CreateVolume creates a new Volume and attaches it to the instance.
func (m *MachineScope) CreateVolume(vpcVolume *infrav1.VPCVolume) (string, error) {
volumeOptions := vpcv1.CreateVolumeOptions{}
var resourceGroupID string
if m.IBMVPCCluster.Status.ResourceGroup != nil {
resourceGroupID = m.IBMVPCCluster.Status.ResourceGroup.ID
} else {
resourceGroupID = m.IBMVPCCluster.Spec.ResourceGroup
}
// TODO: EncryptionKeyCRN is not supported for now, the field is omitted from the manifest
if vpcVolume.Profile != "custom" {
volumeOptions.VolumePrototype = &vpcv1.VolumePrototype{
ResourceGroup: &vpcv1.ResourceGroupIdentityByID{
ID: &resourceGroupID,
},
Profile: &vpcv1.VolumeProfileIdentityByName{
Name: &vpcVolume.Profile,
},
Zone: &vpcv1.ZoneIdentity{
Name: &m.IBMVPCMachine.Spec.Zone,
},
Capacity: &vpcVolume.SizeGiB,
}
} else {
volumeOptions.VolumePrototype = &vpcv1.VolumePrototype{
ResourceGroup: &vpcv1.ResourceGroupIdentityByID{
ID: &resourceGroupID,
},
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 "", fmt.Errorf("error while creating volume: %w", err)
}

return *volumeResult.ID, nil
}

// AttachVolume attaches the given volume to the instance.
func (m *MachineScope) AttachVolume(deleteOnInstanceDelete bool, volumeID, volumeName string) error {
attachmentOptions := vpcv1.CreateInstanceVolumeAttachmentOptions{
InstanceID: &m.IBMVPCMachine.Status.InstanceID,
Volume: &vpcv1.VolumeAttachmentPrototypeVolume{
ID: &volumeID,
},
DeleteVolumeOnInstanceDelete: &deleteOnInstanceDelete,
Name: &volumeName,
}

_, _, err := m.IBMVPCClient.AttachVolumeToInstance(&attachmentOptions)
if err != nil {
err = fmt.Errorf("error while attaching volume to instance: %w", err)
}
return err
}
185 changes: 185 additions & 0 deletions cloud/scope/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ import (
. "github.com/onsi/gomega"
)

var (
volumeName = "foo-volume"
volumeID = "foo-volume-id"
)

func newVPCMachine(clusterName, machineName string) *infrav1.IBMVPCMachine {
return &infrav1.IBMVPCMachine{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1096,3 +1101,183 @@ 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 := infrav1.IBMVPCMachine{
Status: infrav1.IBMVPCMachineStatus{
InstanceID: "foo-instance-id",
},
}
volumeAttachmentName := "foo-volume-attachment"

testVolumeAttachments := vpcv1.VolumeAttachmentCollection{
VolumeAttachments: []vpcv1.VolumeAttachment{{
Name: &volumeAttachmentName,
},
{
Name: &volumeName,
}},
}

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(&testVolumeAttachments, nil, nil)
attachments, err := scope.GetVolumeAttachments()
g.Expect(attachments).To(Equal(testVolumeAttachments.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 TestGetVolumeState(t *testing.T) {
setup := func(t *testing.T) (*gomock.Controller, *mock.MockVpc) {
t.Helper()
return gomock.NewController(t), mock.NewMockVpc(gomock.NewController(t))
}

volumeStatus := vpcv1.VolumeStatusPendingConst

vpcMachine := infrav1.IBMVPCMachine{
Status: infrav1.IBMVPCMachineStatus{
InstanceID: "foo-instance-id",
},
}

vpcVolume := vpcv1.Volume{
Name: &volumeName,
ID: &volumeID,
Status: &volumeStatus,
}
volumeFetchError := errors.New("error while fetching volume")

t.Run("Return correct volume state", 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().GetVolume(gomock.AssignableToTypeOf(&vpcv1.GetVolumeOptions{})).Return(&vpcVolume, nil, nil)
state, err := scope.GetVolumeState(volumeID)
g.Expect(err).To(BeNil())
g.Expect(state).To(Equal(volumeStatus))
})

t.Run("Return error when GetVolumeState returns error", 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().GetVolume(gomock.AssignableToTypeOf(&vpcv1.GetVolumeOptions{})).Return(nil, nil, volumeFetchError)
state, err := scope.GetVolumeState(volumeID)
g.Expect(state).To(BeZero())
g.Expect(errors.Is(err, volumeFetchError)).To(BeTrue())
})
}

func TestCreateVolume(t *testing.T) {
setup := func(t *testing.T) (*gomock.Controller, *mock.MockVpc) {
t.Helper()
return gomock.NewController(t), mock.NewMockVpc(gomock.NewController(t))
}

vpcMachine := infrav1.IBMVPCMachine{
Status: infrav1.IBMVPCMachineStatus{
InstanceID: "foo-instance-id",
},
}

infraVolume := infrav1.VPCVolume{
Name: volumeName,
Profile: "custom",
Iops: 100,
SizeGiB: 50,
}
pendingState := vpcv1.VolumeStatusPendingConst

vpcVolume := vpcv1.Volume{
Name: &volumeName,
ID: &volumeID,
Status: &pendingState,
}

volumeCreationError := errors.New("error while creating volume")
t.Run("Volume creation is successful", func(t *testing.T) {
g := NewWithT(t)
mockController, mockVPC := setup(t)
t.Cleanup(mockController.Finish)
scope := setupMachineScope(clusterName, machineName, mockVPC)
mockVPC.EXPECT().CreateVolume(gomock.AssignableToTypeOf(&vpcv1.CreateVolumeOptions{})).Return(&vpcVolume, nil, nil)
id, err := scope.CreateVolume(&infraVolume)
g.Expect(err).Should(Succeed())
g.Expect(id).Should(Equal(volumeID))
})
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)
id, err := scope.CreateVolume(&infraVolume)
g.Expect(err).ShouldNot(Succeed())
g.Expect(errors.Is(err, volumeCreationError)).To(BeTrue())
g.Expect(id).To(BeZero())
})
}

func TestAttachVolume(t *testing.T) {
setup := func(t *testing.T) (*gomock.Controller, *mock.MockVpc) {
t.Helper()
return gomock.NewController(t), mock.NewMockVpc(gomock.NewController(t))
}

deleteOnInstanceDelete := true
vpcMachine := infrav1.IBMVPCMachine{
Status: infrav1.IBMVPCMachineStatus{
InstanceID: "foo-instance-id",
},
}
volumeAttachmentError := errors.New("error while attaching volume")
t.Run("Volume 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().AttachVolumeToInstance(gomock.AssignableToTypeOf(&vpcv1.CreateInstanceVolumeAttachmentOptions{})).Return(nil, nil, nil)
err := scope.AttachVolume(deleteOnInstanceDelete, volumeID, volumeName)
g.Expect(err).Should(Succeed())
})
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().AttachVolumeToInstance(gomock.AssignableToTypeOf(&vpcv1.CreateInstanceVolumeAttachmentOptions{})).Return(nil, nil, volumeAttachmentError)
err := scope.AttachVolume(deleteOnInstanceDelete, volumeID, volumeName)
g.Expect(err).ShouldNot(Succeed())
g.Expect(errors.Is(err, volumeAttachmentError)).To(BeTrue())
})
}
Loading