From d12d23d34ba1ca06a6b315576f2f0d63ea61063b Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Mon, 5 Nov 2018 23:48:22 +0000 Subject: [PATCH 01/16] OS disk snapshot --- builder/azure/arm/artifact.go | 20 +++++- builder/azure/arm/artifact_test.go | 2 +- builder/azure/arm/azure_client.go | 7 ++ builder/azure/arm/builder.go | 73 +++++++++++++-------- builder/azure/arm/config.go | 22 +++++++ builder/azure/arm/step_snapshot_os_disk.go | 75 ++++++++++++++++++++++ builder/azure/common/constants/stateBag.go | 2 + 7 files changed, 172 insertions(+), 29 deletions(-) create mode 100644 builder/azure/arm/step_snapshot_os_disk.go diff --git a/builder/azure/arm/artifact.go b/builder/azure/arm/artifact.go index 8a234090d..632389a94 100644 --- a/builder/azure/arm/artifact.go +++ b/builder/azure/arm/artifact.go @@ -33,18 +33,22 @@ type Artifact struct { ManagedImageName string ManagedImageLocation string ManagedImageId string + ManagedImageOSDiskSnapshotName string + ManagedImageDataDiskSnapshotPrefix string // Additional Disks AdditionalDisks *[]AdditionalDiskArtifact } -func NewManagedImageArtifact(osType, resourceGroup, name, location, id string) (*Artifact, error) { +func NewManagedImageArtifact(osType, resourceGroup, name, location, id, osDiskSnapshotName, osDiskSnapshotPrefix string) (*Artifact, error) { return &Artifact{ ManagedImageResourceGroupName: resourceGroup, ManagedImageName: name, ManagedImageLocation: location, ManagedImageId: id, OSType: osType, + ManagedImageOSDiskSnapshotName: osDiskSnapshotName, + ManagedImageDataDiskSnapshotPrefix: osDiskSnapshotPrefix, }, nil } @@ -124,6 +128,14 @@ func (a *Artifact) isManagedImage() bool { return a.ManagedImageResourceGroupName != "" } +func (a *Artifact) takeOSDiskSnapshot() bool { + return a.ManagedImageOSDiskSnapshotName != "" +} + +func (a *Artifact) takeDataDiskSnapshot() bool { + return a.ManagedImageDataDiskSnapshotPrefix != "" +} + func (*Artifact) BuilderId() string { return BuilderId } @@ -158,6 +170,12 @@ func (a *Artifact) String() string { buf.WriteString(fmt.Sprintf("ManagedImageName: %s\n", a.ManagedImageName)) buf.WriteString(fmt.Sprintf("ManagedImageId: %s\n", a.ManagedImageId)) buf.WriteString(fmt.Sprintf("ManagedImageLocation: %s\n", a.ManagedImageLocation)) + if a.takeOSDiskSnapshot() { + buf.WriteString(fmt.Sprintf("ManagedImageOSDiskSnapshotName: %s\n", a.ManagedImageOSDiskSnapshotName)) + } + if a.takeDataDiskSnapshot() { + buf.WriteString(fmt.Sprintf("ManagedImageDataDiskSnapshotPrefix: %s\n", a.ManagedImageDataDiskSnapshotPrefix)) + } } else { buf.WriteString(fmt.Sprintf("StorageAccountLocation: %s\n", a.StorageAccountLocation)) buf.WriteString(fmt.Sprintf("OSDiskUri: %s\n", a.OSDiskUri)) diff --git a/builder/azure/arm/artifact_test.go b/builder/azure/arm/artifact_test.go index 0bfd0d58d..f649db827 100644 --- a/builder/azure/arm/artifact_test.go +++ b/builder/azure/arm/artifact_test.go @@ -42,7 +42,7 @@ func TestArtifactIdVHD(t *testing.T) { } func TestArtifactIDManagedImage(t *testing.T) { - artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID") + artifact, err := NewManagedImageArtifact("Linux","fakeResourceGroup","fakeName","fakeLocation","fakeID","fakeOSDiskSnapshotName","faksOSDiskSnapshotPrefix") if err != nil { t.Fatalf("err=%s", err) } diff --git a/builder/azure/arm/azure_client.go b/builder/azure/arm/azure_client.go index f8477d235..b74e9abe3 100644 --- a/builder/azure/arm/azure_client.go +++ b/builder/azure/arm/azure_client.go @@ -40,6 +40,7 @@ type AzureClient struct { common.VaultClient armStorage.AccountsClient compute.DisksClient + compute.SnapshotsClient InspectorMaxLength int Template *CaptureTemplate @@ -189,6 +190,12 @@ func NewAzureClient(subscriptionID, resourceGroupName, storageAccountName string azureClient.VirtualMachinesClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), templateCapture(azureClient), errorCapture(azureClient)) azureClient.VirtualMachinesClient.UserAgent = fmt.Sprintf("%s %s", useragent.String(), azureClient.VirtualMachinesClient.UserAgent) + azureClient.SnapshotsClient = compute.NewSnapshotsClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) + azureClient.SnapshotsClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken) + azureClient.SnapshotsClient.RequestInspector = withInspection(maxlen) + azureClient.SnapshotsClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), templateCapture(azureClient), errorCapture(azureClient)) + azureClient.SnapshotsClient.UserAgent = fmt.Sprintf("%s %s", useragent.String(), azureClient.SnapshotsClient.UserAgent) + azureClient.AccountsClient = armStorage.NewAccountsClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) azureClient.AccountsClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken) azureClient.AccountsClient.RequestInspector = withInspection(maxlen) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 6ba41523f..2dbc2f845 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -166,31 +166,27 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe deploymentName := b.stateBag.Get(constants.ArmDeploymentName).(string) if b.config.OSType == constants.Target_Linux { - steps = []multistep.Step{ - NewStepCreateResourceGroup(azureClient, ui), - NewStepValidateTemplate(azureClient, ui, b.config, GetVirtualMachineDeployment), - NewStepDeployTemplate(azureClient, ui, b.config, deploymentName, GetVirtualMachineDeployment), - NewStepGetIPAddress(azureClient, ui, endpointConnectType), - &communicator.StepConnectSSH{ - Config: &b.config.Comm, - Host: lin.SSHHost, - SSHConfig: b.config.Comm.SSHConfigFunc(), - }, - &packerCommon.StepProvision{}, - &packerCommon.StepCleanupTempKeys{ - Comm: &b.config.Comm, - }, - NewStepGetOSDisk(azureClient, ui), - NewStepGetAdditionalDisks(azureClient, ui), - NewStepPowerOffCompute(azureClient, ui), - NewStepCaptureImage(azureClient, ui), - NewStepDeleteResourceGroup(azureClient, ui), - NewStepDeleteOSDisk(azureClient, ui), - NewStepDeleteAdditionalDisks(azureClient, ui), - } + steps = append(steps, + NewStepCreateResourceGroup(azureClient, ui), + NewStepValidateTemplate(azureClient, ui, b.config, GetVirtualMachineDeployment), + NewStepDeployTemplate(azureClient, ui, b.config, deploymentName, GetVirtualMachineDeployment), + NewStepGetIPAddress(azureClient, ui, endpointConnectType), + &communicator.StepConnectSSH{ + Config: &b.config.Comm, + Host: lin.SSHHost, + SSHConfig: b.config.Comm.SSHConfigFunc(), + }, + &packerCommon.StepProvision{}, + &packerCommon.StepCleanupTempKeys{ + Comm: &b.config.Comm, + }, + NewStepGetOSDisk(azureClient, ui), + NewStepGetAdditionalDisks(azureClient, ui), + NewStepPowerOffCompute(azureClient, ui), + ) } else if b.config.OSType == constants.Target_Windows { keyVaultDeploymentName := b.stateBag.Get(constants.ArmKeyVaultDeploymentName).(string) - steps = []multistep.Step{ + steps = append(steps, NewStepCreateResourceGroup(azureClient, ui), NewStepValidateTemplate(azureClient, ui, b.config, GetKeyVaultDeployment), NewStepDeployTemplate(azureClient, ui, b.config, keyVaultDeploymentName, GetKeyVaultDeployment), @@ -218,14 +214,35 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe &packerCommon.StepProvision{}, NewStepGetOSDisk(azureClient, ui), NewStepGetAdditionalDisks(azureClient, ui), + ) + } else { + return nil, fmt.Errorf("Builder does not support the os_type '%s'", b.config.OSType) + } + + // if managed image create a new step + if b.config.isManagedImage() { + steps = append(steps, + NewStepSnapshotOSDisk(azureClient, ui), + //NewStepSnapshotDataDisk(azureClient, ui), + ) + } + + // then add back the remaining steps + if b.config.OSType == constants.Target_Linux { + steps = append(steps, + NewStepCaptureImage(azureClient, ui), + NewStepDeleteResourceGroup(azureClient, ui), + NewStepDeleteOSDisk(azureClient, ui), + NewStepDeleteAdditionalDisks(azureClient, ui), + ) + } else { + steps = append(steps, NewStepPowerOffCompute(azureClient, ui), NewStepCaptureImage(azureClient, ui), NewStepDeleteResourceGroup(azureClient, ui), NewStepDeleteOSDisk(azureClient, ui), NewStepDeleteAdditionalDisks(azureClient, ui), - } - } else { - return nil, fmt.Errorf("Builder does not support the os_type '%s'", b.config.OSType) + ) } if b.config.PackerDebug { @@ -259,7 +276,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe if b.config.isManagedImage() { managedImageID := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/images/%s", b.config.SubscriptionID, b.config.ManagedImageResourceGroupName, b.config.ManagedImageName) - return NewManagedImageArtifact(b.config.OSType, b.config.ManagedImageResourceGroupName, b.config.ManagedImageName, b.config.manageImageLocation, managedImageID) + return NewManagedImageArtifact(b.config.OSType, b.config.ManagedImageResourceGroupName, b.config.ManagedImageName, b.config.manageImageLocation, managedImageID, b.config.ManagedImageOSDiskSnapshotName, b.config.ManagedImageDataDiskSnapshotPrefix) } else if template, ok := b.stateBag.GetOk(constants.ArmCaptureTemplate); ok { return NewArtifact( template.(*CaptureTemplate), @@ -361,6 +378,8 @@ func (b *Builder) configureStateBag(stateBag multistep.StateBag) { stateBag.Put(constants.ArmIsManagedImage, b.config.isManagedImage()) stateBag.Put(constants.ArmManagedImageResourceGroupName, b.config.ManagedImageResourceGroupName) stateBag.Put(constants.ArmManagedImageName, b.config.ManagedImageName) + stateBag.Put(constants.ArmManagedImageOSDiskSnapshotName, b.config.ManagedImageOSDiskSnapshotName) + stateBag.Put(constants.ArmManagedImageDataDiskSnapshotPrefix, b.config.ManagedImageDataDiskSnapshotPrefix) stateBag.Put(constants.ArmAsyncResourceGroupDelete, b.config.AsyncResourceGroupDelete) } diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index d72d28550..172241e0c 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -56,6 +56,7 @@ var ( reCaptureNamePrefix = regexp.MustCompile("^[A-Za-z0-9][A-Za-z0-9_\\-\\.]{0,23}$") reManagedDiskName = regexp.MustCompile(validManagedDiskName) reResourceGroupName = regexp.MustCompile(validResourceGroupNameRe) + reSnaspshotNameOrPrefix = regexp.MustCompile("^[A-Za-z0-9_]{0,79}$") ) type PlanInformation struct { @@ -107,6 +108,8 @@ type Config struct { ManagedImageResourceGroupName string `mapstructure:"managed_image_resource_group_name"` ManagedImageName string `mapstructure:"managed_image_name"` ManagedImageStorageAccountType string `mapstructure:"managed_image_storage_account_type"` + ManagedImageOSDiskSnapshotName string `mapstructure:"managed_image_os_disk_snapshot_name"` + ManagedImageDataDiskSnapshotPrefix string `mapstructure:"managed_image_data_disk_snapshot_prefix"` managedImageStorageAccountType compute.StorageAccountTypes manageImageLocation string @@ -688,6 +691,18 @@ func assertRequiredParametersSet(c *Config, errs *packer.MultiError) { } } + if c.ManagedImageOSDiskSnapshotName != "" { + if ok, err := assertManagedImageOSDiskSnapshotNameOrPrefix(c.ManagedImageOSDiskSnapshotName, "managed_image_os_disk_snapshot_name"); !ok { + errs = packer.MultiErrorAppend(errs, err) + } + } + + if c.ManagedImageDataDiskSnapshotPrefix != "" { + if ok, err := assertManagedImageOSDiskSnapshotNameOrPrefix(c.ManagedImageDataDiskSnapshotPrefix, "managed_image_data_disk_snapshot_prefix"); !ok { + errs = packer.MultiErrorAppend(errs, err) + } + } + if c.VirtualNetworkName == "" && c.VirtualNetworkResourceGroupName != "" { errs = packer.MultiErrorAppend(errs, fmt.Errorf("If virtual_network_resource_group_name is specified, so must virtual_network_name")) } @@ -741,6 +756,13 @@ func assertManagedImageName(name, setting string) (bool, error) { return true, nil } +func assertManagedImageOSDiskSnapshotNameOrPrefix(name, setting string) (bool, error) { + if !isValidAzureName(reSnaspshotNameOrPrefix, name) { + return false, fmt.Errorf("The setting %s must only contain characters from a-z, A-Z, 0-9 and _ and the maximum length is 80 characters", setting) + } + return true, nil +} + func assertResourceGroupName(rgn, setting string) (bool, error) { if !isValidAzureName(reResourceGroupName, rgn) { return false, fmt.Errorf("The setting %s must match the regular expression %q, and not end with a '-' or '.'.", setting, validResourceGroupNameRe) diff --git a/builder/azure/arm/step_snapshot_os_disk.go b/builder/azure/arm/step_snapshot_os_disk.go new file mode 100644 index 000000000..c954fdf71 --- /dev/null +++ b/builder/azure/arm/step_snapshot_os_disk.go @@ -0,0 +1,75 @@ +package arm + +import ( + "context" + + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute" + "github.com/Azure/go-autorest/autorest/to" + "github.com/hashicorp/packer/builder/azure/common/constants" + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" +) + +type StepSnapshotOSDisk struct { + client *AzureClient + create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error + say func(message string) + error func(e error) +} + +func NewStepSnapshotOSDisk(client *AzureClient, ui packer.Ui) *StepSnapshotOSDisk { + var step = &StepSnapshotOSDisk{ + client: client, + say: func(message string) { ui.Say(message) }, + error: func(e error) { ui.Error(e.Error()) }, + } + + step.create = step.createSnapshot + return step +} + +func (s *StepSnapshotOSDisk) createSnapshot(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error { + + srcVhdToSnapshot := compute.Snapshot{ + DiskProperties: &compute.DiskProperties{ + CreationData: &compute.CreationData{ + CreateOption: compute.Import, + SourceURI: to.StringPtr(srcUriVhd), + }, + }, + Location: to.StringPtr(location), + Tags: tags, + } + + f, err := s.client.SnapshotsClient.CreateOrUpdate(ctx, resourceGroupName, dstSnapshotName, srcVhdToSnapshot) + + if err != nil { + s.say(s.client.LastError.Error()) + } + + return f.WaitForCompletion(ctx, s.client.SnapshotsClient.Client) +} + +func (s *StepSnapshotOSDisk) Run(ctx context.Context, stateBag multistep.StateBag) multistep.StepAction { + s.say("Taking snapshot of OS disk ...") + + var resourceGroupName = stateBag.Get(constants.ArmResourceGroupName).(string) + var location = stateBag.Get(constants.ArmLocation).(string) + var tags = stateBag.Get(constants.ArmTags).(map[string]*string) + var srcUriVhd = stateBag.Get(constants.ArmOSDiskVhd).(string) + var dstSnapshotName = stateBag.Get(constants.ArmManagedImageOSDiskSnapshotName).(string) + + err := s.create(ctx, resourceGroupName, srcUriVhd, location, tags, dstSnapshotName) + + if err != nil { + stateBag.Put(constants.Error, err) + s.error(err) + + return multistep.ActionHalt + } + + return multistep.ActionContinue +} + +func (*StepSnapshotOSDisk) Cleanup(multistep.StateBag) { +} diff --git a/builder/azure/common/constants/stateBag.go b/builder/azure/common/constants/stateBag.go index 5c78912b1..a21863454 100644 --- a/builder/azure/common/constants/stateBag.go +++ b/builder/azure/common/constants/stateBag.go @@ -35,4 +35,6 @@ const ( ArmManagedImageLocation string = "arm.ManagedImageLocation" ArmManagedImageName string = "arm.ManagedImageName" ArmAsyncResourceGroupDelete string = "arm.AsyncResourceGroupDelete" + ArmManagedImageOSDiskSnapshotName string = "arm.ManagedImageOSDiskSnapshotName" + ArmManagedImageDataDiskSnapshotPrefix string = "arm.ManagedImageDataDiskSnapshotPrefix" ) From 1266d5146dd066a1b73dd4e35ff5a5e4dbf20b36 Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Tue, 6 Nov 2018 19:17:03 +0000 Subject: [PATCH 02/16] addressed PR comments + add file for data disk snapshots --- builder/azure/arm/artifact.go | 24 +++--- builder/azure/arm/artifact_test.go | 2 +- builder/azure/arm/azure_client.go | 2 +- builder/azure/arm/builder.go | 79 ++++++++++--------- builder/azure/arm/config.go | 30 ++++--- builder/azure/arm/step_snapshot_data_disks.go | 79 +++++++++++++++++++ 6 files changed, 152 insertions(+), 64 deletions(-) create mode 100644 builder/azure/arm/step_snapshot_data_disks.go diff --git a/builder/azure/arm/artifact.go b/builder/azure/arm/artifact.go index 632389a94..d45c339ea 100644 --- a/builder/azure/arm/artifact.go +++ b/builder/azure/arm/artifact.go @@ -29,11 +29,11 @@ type Artifact struct { TemplateUriReadOnlySas string // Managed Image - ManagedImageResourceGroupName string - ManagedImageName string - ManagedImageLocation string - ManagedImageId string - ManagedImageOSDiskSnapshotName string + ManagedImageResourceGroupName string + ManagedImageName string + ManagedImageLocation string + ManagedImageId string + ManagedImageosDiskSnapshotName string ManagedImageDataDiskSnapshotPrefix string // Additional Disks @@ -47,7 +47,7 @@ func NewManagedImageArtifact(osType, resourceGroup, name, location, id, osDiskSn ManagedImageLocation: location, ManagedImageId: id, OSType: osType, - ManagedImageOSDiskSnapshotName: osDiskSnapshotName, + ManagedImageosDiskSnapshotName: osDiskSnapshotName, ManagedImageDataDiskSnapshotPrefix: osDiskSnapshotPrefix, }, nil } @@ -128,11 +128,11 @@ func (a *Artifact) isManagedImage() bool { return a.ManagedImageResourceGroupName != "" } -func (a *Artifact) takeOSDiskSnapshot() bool { - return a.ManagedImageOSDiskSnapshotName != "" +func (a *Artifact) osDiskSnapshot() bool { + return a.ManagedImageosDiskSnapshotName != "" } -func (a *Artifact) takeDataDiskSnapshot() bool { +func (a *Artifact) dataDiskSnapshot() bool { return a.ManagedImageDataDiskSnapshotPrefix != "" } @@ -170,10 +170,10 @@ func (a *Artifact) String() string { buf.WriteString(fmt.Sprintf("ManagedImageName: %s\n", a.ManagedImageName)) buf.WriteString(fmt.Sprintf("ManagedImageId: %s\n", a.ManagedImageId)) buf.WriteString(fmt.Sprintf("ManagedImageLocation: %s\n", a.ManagedImageLocation)) - if a.takeOSDiskSnapshot() { - buf.WriteString(fmt.Sprintf("ManagedImageOSDiskSnapshotName: %s\n", a.ManagedImageOSDiskSnapshotName)) + if a.osDiskSnapshot() { + buf.WriteString(fmt.Sprintf("ManagedImageosDiskSnapshotName: %s\n", a.ManagedImageosDiskSnapshotName)) } - if a.takeDataDiskSnapshot() { + if a.dataDiskSnapshot() { buf.WriteString(fmt.Sprintf("ManagedImageDataDiskSnapshotPrefix: %s\n", a.ManagedImageDataDiskSnapshotPrefix)) } } else { diff --git a/builder/azure/arm/artifact_test.go b/builder/azure/arm/artifact_test.go index f649db827..340212bcd 100644 --- a/builder/azure/arm/artifact_test.go +++ b/builder/azure/arm/artifact_test.go @@ -42,7 +42,7 @@ func TestArtifactIdVHD(t *testing.T) { } func TestArtifactIDManagedImage(t *testing.T) { - artifact, err := NewManagedImageArtifact("Linux","fakeResourceGroup","fakeName","fakeLocation","fakeID","fakeOSDiskSnapshotName","faksOSDiskSnapshotPrefix") + artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "fakeOSDiskSnapshotName", "fakeOSDiskSnapshotPrefix") if err != nil { t.Fatalf("err=%s", err) } diff --git a/builder/azure/arm/azure_client.go b/builder/azure/arm/azure_client.go index b74e9abe3..a225ee618 100644 --- a/builder/azure/arm/azure_client.go +++ b/builder/azure/arm/azure_client.go @@ -193,7 +193,7 @@ func NewAzureClient(subscriptionID, resourceGroupName, storageAccountName string azureClient.SnapshotsClient = compute.NewSnapshotsClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) azureClient.SnapshotsClient.Authorizer = autorest.NewBearerAuthorizer(servicePrincipalToken) azureClient.SnapshotsClient.RequestInspector = withInspection(maxlen) - azureClient.SnapshotsClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), templateCapture(azureClient), errorCapture(azureClient)) + azureClient.SnapshotsClient.ResponseInspector = byConcatDecorators(byInspecting(maxlen), errorCapture(azureClient)) azureClient.SnapshotsClient.UserAgent = fmt.Sprintf("%s %s", useragent.String(), azureClient.SnapshotsClient.UserAgent) azureClient.AccountsClient = armStorage.NewAccountsClientWithBaseURI(cloud.ResourceManagerEndpoint, subscriptionID) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 2dbc2f845..debc6108b 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -166,27 +166,40 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe deploymentName := b.stateBag.Get(constants.ArmDeploymentName).(string) if b.config.OSType == constants.Target_Linux { - steps = append(steps, - NewStepCreateResourceGroup(azureClient, ui), - NewStepValidateTemplate(azureClient, ui, b.config, GetVirtualMachineDeployment), - NewStepDeployTemplate(azureClient, ui, b.config, deploymentName, GetVirtualMachineDeployment), - NewStepGetIPAddress(azureClient, ui, endpointConnectType), - &communicator.StepConnectSSH{ - Config: &b.config.Comm, - Host: lin.SSHHost, - SSHConfig: b.config.Comm.SSHConfigFunc(), - }, - &packerCommon.StepProvision{}, - &packerCommon.StepCleanupTempKeys{ - Comm: &b.config.Comm, - }, - NewStepGetOSDisk(azureClient, ui), - NewStepGetAdditionalDisks(azureClient, ui), - NewStepPowerOffCompute(azureClient, ui), + steps = []multistep.Step{ + NewStepCreateResourceGroup(azureClient, ui), + NewStepValidateTemplate(azureClient, ui, b.config, GetVirtualMachineDeployment), + NewStepDeployTemplate(azureClient, ui, b.config, deploymentName, GetVirtualMachineDeployment), + NewStepGetIPAddress(azureClient, ui, endpointConnectType), + &communicator.StepConnectSSH{ + Config: &b.config.Comm, + Host: lin.SSHHost, + SSHConfig: b.config.Comm.SSHConfigFunc(), + }, + &packerCommon.StepProvision{}, + &packerCommon.StepCleanupTempKeys{ + Comm: &b.config.Comm, + }, + NewStepGetOSDisk(azureClient, ui), + NewStepGetAdditionalDisks(azureClient, ui), + NewStepPowerOffCompute(azureClient, ui), + } + // if managed image create snapshot + if b.config.isManagedImage() { + steps = append(steps, + NewStepSnapshotOSDisk(azureClient, ui), + NewStepSnapshotDataDisks(azureClient, ui), ) + } + steps = append(steps, + NewStepCaptureImage(azureClient, ui), + NewStepDeleteResourceGroup(azureClient, ui), + NewStepDeleteOSDisk(azureClient, ui), + NewStepDeleteAdditionalDisks(azureClient, ui), + ) } else if b.config.OSType == constants.Target_Windows { keyVaultDeploymentName := b.stateBag.Get(constants.ArmKeyVaultDeploymentName).(string) - steps = append(steps, + steps = []multistep.Step{ NewStepCreateResourceGroup(azureClient, ui), NewStepValidateTemplate(azureClient, ui, b.config, GetKeyVaultDeployment), NewStepDeployTemplate(azureClient, ui, b.config, keyVaultDeploymentName, GetKeyVaultDeployment), @@ -214,28 +227,14 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe &packerCommon.StepProvision{}, NewStepGetOSDisk(azureClient, ui), NewStepGetAdditionalDisks(azureClient, ui), - ) - } else { - return nil, fmt.Errorf("Builder does not support the os_type '%s'", b.config.OSType) - } - - // if managed image create a new step - if b.config.isManagedImage() { - steps = append(steps, - NewStepSnapshotOSDisk(azureClient, ui), - //NewStepSnapshotDataDisk(azureClient, ui), - ) - } - - // then add back the remaining steps - if b.config.OSType == constants.Target_Linux { - steps = append(steps, - NewStepCaptureImage(azureClient, ui), - NewStepDeleteResourceGroup(azureClient, ui), - NewStepDeleteOSDisk(azureClient, ui), - NewStepDeleteAdditionalDisks(azureClient, ui), + } + // if managed image create snapshot + if b.config.isManagedImage() { + steps = append(steps, + NewStepSnapshotOSDisk(azureClient, ui), + NewStepSnapshotDataDisks(azureClient, ui), ) - } else { + } steps = append(steps, NewStepPowerOffCompute(azureClient, ui), NewStepCaptureImage(azureClient, ui), @@ -243,6 +242,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe NewStepDeleteOSDisk(azureClient, ui), NewStepDeleteAdditionalDisks(azureClient, ui), ) + } else { + return nil, fmt.Errorf("Builder does not support the os_type '%s'", b.config.OSType) } if b.config.PackerDebug { diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index 172241e0c..530439cf4 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -56,7 +56,8 @@ var ( reCaptureNamePrefix = regexp.MustCompile("^[A-Za-z0-9][A-Za-z0-9_\\-\\.]{0,23}$") reManagedDiskName = regexp.MustCompile(validManagedDiskName) reResourceGroupName = regexp.MustCompile(validResourceGroupNameRe) - reSnaspshotNameOrPrefix = regexp.MustCompile("^[A-Za-z0-9_]{0,79}$") + reSnaspshotName = regexp.MustCompile("^[A-Za-z0-9_]{10,79}$") + reSnaspshotPrefix = regexp.MustCompile("^[A-Za-z0-9_]{10,59}$") ) type PlanInformation struct { @@ -105,13 +106,13 @@ type Config struct { Location string `mapstructure:"location"` VMSize string `mapstructure:"vm_size"` - ManagedImageResourceGroupName string `mapstructure:"managed_image_resource_group_name"` - ManagedImageName string `mapstructure:"managed_image_name"` - ManagedImageStorageAccountType string `mapstructure:"managed_image_storage_account_type"` - ManagedImageOSDiskSnapshotName string `mapstructure:"managed_image_os_disk_snapshot_name"` + ManagedImageResourceGroupName string `mapstructure:"managed_image_resource_group_name"` + ManagedImageName string `mapstructure:"managed_image_name"` + ManagedImageStorageAccountType string `mapstructure:"managed_image_storage_account_type"` + ManagedImageOSDiskSnapshotName string `mapstructure:"managed_image_os_disk_snapshot_name"` ManagedImageDataDiskSnapshotPrefix string `mapstructure:"managed_image_data_disk_snapshot_prefix"` - managedImageStorageAccountType compute.StorageAccountTypes - manageImageLocation string + managedImageStorageAccountType compute.StorageAccountTypes + manageImageLocation string // Deployment AzureTags map[string]*string `mapstructure:"azure_tags"` @@ -692,13 +693,13 @@ func assertRequiredParametersSet(c *Config, errs *packer.MultiError) { } if c.ManagedImageOSDiskSnapshotName != "" { - if ok, err := assertManagedImageOSDiskSnapshotNameOrPrefix(c.ManagedImageOSDiskSnapshotName, "managed_image_os_disk_snapshot_name"); !ok { + if ok, err := assertManagedImageOSDiskSnapshotName(c.ManagedImageOSDiskSnapshotName, "managed_image_os_disk_snapshot_name"); !ok { errs = packer.MultiErrorAppend(errs, err) } } if c.ManagedImageDataDiskSnapshotPrefix != "" { - if ok, err := assertManagedImageOSDiskSnapshotNameOrPrefix(c.ManagedImageDataDiskSnapshotPrefix, "managed_image_data_disk_snapshot_prefix"); !ok { + if ok, err := assertManagedImageDataDiskSnapshotName(c.ManagedImageDataDiskSnapshotPrefix, "managed_image_data_disk_snapshot_prefix"); !ok { errs = packer.MultiErrorAppend(errs, err) } } @@ -756,13 +757,20 @@ func assertManagedImageName(name, setting string) (bool, error) { return true, nil } -func assertManagedImageOSDiskSnapshotNameOrPrefix(name, setting string) (bool, error) { - if !isValidAzureName(reSnaspshotNameOrPrefix, name) { +func assertManagedImageOSDiskSnapshotName(name, setting string) (bool, error) { + if !isValidAzureName(reSnaspshotName, name) { return false, fmt.Errorf("The setting %s must only contain characters from a-z, A-Z, 0-9 and _ and the maximum length is 80 characters", setting) } return true, nil } +func assertManagedImageDataDiskSnapshotName(name, setting string) (bool, error) { + if !isValidAzureName(reSnaspshotPrefix, name) { + return false, fmt.Errorf("The setting %s must only contain characters from a-z, A-Z, 0-9 and _ and the maximum length (excluding the prefix) is 60 characters", setting) + } + return true, nil +} + func assertResourceGroupName(rgn, setting string) (bool, error) { if !isValidAzureName(reResourceGroupName, rgn) { return false, fmt.Errorf("The setting %s must match the regular expression %q, and not end with a '-' or '.'.", setting, validResourceGroupNameRe) diff --git a/builder/azure/arm/step_snapshot_data_disks.go b/builder/azure/arm/step_snapshot_data_disks.go new file mode 100644 index 000000000..6535e656b --- /dev/null +++ b/builder/azure/arm/step_snapshot_data_disks.go @@ -0,0 +1,79 @@ +package arm + +import ( + "context" + "strconv" + + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute" + "github.com/Azure/go-autorest/autorest/to" + "github.com/hashicorp/packer/builder/azure/common/constants" + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" +) + +type StepSnapshotDataDisks struct { + client *AzureClient + create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error + say func(message string) + error func(e error) +} + +func NewStepSnapshotDataDisks(client *AzureClient, ui packer.Ui) *StepSnapshotDataDisks { + var step = &StepSnapshotDataDisks{ + client: client, + say: func(message string) { ui.Say(message) }, + error: func(e error) { ui.Error(e.Error()) }, + } + + step.create = step.createDataDiskSnapshot + return step +} + +func (s *StepSnapshotDataDisks) createDataDiskSnapshot(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error { + + srcVhdToSnapshot := compute.Snapshot{ + DiskProperties: &compute.DiskProperties{ + CreationData: &compute.CreationData{ + CreateOption: compute.Import, + SourceURI: to.StringPtr(srcUriVhd), + }, + }, + Location: to.StringPtr(location), + Tags: tags, + } + + f, err := s.client.SnapshotsClient.CreateOrUpdate(ctx, resourceGroupName, dstSnapshotName, srcVhdToSnapshot) + + if err != nil { + s.say(s.client.LastError.Error()) + } + + return f.WaitForCompletion(ctx, s.client.SnapshotsClient.Client) +} + +func (s *StepSnapshotDataDisks) Run(ctx context.Context, stateBag multistep.StateBag) multistep.StepAction { + s.say("Taking snapshot of OS disk ...") + + var resourceGroupName = stateBag.Get(constants.ArmResourceGroupName).(string) + var location = stateBag.Get(constants.ArmLocation).(string) + var tags = stateBag.Get(constants.ArmTags).(map[string]*string) + var additionalDisks = stateBag.Get(constants.ArmAdditionalDiskVhds).([]string) + var dstSnapshotPrefix = stateBag.Get(constants.ArmManagedImageDataDiskSnapshotPrefix).(string) + + for i, disk := range additionalDisks { + dstSnapshotName := dstSnapshotPrefix + strconv.Itoa(i) + err := s.create(ctx, resourceGroupName, disk, location, tags, dstSnapshotName) + + if err != nil { + stateBag.Put(constants.Error, err) + s.error(err) + + return multistep.ActionHalt + } + } + + return multistep.ActionContinue +} + +func (*StepSnapshotDataDisks) Cleanup(multistep.StateBag) { +} From 593363c5be2caffce5356325e3ab98aa8a7a676f Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Tue, 6 Nov 2018 19:23:15 +0000 Subject: [PATCH 03/16] fix typo in last commit --- builder/azure/arm/artifact.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/builder/azure/arm/artifact.go b/builder/azure/arm/artifact.go index d45c339ea..b3a69b624 100644 --- a/builder/azure/arm/artifact.go +++ b/builder/azure/arm/artifact.go @@ -33,7 +33,7 @@ type Artifact struct { ManagedImageName string ManagedImageLocation string ManagedImageId string - ManagedImageosDiskSnapshotName string + ManagedImageOSDiskSnapshotName string ManagedImageDataDiskSnapshotPrefix string // Additional Disks @@ -47,7 +47,7 @@ func NewManagedImageArtifact(osType, resourceGroup, name, location, id, osDiskSn ManagedImageLocation: location, ManagedImageId: id, OSType: osType, - ManagedImageosDiskSnapshotName: osDiskSnapshotName, + ManagedImageOSDiskSnapshotName: osDiskSnapshotName, ManagedImageDataDiskSnapshotPrefix: osDiskSnapshotPrefix, }, nil } @@ -129,7 +129,7 @@ func (a *Artifact) isManagedImage() bool { } func (a *Artifact) osDiskSnapshot() bool { - return a.ManagedImageosDiskSnapshotName != "" + return a.ManagedImageOSDiskSnapshotName != "" } func (a *Artifact) dataDiskSnapshot() bool { @@ -171,7 +171,7 @@ func (a *Artifact) String() string { buf.WriteString(fmt.Sprintf("ManagedImageId: %s\n", a.ManagedImageId)) buf.WriteString(fmt.Sprintf("ManagedImageLocation: %s\n", a.ManagedImageLocation)) if a.osDiskSnapshot() { - buf.WriteString(fmt.Sprintf("ManagedImageosDiskSnapshotName: %s\n", a.ManagedImageosDiskSnapshotName)) + buf.WriteString(fmt.Sprintf("ManagedImageOSDiskSnapshotName: %s\n", a.ManagedImageOSDiskSnapshotName)) } if a.dataDiskSnapshot() { buf.WriteString(fmt.Sprintf("ManagedImageDataDiskSnapshotPrefix: %s\n", a.ManagedImageDataDiskSnapshotPrefix)) From 369b2dae5eb115df45cfd67bb2cd3a897b61deb9 Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Tue, 6 Nov 2018 21:50:53 +0000 Subject: [PATCH 04/16] Inlining suggested functions --- builder/azure/arm/artifact.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/builder/azure/arm/artifact.go b/builder/azure/arm/artifact.go index b3a69b624..fe9d46b0a 100644 --- a/builder/azure/arm/artifact.go +++ b/builder/azure/arm/artifact.go @@ -128,14 +128,6 @@ func (a *Artifact) isManagedImage() bool { return a.ManagedImageResourceGroupName != "" } -func (a *Artifact) osDiskSnapshot() bool { - return a.ManagedImageOSDiskSnapshotName != "" -} - -func (a *Artifact) dataDiskSnapshot() bool { - return a.ManagedImageDataDiskSnapshotPrefix != "" -} - func (*Artifact) BuilderId() string { return BuilderId } @@ -170,10 +162,10 @@ func (a *Artifact) String() string { buf.WriteString(fmt.Sprintf("ManagedImageName: %s\n", a.ManagedImageName)) buf.WriteString(fmt.Sprintf("ManagedImageId: %s\n", a.ManagedImageId)) buf.WriteString(fmt.Sprintf("ManagedImageLocation: %s\n", a.ManagedImageLocation)) - if a.osDiskSnapshot() { + if a.ManagedImageOSDiskSnapshotName != "" { buf.WriteString(fmt.Sprintf("ManagedImageOSDiskSnapshotName: %s\n", a.ManagedImageOSDiskSnapshotName)) } - if a.dataDiskSnapshot() { + if a.ManagedImageDataDiskSnapshotPrefix != "" { buf.WriteString(fmt.Sprintf("ManagedImageDataDiskSnapshotPrefix: %s\n", a.ManagedImageDataDiskSnapshotPrefix)) } } else { From b2d1675d399d2a1f0c177b12bd240019ee64895a Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Wed, 7 Nov 2018 03:23:17 +0000 Subject: [PATCH 05/16] Added tests --- .../arm/step_snapshot_data_disks_test.go | 69 ++++++++++++++++++ .../azure/arm/step_snapshot_os_disk_test.go | 70 +++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 builder/azure/arm/step_snapshot_data_disks_test.go create mode 100644 builder/azure/arm/step_snapshot_os_disk_test.go diff --git a/builder/azure/arm/step_snapshot_data_disks_test.go b/builder/azure/arm/step_snapshot_data_disks_test.go new file mode 100644 index 000000000..0eefa924d --- /dev/null +++ b/builder/azure/arm/step_snapshot_data_disks_test.go @@ -0,0 +1,69 @@ +package arm + +import ( + "context" + "fmt" + "github.com/hashicorp/packer/builder/azure/common/constants" + "github.com/hashicorp/packer/helper/multistep" + "testing" +) + +func TestStepSnapshotDataDisksShouldFailIfSnapshotFails(t *testing.T) { + var testSubject = &StepSnapshotDataDisks{ + create: func(context.Context, string, string, string, map[string]*string, string) error { + return fmt.Errorf("!! Unit Test FAIL !!") + }, + say: func(message string) {}, + error: func(e error) {}, + } + + stateBag := createTestStateBagStepSnapshotDataDisks() + + var result = testSubject.Run(context.Background(), stateBag) + if result != multistep.ActionHalt { + t.Fatalf("Expected the step to return 'ActionHalt', but got '%d'.", result) + } + + if _, ok := stateBag.GetOk(constants.Error); ok == false { + t.Fatalf("Expected the step to set stateBag['%s'], but it was not.", constants.Error) + } +} + +func TestStepSnapshotDataDisksShouldPassIfSnapshotPasses(t *testing.T) { + var testSubject = &StepSnapshotDataDisks{ + create: func(context.Context, string, string, string, map[string]*string, string) error { + return nil + }, + say: func(message string) {}, + error: func(e error) {}, + } + + stateBag := createTestStateBagStepSnapshotDataDisks() + + var result = testSubject.Run(context.Background(), stateBag) + if result != multistep.ActionContinue { + t.Fatalf("Expected the step to return 'ActionContinue', but got '%d'.", result) + } + + if _, ok := stateBag.GetOk(constants.Error); ok == true { + t.Fatalf("Expected the step to not set stateBag['%s'], but it was.", constants.Error) + } +} + +func createTestStateBagStepSnapshotDataDisks() multistep.StateBag { + stateBag := new(multistep.BasicStateBag) + + stateBag.Put(constants.ArmResourceGroupName, "Unit Test: ResourceGroupName") + stateBag.Put(constants.ArmLocation, "Unit Test: Location") + + value := "Unit Test: Tags" + tags := map[string]*string{ + "tag01": &value, + } + stateBag.Put(constants.ArmTags, tags) + + stateBag.Put(constants.ArmAdditionalDiskVhds, []string{"subscriptions/123-456-789/resourceGroups/existingresourcegroup/providers/Microsoft.Compute/disks/osdisk"}) + stateBag.Put(constants.ArmManagedImageDataDiskSnapshotPrefix, "Unit Test: ManagedImageDataDiskSnapshotPrefix") + + return stateBag +} diff --git a/builder/azure/arm/step_snapshot_os_disk_test.go b/builder/azure/arm/step_snapshot_os_disk_test.go new file mode 100644 index 000000000..08f5038ac --- /dev/null +++ b/builder/azure/arm/step_snapshot_os_disk_test.go @@ -0,0 +1,70 @@ +package arm + +import ( + "context" + "fmt" + "github.com/hashicorp/packer/builder/azure/common/constants" + "github.com/hashicorp/packer/helper/multistep" + "testing" +) + +func TestStepSnapshotOSDiskShouldFailIfSnapshotFails(t *testing.T) { + var testSubject = &StepSnapshotOSDisk{ + create: func(context.Context, string, string, string, map[string]*string, string) error { + return fmt.Errorf("!! Unit Test FAIL !!") + }, + say: func(message string) {}, + error: func(e error) {}, + } + + stateBag := createTestStateBagStepSnapshotOSDisk() + + var result = testSubject.Run(context.Background(), stateBag) + if result != multistep.ActionHalt { + t.Fatalf("Expected the step to return 'ActionHalt', but got '%d'.", result) + } + + if _, ok := stateBag.GetOk(constants.Error); ok == false { + t.Fatalf("Expected the step to set stateBag['%s'], but it was not.", constants.Error) + } +} + +func TestStepSnapshotOSDiskShouldPassIfSnapshotPasses(t *testing.T) { + var testSubject = &StepSnapshotOSDisk{ + create: func(context.Context, string, string, string, map[string]*string, string) error { + return nil + }, + say: func(message string) {}, + error: func(e error) {}, + } + + stateBag := createTestStateBagStepSnapshotOSDisk() + + var result = testSubject.Run(context.Background(), stateBag) + if result != multistep.ActionContinue { + t.Fatalf("Expected the step to return 'ActionContinue', but got '%d'.", result) + } + + if _, ok := stateBag.GetOk(constants.Error); ok == true { + t.Fatalf("Expected the step to not set stateBag['%s'], but it was.", constants.Error) + } +} + +func createTestStateBagStepSnapshotOSDisk() multistep.StateBag { + stateBag := new(multistep.BasicStateBag) + + stateBag.Put(constants.ArmResourceGroupName, "Unit Test: ResourceGroupName") + stateBag.Put(constants.ArmLocation, "Unit Test: Location") + + value := "Unit Test: Tags" + tags := map[string]*string{ + "tag01": &value, + } + + stateBag.Put(constants.ArmTags, tags) + + stateBag.Put(constants.ArmOSDiskVhd, "subscriptions/123-456-789/resourceGroups/existingresourcegroup/providers/Microsoft.Compute/disks/osdisk") + stateBag.Put(constants.ArmManagedImageOSDiskSnapshotName, "Unit Test: ManagedImageOSDiskSnapshotName") + + return stateBag +} \ No newline at end of file From 8c326dbab74d0733fcb9ec498e739dec284c077a Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Wed, 7 Nov 2018 18:08:15 +0000 Subject: [PATCH 06/16] Formatting changes --- builder/azure/arm/config_test.go | 2 +- builder/azure/arm/resource_resolver.go | 2 +- builder/azure/arm/step_snapshot_data_disks.go | 4 ++-- builder/azure/arm/step_snapshot_os_disk.go | 4 ++-- builder/azure/arm/step_snapshot_os_disk_test.go | 2 +- builder/azure/arm/template_factory_test.go | 8 ++++---- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go index d25329dc7..60ef609d3 100644 --- a/builder/azure/arm/config_test.go +++ b/builder/azure/arm/config_test.go @@ -799,7 +799,7 @@ func TestConfigShouldRejectCustomAndPlatformManagedImageBuild(t *testing.T) { func TestConfigShouldRejectCustomAndImageUrlForManagedImageBuild(t *testing.T) { config := map[string]interface{}{ - "image_url": "ignore", + "image_url": "ignore", "custom_managed_image_resource_group_name": "ignore", "custom_managed_image_name": "ignore", "location": "ignore", diff --git a/builder/azure/arm/resource_resolver.go b/builder/azure/arm/resource_resolver.go index 8b89d78af..80c55bbb1 100644 --- a/builder/azure/arm/resource_resolver.go +++ b/builder/azure/arm/resource_resolver.go @@ -24,7 +24,7 @@ type resourceResolver struct { func newResourceResolver(client *AzureClient) *resourceResolver { return &resourceResolver{ - client: client, + client: client, findVirtualNetworkResourceGroup: findVirtualNetworkResourceGroup, findVirtualNetworkSubnet: findVirtualNetworkSubnet, } diff --git a/builder/azure/arm/step_snapshot_data_disks.go b/builder/azure/arm/step_snapshot_data_disks.go index 6535e656b..3e3533fc8 100644 --- a/builder/azure/arm/step_snapshot_data_disks.go +++ b/builder/azure/arm/step_snapshot_data_disks.go @@ -13,7 +13,7 @@ import ( type StepSnapshotDataDisks struct { client *AzureClient - create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error + create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error say func(message string) error func(e error) } @@ -35,7 +35,7 @@ func (s *StepSnapshotDataDisks) createDataDiskSnapshot(ctx context.Context, reso DiskProperties: &compute.DiskProperties{ CreationData: &compute.CreationData{ CreateOption: compute.Import, - SourceURI: to.StringPtr(srcUriVhd), + SourceURI: to.StringPtr(srcUriVhd), }, }, Location: to.StringPtr(location), diff --git a/builder/azure/arm/step_snapshot_os_disk.go b/builder/azure/arm/step_snapshot_os_disk.go index c954fdf71..ce73f3f96 100644 --- a/builder/azure/arm/step_snapshot_os_disk.go +++ b/builder/azure/arm/step_snapshot_os_disk.go @@ -12,7 +12,7 @@ import ( type StepSnapshotOSDisk struct { client *AzureClient - create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error + create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error say func(message string) error func(e error) } @@ -34,7 +34,7 @@ func (s *StepSnapshotOSDisk) createSnapshot(ctx context.Context, resourceGroupNa DiskProperties: &compute.DiskProperties{ CreationData: &compute.CreationData{ CreateOption: compute.Import, - SourceURI: to.StringPtr(srcUriVhd), + SourceURI: to.StringPtr(srcUriVhd), }, }, Location: to.StringPtr(location), diff --git a/builder/azure/arm/step_snapshot_os_disk_test.go b/builder/azure/arm/step_snapshot_os_disk_test.go index 08f5038ac..786e6a36e 100644 --- a/builder/azure/arm/step_snapshot_os_disk_test.go +++ b/builder/azure/arm/step_snapshot_os_disk_test.go @@ -67,4 +67,4 @@ func createTestStateBagStepSnapshotOSDisk() multistep.StateBag { stateBag.Put(constants.ArmManagedImageOSDiskSnapshotName, "Unit Test: ManagedImageOSDiskSnapshotName") return stateBag -} \ No newline at end of file +} diff --git a/builder/azure/arm/template_factory_test.go b/builder/azure/arm/template_factory_test.go index ac00d33fd..fcf431336 100644 --- a/builder/azure/arm/template_factory_test.go +++ b/builder/azure/arm/template_factory_test.go @@ -261,10 +261,10 @@ growpart: // Ensure the VM template is correct when building from a custom managed image. func TestVirtualMachineDeployment08(t *testing.T) { config := map[string]interface{}{ - "location": "ignore", - "subscription_id": "ignore", - "os_type": constants.Target_Linux, - "communicator": "none", + "location": "ignore", + "subscription_id": "ignore", + "os_type": constants.Target_Linux, + "communicator": "none", "custom_managed_image_resource_group_name": "CustomManagedImageResourceGroupName", "custom_managed_image_name": "CustomManagedImageName", "managed_image_name": "ManagedImageName", From 1fa9f1ef1188e216a98eeaed1b4266973b912648 Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Wed, 7 Nov 2018 18:11:48 +0000 Subject: [PATCH 07/16] formatting changes in stateBag.go --- builder/azure/common/constants/stateBag.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/builder/azure/common/constants/stateBag.go b/builder/azure/common/constants/stateBag.go index a21863454..f57f5e5a5 100644 --- a/builder/azure/common/constants/stateBag.go +++ b/builder/azure/common/constants/stateBag.go @@ -30,11 +30,11 @@ const ( ArmVirtualMachineCaptureParameters string = "arm.VirtualMachineCaptureParameters" ArmIsExistingResourceGroup string = "arm.IsExistingResourceGroup" - ArmIsManagedImage string = "arm.IsManagedImage" - ArmManagedImageResourceGroupName string = "arm.ManagedImageResourceGroupName" - ArmManagedImageLocation string = "arm.ManagedImageLocation" - ArmManagedImageName string = "arm.ManagedImageName" - ArmAsyncResourceGroupDelete string = "arm.AsyncResourceGroupDelete" - ArmManagedImageOSDiskSnapshotName string = "arm.ManagedImageOSDiskSnapshotName" - ArmManagedImageDataDiskSnapshotPrefix string = "arm.ManagedImageDataDiskSnapshotPrefix" + ArmIsManagedImage string = "arm.IsManagedImage" + ArmManagedImageResourceGroupName string = "arm.ManagedImageResourceGroupName" + ArmManagedImageLocation string = "arm.ManagedImageLocation" + ArmManagedImageName string = "arm.ManagedImageName" + ArmAsyncResourceGroupDelete string = "arm.AsyncResourceGroupDelete" + ArmManagedImageOSDiskSnapshotName string = "arm.ManagedImageOSDiskSnapshotName" + ArmManagedImageDataDiskSnapshotPrefix string = "arm.ManagedImageDataDiskSnapshotPrefix" ) From 8d8c86366bd51b4b8e699470ecf1bb3fcb8b632d Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Wed, 7 Nov 2018 22:23:22 +0000 Subject: [PATCH 08/16] Config tests + typo fix --- builder/azure/arm/config.go | 8 +- builder/azure/arm/config_test.go | 203 +++++++++++++++++++++++++++++++ 2 files changed, 207 insertions(+), 4 deletions(-) diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index 530439cf4..7eff6d831 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -56,8 +56,8 @@ var ( reCaptureNamePrefix = regexp.MustCompile("^[A-Za-z0-9][A-Za-z0-9_\\-\\.]{0,23}$") reManagedDiskName = regexp.MustCompile(validManagedDiskName) reResourceGroupName = regexp.MustCompile(validResourceGroupNameRe) - reSnaspshotName = regexp.MustCompile("^[A-Za-z0-9_]{10,79}$") - reSnaspshotPrefix = regexp.MustCompile("^[A-Za-z0-9_]{10,59}$") + reSnapshotName = regexp.MustCompile("^[A-Za-z0-9_]{10,79}$") + reSnapshotPrefix = regexp.MustCompile("^[A-Za-z0-9_]{10,59}$") ) type PlanInformation struct { @@ -758,14 +758,14 @@ func assertManagedImageName(name, setting string) (bool, error) { } func assertManagedImageOSDiskSnapshotName(name, setting string) (bool, error) { - if !isValidAzureName(reSnaspshotName, name) { + if !isValidAzureName(reSnapshotName, name) { return false, fmt.Errorf("The setting %s must only contain characters from a-z, A-Z, 0-9 and _ and the maximum length is 80 characters", setting) } return true, nil } func assertManagedImageDataDiskSnapshotName(name, setting string) (bool, error) { - if !isValidAzureName(reSnaspshotPrefix, name) { + if !isValidAzureName(reSnapshotPrefix, name) { return false, fmt.Errorf("The setting %s must only contain characters from a-z, A-Z, 0-9 and _ and the maximum length (excluding the prefix) is 60 characters", setting) } return true, nil diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go index 60ef609d3..9dce2a6dd 100644 --- a/builder/azure/arm/config_test.go +++ b/builder/azure/arm/config_test.go @@ -547,6 +547,107 @@ func TestConfigShouldRejectMalformedCaptureContainerName(t *testing.T) { } } +func TestConfigShouldRejectMalformedManagedImageOSDiskSnapshotName(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "managed_image_resource_group_name": "ignore", + "managed_image_name": "ignore", + "managed_image_os_disk_snapshot_name": "ignore", + // Does not matter for this test case, just pick one. + "os_type": constants.Target_Linux, + } + + wellFormedManagedImageOSDiskSnapshotName := []string{ + "AbcdefghijklmnopqrstuvwX", + "underscore_underscore", + "0leading_number", + "really_loooooooooooooooooooooooooooooooooooooooooooooooooong", + } + + for _, x := range wellFormedManagedImageOSDiskSnapshotName { + config["managed_image_os_disk_snapshot_name"] = x + _, _, err := newConfig(config, getPackerConfiguration()) + + if err != nil { + t.Errorf("Expected test to pass, but it failed with the well-formed managed_image_os_disk_snapshot_name set to %q.", x) + } + } + + malformedManagedImageOSDiskSnapshotName := []string{ + "min_ten", + "-leading-hyphen", + "trailing-hyphen-", + "trailing-period.", + "punc-!@#$%^&*()_+-=-punc", + "really_looooooooooooooooooooooooooooooooooooooooooooooooooooooong_exceeding_80_char_limit", + } + + for _, x := range malformedManagedImageOSDiskSnapshotName { + config["managed_image_os_disk_snapshot_name"] = x + _, _, err := newConfig(config, getPackerConfiguration()) + + if err == nil { + t.Errorf("Expected test to fail, but it succeeded with the malformed managed_image_os_disk_snapshot_name set to %q.", x) + } + } +} + +func TestConfigShouldRejectMalformedManagedImageDataDiskSnapshotPrefix(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "managed_image_resource_group_name": "ignore", + "managed_image_name": "ignore", + "managed_image_data_disk_snapshot_prefix": "ignore", + // Does not matter for this test case, just pick one. + "os_type": constants.Target_Linux, + } + + wellFormedManagedImageDataDiskSnapshotPrefix := []string{ + "min_ten_chars", + "AbcdefghijklmnopqrstuvwX", + "underscore_underscore", + "0leading_number", + "less_than_sixty_characters", + } + + for _, x := range wellFormedManagedImageDataDiskSnapshotPrefix { + config["managed_image_data_disk_snapshot_prefix"] = x + _, _, err := newConfig(config, getPackerConfiguration()) + + if err != nil { + t.Errorf("Expected test to pass, but it failed with the well-formed managed_image_data_disk_snapshot_prefix set to %q.", x) + } + } + + malformedManagedImageDataDiskSnapshotPrefix := []string{ + "more_ten", + "-leading-hyphen", + "trailing-hyphen-", + "trailing-period.", + "punc-!@#$%^&*()_+-=-punc", + "really_looooooooooooooooooooooooooooooooooooooooooooooooooooooong_exceeding_60_char_limit", + } + + for _, x := range malformedManagedImageDataDiskSnapshotPrefix { + config["managed_image_data_disk_snapshot_prefix"] = x + _, _, err := newConfig(config, getPackerConfiguration()) + + if err == nil { + t.Errorf("Expected test to fail, but it succeeded with the malformed managed_image_data_disk_snapshot_prefix set to %q.", x) + } + } +} + func TestConfigShouldAcceptTags(t *testing.T) { config := map[string]interface{}{ "capture_name_prefix": "ignore", @@ -709,6 +810,108 @@ func TestConfigShouldRejectMissingCustomDataFile(t *testing.T) { } } +func TestConfigShouldRejectManagedImageOSDiskSnapshotNameWithoutManagedImageName(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "managed_image_resource_group_name": "ignore", + "managed_image_os_disk_snapshot_name": "ignore", + // Does not matter for this test case, just pick one. + "os_type": constants.Target_Linux, + } + + _, _, err := newConfig(config, getPackerConfiguration()) + if err == nil { + t.Fatal("expected config to reject Managed Image build with OS disk snapshot name but without managed image name") + } +} + +func TestConfigShouldRejectManagedImageOSDiskSnapshotNameWithoutManagedImageResourceGroupName(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "managed_image_name": "ignore", + "managed_image_os_disk_snapshot_name": "ignore", + // Does not matter for this test case, just pick one. + "os_type": constants.Target_Linux, + } + + _, _, err := newConfig(config, getPackerConfiguration()) + if err == nil { + t.Fatal("expected config to reject Managed Image build with OS disk snapshot name but without managed image resource group name") + } +} + +func TestConfigShouldRejectImageDataDiskSnapshotPrefixWithoutManagedImageName(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "managed_image_resource_group_name": "ignore", + "managed_image_data_disk_snapshot_prefix": "ignore", + // Does not matter for this test case, just pick one. + "os_type": constants.Target_Linux, + } + + _, _, err := newConfig(config, getPackerConfiguration()) + if err == nil { + t.Fatal("expected config to reject Managed Image build with data disk snapshot prefix but without managed image name") + } +} + +func TestConfigShouldRejectImageDataDiskSnapshotPrefixWithoutManagedImageResourceGroupName(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "managed_image_name": "ignore", + "managed_image_data_disk_snapshot_prefix": "ignore", + // Does not matter for this test case, just pick one. + "os_type": constants.Target_Linux, + } + + _, _, err := newConfig(config, getPackerConfiguration()) + if err == nil { + t.Fatal("expected config to reject Managed Image build with data disk snapshot prefix but without managed image resource group name") + } +} + +func TestConfigShouldAcceptManagedImageOSDiskSnapshotNameAndManagedImageDataDiskSnapshotPrefix(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "managed_image_resource_group_name": "ignore", + "managed_image_name": "ignore", + "managed_image_os_disk_snapshot_name": "ignore_ignore", + "managed_image_data_disk_snapshot_prefix": "ignore_ignore", + // Does not matter for this test case, just pick one. + "os_type": constants.Target_Linux, + } + + _, _, err := newConfig(config, getPackerConfiguration()) + if err != nil { + t.Fatal("expected config to accept platform managed image build") + } +} + func TestConfigShouldAcceptPlatformManagedImageBuild(t *testing.T) { config := map[string]interface{}{ "image_offer": "ignore", From 8881bc2d64402319bcc7777aab9e78df1359ca4c Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Fri, 9 Nov 2018 03:30:57 +0000 Subject: [PATCH 09/16] Fix bug in snapshot client creation --- builder/azure/arm/step_snapshot_data_disks.go | 4 ++-- builder/azure/arm/step_snapshot_os_disk.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/builder/azure/arm/step_snapshot_data_disks.go b/builder/azure/arm/step_snapshot_data_disks.go index 3e3533fc8..df9a5bfaf 100644 --- a/builder/azure/arm/step_snapshot_data_disks.go +++ b/builder/azure/arm/step_snapshot_data_disks.go @@ -34,8 +34,8 @@ func (s *StepSnapshotDataDisks) createDataDiskSnapshot(ctx context.Context, reso srcVhdToSnapshot := compute.Snapshot{ DiskProperties: &compute.DiskProperties{ CreationData: &compute.CreationData{ - CreateOption: compute.Import, - SourceURI: to.StringPtr(srcUriVhd), + CreateOption: compute.Copy, + SourceResourceID: to.StringPtr(srcUriVhd), }, }, Location: to.StringPtr(location), diff --git a/builder/azure/arm/step_snapshot_os_disk.go b/builder/azure/arm/step_snapshot_os_disk.go index ce73f3f96..5fa9f6b0b 100644 --- a/builder/azure/arm/step_snapshot_os_disk.go +++ b/builder/azure/arm/step_snapshot_os_disk.go @@ -33,8 +33,8 @@ func (s *StepSnapshotOSDisk) createSnapshot(ctx context.Context, resourceGroupNa srcVhdToSnapshot := compute.Snapshot{ DiskProperties: &compute.DiskProperties{ CreationData: &compute.CreationData{ - CreateOption: compute.Import, - SourceURI: to.StringPtr(srcUriVhd), + CreateOption: compute.Copy, + SourceResourceID: to.StringPtr(srcUriVhd), }, }, Location: to.StringPtr(location), From 06525dd8855f3e0532918b11e61a3a9d2ad4b4d7 Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Fri, 9 Nov 2018 19:28:37 +0000 Subject: [PATCH 10/16] adding logs in the steps --- builder/azure/arm/step_snapshot_data_disks.go | 23 +++++++++++++++++-- builder/azure/arm/step_snapshot_os_disk.go | 23 ++++++++++++++++--- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/builder/azure/arm/step_snapshot_data_disks.go b/builder/azure/arm/step_snapshot_data_disks.go index df9a5bfaf..4294cc489 100644 --- a/builder/azure/arm/step_snapshot_data_disks.go +++ b/builder/azure/arm/step_snapshot_data_disks.go @@ -2,6 +2,7 @@ package arm import ( "context" + "fmt" "strconv" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute" @@ -46,15 +47,32 @@ func (s *StepSnapshotDataDisks) createDataDiskSnapshot(ctx context.Context, reso if err != nil { s.say(s.client.LastError.Error()) + return err } - return f.WaitForCompletion(ctx, s.client.SnapshotsClient.Client) + err = f.WaitForCompletion(ctx, s.client.SnapshotsClient.Client) + + if err != nil { + s.say(s.client.LastError.Error()) + return err + } + + createdSnapshot, err := f.Result(s.client.SnapshotsClient) + + if err != nil { + s.say(s.client.LastError.Error()) + return err + } + + s.say(fmt.Sprintf(" -> Managed Image OS Disk Snapshot : '%s'", *(createdSnapshot.ID))) + + return nil } func (s *StepSnapshotDataDisks) Run(ctx context.Context, stateBag multistep.StateBag) multistep.StepAction { s.say("Taking snapshot of OS disk ...") - var resourceGroupName = stateBag.Get(constants.ArmResourceGroupName).(string) + var resourceGroupName = stateBag.Get(constants.ArmManagedImageResourceGroupName).(string) var location = stateBag.Get(constants.ArmLocation).(string) var tags = stateBag.Get(constants.ArmTags).(map[string]*string) var additionalDisks = stateBag.Get(constants.ArmAdditionalDiskVhds).([]string) @@ -70,6 +88,7 @@ func (s *StepSnapshotDataDisks) Run(ctx context.Context, stateBag multistep.Stat return multistep.ActionHalt } + } return multistep.ActionContinue diff --git a/builder/azure/arm/step_snapshot_os_disk.go b/builder/azure/arm/step_snapshot_os_disk.go index 5fa9f6b0b..ebb5e0486 100644 --- a/builder/azure/arm/step_snapshot_os_disk.go +++ b/builder/azure/arm/step_snapshot_os_disk.go @@ -2,7 +2,7 @@ package arm import ( "context" - + "fmt" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute" "github.com/Azure/go-autorest/autorest/to" "github.com/hashicorp/packer/builder/azure/common/constants" @@ -45,15 +45,32 @@ func (s *StepSnapshotOSDisk) createSnapshot(ctx context.Context, resourceGroupNa if err != nil { s.say(s.client.LastError.Error()) + return err + } + + err = f.WaitForCompletion(ctx, s.client.SnapshotsClient.Client) + + if err != nil { + s.say(s.client.LastError.Error()) + return err } - return f.WaitForCompletion(ctx, s.client.SnapshotsClient.Client) + createdSnapshot, err := f.Result(s.client.SnapshotsClient) + + if err != nil { + s.say(s.client.LastError.Error()) + return err + } + + s.say(fmt.Sprintf(" -> Managed Image OS Disk Snapshot : '%s'", *(createdSnapshot.ID))) + + return nil } func (s *StepSnapshotOSDisk) Run(ctx context.Context, stateBag multistep.StateBag) multistep.StepAction { s.say("Taking snapshot of OS disk ...") - var resourceGroupName = stateBag.Get(constants.ArmResourceGroupName).(string) + var resourceGroupName = stateBag.Get(constants.ArmManagedImageResourceGroupName).(string) var location = stateBag.Get(constants.ArmLocation).(string) var tags = stateBag.Get(constants.ArmTags).(map[string]*string) var srcUriVhd = stateBag.Get(constants.ArmOSDiskVhd).(string) From a297d73b980ce7ecbb33dbfb1a132fb927e69b3a Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Fri, 9 Nov 2018 19:29:57 +0000 Subject: [PATCH 11/16] correcting wrong log message --- builder/azure/arm/step_snapshot_data_disks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/azure/arm/step_snapshot_data_disks.go b/builder/azure/arm/step_snapshot_data_disks.go index 4294cc489..37f0342ac 100644 --- a/builder/azure/arm/step_snapshot_data_disks.go +++ b/builder/azure/arm/step_snapshot_data_disks.go @@ -64,7 +64,7 @@ func (s *StepSnapshotDataDisks) createDataDiskSnapshot(ctx context.Context, reso return err } - s.say(fmt.Sprintf(" -> Managed Image OS Disk Snapshot : '%s'", *(createdSnapshot.ID))) + s.say(fmt.Sprintf(" -> Managed Image Data Disk Snapshot : '%s'", *(createdSnapshot.ID))) return nil } From 213fbbae0074acf9b0917cd12dd6b76912dee659 Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Fri, 9 Nov 2018 22:41:48 +0000 Subject: [PATCH 12/16] Updated go version and reformatted --- builder/azure/arm/artifact.go | 10 ++--- builder/azure/arm/config_test.go | 46 +++++++++++----------- builder/azure/arm/resource_resolver.go | 2 +- builder/azure/arm/template_factory_test.go | 8 ++-- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/builder/azure/arm/artifact.go b/builder/azure/arm/artifact.go index fe9d46b0a..d90a178f2 100644 --- a/builder/azure/arm/artifact.go +++ b/builder/azure/arm/artifact.go @@ -42,11 +42,11 @@ type Artifact struct { func NewManagedImageArtifact(osType, resourceGroup, name, location, id, osDiskSnapshotName, osDiskSnapshotPrefix string) (*Artifact, error) { return &Artifact{ - ManagedImageResourceGroupName: resourceGroup, - ManagedImageName: name, - ManagedImageLocation: location, - ManagedImageId: id, - OSType: osType, + ManagedImageResourceGroupName: resourceGroup, + ManagedImageName: name, + ManagedImageLocation: location, + ManagedImageId: id, + OSType: osType, ManagedImageOSDiskSnapshotName: osDiskSnapshotName, ManagedImageDataDiskSnapshotPrefix: osDiskSnapshotPrefix, }, nil diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go index 9dce2a6dd..e99761f84 100644 --- a/builder/azure/arm/config_test.go +++ b/builder/azure/arm/config_test.go @@ -599,14 +599,14 @@ func TestConfigShouldRejectMalformedManagedImageOSDiskSnapshotName(t *testing.T) func TestConfigShouldRejectMalformedManagedImageDataDiskSnapshotPrefix(t *testing.T) { config := map[string]interface{}{ - "image_offer": "ignore", - "image_publisher": "ignore", - "image_sku": "ignore", - "location": "ignore", - "subscription_id": "ignore", - "communicator": "none", - "managed_image_resource_group_name": "ignore", - "managed_image_name": "ignore", + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "managed_image_resource_group_name": "ignore", + "managed_image_name": "ignore", "managed_image_data_disk_snapshot_prefix": "ignore", // Does not matter for this test case, just pick one. "os_type": constants.Target_Linux, @@ -852,13 +852,13 @@ func TestConfigShouldRejectManagedImageOSDiskSnapshotNameWithoutManagedImageReso func TestConfigShouldRejectImageDataDiskSnapshotPrefixWithoutManagedImageName(t *testing.T) { config := map[string]interface{}{ - "image_offer": "ignore", - "image_publisher": "ignore", - "image_sku": "ignore", - "location": "ignore", - "subscription_id": "ignore", - "communicator": "none", - "managed_image_resource_group_name": "ignore", + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "managed_image_resource_group_name": "ignore", "managed_image_data_disk_snapshot_prefix": "ignore", // Does not matter for this test case, just pick one. "os_type": constants.Target_Linux, @@ -872,13 +872,13 @@ func TestConfigShouldRejectImageDataDiskSnapshotPrefixWithoutManagedImageName(t func TestConfigShouldRejectImageDataDiskSnapshotPrefixWithoutManagedImageResourceGroupName(t *testing.T) { config := map[string]interface{}{ - "image_offer": "ignore", - "image_publisher": "ignore", - "image_sku": "ignore", - "location": "ignore", - "subscription_id": "ignore", - "communicator": "none", - "managed_image_name": "ignore", + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "managed_image_name": "ignore", "managed_image_data_disk_snapshot_prefix": "ignore", // Does not matter for this test case, just pick one. "os_type": constants.Target_Linux, @@ -1002,7 +1002,7 @@ func TestConfigShouldRejectCustomAndPlatformManagedImageBuild(t *testing.T) { func TestConfigShouldRejectCustomAndImageUrlForManagedImageBuild(t *testing.T) { config := map[string]interface{}{ - "image_url": "ignore", + "image_url": "ignore", "custom_managed_image_resource_group_name": "ignore", "custom_managed_image_name": "ignore", "location": "ignore", diff --git a/builder/azure/arm/resource_resolver.go b/builder/azure/arm/resource_resolver.go index 80c55bbb1..8b89d78af 100644 --- a/builder/azure/arm/resource_resolver.go +++ b/builder/azure/arm/resource_resolver.go @@ -24,7 +24,7 @@ type resourceResolver struct { func newResourceResolver(client *AzureClient) *resourceResolver { return &resourceResolver{ - client: client, + client: client, findVirtualNetworkResourceGroup: findVirtualNetworkResourceGroup, findVirtualNetworkSubnet: findVirtualNetworkSubnet, } diff --git a/builder/azure/arm/template_factory_test.go b/builder/azure/arm/template_factory_test.go index fcf431336..ac00d33fd 100644 --- a/builder/azure/arm/template_factory_test.go +++ b/builder/azure/arm/template_factory_test.go @@ -261,10 +261,10 @@ growpart: // Ensure the VM template is correct when building from a custom managed image. func TestVirtualMachineDeployment08(t *testing.T) { config := map[string]interface{}{ - "location": "ignore", - "subscription_id": "ignore", - "os_type": constants.Target_Linux, - "communicator": "none", + "location": "ignore", + "subscription_id": "ignore", + "os_type": constants.Target_Linux, + "communicator": "none", "custom_managed_image_resource_group_name": "CustomManagedImageResourceGroupName", "custom_managed_image_name": "CustomManagedImageName", "managed_image_name": "ManagedImageName", From 4822d6cfb962e188ea34a343f9666396e624864b Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Fri, 9 Nov 2018 23:05:28 +0000 Subject: [PATCH 13/16] Updating documentation --- website/source/docs/builders/azure.html.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/website/source/docs/builders/azure.html.md b/website/source/docs/builders/azure.html.md index a81406cc7..1e808794e 100644 --- a/website/source/docs/builders/azure.html.md +++ b/website/source/docs/builders/azure.html.md @@ -303,6 +303,14 @@ Providing `temp_resource_group_name` or `location` in combination with value and defaults to false. **Important** Setting this true means that your builds are faster, however any failed deletes are not reported. +- `managed_image_os_disk_snapshot_name` (string) If managed\_image\_os\_disk\_snapshot\_name + is set, a snapshot of the OS disk is created with the same name as this value before the + VM is captured. + +- `managed_image_data_disk_snapshot_prefix` (string) If managed\_image\_data\_disk\_snapshot\_prefix + is set, snapshot of the data disk(s) is created with the same prefix as this value before the VM + is captured. + ## Basic Example Here is a basic example for Azure. From 2d6b18e63e1f2a8063d43da40b6785fdb9badf2d Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Sat, 10 Nov 2018 00:05:25 +0000 Subject: [PATCH 14/16] Fix tests after bug fix --- builder/azure/arm/step_snapshot_data_disks_test.go | 2 +- builder/azure/arm/step_snapshot_os_disk_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builder/azure/arm/step_snapshot_data_disks_test.go b/builder/azure/arm/step_snapshot_data_disks_test.go index 0eefa924d..aee7523ec 100644 --- a/builder/azure/arm/step_snapshot_data_disks_test.go +++ b/builder/azure/arm/step_snapshot_data_disks_test.go @@ -53,7 +53,7 @@ func TestStepSnapshotDataDisksShouldPassIfSnapshotPasses(t *testing.T) { func createTestStateBagStepSnapshotDataDisks() multistep.StateBag { stateBag := new(multistep.BasicStateBag) - stateBag.Put(constants.ArmResourceGroupName, "Unit Test: ResourceGroupName") + stateBag.Put(constants.ArmManagedImageResourceGroupName, "Unit Test: ResourceGroupName") stateBag.Put(constants.ArmLocation, "Unit Test: Location") value := "Unit Test: Tags" diff --git a/builder/azure/arm/step_snapshot_os_disk_test.go b/builder/azure/arm/step_snapshot_os_disk_test.go index 786e6a36e..f4e7eeafb 100644 --- a/builder/azure/arm/step_snapshot_os_disk_test.go +++ b/builder/azure/arm/step_snapshot_os_disk_test.go @@ -53,7 +53,7 @@ func TestStepSnapshotOSDiskShouldPassIfSnapshotPasses(t *testing.T) { func createTestStateBagStepSnapshotOSDisk() multistep.StateBag { stateBag := new(multistep.BasicStateBag) - stateBag.Put(constants.ArmResourceGroupName, "Unit Test: ResourceGroupName") + stateBag.Put(constants.ArmManagedImageResourceGroupName, "Unit Test: ResourceGroupName") stateBag.Put(constants.ArmLocation, "Unit Test: Location") value := "Unit Test: Tags" From 67342750a374e89f064da834ad44618c0ff71e35 Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Wed, 14 Nov 2018 01:47:48 +0000 Subject: [PATCH 15/16] Addressed PR comments --- builder/azure/arm/builder.go | 26 +++------- builder/azure/arm/config_test.go | 42 ++++++++++++++++ builder/azure/arm/step_snapshot_data_disks.go | 48 ++++++++++--------- .../arm/step_snapshot_data_disks_test.go | 10 ++-- builder/azure/arm/step_snapshot_os_disk.go | 43 +++++++++-------- .../azure/arm/step_snapshot_os_disk_test.go | 10 ++-- 6 files changed, 110 insertions(+), 69 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index debc6108b..e278052f6 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -183,20 +183,13 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe NewStepGetOSDisk(azureClient, ui), NewStepGetAdditionalDisks(azureClient, ui), NewStepPowerOffCompute(azureClient, ui), - } - // if managed image create snapshot - if b.config.isManagedImage() { - steps = append(steps, - NewStepSnapshotOSDisk(azureClient, ui), - NewStepSnapshotDataDisks(azureClient, ui), - ) - } - steps = append(steps, + NewStepSnapshotOSDisk(azureClient, ui, b.config.isManagedImage()), + NewStepSnapshotDataDisks(azureClient, ui, b.config.isManagedImage()), NewStepCaptureImage(azureClient, ui), NewStepDeleteResourceGroup(azureClient, ui), NewStepDeleteOSDisk(azureClient, ui), NewStepDeleteAdditionalDisks(azureClient, ui), - ) + } } else if b.config.OSType == constants.Target_Windows { keyVaultDeploymentName := b.stateBag.Get(constants.ArmKeyVaultDeploymentName).(string) steps = []multistep.Step{ @@ -227,21 +220,14 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe &packerCommon.StepProvision{}, NewStepGetOSDisk(azureClient, ui), NewStepGetAdditionalDisks(azureClient, ui), - } - // if managed image create snapshot - if b.config.isManagedImage() { - steps = append(steps, - NewStepSnapshotOSDisk(azureClient, ui), - NewStepSnapshotDataDisks(azureClient, ui), - ) - } - steps = append(steps, + NewStepSnapshotOSDisk(azureClient, ui, b.config.isManagedImage()), + NewStepSnapshotDataDisks(azureClient, ui, b.config.isManagedImage()), NewStepPowerOffCompute(azureClient, ui), NewStepCaptureImage(azureClient, ui), NewStepDeleteResourceGroup(azureClient, ui), NewStepDeleteOSDisk(azureClient, ui), NewStepDeleteAdditionalDisks(azureClient, ui), - ) + } } else { return nil, fmt.Errorf("Builder does not support the os_type '%s'", b.config.OSType) } diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go index e99761f84..1d2f711fd 100644 --- a/builder/azure/arm/config_test.go +++ b/builder/azure/arm/config_test.go @@ -912,6 +912,48 @@ func TestConfigShouldAcceptManagedImageOSDiskSnapshotNameAndManagedImageDataDisk } } +func TestConfigShouldRejectManagedImageOSDiskSnapshotNameAndManagedImageDataDiskSnapshotPrefixWithCaptureContainerName(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "capture_container_name": "ignore", + "managed_image_os_disk_snapshot_name": "ignore_ignore", + "managed_image_data_disk_snapshot_prefix": "ignore_ignore", + // Does not matter for this test case, just pick one. + "os_type": constants.Target_Linux, + } + + _, _, err := newConfig(config, getPackerConfiguration()) + if err == nil { + t.Fatal("expected config to reject Managed Image build with data disk snapshot prefix and OS disk snapshot name with capture container name") + } +} + +func TestConfigShouldRejectManagedImageOSDiskSnapshotNameAndManagedImageDataDiskSnapshotPrefixWithCaptureNamePrefix(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "capture_name_prefix": "ignore", + "managed_image_os_disk_snapshot_name": "ignore_ignore", + "managed_image_data_disk_snapshot_prefix": "ignore_ignore", + // Does not matter for this test case, just pick one. + "os_type": constants.Target_Linux, + } + + _, _, err := newConfig(config, getPackerConfiguration()) + if err == nil { + t.Fatal("expected config to reject Managed Image build with data disk snapshot prefix and OS disk snapshot name with capture name prefix") + } +} + func TestConfigShouldAcceptPlatformManagedImageBuild(t *testing.T) { config := map[string]interface{}{ "image_offer": "ignore", diff --git a/builder/azure/arm/step_snapshot_data_disks.go b/builder/azure/arm/step_snapshot_data_disks.go index 37f0342ac..baf56b443 100644 --- a/builder/azure/arm/step_snapshot_data_disks.go +++ b/builder/azure/arm/step_snapshot_data_disks.go @@ -13,17 +13,19 @@ import ( ) type StepSnapshotDataDisks struct { - client *AzureClient - create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error - say func(message string) - error func(e error) + client *AzureClient + create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error + say func(message string) + error func(e error) + isManagedImage bool } -func NewStepSnapshotDataDisks(client *AzureClient, ui packer.Ui) *StepSnapshotDataDisks { +func NewStepSnapshotDataDisks(client *AzureClient, ui packer.Ui, isManagedImage bool) *StepSnapshotDataDisks { var step = &StepSnapshotDataDisks{ - client: client, - say: func(message string) { ui.Say(message) }, - error: func(e error) { ui.Error(e.Error()) }, + client: client, + say: func(message string) { ui.Say(message) }, + error: func(e error) { ui.Error(e.Error()) }, + isManagedImage: isManagedImage, } step.create = step.createDataDiskSnapshot @@ -70,25 +72,27 @@ func (s *StepSnapshotDataDisks) createDataDiskSnapshot(ctx context.Context, reso } func (s *StepSnapshotDataDisks) Run(ctx context.Context, stateBag multistep.StateBag) multistep.StepAction { - s.say("Taking snapshot of OS disk ...") + if s.isManagedImage { - var resourceGroupName = stateBag.Get(constants.ArmManagedImageResourceGroupName).(string) - var location = stateBag.Get(constants.ArmLocation).(string) - var tags = stateBag.Get(constants.ArmTags).(map[string]*string) - var additionalDisks = stateBag.Get(constants.ArmAdditionalDiskVhds).([]string) - var dstSnapshotPrefix = stateBag.Get(constants.ArmManagedImageDataDiskSnapshotPrefix).(string) + s.say("Taking snapshot of data disk ...") - for i, disk := range additionalDisks { - dstSnapshotName := dstSnapshotPrefix + strconv.Itoa(i) - err := s.create(ctx, resourceGroupName, disk, location, tags, dstSnapshotName) + var resourceGroupName = stateBag.Get(constants.ArmManagedImageResourceGroupName).(string) + var location = stateBag.Get(constants.ArmLocation).(string) + var tags = stateBag.Get(constants.ArmTags).(map[string]*string) + var additionalDisks = stateBag.Get(constants.ArmAdditionalDiskVhds).([]string) + var dstSnapshotPrefix = stateBag.Get(constants.ArmManagedImageDataDiskSnapshotPrefix).(string) - if err != nil { - stateBag.Put(constants.Error, err) - s.error(err) + for i, disk := range additionalDisks { + dstSnapshotName := dstSnapshotPrefix + strconv.Itoa(i) + err := s.create(ctx, resourceGroupName, disk, location, tags, dstSnapshotName) - return multistep.ActionHalt - } + if err != nil { + stateBag.Put(constants.Error, err) + s.error(err) + return multistep.ActionHalt + } + } } return multistep.ActionContinue diff --git a/builder/azure/arm/step_snapshot_data_disks_test.go b/builder/azure/arm/step_snapshot_data_disks_test.go index aee7523ec..e7203aade 100644 --- a/builder/azure/arm/step_snapshot_data_disks_test.go +++ b/builder/azure/arm/step_snapshot_data_disks_test.go @@ -13,8 +13,9 @@ func TestStepSnapshotDataDisksShouldFailIfSnapshotFails(t *testing.T) { create: func(context.Context, string, string, string, map[string]*string, string) error { return fmt.Errorf("!! Unit Test FAIL !!") }, - say: func(message string) {}, - error: func(e error) {}, + say: func(message string) {}, + error: func(e error) {}, + isManagedImage: true, } stateBag := createTestStateBagStepSnapshotDataDisks() @@ -34,8 +35,9 @@ func TestStepSnapshotDataDisksShouldPassIfSnapshotPasses(t *testing.T) { create: func(context.Context, string, string, string, map[string]*string, string) error { return nil }, - say: func(message string) {}, - error: func(e error) {}, + say: func(message string) {}, + error: func(e error) {}, + isManagedImage: true, } stateBag := createTestStateBagStepSnapshotDataDisks() diff --git a/builder/azure/arm/step_snapshot_os_disk.go b/builder/azure/arm/step_snapshot_os_disk.go index ebb5e0486..7f349c07e 100644 --- a/builder/azure/arm/step_snapshot_os_disk.go +++ b/builder/azure/arm/step_snapshot_os_disk.go @@ -11,17 +11,19 @@ import ( ) type StepSnapshotOSDisk struct { - client *AzureClient - create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error - say func(message string) - error func(e error) + client *AzureClient + create func(ctx context.Context, resourceGroupName string, srcUriVhd string, location string, tags map[string]*string, dstSnapshotName string) error + say func(message string) + error func(e error) + isManagedImage bool } -func NewStepSnapshotOSDisk(client *AzureClient, ui packer.Ui) *StepSnapshotOSDisk { +func NewStepSnapshotOSDisk(client *AzureClient, ui packer.Ui, isManagedImage bool) *StepSnapshotOSDisk { var step = &StepSnapshotOSDisk{ - client: client, - say: func(message string) { ui.Say(message) }, - error: func(e error) { ui.Error(e.Error()) }, + client: client, + say: func(message string) { ui.Say(message) }, + error: func(e error) { ui.Error(e.Error()) }, + isManagedImage: isManagedImage, } step.create = step.createSnapshot @@ -68,21 +70,24 @@ func (s *StepSnapshotOSDisk) createSnapshot(ctx context.Context, resourceGroupNa } func (s *StepSnapshotOSDisk) Run(ctx context.Context, stateBag multistep.StateBag) multistep.StepAction { - s.say("Taking snapshot of OS disk ...") + if s.isManagedImage { - var resourceGroupName = stateBag.Get(constants.ArmManagedImageResourceGroupName).(string) - var location = stateBag.Get(constants.ArmLocation).(string) - var tags = stateBag.Get(constants.ArmTags).(map[string]*string) - var srcUriVhd = stateBag.Get(constants.ArmOSDiskVhd).(string) - var dstSnapshotName = stateBag.Get(constants.ArmManagedImageOSDiskSnapshotName).(string) + s.say("Taking snapshot of OS disk ...") - err := s.create(ctx, resourceGroupName, srcUriVhd, location, tags, dstSnapshotName) + var resourceGroupName = stateBag.Get(constants.ArmManagedImageResourceGroupName).(string) + var location = stateBag.Get(constants.ArmLocation).(string) + var tags = stateBag.Get(constants.ArmTags).(map[string]*string) + var srcUriVhd = stateBag.Get(constants.ArmOSDiskVhd).(string) + var dstSnapshotName = stateBag.Get(constants.ArmManagedImageOSDiskSnapshotName).(string) - if err != nil { - stateBag.Put(constants.Error, err) - s.error(err) + err := s.create(ctx, resourceGroupName, srcUriVhd, location, tags, dstSnapshotName) + + if err != nil { + stateBag.Put(constants.Error, err) + s.error(err) - return multistep.ActionHalt + return multistep.ActionHalt + } } return multistep.ActionContinue diff --git a/builder/azure/arm/step_snapshot_os_disk_test.go b/builder/azure/arm/step_snapshot_os_disk_test.go index f4e7eeafb..b7342319b 100644 --- a/builder/azure/arm/step_snapshot_os_disk_test.go +++ b/builder/azure/arm/step_snapshot_os_disk_test.go @@ -13,8 +13,9 @@ func TestStepSnapshotOSDiskShouldFailIfSnapshotFails(t *testing.T) { create: func(context.Context, string, string, string, map[string]*string, string) error { return fmt.Errorf("!! Unit Test FAIL !!") }, - say: func(message string) {}, - error: func(e error) {}, + say: func(message string) {}, + error: func(e error) {}, + isManagedImage: true, } stateBag := createTestStateBagStepSnapshotOSDisk() @@ -34,8 +35,9 @@ func TestStepSnapshotOSDiskShouldPassIfSnapshotPasses(t *testing.T) { create: func(context.Context, string, string, string, map[string]*string, string) error { return nil }, - say: func(message string) {}, - error: func(e error) {}, + say: func(message string) {}, + error: func(e error) {}, + isManagedImage: true, } stateBag := createTestStateBagStepSnapshotOSDisk() From b8def0b3fb06b3c73bdf2e55f9fd49be626d0c70 Mon Sep 17 00:00:00 2001 From: Amrita Dutta Date: Thu, 15 Nov 2018 22:01:16 +0000 Subject: [PATCH 16/16] Added test to check for missing OS disk snapshot name and data disk snapshot prefix --- builder/azure/arm/artifact.go | 4 +- builder/azure/arm/artifact_test.go | 59 ++++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/builder/azure/arm/artifact.go b/builder/azure/arm/artifact.go index d90a178f2..f9bb25d81 100644 --- a/builder/azure/arm/artifact.go +++ b/builder/azure/arm/artifact.go @@ -40,7 +40,7 @@ type Artifact struct { AdditionalDisks *[]AdditionalDiskArtifact } -func NewManagedImageArtifact(osType, resourceGroup, name, location, id, osDiskSnapshotName, osDiskSnapshotPrefix string) (*Artifact, error) { +func NewManagedImageArtifact(osType, resourceGroup, name, location, id, osDiskSnapshotName, dataDiskSnapshotPrefix string) (*Artifact, error) { return &Artifact{ ManagedImageResourceGroupName: resourceGroup, ManagedImageName: name, @@ -48,7 +48,7 @@ func NewManagedImageArtifact(osType, resourceGroup, name, location, id, osDiskSn ManagedImageId: id, OSType: osType, ManagedImageOSDiskSnapshotName: osDiskSnapshotName, - ManagedImageDataDiskSnapshotPrefix: osDiskSnapshotPrefix, + ManagedImageDataDiskSnapshotPrefix: dataDiskSnapshotPrefix, }, nil } diff --git a/builder/azure/arm/artifact_test.go b/builder/azure/arm/artifact_test.go index 340212bcd..11b8c3b2a 100644 --- a/builder/azure/arm/artifact_test.go +++ b/builder/azure/arm/artifact_test.go @@ -42,14 +42,67 @@ func TestArtifactIdVHD(t *testing.T) { } func TestArtifactIDManagedImage(t *testing.T) { - artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "fakeOSDiskSnapshotName", "fakeOSDiskSnapshotPrefix") + artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "fakeOsDiskSnapshotName", "fakeDataDiskSnapshotPrefix") if err != nil { t.Fatalf("err=%s", err) } - expected := "fakeID" + expected := `Azure.ResourceManagement.VMImage: - result := artifact.Id() +OSType: Linux +ManagedImageResourceGroupName: fakeResourceGroup +ManagedImageName: fakeName +ManagedImageId: fakeID +ManagedImageLocation: fakeLocation +ManagedImageOSDiskSnapshotName: fakeOsDiskSnapshotName +ManagedImageDataDiskSnapshotPrefix: fakeDataDiskSnapshotPrefix +` + + result := artifact.String() + if result != expected { + t.Fatalf("bad: %s", result) + } +} + +func TestArtifactIDManagedImageWithoutOSDiskSnapshotName(t *testing.T) { + artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "", "fakeDataDiskSnapshotPrefix") + if err != nil { + t.Fatalf("err=%s", err) + } + + expected := `Azure.ResourceManagement.VMImage: + +OSType: Linux +ManagedImageResourceGroupName: fakeResourceGroup +ManagedImageName: fakeName +ManagedImageId: fakeID +ManagedImageLocation: fakeLocation +ManagedImageDataDiskSnapshotPrefix: fakeDataDiskSnapshotPrefix +` + + result := artifact.String() + if result != expected { + t.Fatalf("bad: %s", result) + } +} + +func TestArtifactIDManagedImageWithoutDataDiskSnapshotPrefix(t *testing.T) { + artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "fakeOsDiskSnapshotName", "") + if err != nil { + t.Fatalf("err=%s", err) + } + + expected := `Azure.ResourceManagement.VMImage: + +OSType: Linux +ManagedImageResourceGroupName: fakeResourceGroup +ManagedImageName: fakeName +ManagedImageId: fakeID +ManagedImageLocation: fakeLocation +ManagedImageOSDiskSnapshotName: fakeOsDiskSnapshotName +` + + result := artifact.String() if result != expected { t.Fatalf("bad: %s", result) }