diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 5cb066219e..8232d64d7a 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -792,6 +792,11 @@ var ( // InstanceStateDeleted is the string representing an instance in a deleted state. InstanceStateDeleted = InstanceState("DELETED") + // InstanceStateSoftDeleted is the string representing an instance in a soft-deleted state. + // This state occurs when OpenStack is configured with a reclaim_instance_interval > 0, + // allowing recovery of deleted instances within the reclaim period. + InstanceStateSoftDeleted = InstanceState("SOFT_DELETED") + // InstanceStateUndefined is the string representing an undefined instance state. InstanceStateUndefined = InstanceState("") ) diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go index 255faf6d5c..cd367eb797 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -491,10 +491,15 @@ func (s *Service) DeleteInstance(eventObject runtime.Object, instanceStatus *Ins if err != nil { return false, err } - if i != nil { - return false, nil + // Server not found means it has been permanently deleted + if i == nil { + return true, nil + } + // Server in SOFT_DELETED or DELETED state means deletion succeeded. This respects OpenStack's soft delete policy. + if i.State() == infrav1.InstanceStateSoftDeleted || i.State() == infrav1.InstanceStateDeleted { + return true, nil } - return true, nil + return false, nil }) if err != nil { record.Warnf(eventObject, "FailedDeleteServer", "Failed to delete server %s with id %s: %v", instance.Name, instance.ID, err) diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go index 32adcea7a4..da86f66117 100644 --- a/pkg/cloud/services/compute/instance_test.go +++ b/pkg/cloud/services/compute/instance_test.go @@ -26,6 +26,7 @@ import ( "github.com/go-logr/logr/testr" "github.com/google/go-cmp/cmp" + "github.com/gophercloud/gophercloud/v2" "github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/keypairs" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" @@ -1021,3 +1022,101 @@ func TestService_ReconcileInstance(t *testing.T) { }) } } + +func TestService_DeleteInstance(t *testing.T) { + const ( + serverID = "ce96e584-7ebc-46d6-9e55-987d72e3806c" + serverName = "test-server" + ) + + tests := []struct { + name string + expect func(m *mock.MockComputeClientMockRecorder) + wantErr bool + }{ + { + name: "Server not found after delete", + expect: func(m *mock.MockComputeClientMockRecorder) { + m.DeleteServer(serverID).Return(nil) + m.GetServer(serverID).Return(nil, &gophercloud.ErrResourceNotFound{}) + }, + wantErr: false, + }, + { + name: "Server in SOFT_DELETED state", + expect: func(m *mock.MockComputeClientMockRecorder) { + m.DeleteServer(serverID).Return(nil) + m.GetServer(serverID).Return(&servers.Server{ + ID: serverID, + Name: serverName, + Status: "SOFT_DELETED", + }, nil) + }, + wantErr: false, + }, + { + name: "Server in DELETED state", + expect: func(m *mock.MockComputeClientMockRecorder) { + m.DeleteServer(serverID).Return(nil) + m.GetServer(serverID).Return(&servers.Server{ + ID: serverID, + Name: serverName, + Status: "DELETED", + }, nil) + }, + wantErr: false, + }, + { + name: "Delete API returns not found", + expect: func(m *mock.MockComputeClientMockRecorder) { + m.DeleteServer(serverID).Return(&gophercloud.ErrResourceNotFound{}) + }, + wantErr: false, + }, + { + name: "Delete API returns error", + expect: func(m *mock.MockComputeClientMockRecorder) { + m.DeleteServer(serverID).Return(errors.New("API error")) + }, + wantErr: true, + }, + { + name: "GetServer returns error", + expect: func(m *mock.MockComputeClientMockRecorder) { + m.DeleteServer(serverID).Return(nil) + m.GetServer(serverID).Return(nil, errors.New("API error")) + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockCtrl := gomock.NewController(t) + log := testr.New(t) + mockScopeFactory := scope.NewMockScopeFactory(mockCtrl, "") + + tt.expect(mockScopeFactory.ComputeClient.EXPECT()) + + s, err := NewService(scope.NewWithLogger(mockScopeFactory, log)) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + instanceStatus := &InstanceStatus{ + server: &servers.Server{ + ID: serverID, + Name: serverName, + }, + logger: log, + } + + eventObject := &infrav1.OpenStackMachine{} + err = s.DeleteInstance(eventObject, instanceStatus) + if (err != nil) != tt.wantErr { + t.Errorf("Service.DeleteInstance() error = %v, wantErr %v", err, tt.wantErr) + return + } + }) + } +}