diff --git a/builder/azure/chroot/diskattacher.go b/builder/azure/chroot/diskattacher.go index c52e294eb..abeba5af6 100644 --- a/builder/azure/chroot/diskattacher.go +++ b/builder/azure/chroot/diskattacher.go @@ -26,7 +26,7 @@ type DiskAttacher interface { WaitForDetach(ctx context.Context, diskID string) error } -func NewDiskAttacher(azureClient client.AzureClientSet) DiskAttacher { +var NewDiskAttacher = func (azureClient client.AzureClientSet) DiskAttacher { return &diskAttacher{ azcli: azureClient, } diff --git a/builder/azure/chroot/step_attach_disk_test.go b/builder/azure/chroot/step_attach_disk_test.go new file mode 100644 index 000000000..e644b6a78 --- /dev/null +++ b/builder/azure/chroot/step_attach_disk_test.go @@ -0,0 +1,131 @@ +package chroot + +import ( + "context" + "errors" + "io/ioutil" + "net/http" + "reflect" + "strings" + "testing" + + "github.com/Azure/azure-sdk-for-go/profiles/latest/compute/mgmt/compute" + "github.com/Azure/go-autorest/autorest" + "github.com/hashicorp/packer/builder/azure/common/client" + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" +) + +func TestStepAttachDisk_Run(t *testing.T) { + type fields struct { + GetDiskResponseCode int + GetDiskResponseBody string + + attachError error + waitForDeviceError error + } + tests := []struct { + name string + fields fields + want multistep.StepAction + }{ + { + name: "HappyPath", + want: multistep.ActionContinue, + }, + { + name: "AttachError", + fields: fields{ + attachError: errors.New("unit test"), + }, + want: multistep.ActionHalt, + }, + { + name: "WaitForDeviceError", + fields: fields{ + waitForDeviceError: errors.New("unit test"), + }, + want: multistep.ActionHalt, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &StepAttachDisk{} + + NewDiskAttacher = func(azcli client.AzureClientSet) DiskAttacher { + return &fakeDiskAttacher{ + attachError: tt.fields.attachError, + waitForDeviceError: tt.fields.waitForDeviceError, + } + } + + dm := compute.NewDisksClient("subscriptionId") + dm.Sender = autorest.SenderFunc(func(r *http.Request) (*http.Response, error) { + return &http.Response{ + Request: r, + Body: ioutil.NopCloser(strings.NewReader(tt.fields.GetDiskResponseBody)), + StatusCode: tt.fields.GetDiskResponseCode, + }, nil + }) + + errorBuffer := &strings.Builder{} + ui := &packer.BasicUi{ + Reader: strings.NewReader(""), + Writer: ioutil.Discard, + ErrorWriter: errorBuffer, + } + + state := new(multistep.BasicStateBag) + state.Put("azureclient", &client.AzureClientSetMock{}) + state.Put("ui", ui) + state.Put("os_disk_resource_id", "/subscriptions/12345/resourceGroups/group1/providers/Microsoft.Compute/disks/disk1") + + got := s.Run(context.TODO(), state) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("StepAttachDisk.Run() = %v, want %v", got, tt.want) + } + + if got == multistep.ActionHalt { + if _, ok := state.GetOk("error"); !ok { + t.Fatal("Expected 'error' to be set in statebag after failure") + } + } + }) + } +} + +type fakeDiskAttacher struct { + attachError error + waitForDeviceError error +} + +var _ DiskAttacher = &fakeDiskAttacher{} + +func (da *fakeDiskAttacher) AttachDisk(ctx context.Context, disk string) (lun int32, err error) { + if da.attachError != nil { + return 0, da.attachError + } + return 3, nil +} + +func (da *fakeDiskAttacher) DiskPathForLun(lun int32) string { + panic("not implemented") +} + +func (da *fakeDiskAttacher) WaitForDevice(ctx context.Context, lun int32) (device string, err error) { + if da.waitForDeviceError != nil { + return "", da.waitForDeviceError + } + if lun == 3 { + return "/dev/sdq", nil + } + panic("expected lun==3") +} + +func (da *fakeDiskAttacher) DetachDisk(ctx context.Context, disk string) (err error) { + panic("not implemented") +} + +func (da *fakeDiskAttacher) WaitForDetach(ctx context.Context, diskID string) error { + panic("not implemented") +} diff --git a/builder/azure/chroot/step_verify_source_disk.go b/builder/azure/chroot/step_verify_source_disk.go index b949447cb..04879abd9 100644 --- a/builder/azure/chroot/step_verify_source_disk.go +++ b/builder/azure/chroot/step_verify_source_disk.go @@ -3,6 +3,7 @@ package chroot import ( "context" "fmt" + "log" "strings" "github.com/Azure/go-autorest/autorest/azure" @@ -26,36 +27,51 @@ func (s StepVerifySourceDisk) Run(ctx context.Context, state multistep.StateBag) ui.Say("Checking source disk location") resource, err := azure.ParseResourceID(s.SourceDiskResourceID) if err != nil { - ui.Error(fmt.Sprintf("Could not parse resource id %q: %s", s.SourceDiskResourceID, err)) + log.Printf("StepVerifySourceDisk.Run: error: %+v", err) + err := fmt.Errorf("Could not parse resource id %q: %s", s.SourceDiskResourceID, err) + state.Put("error", err) + ui.Error(err.Error()) return multistep.ActionHalt } if !strings.EqualFold(resource.SubscriptionID, s.SubscriptionID) { - ui.Error(fmt.Sprintf("Source disk resource %q is in a different subscription than this VM (%q). "+ + err := fmt.Errorf("Source disk resource %q is in a different subscription than this VM (%q). "+ "Packer does not know how to handle that.", - s.SourceDiskResourceID, s.SubscriptionID)) + s.SourceDiskResourceID, s.SubscriptionID) + log.Printf("StepVerifySourceDisk.Run: error: %+v", err) + state.Put("error", err) + ui.Error(err.Error()) return multistep.ActionHalt } if !(strings.EqualFold(resource.Provider, "Microsoft.Compute") && strings.EqualFold(resource.ResourceType, "disks")) { - ui.Error(fmt.Sprintf("Resource ID %q is not a managed disk resource", s.SourceDiskResourceID)) + err := fmt.Errorf("Resource ID %q is not a managed disk resource", s.SourceDiskResourceID) + log.Printf("StepVerifySourceDisk.Run: error: %+v", err) + state.Put("error", err) + ui.Error(err.Error()) return multistep.ActionHalt } disk, err := azcli.DisksClient().Get(ctx, resource.ResourceGroup, resource.ResourceName) if err != nil { - ui.Error(fmt.Sprintf("Unable to retrieve disk (%q): %s", s.SourceDiskResourceID, err)) + err := fmt.Errorf("Unable to retrieve disk (%q): %s", s.SourceDiskResourceID, err) + log.Printf("StepVerifySourceDisk.Run: error: %+v", err) + state.Put("error", err) + ui.Error(err.Error()) return multistep.ActionHalt } location := to.String(disk.Location) if !strings.EqualFold(location, s.Location) { - ui.Error(fmt.Sprintf("Source disk resource %q is in a different location (%q) than this VM (%q). "+ + err := fmt.Errorf("Source disk resource %q is in a different location (%q) than this VM (%q). "+ "Packer does not know how to handle that.", s.SourceDiskResourceID, location, - s.Location)) + s.Location) + log.Printf("StepVerifySourceDisk.Run: error: %+v", err) + state.Put("error", err) + ui.Error(err.Error()) return multistep.ActionHalt } diff --git a/builder/azure/chroot/step_verify_source_disk_test.go b/builder/azure/chroot/step_verify_source_disk_test.go index 113fd3f73..3f60e5c57 100644 --- a/builder/azure/chroot/step_verify_source_disk_test.go +++ b/builder/azure/chroot/step_verify_source_disk_test.go @@ -47,6 +47,19 @@ func Test_StepVerifySourceDisk_Run(t *testing.T) { }, want: multistep.ActionContinue, }, + { + name: "NotAResourceID", + fields: fields{ + SubscriptionID: "subid1", + SourceDiskResourceID: "/other", + Location: "westus2", + + GetDiskResponseCode: 200, + GetDiskResponseBody: `{"location":"westus2"}`, + }, + want: multistep.ActionHalt, + errormatch: "Could not parse resource id", + }, { name: "DiskNotFound", fields: fields{ @@ -115,6 +128,7 @@ func Test_StepVerifySourceDisk_Run(t *testing.T) { StatusCode: tt.fields.GetDiskResponseCode, }, nil }) + errorBuffer := &strings.Builder{} ui := &packer.BasicUi{ Reader: strings.NewReader(""), @@ -128,14 +142,22 @@ func Test_StepVerifySourceDisk_Run(t *testing.T) { }) state.Put("ui", ui) - if got := s.Run(context.TODO(), state); !reflect.DeepEqual(got, tt.want) { + got := s.Run(context.TODO(), state); + if !reflect.DeepEqual(got, tt.want) { t.Errorf("StepVerifySourceDisk.Run() = %v, want %v", got, tt.want) } + if tt.errormatch != "" { if !regexp.MustCompile(tt.errormatch).MatchString(errorBuffer.String()) { t.Errorf("Expected the error output (%q) to match %q", errorBuffer.String(), tt.errormatch) } } + + if got == multistep.ActionHalt { + if _, ok := state.GetOk("error"); !ok { + t.Fatal("Expected 'error' to be set in statebag after failure") + } + } }) } }