Skip to content

fix(target): track owned volume attachments #155

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.

Sign up for GitHub

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
Rico Lin (ricolin) wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(target): track owned volume attachments #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
70 changes: 41 additions & 29 deletions internal/target/openstack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Expand All @@ -195,23 +217,22 @@ 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 {
select {
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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
Expand Down
198 changes: 198 additions & 0 deletions internal/target/openstack_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
}
}