From 82a1f017aa608ea9b5d1f64b90fc307fc7ae5261 Mon Sep 17 00:00:00 2001 From: Contigo Red Date: Mon, 5 Oct 2020 21:54:03 +0300 Subject: [PATCH 1/2] azure: arm builder: adding keep_os_disk parameter to control OS disk deletion. keep_os_disk: auto generated help azure: arm builder: add disk os to artifact azure: arm builder: fmt'ed artifact_test.go --- builder/azure/arm/artifact.go | 32 +++++- builder/azure/arm/artifact_test.go | 98 ++++++++++++++++++- builder/azure/arm/builder.go | 39 +++++--- builder/azure/arm/config.go | 4 + builder/azure/arm/config.hcl2spec.go | 2 + builder/azure/arm/step_deploy_template.go | 15 +-- builder/azure/common/constants/stateBag.go | 1 + .../builder/azure/arm/Config-not-required.mdx | 4 + 8 files changed, 166 insertions(+), 29 deletions(-) diff --git a/builder/azure/arm/artifact.go b/builder/azure/arm/artifact.go index 6830e9172..b1ae13e3a 100644 --- a/builder/azure/arm/artifact.go +++ b/builder/azure/arm/artifact.go @@ -46,8 +46,8 @@ type Artifact struct { StateData map[string]interface{} } -func NewManagedImageArtifact(osType, resourceGroup, name, location, id, osDiskSnapshotName, dataDiskSnapshotPrefix string, generatedData map[string]interface{}) (*Artifact, error) { - return &Artifact{ +func NewManagedImageArtifact(osType, resourceGroup, name, location, id, osDiskSnapshotName, dataDiskSnapshotPrefix string, generatedData map[string]interface{}, keepOSDisk bool, template *CaptureTemplate, getSasUrl func(name string) string) (*Artifact, error) { + res := Artifact{ ManagedImageResourceGroupName: resourceGroup, ManagedImageName: name, ManagedImageLocation: location, @@ -56,7 +56,27 @@ func NewManagedImageArtifact(osType, resourceGroup, name, location, id, osDiskSn ManagedImageOSDiskSnapshotName: osDiskSnapshotName, ManagedImageDataDiskSnapshotPrefix: dataDiskSnapshotPrefix, StateData: generatedData, - }, nil + } + + if keepOSDisk { + if template == nil { + return nil, fmt.Errorf("nil capture template") + } + + if len(template.Resources) != 1 { + return nil, fmt.Errorf("malformed capture template, expected one resource") + } + + vhdUri, err := url.Parse(template.Resources[0].Properties.StorageProfile.OSDisk.Image.Uri) + if err != nil { + return nil, err + } + + res.OSDiskUri = vhdUri.String() + res.OSDiskUriReadOnlySas = getSasUrl(getStorageUrlPath(vhdUri)) + } + + return &res, nil } func NewManagedImageArtifactWithSIGAsDestination(osType, resourceGroup, name, location, id, osDiskSnapshotName, dataDiskSnapshotPrefix, destinationSharedImageGalleryId string, generatedData map[string]interface{}) (*Artifact, error) { @@ -198,6 +218,12 @@ func (a *Artifact) String() string { if a.ManagedImageSharedImageGalleryId != "" { buf.WriteString(fmt.Sprintf("ManagedImageSharedImageGalleryId: %s\n", a.ManagedImageSharedImageGalleryId)) } + if a.OSDiskUri != "" { + buf.WriteString(fmt.Sprintf("OSDiskUri: %s\n", a.OSDiskUri)) + } + if a.OSDiskUriReadOnlySas != "" { + buf.WriteString(fmt.Sprintf("OSDiskUriReadOnlySas: %s\n", a.OSDiskUriReadOnlySas)) + } } 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 73f82e626..81acdb101 100644 --- a/builder/azure/arm/artifact_test.go +++ b/builder/azure/arm/artifact_test.go @@ -46,7 +46,24 @@ func TestArtifactIdVHD(t *testing.T) { } func TestArtifactIDManagedImage(t *testing.T) { - artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "fakeOsDiskSnapshotName", "fakeDataDiskSnapshotPrefix", generatedData()) + template := CaptureTemplate{ + Resources: []CaptureResources{ + { + Properties: CaptureProperties{ + StorageProfile: CaptureStorageProfile{ + OSDisk: CaptureDisk{ + Image: CaptureUri{ + Uri: "https://storage.blob.core.windows.net/system/Microsoft.Compute/Images/images/packer-osDisk.4085bb15-3644-4641-b9cd-f575918640b4.vhd", + }, + }, + }, + }, + Location: "southcentralus", + }, + }, + } + + artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "fakeOsDiskSnapshotName", "fakeDataDiskSnapshotPrefix", generatedData(), false, &template, getFakeSasUrl) if err != nil { t.Fatalf("err=%s", err) } @@ -69,7 +86,24 @@ ManagedImageDataDiskSnapshotPrefix: fakeDataDiskSnapshotPrefix } func TestArtifactIDManagedImageWithoutOSDiskSnapshotName(t *testing.T) { - artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "", "fakeDataDiskSnapshotPrefix", generatedData()) + template := CaptureTemplate{ + Resources: []CaptureResources{ + { + Properties: CaptureProperties{ + StorageProfile: CaptureStorageProfile{ + OSDisk: CaptureDisk{ + Image: CaptureUri{ + Uri: "https://storage.blob.core.windows.net/system/Microsoft.Compute/Images/images/packer-osDisk.4085bb15-3644-4641-b9cd-f575918640b4.vhd", + }, + }, + }, + }, + Location: "southcentralus", + }, + }, + } + + artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "", "fakeDataDiskSnapshotPrefix", generatedData(), false, &template, getFakeSasUrl) if err != nil { t.Fatalf("err=%s", err) } @@ -91,7 +125,24 @@ ManagedImageDataDiskSnapshotPrefix: fakeDataDiskSnapshotPrefix } func TestArtifactIDManagedImageWithoutDataDiskSnapshotPrefix(t *testing.T) { - artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "fakeOsDiskSnapshotName", "", generatedData()) + template := CaptureTemplate{ + Resources: []CaptureResources{ + { + Properties: CaptureProperties{ + StorageProfile: CaptureStorageProfile{ + OSDisk: CaptureDisk{ + Image: CaptureUri{ + Uri: "https://storage.blob.core.windows.net/system/Microsoft.Compute/Images/images/packer-osDisk.4085bb15-3644-4641-b9cd-f575918640b4.vhd", + }, + }, + }, + }, + Location: "southcentralus", + }, + }, + } + + artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "fakeOsDiskSnapshotName", "", generatedData(), false, &template, getFakeSasUrl) if err != nil { t.Fatalf("err=%s", err) } @@ -112,6 +163,47 @@ ManagedImageOSDiskSnapshotName: fakeOsDiskSnapshotName } } +func TestArtifactIDManagedImageWithKeepingTheOSDisk(t *testing.T) { + template := CaptureTemplate{ + Resources: []CaptureResources{ + { + Properties: CaptureProperties{ + StorageProfile: CaptureStorageProfile{ + OSDisk: CaptureDisk{ + Image: CaptureUri{ + Uri: "https://storage.blob.core.windows.net/system/Microsoft.Compute/Images/images/packer-osDisk.4085bb15-3644-4641-b9cd-f575918640b4.vhd", + }, + }, + }, + }, + Location: "southcentralus", + }, + }, + } + + artifact, err := NewManagedImageArtifact("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "fakeOsDiskSnapshotName", "", generatedData(), true, &template, getFakeSasUrl) + if err != nil { + t.Fatalf("err=%s", err) + } + + expected := `Azure.ResourceManagement.VMImage: + +OSType: Linux +ManagedImageResourceGroupName: fakeResourceGroup +ManagedImageName: fakeName +ManagedImageId: fakeID +ManagedImageLocation: fakeLocation +ManagedImageOSDiskSnapshotName: fakeOsDiskSnapshotName +OSDiskUri: https://storage.blob.core.windows.net/system/Microsoft.Compute/Images/images/packer-osDisk.4085bb15-3644-4641-b9cd-f575918640b4.vhd +OSDiskUriReadOnlySas: SAS-Images/images/packer-osDisk.4085bb15-3644-4641-b9cd-f575918640b4.vhd +` + + result := artifact.String() + if result != expected { + t.Fatalf("bad: %s", result) + } +} + func TestArtifactIDManagedImageWithSharedImageGalleryId(t *testing.T) { artifact, err := NewManagedImageArtifactWithSIGAsDestination("Linux", "fakeResourceGroup", "fakeName", "fakeLocation", "fakeID", "fakeOsDiskSnapshotName", "fakeDataDiskSnapshotPrefix", "fakeSharedImageGallery", generatedData()) if err != nil { diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index e35ee2c3f..7ca702884 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -296,6 +296,15 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) return nil, errors.New("Build was halted.") } + getSasUrlFunc := func(name string) string { + blob := azureClient.BlobStorageClient.GetContainerReference(DefaultSasBlobContainer).GetBlobReference(name) + options := storage.BlobSASOptions{} + options.BlobServiceSASPermissions.Read = true + options.Expiry = time.Now().AddDate(0, 1, 0).UTC() // one month + sasUrl, _ := blob.GetSASURI(options) + return sasUrl + } + generatedData := map[string]interface{}{"generated_data": b.stateBag.Get("generated_data")} if b.config.isManagedImage() { managedImageID := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/images/%s", @@ -310,26 +319,23 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) b.config.ManagedImageDataDiskSnapshotPrefix, b.stateBag.Get(constants.ArmManagedImageSharedGalleryId).(string), generatedData) + } else if template, ok := b.stateBag.GetOk(constants.ArmCaptureTemplate); ok { + return NewManagedImageArtifact(b.config.OSType, + b.config.ManagedImageResourceGroupName, + b.config.ManagedImageName, + b.config.Location, + managedImageID, + b.config.ManagedImageOSDiskSnapshotName, + b.config.ManagedImageDataDiskSnapshotPrefix, + generatedData, + b.stateBag.Get(constants.ArmKeepOSDisk).(bool), + template.(*CaptureTemplate), + getSasUrlFunc) } - return NewManagedImageArtifact(b.config.OSType, - b.config.ManagedImageResourceGroupName, - b.config.ManagedImageName, - b.config.Location, - managedImageID, - b.config.ManagedImageOSDiskSnapshotName, - b.config.ManagedImageDataDiskSnapshotPrefix, - generatedData) } else if template, ok := b.stateBag.GetOk(constants.ArmCaptureTemplate); ok { return NewArtifact( template.(*CaptureTemplate), - func(name string) string { - blob := azureClient.BlobStorageClient.GetContainerReference(DefaultSasBlobContainer).GetBlobReference(name) - options := storage.BlobSASOptions{} - options.BlobServiceSASPermissions.Read = true - options.Expiry = time.Now().AddDate(0, 1, 0).UTC() // one month - sasUrl, _ := blob.GetSASURI(options) - return sasUrl - }, + getSasUrlFunc, b.config.OSType, generatedData) } @@ -425,6 +431,7 @@ func (b *Builder) configureStateBag(stateBag multistep.StateBag) { stateBag.Put(constants.ArmManagedImageOSDiskSnapshotName, b.config.ManagedImageOSDiskSnapshotName) stateBag.Put(constants.ArmManagedImageDataDiskSnapshotPrefix, b.config.ManagedImageDataDiskSnapshotPrefix) stateBag.Put(constants.ArmAsyncResourceGroupDelete, b.config.AsyncResourceGroupDelete) + stateBag.Put(constants.ArmKeepOSDisk, b.config.KeepOSDisk) if b.config.isManagedImage() && b.config.SharedGalleryDestination.SigDestinationGalleryName != "" { stateBag.Put(constants.ArmManagedImageSigPublishResourceGroup, b.config.SharedGalleryDestination.SigDestinationResourceGroup) diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index 4392248cb..7d9a2ac4a 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -279,6 +279,10 @@ type Config struct { // disk(s) is created with the same prefix as this value before the VM is // captured. ManagedImageDataDiskSnapshotPrefix string `mapstructure:"managed_image_data_disk_snapshot_prefix" required:"false"` + // If + // keep_os_disk is set, the OS disk is not deleted. + // The default is false. + KeepOSDisk bool `mapstructure:"keep_os_disk" required:"false"` // Store the image in zone-resilient storage. You need to create it in a // region that supports [availability // zones](https://docs.microsoft.com/en-us/azure/availability-zones/az-overview). diff --git a/builder/azure/arm/config.hcl2spec.go b/builder/azure/arm/config.hcl2spec.go index 3cf374c1b..73ddb834a 100644 --- a/builder/azure/arm/config.hcl2spec.go +++ b/builder/azure/arm/config.hcl2spec.go @@ -52,6 +52,7 @@ type FlatConfig struct { ManagedImageStorageAccountType *string `mapstructure:"managed_image_storage_account_type" required:"false" cty:"managed_image_storage_account_type" hcl:"managed_image_storage_account_type"` ManagedImageOSDiskSnapshotName *string `mapstructure:"managed_image_os_disk_snapshot_name" required:"false" cty:"managed_image_os_disk_snapshot_name" hcl:"managed_image_os_disk_snapshot_name"` ManagedImageDataDiskSnapshotPrefix *string `mapstructure:"managed_image_data_disk_snapshot_prefix" required:"false" cty:"managed_image_data_disk_snapshot_prefix" hcl:"managed_image_data_disk_snapshot_prefix"` + KeepOSDisk *bool `mapstructure:"keep_os_disk" required:"false" cty:"keep_os_disk" hcl:"keep_os_disk"` ManagedImageZoneResilient *bool `mapstructure:"managed_image_zone_resilient" required:"false" cty:"managed_image_zone_resilient" hcl:"managed_image_zone_resilient"` AzureTags map[string]string `mapstructure:"azure_tags" required:"false" cty:"azure_tags" hcl:"azure_tags"` AzureTag []config.FlatNameValue `mapstructure:"azure_tag" required:"false" cty:"azure_tag" hcl:"azure_tag"` @@ -183,6 +184,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "managed_image_storage_account_type": &hcldec.AttrSpec{Name: "managed_image_storage_account_type", Type: cty.String, Required: false}, "managed_image_os_disk_snapshot_name": &hcldec.AttrSpec{Name: "managed_image_os_disk_snapshot_name", Type: cty.String, Required: false}, "managed_image_data_disk_snapshot_prefix": &hcldec.AttrSpec{Name: "managed_image_data_disk_snapshot_prefix", Type: cty.String, Required: false}, + "keep_os_disk": &hcldec.AttrSpec{Name: "keep_os_disk", Type: cty.Bool, Required: false}, "managed_image_zone_resilient": &hcldec.AttrSpec{Name: "managed_image_zone_resilient", Type: cty.Bool, Required: false}, "azure_tags": &hcldec.AttrSpec{Name: "azure_tags", Type: cty.Map(cty.String), Required: false}, "azure_tag": &hcldec.BlockListSpec{TypeName: "azure_tag", Nested: hcldec.ObjectSpec((*config.FlatNameValue)(nil).HCL2Spec())}, diff --git a/builder/azure/arm/step_deploy_template.go b/builder/azure/arm/step_deploy_template.go index 0390ff985..9c40c732f 100644 --- a/builder/azure/arm/step_deploy_template.go +++ b/builder/azure/arm/step_deploy_template.go @@ -92,13 +92,14 @@ func (s *StepDeployTemplate) Cleanup(state multistep.StateBag) { "Error: %s", computeName, err)) return } - - ui.Say(fmt.Sprintf(" Deleting -> %s : '%s'", imageType, imageName)) - err = s.deleteDisk(context.TODO(), imageType, imageName, resourceGroupName) - if err != nil { - ui.Error(fmt.Sprintf("Error deleting resource. Please delete manually.\n\n"+ - "Name: %s\n"+ - "Error: %s", imageName, err)) + if !state.Get(constants.ArmKeepOSDisk).(bool) { + ui.Say(fmt.Sprintf(" Deleting -> %s : '%s'", imageType, imageName)) + err = s.deleteDisk(context.TODO(), imageType, imageName, resourceGroupName) + if err != nil { + ui.Error(fmt.Sprintf("Error deleting resource. Please delete manually.\n\n"+ + "Name: %s\n"+ + "Error: %s", imageName, err)) + } } } diff --git a/builder/azure/common/constants/stateBag.go b/builder/azure/common/constants/stateBag.go index 7b1b8cfce..948bbf67b 100644 --- a/builder/azure/common/constants/stateBag.go +++ b/builder/azure/common/constants/stateBag.go @@ -54,4 +54,5 @@ const ( ArmAsyncResourceGroupDelete string = "arm.AsyncResourceGroupDelete" ArmManagedImageOSDiskSnapshotName string = "arm.ManagedImageOSDiskSnapshotName" ArmManagedImageDataDiskSnapshotPrefix string = "arm.ManagedImageDataDiskSnapshotPrefix" + ArmKeepOSDisk string = "arm.KeepOSDisk" ) diff --git a/website/content/partials/builder/azure/arm/Config-not-required.mdx b/website/content/partials/builder/azure/arm/Config-not-required.mdx index 8181cd625..eec1fe16c 100644 --- a/website/content/partials/builder/azure/arm/Config-not-required.mdx +++ b/website/content/partials/builder/azure/arm/Config-not-required.mdx @@ -133,6 +133,10 @@ disk(s) is created with the same prefix as this value before the VM is captured. +- `keep_os_disk` (bool) - If + keep_os_disk is set, the OS disk is not deleted. + The default is false. + - `managed_image_zone_resilient` (bool) - Store the image in zone-resilient storage. You need to create it in a region that supports [availability zones](https://docs.microsoft.com/en-us/azure/availability-zones/az-overview). From 1788d29567477dea2c8021792c4e3e0932df9b3b Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 23 Apr 2021 16:50:41 -0700 Subject: [PATCH 2/2] fix --- builder/azure/arm/artifact.go | 10 +++++++--- builder/azure/arm/builder.go | 11 +++++++++++ builder/azure/arm/step_deploy_template_test.go | 4 ++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/builder/azure/arm/artifact.go b/builder/azure/arm/artifact.go index b1ae13e3a..6a3463ec3 100644 --- a/builder/azure/arm/artifact.go +++ b/builder/azure/arm/artifact.go @@ -3,6 +3,7 @@ package arm import ( "bytes" "fmt" + "log" "net/url" "path" "strings" @@ -60,16 +61,19 @@ func NewManagedImageArtifact(osType, resourceGroup, name, location, id, osDiskSn if keepOSDisk { if template == nil { - return nil, fmt.Errorf("nil capture template") + log.Printf("artifact error: nil capture template") + return &res, nil } if len(template.Resources) != 1 { - return nil, fmt.Errorf("malformed capture template, expected one resource") + log.Printf("artifact error: malformed capture template, expected one resource") + return &res, nil } vhdUri, err := url.Parse(template.Resources[0].Properties.StorageProfile.OSDisk.Image.Uri) if err != nil { - return nil, err + log.Printf("artifact error: Error parsing osdisk url: %s", err) + return &res, nil } res.OSDiskUri = vhdUri.String() diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 7ca702884..7b67f4862 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -332,6 +332,17 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) template.(*CaptureTemplate), getSasUrlFunc) } + return NewManagedImageArtifact(b.config.OSType, + b.config.ManagedImageResourceGroupName, + b.config.ManagedImageName, + b.config.Location, + managedImageID, + b.config.ManagedImageOSDiskSnapshotName, + b.config.ManagedImageDataDiskSnapshotPrefix, + generatedData, + b.stateBag.Get(constants.ArmKeepOSDisk).(bool), + nil, + getSasUrlFunc) } else if template, ok := b.stateBag.GetOk(constants.ArmCaptureTemplate); ok { return NewArtifact( template.(*CaptureTemplate), diff --git a/builder/azure/arm/step_deploy_template_test.go b/builder/azure/arm/step_deploy_template_test.go index a29ef2a0c..088b9901a 100644 --- a/builder/azure/arm/step_deploy_template_test.go +++ b/builder/azure/arm/step_deploy_template_test.go @@ -117,6 +117,7 @@ func TestStepDeployTemplateCleanupShouldDeleteManagedOSImageInExistingResourceGr stateBag.Put(constants.ArmIsManagedImage, true) stateBag.Put(constants.ArmIsExistingResourceGroup, true) stateBag.Put(constants.ArmIsResourceGroupCreated, true) + stateBag.Put(constants.ArmKeepOSDisk, false) stateBag.Put("ui", packersdk.TestUi(t)) testSubject.Cleanup(stateBag) @@ -133,6 +134,7 @@ func TestStepDeployTemplateCleanupShouldDeleteManagedOSImageInTemporaryResourceG stateBag.Put(constants.ArmIsManagedImage, true) stateBag.Put(constants.ArmIsExistingResourceGroup, false) stateBag.Put(constants.ArmIsResourceGroupCreated, true) + stateBag.Put(constants.ArmKeepOSDisk, false) stateBag.Put("ui", packersdk.TestUi(t)) testSubject.Cleanup(stateBag) @@ -149,6 +151,7 @@ func TestStepDeployTemplateCleanupShouldDeleteVHDOSImageInExistingResourceGroup( stateBag.Put(constants.ArmIsManagedImage, false) stateBag.Put(constants.ArmIsExistingResourceGroup, true) stateBag.Put(constants.ArmIsResourceGroupCreated, true) + stateBag.Put(constants.ArmKeepOSDisk, false) stateBag.Put("ui", packersdk.TestUi(t)) testSubject.Cleanup(stateBag) @@ -165,6 +168,7 @@ func TestStepDeployTemplateCleanupShouldVHDOSImageInTemporaryResourceGroup(t *te stateBag.Put(constants.ArmIsManagedImage, false) stateBag.Put(constants.ArmIsExistingResourceGroup, false) stateBag.Put(constants.ArmIsResourceGroupCreated, true) + stateBag.Put(constants.ArmKeepOSDisk, false) stateBag.Put("ui", packersdk.TestUi(t)) testSubject.Cleanup(stateBag)