clean up azure temporary managed os disk (#10713)

* clean up temporary managed os disk

* improve message for skipping disk deletion

Co-authored-by: Wilken Rivera <dev@wilkenrivera.com>

* re-arrange osdisk/additional disk cleanup

Co-authored-by: Wilken Rivera <dev@wilkenrivera.com>

* remove additional disk cleanup

* add some validation on scenarios

* alway clean up resources inside template cleanup

* tidy to master

* clarify naming and comments

Co-authored-by: Wilken Rivera <dev@wilkenrivera.com>

* test: add acceptance testing for azure cleanup

* revert template changes

* fix err check

* delete resources in parallel with retry to avoid serial deletion

* nit: improve log message for transient deletion errors

* fix: typo

* remove unused func to make lint happy

Co-authored-by: Wilken Rivera <dev@wilkenrivera.com>
This commit is contained in:
Ace Eldeib 2021-03-18 09:09:57 -07:00 committed by GitHub
parent 0f541aaf5e
commit 3227d3da43
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 280 additions and 50 deletions

View File

@ -20,12 +20,18 @@ package arm
// go test -v -timeout 90m -run TestBuilderAcc_.*
import (
"testing"
"bytes"
"context"
"errors"
"fmt"
"os"
"strings"
"testing"
"github.com/Azure/go-autorest/autorest"
"github.com/Azure/go-autorest/autorest/azure/auth"
builderT "github.com/hashicorp/packer-plugin-sdk/acctest"
packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
)
const DeviceLoginAcceptanceTest = "DEVICELOGIN_TEST"
@ -96,10 +102,15 @@ func TestBuilderAcc_ManagedDisk_Linux_AzureCLI(t *testing.T) {
return
}
var b Builder
builderT.Test(t, builderT.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Builder: &Builder{},
PreCheck: func() { testAuthPreCheck(t) },
Builder: &b,
Template: testBuilderAccManagedDiskLinuxAzureCLI,
Check: func([]packersdk.Artifact) error {
checkTemporaryGroupDeleted(t, &b)
return nil
},
})
}
@ -112,15 +123,108 @@ func TestBuilderAcc_Blob_Windows(t *testing.T) {
}
func TestBuilderAcc_Blob_Linux(t *testing.T) {
var b Builder
builderT.Test(t, builderT.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Builder: &Builder{},
PreCheck: func() { testAuthPreCheck(t) },
Builder: &b,
Template: testBuilderAccBlobLinux,
Check: func([]packersdk.Artifact) error {
checkUnmanagedVHDDeleted(t, &b)
return nil
},
})
}
func testAccPreCheck(*testing.T) {}
func testAuthPreCheck(t *testing.T) {
_, err := auth.NewAuthorizerFromEnvironment()
if err != nil {
t.Fatalf("failed to auth to azure: %s", err)
}
}
func checkTemporaryGroupDeleted(t *testing.T, b *Builder) {
ui := testUi()
spnCloud, spnKeyVault, err := b.getServicePrincipalTokens(ui.Say)
if err != nil {
t.Fatalf("failed getting azure tokens: %s", err)
}
ui.Message("Creating test Azure Resource Manager (ARM) client ...")
azureClient, err := NewAzureClient(
b.config.ClientConfig.SubscriptionID,
b.config.SharedGalleryDestination.SigDestinationSubscription,
b.config.ResourceGroupName,
b.config.StorageAccount,
b.config.ClientConfig.CloudEnvironment(),
b.config.SharedGalleryTimeout,
b.config.PollingDurationTimeout,
spnCloud,
spnKeyVault)
if err != nil {
t.Fatalf("failed to create azure client: %s", err)
}
// Validate resource group has been deleted
_, err = azureClient.GroupsClient.Get(context.Background(), b.config.tmpResourceGroupName)
if err == nil || !resourceNotFound(err) {
t.Fatalf("failed validating resource group deletion: %s", err)
}
}
func checkUnmanagedVHDDeleted(t *testing.T, b *Builder) {
ui := testUi()
spnCloud, spnKeyVault, err := b.getServicePrincipalTokens(ui.Say)
if err != nil {
t.Fatalf("failed getting azure tokens: %s", err)
}
azureClient, err := NewAzureClient(
b.config.ClientConfig.SubscriptionID,
b.config.SharedGalleryDestination.SigDestinationSubscription,
b.config.ResourceGroupName,
b.config.StorageAccount,
b.config.ClientConfig.CloudEnvironment(),
b.config.SharedGalleryTimeout,
b.config.PollingDurationTimeout,
spnCloud,
spnKeyVault)
if err != nil {
t.Fatalf("failed to create azure client: %s", err)
}
// validate temporary os blob was deleted
blob := azureClient.BlobStorageClient.GetContainerReference("images").GetBlobReference(b.config.tmpOSDiskName)
_, err = blob.BreakLease(nil)
if err != nil && !strings.Contains(err.Error(), "BlobNotFound") {
t.Fatalf("failed validating deletion of unmanaged vhd: %s", err)
}
// Validate resource group has been deleted
_, err = azureClient.GroupsClient.Get(context.Background(), b.config.tmpResourceGroupName)
if err == nil || !resourceNotFound(err) {
t.Fatalf("failed validating resource group deletion: %s", err)
}
}
func resourceNotFound(err error) bool {
derr := autorest.DetailedError{}
return errors.As(err, &derr) && derr.StatusCode == 404
}
func testUi() *packersdk.BasicUi {
return &packersdk.BasicUi{
Reader: new(bytes.Buffer),
Writer: new(bytes.Buffer),
ErrorWriter: new(bytes.Buffer),
}
}
const testBuilderAccManagedDiskWindows = `
{
"variables": {
@ -155,6 +259,7 @@ const testBuilderAccManagedDiskWindows = `
}]
}
`
const testBuilderAccManagedDiskWindowsBuildResourceGroup = `
{
"variables": {
@ -287,6 +392,7 @@ const testBuilderAccManagedDiskLinux = `
}]
}
`
const testBuilderAccManagedDiskLinuxDeviceLogin = `
{
"variables": {
@ -379,6 +485,7 @@ const testBuilderAccBlobLinux = `
}]
}
`
const testBuilderAccManagedDiskLinuxAzureCLI = `
{
"builders": [{
@ -388,7 +495,8 @@ const testBuilderAccManagedDiskLinuxAzureCLI = `
"managed_image_resource_group_name": "packer-acceptance-test",
"managed_image_name": "testBuilderAccManagedDiskLinuxAzureCLI-{{timestamp}}",
"temp_resource_group_name": "packer-acceptance-test-managed-cli",
"os_type": "Linux",
"image_publisher": "Canonical",
"image_offer": "UbuntuServer",

View File

@ -6,6 +6,7 @@ import (
"fmt"
"net/url"
"strings"
"sync"
"time"
"github.com/hashicorp/packer-plugin-sdk/multistep"
@ -15,16 +16,17 @@ import (
)
type StepDeployTemplate struct {
client *AzureClient
deploy func(ctx context.Context, resourceGroupName string, deploymentName string) error
delete func(ctx context.Context, deploymentName, resourceGroupName string) error
disk func(ctx context.Context, resourceGroupName string, computeName string) (string, string, error)
deleteDisk func(ctx context.Context, imageType string, imageName string, resourceGroupName string) error
say func(message string)
error func(e error)
config *Config
factory templateFactoryFunc
name string
client *AzureClient
deploy func(ctx context.Context, resourceGroupName string, deploymentName string) error
delete func(ctx context.Context, deploymentName, resourceGroupName string) error
disk func(ctx context.Context, resourceGroupName string, computeName string) (string, string, error)
deleteDisk func(ctx context.Context, imageType string, imageName string, resourceGroupName string) error
deleteDeployment func(ctx context.Context, state multistep.StateBag) error
say func(message string)
error func(e error)
config *Config
factory templateFactoryFunc
name string
}
func NewStepDeployTemplate(client *AzureClient, ui packersdk.Ui, config *Config, deploymentName string, factory templateFactoryFunc) *StepDeployTemplate {
@ -41,6 +43,7 @@ func NewStepDeployTemplate(client *AzureClient, ui packersdk.Ui, config *Config,
step.delete = step.deleteDeploymentResources
step.disk = step.getImageDetails
step.deleteDisk = step.deleteImage
step.deleteDeployment = step.deleteDeploymentObject
return step
}
@ -58,32 +61,45 @@ func (s *StepDeployTemplate) Run(ctx context.Context, state multistep.StateBag)
func (s *StepDeployTemplate) Cleanup(state multistep.StateBag) {
defer func() {
err := s.deleteTemplate(context.Background(), state)
err := s.deleteDeployment(context.Background(), state)
if err != nil {
s.say(s.client.LastError.Error())
s.say(err.Error())
}
}()
// Only clean up if this is an existing resource group that has been verified to exist.
// ArmIsResourceGroupCreated is set in step_create_resource_group to true, when Packer has verified that the resource group exists.
// ArmIsExistingResourceGroup is set to true when build_resource_group is set in the Packer configuration.
existingResourceGroup := state.Get(constants.ArmIsExistingResourceGroup).(bool)
resourceGroupCreated := state.Get(constants.ArmIsResourceGroupCreated).(bool)
if !existingResourceGroup || !resourceGroupCreated {
return
}
ui := state.Get("ui").(packersdk.Ui)
ui.Say("\nThe resource group was not created by Packer, deleting individual resources ...")
ui.Say("\nDeleting individual resources ...")
deploymentName := s.name
resourceGroupName := state.Get(constants.ArmResourceGroupName).(string)
err := s.deleteDeploymentResources(context.TODO(), deploymentName, resourceGroupName)
// Get image disk details before deleting the image; otherwise we won't be able to
// delete the disk as the image request will return a 404
computeName := state.Get(constants.ArmComputeName).(string)
imageType, imageName, err := s.disk(context.TODO(), resourceGroupName, computeName)
if err != nil && !strings.Contains(err.Error(), "ResourceNotFound") {
ui.Error(fmt.Sprintf("Could not retrieve OS Image details: %s", err))
}
err = s.delete(context.TODO(), deploymentName, resourceGroupName)
if err != nil {
s.reportIfError(err, resourceGroupName)
}
NewStepDeleteAdditionalDisks(s.client, ui).Run(context.TODO(), state)
// The disk was not found on the VM, this is an error.
if imageType == "" && imageName == "" {
ui.Error(fmt.Sprintf("Failed to find temporary OS disk on VM. Please delete manually.\n\n"+
"VM Name: %s\n"+
"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))
}
}
func (s *StepDeployTemplate) deployTemplate(ctx context.Context, resourceGroupName string, deploymentName string) error {
@ -106,7 +122,7 @@ func (s *StepDeployTemplate) deployTemplate(ctx context.Context, resourceGroupNa
return err
}
func (s *StepDeployTemplate) deleteTemplate(ctx context.Context, state multistep.StateBag) error {
func (s *StepDeployTemplate) deleteDeploymentObject(ctx context.Context, state multistep.StateBag) error {
deploymentName := s.name
resourceGroupName := state.Get(constants.ArmResourceGroupName).(string)
ui := state.Get("ui").(packersdk.Ui)
@ -222,6 +238,8 @@ func (s *StepDeployTemplate) deleteDeploymentResources(ctx context.Context, depl
return err
}
resources := map[string]string{}
for deploymentOperations.NotDone() {
deploymentOperation := deploymentOperations.Value()
// Sometimes an empty operation is added to the list by Azure
@ -233,30 +251,47 @@ func (s *StepDeployTemplate) deleteDeploymentResources(ctx context.Context, depl
resourceName := *deploymentOperation.Properties.TargetResource.ResourceName
resourceType := *deploymentOperation.Properties.TargetResource.ResourceType
s.say(fmt.Sprintf(" -> %s : '%s'", resourceType, resourceName))
err = retry.Config{
Tries: 10,
RetryDelay: (&retry.Backoff{InitialBackoff: 10 * time.Second, MaxBackoff: 600 * time.Second, Multiplier: 2}).Linear,
}.Run(ctx, func(ctx context.Context) error {
err := deleteResource(ctx, s.client,
resourceType,
resourceName,
resourceGroupName)
if err != nil {
s.reportIfError(err, resourceName)
}
return nil
})
if err != nil {
s.reportIfError(err, resourceName)
}
s.say(fmt.Sprintf("Adding to deletion queue -> %s : '%s'", resourceType, resourceName))
resources[resourceType] = resourceName
if err = deploymentOperations.Next(); err != nil {
return err
}
}
var wg sync.WaitGroup
wg.Add(len(resources))
for resourceType, resourceName := range resources {
go func(resourceType, resourceName string) {
defer wg.Done()
retryConfig := retry.Config{
Tries: 10,
RetryDelay: (&retry.Backoff{InitialBackoff: 10 * time.Second, MaxBackoff: 600 * time.Second, Multiplier: 2}).Linear,
}
err = retryConfig.Run(ctx, func(ctx context.Context) error {
s.say(fmt.Sprintf("Attempting deletion -> %s : '%s'", resourceType, resourceName))
err := deleteResource(ctx, s.client,
resourceType,
resourceName,
resourceGroupName)
if err != nil {
s.say(fmt.Sprintf("Error deleting resource. Will retry.\n"+
"Name: %s\n"+
"Error: %s\n", resourceName, err.Error()))
}
return err
})
if err != nil {
s.reportIfError(err, resourceName)
}
}(resourceType, resourceName)
}
s.say("Waiting for deletion of all resources...")
wg.Wait()
return nil
}

View File

@ -6,6 +6,7 @@ import (
"testing"
"github.com/hashicorp/packer-plugin-sdk/multistep"
packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
"github.com/hashicorp/packer/builder/azure/common/constants"
)
@ -108,11 +109,97 @@ func TestStepDeployTemplateDeleteImageShouldFailWithInvalidImage(t *testing.T) {
}
}
func TestStepDeployTemplateCleanupShouldDeleteManagedOSImageInExistingResourceGroup(t *testing.T) {
var deleteDiskCounter = 0
var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter)
stateBag := createTestStateBagStepDeployTemplate()
stateBag.Put(constants.ArmIsManagedImage, true)
stateBag.Put(constants.ArmIsExistingResourceGroup, true)
stateBag.Put(constants.ArmIsResourceGroupCreated, true)
stateBag.Put("ui", packersdk.TestUi(t))
testSubject.Cleanup(stateBag)
if deleteDiskCounter != 1 {
t.Fatalf("Expected DeployTemplate Cleanup to invoke deleteDisk 1 time, but invoked %d times", deleteDiskCounter)
}
}
func TestStepDeployTemplateCleanupShouldDeleteManagedOSImageInTemporaryResourceGroup(t *testing.T) {
var deleteDiskCounter = 0
var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter)
stateBag := createTestStateBagStepDeployTemplate()
stateBag.Put(constants.ArmIsManagedImage, true)
stateBag.Put(constants.ArmIsExistingResourceGroup, false)
stateBag.Put(constants.ArmIsResourceGroupCreated, true)
stateBag.Put("ui", packersdk.TestUi(t))
testSubject.Cleanup(stateBag)
if deleteDiskCounter != 1 {
t.Fatalf("Expected DeployTemplate Cleanup to invoke deleteDisk 1 times, but invoked %d times", deleteDiskCounter)
}
}
func TestStepDeployTemplateCleanupShouldDeleteVHDOSImageInExistingResourceGroup(t *testing.T) {
var deleteDiskCounter = 0
var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter)
stateBag := createTestStateBagStepDeployTemplate()
stateBag.Put(constants.ArmIsManagedImage, false)
stateBag.Put(constants.ArmIsExistingResourceGroup, true)
stateBag.Put(constants.ArmIsResourceGroupCreated, true)
stateBag.Put("ui", packersdk.TestUi(t))
testSubject.Cleanup(stateBag)
if deleteDiskCounter != 1 {
t.Fatalf("Expected DeployTemplate Cleanup to invoke deleteDisk 1 time, but invoked %d times", deleteDiskCounter)
}
}
func TestStepDeployTemplateCleanupShouldVHDOSImageInTemporaryResourceGroup(t *testing.T) {
var deleteDiskCounter = 0
var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter)
stateBag := createTestStateBagStepDeployTemplate()
stateBag.Put(constants.ArmIsManagedImage, false)
stateBag.Put(constants.ArmIsExistingResourceGroup, false)
stateBag.Put(constants.ArmIsResourceGroupCreated, true)
stateBag.Put("ui", packersdk.TestUi(t))
testSubject.Cleanup(stateBag)
if deleteDiskCounter != 1 {
t.Fatalf("Expected DeployTemplate Cleanup to invoke deleteDisk 1 times, but invoked %d times", deleteDiskCounter)
}
}
func createTestStateBagStepDeployTemplate() multistep.StateBag {
stateBag := new(multistep.BasicStateBag)
stateBag.Put(constants.ArmDeploymentName, "Unit Test: DeploymentName")
stateBag.Put(constants.ArmResourceGroupName, "Unit Test: ResourceGroupName")
stateBag.Put(constants.ArmComputeName, "Unit Test: ComputeName")
return stateBag
}
func createTestStepDeployTemplateDeleteOSImage(deleteDiskCounter *int) *StepDeployTemplate {
return &StepDeployTemplate{
deploy: func(context.Context, string, string) error { return nil },
say: func(message string) {},
error: func(e error) {},
deleteDisk: func(ctx context.Context, imageType string, imageName string, resourceGroupName string) error {
*deleteDiskCounter++
return nil
},
disk: func(ctx context.Context, resourceGroupName, computeName string) (string, string, error) {
return "Microsoft.Compute/disks", "", nil
},
delete: func(ctx context.Context, deploymentName, resourceGroupName string) error {
return nil
},
deleteDeployment: func(ctx context.Context, state multistep.StateBag) error {
return nil
},
}
}