diff --git a/internal/target/openstack.go b/internal/target/openstack.go index 20ce4d6..ad1e276 100644 --- a/internal/target/openstack.go +++ b/internal/target/openstack.go @@ -21,9 +21,11 @@ import ( ) type OpenStack struct { - VirtualMachine *object.VirtualMachine - Disk *types.VirtualDisk - ClientSet *openstack.ClientSet + VirtualMachine *object.VirtualMachine + Disk *types.VirtualDisk + ClientSet *openstack.ClientSet + attachedInstanceUUID string + attachedVolumeID string } type VolumeCreateOpts struct { @@ -32,6 +34,26 @@ type VolumeCreateOpts struct { BusType string } +var ( + getVolumeForDisk = func(ctx context.Context, clientSet *openstack.ClientSet, vm *object.VirtualMachine, disk *types.VirtualDisk) (*volumes.Volume, error) { + return clientSet.GetVolumeForDisk(ctx, vm, disk) + } + getCurrentInstanceUUID = openstack.GetCurrentInstanceUUID + findVolumeDevice = findDevice + attachVolume = func(ctx context.Context, t *OpenStack, instanceUUID, volumeID string) error { + _, err := volumeattach.Create(ctx, t.ClientSet.Compute, instanceUUID, volumeattach.CreateOpts{ + VolumeID: volumeID, + }).Extract() + return err + } + detachVolume = func(ctx context.Context, t *OpenStack, instanceUUID, volumeID string) error { + return volumeattach.Delete(ctx, t.ClientSet.Compute, instanceUUID, volumeID).ExtractErr() + } + waitVolumeStatus = volumes.WaitForStatus + attachPollTimeout = 2 * time.Minute + attachPollEvery = time.Second +) + func NewOpenStack(ctx context.Context, vm *object.VirtualMachine, disk *types.VirtualDisk) (*OpenStack, error) { clientSet, err := openstack.NewClientSet(ctx) if err != nil { @@ -70,7 +92,7 @@ func findDevice(volumeID string) (string, error) { } func (t *OpenStack) Connect(ctx context.Context) error { - volume, err := t.ClientSet.GetVolumeForDisk(ctx, t.VirtualMachine, t.Disk) + volume, err := getVolumeForDisk(ctx, t.ClientSet, t.VirtualMachine, t.Disk) volumeMetadata := map[string]string{ "migrate_kit": "true", "vm": t.VirtualMachine.Reference().Value, @@ -186,7 +208,7 @@ func (t *OpenStack) Connect(ctx context.Context) error { } if path == "" { - instanceUUID, err := openstack.GetCurrentInstanceUUID() + instanceUUID, err := getCurrentInstanceUUID() if err != nil { return err } @@ -195,15 +217,14 @@ func (t *OpenStack) Connect(ctx context.Context) error { "instance_uuid": instanceUUID, }).Info("Detected instance UUID, attaching volume...") - _, err = volumeattach.Create(ctx, t.ClientSet.Compute, instanceUUID, volumeattach.CreateOpts{ - VolumeID: volume.ID, - }).Extract() - if err != nil { + if err := attachVolume(ctx, t, instanceUUID, volume.ID); err != nil { return err } + t.attachedInstanceUUID = instanceUUID + t.attachedVolumeID = volume.ID - timeoutTimer := time.After(2 * time.Minute) - ticker := time.NewTicker(1 * time.Second) + timeoutTimer := time.After(attachPollTimeout) + ticker := time.NewTicker(attachPollEvery) defer ticker.Stop() for { @@ -211,7 +232,7 @@ func (t *OpenStack) Connect(ctx context.Context) error { case <-timeoutTimer: return errors.New("timed out waiting for volume to attach") case <-ticker.C: - devicePath, err := findDevice(volume.ID) + devicePath, err := findVolumeDevice(volume.ID) if err != nil { return err } @@ -260,12 +281,12 @@ func (t *OpenStack) createVolume(ctx context.Context, opts *VolumeCreateOpts, me } func (t *OpenStack) GetPath(ctx context.Context) (string, error) { - volume, err := t.ClientSet.GetVolumeForDisk(ctx, t.VirtualMachine, t.Disk) + volume, err := getVolumeForDisk(ctx, t.ClientSet, t.VirtualMachine, t.Disk) if err != nil { return "", err } - devicePath, err := findDevice(volume.ID) + devicePath, err := findVolumeDevice(volume.ID) if err != nil { return "", err } @@ -274,33 +295,24 @@ func (t *OpenStack) GetPath(ctx context.Context) (string, error) { } func (t *OpenStack) Disconnect(ctx context.Context) error { - volume, err := t.ClientSet.GetVolumeForDisk(ctx, t.VirtualMachine, t.Disk) + volume, err := getVolumeForDisk(ctx, t.ClientSet, t.VirtualMachine, t.Disk) if errors.Is(err, openstack.ErrorVolumeNotFound) { return nil } else if err != nil { return err } - devicePath, err := findDevice(volume.ID) - if err != nil { - return err - } - - if devicePath != "" { - instanceUUID, err := openstack.GetCurrentInstanceUUID() - if err != nil { - return err - } - - err = volumeattach.Delete(ctx, t.ClientSet.Compute, instanceUUID, volume.ID).ExtractErr() - if err != nil { + if t.attachedVolumeID != "" { + if err := detachVolume(ctx, t, t.attachedInstanceUUID, t.attachedVolumeID); err != nil { return err } + t.attachedInstanceUUID = "" + t.attachedVolumeID = "" ctx, cancel := context.WithTimeout(ctx, 60*time.Second) defer cancel() - err = volumes.WaitForStatus(ctx, t.ClientSet.BlockStorage, volume.ID, "available") + err = waitVolumeStatus(ctx, t.ClientSet.BlockStorage, volume.ID, "available") if err != nil { return errors.Join(errors.New("timed out waiting for volume to be available"), err) } diff --git a/internal/target/openstack_test.go b/internal/target/openstack_test.go new file mode 100644 index 0000000..8f9358e --- /dev/null +++ b/internal/target/openstack_test.go @@ -0,0 +1,198 @@ +package target + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/gophercloud/gophercloud/v2" + "github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes" + "github.com/vexxhost/migratekit/internal/openstack" + "github.com/vmware/govmomi/object" + "github.com/vmware/govmomi/vim25/types" +) + +func testContext() context.Context { + ctx := context.Background() + ctx = context.WithValue(ctx, "volumeCreateOpts", &VolumeCreateOpts{}) + return ctx +} + +func testTarget() *OpenStack { + return &OpenStack{ + ClientSet: &openstack.ClientSet{}, + VirtualMachine: object.NewVirtualMachine(nil, types.ManagedObjectReference{ + Type: "VirtualMachine", + Value: "vm-123", + }), + Disk: &types.VirtualDisk{ + VirtualDevice: types.VirtualDevice{ + Key: 2000, + }, + }, + } +} + +func stubVolumeHooks(t *testing.T, volume *volumes.Volume) { + t.Helper() + + oldGetVolumeForDisk := getVolumeForDisk + oldGetCurrentInstanceUUID := getCurrentInstanceUUID + oldFindVolumeDevice := findVolumeDevice + oldAttachVolume := attachVolume + oldDetachVolume := detachVolume + oldWaitVolumeStatus := waitVolumeStatus + oldAttachPollTimeout := attachPollTimeout + oldAttachPollEvery := attachPollEvery + + getVolumeForDisk = func(context.Context, *openstack.ClientSet, *object.VirtualMachine, *types.VirtualDisk) (*volumes.Volume, error) { + return volume, nil + } + getCurrentInstanceUUID = func() (string, error) { + return "", errors.New("unexpected metadata lookup") + } + findVolumeDevice = func(string) (string, error) { + return "", errors.New("unexpected device lookup") + } + attachVolume = func(context.Context, *OpenStack, string, string) error { + return errors.New("unexpected volume attach") + } + detachVolume = func(context.Context, *OpenStack, string, string) error { + return errors.New("unexpected volume detach") + } + waitVolumeStatus = func(context.Context, *gophercloud.ServiceClient, string, string) error { + return errors.New("unexpected volume status wait") + } + attachPollTimeout = time.Second + attachPollEvery = time.Millisecond + + t.Cleanup(func() { + getVolumeForDisk = oldGetVolumeForDisk + getCurrentInstanceUUID = oldGetCurrentInstanceUUID + findVolumeDevice = oldFindVolumeDevice + attachVolume = oldAttachVolume + detachVolume = oldDetachVolume + waitVolumeStatus = oldWaitVolumeStatus + attachPollTimeout = oldAttachPollTimeout + attachPollEvery = oldAttachPollEvery + }) +} + +func TestConnectPreAttachedVolumeSkipsMetadataAndAttach(t *testing.T) { + volume := &volumes.Volume{ID: "volume-123"} + stubVolumeHooks(t, volume) + + var deviceLookups int + findVolumeDevice = func(volumeID string) (string, error) { + deviceLookups++ + if volumeID != volume.ID { + t.Fatalf("volume ID = %q, want %q", volumeID, volume.ID) + } + return "/dev/sdb", nil + } + + target := testTarget() + if err := target.Connect(testContext()); err != nil { + t.Fatalf("Connect returned error: %v", err) + } + + if deviceLookups != 1 { + t.Fatalf("device lookups = %d, want 1", deviceLookups) + } + if target.attachedInstanceUUID != "" || target.attachedVolumeID != "" { + t.Fatalf("target recorded attachment ownership: instance=%q volume=%q", target.attachedInstanceUUID, target.attachedVolumeID) + } +} + +func TestConnectAutoAttachUsesMetadataAndRecordsOwnership(t *testing.T) { + volume := &volumes.Volume{ID: "volume-123"} + stubVolumeHooks(t, volume) + + var deviceLookups int + findVolumeDevice = func(volumeID string) (string, error) { + deviceLookups++ + if volumeID != volume.ID { + t.Fatalf("volume ID = %q, want %q", volumeID, volume.ID) + } + if deviceLookups == 1 { + return "", nil + } + return "/dev/sdb", nil + } + + var metadataLookups int + getCurrentInstanceUUID = func() (string, error) { + metadataLookups++ + return "instance-123", nil + } + + var attachedInstanceUUID, attachedVolumeID string + attachVolume = func(_ context.Context, _ *OpenStack, instanceUUID, volumeID string) error { + attachedInstanceUUID = instanceUUID + attachedVolumeID = volumeID + return nil + } + + target := testTarget() + if err := target.Connect(testContext()); err != nil { + t.Fatalf("Connect returned error: %v", err) + } + + if metadataLookups != 1 { + t.Fatalf("metadata lookups = %d, want 1", metadataLookups) + } + if attachedInstanceUUID != "instance-123" || attachedVolumeID != volume.ID { + t.Fatalf("attached instance=%q volume=%q, want instance-123/%s", attachedInstanceUUID, attachedVolumeID, volume.ID) + } + if target.attachedInstanceUUID != "instance-123" || target.attachedVolumeID != volume.ID { + t.Fatalf("target ownership instance=%q volume=%q, want instance-123/%s", target.attachedInstanceUUID, target.attachedVolumeID, volume.ID) + } +} + +func TestDisconnectPreAttachedVolumeDoesNotDetach(t *testing.T) { + volume := &volumes.Volume{ID: "volume-123"} + stubVolumeHooks(t, volume) + + target := testTarget() + if err := target.Disconnect(context.Background()); err != nil { + t.Fatalf("Disconnect returned error: %v", err) + } +} + +func TestDisconnectAutoAttachedVolumeDetachesSavedAttachment(t *testing.T) { + volume := &volumes.Volume{ID: "volume-123"} + stubVolumeHooks(t, volume) + + var detachedInstanceUUID, detachedVolumeID string + detachVolume = func(_ context.Context, _ *OpenStack, instanceUUID, volumeID string) error { + detachedInstanceUUID = instanceUUID + detachedVolumeID = volumeID + return nil + } + + var waitedVolumeID, waitedStatus string + waitVolumeStatus = func(_ context.Context, _ *gophercloud.ServiceClient, volumeID, status string) error { + waitedVolumeID = volumeID + waitedStatus = status + return nil + } + + target := testTarget() + target.attachedInstanceUUID = "instance-123" + target.attachedVolumeID = volume.ID + + if err := target.Disconnect(context.Background()); err != nil { + t.Fatalf("Disconnect returned error: %v", err) + } + + if detachedInstanceUUID != "instance-123" || detachedVolumeID != volume.ID { + t.Fatalf("detached instance=%q volume=%q, want instance-123/%s", detachedInstanceUUID, detachedVolumeID, volume.ID) + } + if waitedVolumeID != volume.ID || waitedStatus != "available" { + t.Fatalf("waited for volume=%q status=%q, want %s/available", waitedVolumeID, waitedStatus, volume.ID) + } + if target.attachedInstanceUUID != "" || target.attachedVolumeID != "" { + t.Fatalf("target ownership was not cleared: instance=%q volume=%q", target.attachedInstanceUUID, target.attachedVolumeID) + } +}