diff --git a/cmd/stackit-csi-plugin/main.go b/cmd/stackit-csi-plugin/main.go index 302aee97..534a5583 100644 --- a/cmd/stackit-csi-plugin/main.go +++ b/cmd/stackit-csi-plugin/main.go @@ -23,14 +23,15 @@ import ( ) var ( - endpoint string - cloudConfig string - cluster string - metricsAddress string - provideControllerService bool - provideNodeService bool - legacyStorageMode bool - legacyVolumeCreation bool + endpoint string + cloudConfig string + cluster string + metricsAddress string + provideControllerService bool + provideNodeService bool + legacyStorageMode bool + legacyVolumeCreation bool + deleteVolumesInErrorState bool ) func main() { @@ -85,6 +86,7 @@ func main() { cmd.PersistentFlags().BoolVar(&legacyStorageMode, "legacy-storage-mode", false, "Configures the CSI to listen to the legacy storage driverName cinder.csi.openstack.org instead") cmd.PersistentFlags().BoolVar(&legacyVolumeCreation, "legacy-volume-creation", true, "Enable or disable support for creating volumes with the old driverName (cinder.csi.openstack.org)") + cmd.PersistentFlags().BoolVar(&deleteVolumesInErrorState, "delete-volumes-in-error", false, "Delete volumes in error state when creating") stackitclient.AddExtraFlags(pflag.CommandLine) @@ -117,6 +119,10 @@ func handle(ctx context.Context) { driverOpts.BlockVolumeCreation = true } + if deleteVolumesInErrorState { + driverOpts.DeleteVolumesInErrorState = true + } + d := blockstorage.NewDriver(driverOpts) if provideControllerService { diff --git a/pkg/csi/blockstorage/controllerserver.go b/pkg/csi/blockstorage/controllerserver.go index 367d8340..2065ca52 100644 --- a/pkg/csi/blockstorage/controllerserver.go +++ b/pkg/csi/blockstorage/controllerserver.go @@ -137,6 +137,9 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol return nil, status.Error(codes.AlreadyExists, "Volume Already exists with same name and different capacity") } if *vols[0].Status != stackitclient.VolumeAvailableStatus { + if cs.Driver.deleteVolumesInErrorState { + cs.deleteVolumeInError(ctx, &vols[0]) + } return nil, status.Error(codes.Internal, fmt.Sprintf("Volume %s is not in available state", *vols[0].Id)) } klog.V(4).Infof("Volume %s already exists in Availability Zone: %s of size %d GiB", *vols[0].Id, vols[0].AvailabilityZone, *vols[0].Size) @@ -265,14 +268,17 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol targetStatus := []string{stackitclient.VolumeAvailableStatus} // Recheck after: 0s (immediate), 20s, 45.6s, 78.36s, 120.31s - err = cloud.WaitVolumeTargetStatusWithCustomBackoff(ctx, *vol.Id, targetStatus, + err = cloud.WaitVolumeTargetStatusWithCustomBackoff(ctx, &vol, targetStatus, &wait.Backoff{ Duration: 20 * time.Second, Steps: 5, Factor: 1.28, }) if err != nil { - klog.Errorf("Failed to WaitVolumeTargetStatus of volume %s: %v", *vol.Id, err) + klog.Errorf("Failed to WaitVolumeTargetStatus of volume %s: %v", vol.GetId(), err) + if cs.Driver.deleteVolumesInErrorState { + cs.deleteVolumeInError(ctx, vol) + } return nil, status.Error(codes.Internal, fmt.Sprintf("CreateVolume Volume %s failed getting available in time: %v", *vol.Id, err)) } @@ -281,6 +287,18 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol return cs.getCreateVolumeResponse(vol), nil } +func (cs *controllerServer) deleteVolumeInError(ctx context.Context, vol *iaas.Volume) { + cloud := cs.Instance + if vol.GetStatus() == stackitclient.VolumeErrorStatus { + klog.Warningf("Volume %s entered ERROR status, attempting cleanup deletion...", vol.GetId()) + if deleteErr := cloud.DeleteVolume(ctx, vol.GetId()); deleteErr != nil { + klog.Errorf("Failed to delete erroneous volume %s: %v", vol.GetId(), deleteErr) + } else { + klog.Infof("Successfully deleted erroneous volume %s", vol.GetId()) + } + } +} + func setVolumeEncryptionParameters(opts *iaas.CreateVolumePayload, volParams *stackitParameterConfig) error { err := validateEncryptionConfig(volParams) if err != nil { diff --git a/pkg/csi/blockstorage/controllerserver_test.go b/pkg/csi/blockstorage/controllerserver_test.go index c4f91be9..edcde647 100644 --- a/pkg/csi/blockstorage/controllerserver_test.go +++ b/pkg/csi/blockstorage/controllerserver_test.go @@ -67,13 +67,15 @@ var _ = Describe("ControllerServer test", Ordered, func() { iaasClient.EXPECT().GetVolumesByName(gomock.Any(), "new volume").Return([]iaas.Volume{}, nil) - iaasClient.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(&iaas.Volume{ + vol := &iaas.Volume{ Id: new("volume-id"), Name: new("new volume"), AvailabilityZone: "eu01", Size: new(int64(20)), - }, nil) - iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), "volume-id", gomock.Any(), gomock.Any()).Return(nil) + } + + iaasClient.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(vol, nil) + iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), &vol, gomock.Any(), gomock.Any()).Return(nil) resp, err := fakeCs.CreateVolume(context.Background(), req) Expect(err).ToNot(HaveOccurred()) @@ -122,13 +124,15 @@ var _ = Describe("ControllerServer test", Ordered, func() { iaasClient.EXPECT().GetVolumesByName(gomock.Any(), "volume name").Return([]iaas.Volume{}, nil) - iaasClient.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(&iaas.Volume{ + vol := &iaas.Volume{ Id: new("volume-id"), Name: new("volume name"), AvailabilityZone: "zone-from-parameters", Size: new(int64(20)), - }, nil) - iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), "volume-id", gomock.Any(), gomock.Any()).Return(nil) + } + + iaasClient.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(vol, nil) + iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), &vol, gomock.Any(), gomock.Any()).Return(nil) _, err := fakeCs.CreateVolume(context.Background(), req) Expect(err).ToNot(HaveOccurred()) @@ -150,13 +154,15 @@ var _ = Describe("ControllerServer test", Ordered, func() { iaasClient.EXPECT().GetVolumesByName(gomock.Any(), "volume name").Return([]iaas.Volume{}, nil) - iaasClient.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(&iaas.Volume{ + vol := &iaas.Volume{ Id: new("volume-id"), Name: new("volume name"), AvailabilityZone: "zone-from-accessibility-reqs", Size: new(int64(20)), - }, nil) - iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), "volume-id", gomock.Any(), gomock.Any()).Return(nil) + } + + iaasClient.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(vol, nil) + iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), &vol, gomock.Any(), gomock.Any()).Return(nil) _, err := fakeCs.CreateVolume(context.Background(), req) Expect(err).ToNot(HaveOccurred()) @@ -303,24 +309,23 @@ var _ = Describe("ControllerServer test", Ordered, func() { VolumeId: "snapshot-volume-id", AvailabilityZone: new("eu01"), }, nil) + + vol := &iaas.Volume{ + Id: new("volume-id"), + Name: new("new volume"), + AvailabilityZone: "eu01", + Size: new(int64(20)), + } + iaasClient.EXPECT(). CreateVolume(gomock.Any(), gomock.Any()). DoAndReturn(func(_ context.Context, opts iaas.CreateVolumePayload) (*iaas.Volume, error) { Expect(opts.Source.Id).To(Equal("snapshot-id")) Expect(opts.Source.Type).To(Equal("snapshot")) - volumeID := "volume-id" - name := "new volume" - size := int64(20) - - return &iaas.Volume{ - Id: &volumeID, - Name: &name, - AvailabilityZone: "eu01", - Size: &size, - }, nil + return vol, nil }) - iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), "volume-id", gomock.Any(), gomock.Any()).Return(nil) + iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), &vol, gomock.Any(), gomock.Any()).Return(nil) _, err := fakeCs.CreateVolume(context.Background(), req) Expect(err).ToNot(HaveOccurred()) @@ -379,24 +384,23 @@ var _ = Describe("ControllerServer test", Ordered, func() { Status: new("AVAILABLE"), AvailabilityZone: new("eu01"), }, nil) + + vol := &iaas.Volume{ + Id: new("volume-id"), + Name: new("new volume"), + AvailabilityZone: "eu01", + Size: new(int64(20)), + } + iaasClient.EXPECT(). CreateVolume(gomock.Any(), gomock.Any()). DoAndReturn(func(_ context.Context, opts iaas.CreateVolumePayload) (*iaas.Volume, error) { Expect(opts.Source.Id).To(Equal("snapshot-id")) Expect(opts.Source.Type).To(Equal("backup")) - volumeID := "volume-id" - name := "new volume" - size := int64(20) - - return &iaas.Volume{ - Id: &volumeID, - Name: &name, - AvailabilityZone: "eu01", - Size: &size, - }, nil + return vol, nil }) - iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), "volume-id", gomock.Any(), gomock.Any()).Return(nil) + iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), &vol, gomock.Any(), gomock.Any()).Return(nil) _, err := fakeCs.CreateVolume(context.Background(), req) Expect(err).ToNot(HaveOccurred()) @@ -490,24 +494,23 @@ var _ = Describe("ControllerServer test", Ordered, func() { Status: new("AVAILABLE"), AvailabilityZone: "eu01", }, nil) + + vol := &iaas.Volume{ + Id: new("volume-id"), + Name: new("new volume"), + AvailabilityZone: "eu01", + Size: new(int64(20)), + } + iaasClient.EXPECT(). CreateVolume(gomock.Any(), gomock.Any()). DoAndReturn(func(_ context.Context, opts iaas.CreateVolumePayload) (*iaas.Volume, error) { Expect(opts.Source.Id).To(Equal("volume-source-id")) Expect(opts.Source.Type).To(Equal("volume")) - name := "new volume" - volumeID := "volume-id" - size := int64(20) - - return &iaas.Volume{ - Id: &volumeID, - Name: &name, - AvailabilityZone: "eu01", - Size: &size, - }, nil + return vol, nil }) - iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), "volume-id", gomock.Any(), gomock.Any()).Return(nil) + iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), &vol, gomock.Any(), gomock.Any()).Return(nil) _, err := fakeCs.CreateVolume(context.Background(), req) Expect(err).ToNot(HaveOccurred()) @@ -578,13 +581,15 @@ var _ = Describe("ControllerServer test", Ordered, func() { iaasClient.EXPECT().GetVolumesByName(gomock.Any(), "new volume").Return([]iaas.Volume{}, nil) - iaasClient.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(&iaas.Volume{ + vol := &iaas.Volume{ Id: new("volume-id"), Name: new("new volume"), AvailabilityZone: "eu01", Size: new(int64(20)), - }, nil) - iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), "volume-id", gomock.Any(), gomock.Any()). + } + + iaasClient.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(vol, nil) + iaasClient.EXPECT().WaitVolumeTargetStatusWithCustomBackoff(gomock.Any(), &vol, gomock.Any(), gomock.Any()). Return(fmt.Errorf("injected error")) _, err := fakeCs.CreateVolume(context.Background(), req) diff --git a/pkg/csi/blockstorage/driver.go b/pkg/csi/blockstorage/driver.go index 60e3b6f9..41948708 100644 --- a/pkg/csi/blockstorage/driver.go +++ b/pkg/csi/blockstorage/driver.go @@ -31,12 +31,13 @@ var ( ) type Driver struct { - name string - fqVersion string // Fully qualified version in format {Version}@{CPO version} - endpoint string - clusterID string - legacyDriver bool - blockVolumeCreation bool + name string + fqVersion string // Fully qualified version in format {Version}@{CPO version} + endpoint string + clusterID string + legacyDriver bool + blockVolumeCreation bool + deleteVolumesInErrorState bool ids *identityServer cs *controllerServer @@ -51,10 +52,11 @@ type Driver struct { } type DriverOpts struct { - ClusterID string - Endpoint string - LegacyDriverName bool - BlockVolumeCreation bool + ClusterID string + Endpoint string + LegacyDriverName bool + BlockVolumeCreation bool + DeleteVolumesInErrorState bool PVCLister corev1.PersistentVolumeClaimLister } @@ -73,6 +75,10 @@ func NewDriver(o *DriverOpts) *Driver { d.legacyDriver = true } + if o.DeleteVolumesInErrorState { + d.deleteVolumesInErrorState = true + } + if o.BlockVolumeCreation { d.blockVolumeCreation = true } diff --git a/pkg/stackit/client/iaas.go b/pkg/stackit/client/iaas.go index 56ed1588..0da8fc84 100644 --- a/pkg/stackit/client/iaas.go +++ b/pkg/stackit/client/iaas.go @@ -49,12 +49,13 @@ type IaaSClient interface { WaitVolumeTargetStatus(ctx context.Context, volumeID string, tStatus []string) error WaitDiskAttached(ctx context.Context, instanceID, volumeID string) error WaitDiskDetached(ctx context.Context, instanceID, volumeID string) error - WaitVolumeTargetStatusWithCustomBackoff(ctx context.Context, volumeID string, tStatus []string, backoff *wait.Backoff) error + WaitVolumeTargetStatusWithCustomBackoff(ctx context.Context, vol **iaas.Volume, tStatus []string, backoff *wait.Backoff) error } const ( VolumeAvailableStatus = "AVAILABLE" VolumeAttachedStatus = "ATTACHED" + VolumeErrorStatus = "ERROR" operationFinishInitDelay = 1 * time.Second operationFinishFactor = 1.1 operationFinishSteps = 10 @@ -542,25 +543,31 @@ func (i *iaasClient) DetachVolume(ctx context.Context, serverID, volumeID string return nil } -func (i *iaasClient) WaitVolumeTargetStatusWithCustomBackoff(ctx context.Context, volumeID string, tStatus []string, backoff *wait.Backoff) error { +func (i *iaasClient) WaitVolumeTargetStatusWithCustomBackoff(ctx context.Context, vol **iaas.Volume, tStatus []string, backoff *wait.Backoff) error { + volID := (*vol).GetId() + waitErr := wait.ExponentialBackoff(*backoff, func() (bool, error) { - vol, err := i.GetVolume(ctx, volumeID) + updatedVol, err := i.GetVolume(ctx, volID) if err != nil { return false, err } - if slices.Contains(tStatus, *vol.Status) { + + // Update vol so we can skip having another request + *vol = updatedVol + + if slices.Contains(tStatus, updatedVol.GetStatus()) { return true, nil } for _, eState := range volumeErrorStates { - if *vol.Status == eState { - return false, fmt.Errorf("volume is in error state: %s", *vol.Status) + if updatedVol.GetStatus() == eState { + return false, fmt.Errorf("volume is in error state: %s", updatedVol.GetStatus()) } } return false, nil }) if wait.Interrupted(waitErr) { - waitErr = fmt.Errorf("timeout on waiting for volume %s status to be in %v", volumeID, tStatus) + waitErr = fmt.Errorf("timeout on waiting for volume %s status to be in %v", volID, tStatus) } return waitErr diff --git a/pkg/stackit/client/mock/iaas_mock.go b/pkg/stackit/client/mock/iaas_mock.go index a538deb4..8e25dc83 100644 --- a/pkg/stackit/client/mock/iaas_mock.go +++ b/pkg/stackit/client/mock/iaas_mock.go @@ -973,17 +973,17 @@ func (c *MockIaaSClientWaitVolumeTargetStatusCall) DoAndReturn(f func(context.Co } // WaitVolumeTargetStatusWithCustomBackoff mocks base method. -func (m *MockIaaSClient) WaitVolumeTargetStatusWithCustomBackoff(ctx context.Context, volumeID string, tStatus []string, backoff *wait.Backoff) error { +func (m *MockIaaSClient) WaitVolumeTargetStatusWithCustomBackoff(ctx context.Context, vol **v2api.Volume, tStatus []string, backoff *wait.Backoff) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "WaitVolumeTargetStatusWithCustomBackoff", ctx, volumeID, tStatus, backoff) + ret := m.ctrl.Call(m, "WaitVolumeTargetStatusWithCustomBackoff", ctx, vol, tStatus, backoff) ret0, _ := ret[0].(error) return ret0 } // WaitVolumeTargetStatusWithCustomBackoff indicates an expected call of WaitVolumeTargetStatusWithCustomBackoff. -func (mr *MockIaaSClientMockRecorder) WaitVolumeTargetStatusWithCustomBackoff(ctx, volumeID, tStatus, backoff any) *MockIaaSClientWaitVolumeTargetStatusWithCustomBackoffCall { +func (mr *MockIaaSClientMockRecorder) WaitVolumeTargetStatusWithCustomBackoff(ctx, vol, tStatus, backoff any) *MockIaaSClientWaitVolumeTargetStatusWithCustomBackoffCall { mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WaitVolumeTargetStatusWithCustomBackoff", reflect.TypeOf((*MockIaaSClient)(nil).WaitVolumeTargetStatusWithCustomBackoff), ctx, volumeID, tStatus, backoff) + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WaitVolumeTargetStatusWithCustomBackoff", reflect.TypeOf((*MockIaaSClient)(nil).WaitVolumeTargetStatusWithCustomBackoff), ctx, vol, tStatus, backoff) return &MockIaaSClientWaitVolumeTargetStatusWithCustomBackoffCall{Call: call} } @@ -999,13 +999,13 @@ func (c *MockIaaSClientWaitVolumeTargetStatusWithCustomBackoffCall) Return(arg0 } // Do rewrite *gomock.Call.Do -func (c *MockIaaSClientWaitVolumeTargetStatusWithCustomBackoffCall) Do(f func(context.Context, string, []string, *wait.Backoff) error) *MockIaaSClientWaitVolumeTargetStatusWithCustomBackoffCall { +func (c *MockIaaSClientWaitVolumeTargetStatusWithCustomBackoffCall) Do(f func(context.Context, **v2api.Volume, []string, *wait.Backoff) error) *MockIaaSClientWaitVolumeTargetStatusWithCustomBackoffCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockIaaSClientWaitVolumeTargetStatusWithCustomBackoffCall) DoAndReturn(f func(context.Context, string, []string, *wait.Backoff) error) *MockIaaSClientWaitVolumeTargetStatusWithCustomBackoffCall { +func (c *MockIaaSClientWaitVolumeTargetStatusWithCustomBackoffCall) DoAndReturn(f func(context.Context, **v2api.Volume, []string, *wait.Backoff) error) *MockIaaSClientWaitVolumeTargetStatusWithCustomBackoffCall { c.Call = c.Call.DoAndReturn(f) return c }