From 0a75bca694a9835ed9a12be9e570374848eede2b Mon Sep 17 00:00:00 2001 From: Andrew Jackura Date: Thu, 18 Jan 2018 17:11:08 -0800 Subject: [PATCH] Images in state of OBSOLETE or DELETED still exist, use new typed error (#288) Images in state of OBSOLETE or DELETED still exist and should be added to the registry This allows DeleteResources to delete images in this state --- daisy/disk_test.go | 5 +++++ daisy/error.go | 9 +++++---- daisy/image.go | 25 +++++++++++++------------ daisy/resource_registry.go | 10 ++++++---- daisy/step_delete_resources.go | 5 +++-- daisy/step_delete_resources_test.go | 2 +- daisy/test_common_test.go | 13 +++++++++++-- 7 files changed, 44 insertions(+), 25 deletions(-) diff --git a/daisy/disk_test.go b/daisy/disk_test.go index 51afa21a7..6db1b9b04 100644 --- a/daisy/disk_test.go +++ b/daisy/disk_test.go @@ -369,6 +369,11 @@ func TestDiskValidate(t *testing.T) { &Disk{Disk: compute.Disk{Name: "d4", SizeGb: 1, Type: ty}}, false, }, + { + "image OBSOLETE case", + &Disk{Disk: compute.Disk{Name: "d1", SourceImage: fmt.Sprintf("projects/foo/global/images/%s", testImage), Type: ty}}, + true, + }, { "source image dne case", &Disk{Disk: compute.Disk{Name: "d5", SourceImage: "dne", Type: ty}}, diff --git a/daisy/error.go b/daisy/error.go index c7268e5a8..4670b451a 100644 --- a/daisy/error.go +++ b/daisy/error.go @@ -20,10 +20,11 @@ import ( ) const ( - untypedError = "" - multiError = "MultiError" - fileIOError = "FileIOError" - resourceDNEError = "ResourceDoesNotExist" + untypedError = "" + multiError = "MultiError" + fileIOError = "FileIOError" + resourceDNEError = "ResourceDoesNotExist" + imageObsoleteDeletedError = "ImageObsoleteOrDeleted" apiError = "APIError" apiError404 = "APIError404" diff --git a/daisy/image.go b/daisy/image.go index ec80f31a8..d892f6a14 100644 --- a/daisy/image.go +++ b/daisy/image.go @@ -29,7 +29,7 @@ import ( var ( imageCache struct { - exists map[string][]string + exists map[string][]*compute.Image mu sync.Mutex } imageFamilyCache struct { @@ -64,7 +64,7 @@ func imageExists(client daisyCompute.Client, project, family, name string) (bool } if img.Deprecated != nil { if img.Deprecated.State == "OBSOLETE" || img.Deprecated.State == "DELETED" { - return false, nil + return true, typedErrf(imageObsoleteDeletedError, "image %q in state %q", img.Name, img.Deprecated.State) } } imageFamilyCache.exists[project] = append(imageFamilyCache.exists[project], name) @@ -77,25 +77,26 @@ func imageExists(client daisyCompute.Client, project, family, name string) (bool imageCache.mu.Lock() defer imageCache.mu.Unlock() if imageCache.exists == nil { - imageCache.exists = map[string][]string{} + imageCache.exists = map[string][]*compute.Image{} } if _, ok := imageCache.exists[project]; !ok { il, err := client.ListImages(project) if err != nil { return false, errf("error listing images for project %q: %v", project, err) } - var images []string - for _, i := range il { - if i.Deprecated != nil { - if i.Deprecated.State == "OBSOLETE" || i.Deprecated.State == "DELETED" { - continue - } + imageCache.exists[project] = il + } + + for _, i := range imageCache.exists[project] { + if name == i.Name { + if i.Deprecated != nil && (i.Deprecated.State == "OBSOLETE" || i.Deprecated.State == "DELETED") { + return true, typedErrf(imageObsoleteDeletedError, "image %q in state %q", name, i.Deprecated.State) } - images = append(images, i.Name) + return true, nil } - imageCache.exists[project] = images } - return strIn(name, imageCache.exists[project]), nil + + return false, nil } // Image is used to create a GCE image. diff --git a/daisy/resource_registry.go b/daisy/resource_registry.go index b88e648ca..accfc29b9 100644 --- a/daisy/resource_registry.go +++ b/daisy/resource_registry.go @@ -148,16 +148,18 @@ func (r *baseResourceRegistry) registerExisting(url string) (*Resource, dErr) { if r, ok := r.m[url]; ok { return r, nil } - if exists, err := resourceExists(r.w.ComputeClient, url); err != nil { - return nil, err - } else if !exists { + exists, err := resourceExists(r.w.ComputeClient, url) + if !exists { + if err != nil { + return nil, err + } return nil, typedErrf(resourceDNEError, "%s does not exist", url) } parts := strings.Split(url, "/") res := &Resource{RealName: parts[len(parts)-1], link: url, NoCleanup: true} r.m[url] = res - return res, nil + return res, err } func (r *baseResourceRegistry) registerUsage(name string, s *Step) (*Resource, dErr) { diff --git a/daisy/step_delete_resources.go b/daisy/step_delete_resources.go index 959600cc7..8a954a0d0 100644 --- a/daisy/step_delete_resources.go +++ b/daisy/step_delete_resources.go @@ -16,9 +16,8 @@ package daisy import ( "context" - "sync" - "log" + "sync" compute "google.golang.org/api/compute/v1" ) @@ -78,6 +77,8 @@ func (d *DeleteResources) checkError(err dErr, logger *log.Logger) dErr { if err != nil && err.Type() == resourceDNEError { logger.Printf("DeleteResources WARNING: Error validating deletion: %v", err) return nil + } else if err != nil && err.Type() == imageObsoleteDeletedError { + return nil } return err } diff --git a/daisy/step_delete_resources_test.go b/daisy/step_delete_resources_test.go index eb8ae2765..e47ee4d46 100644 --- a/daisy/step_delete_resources_test.go +++ b/daisy/step_delete_resources_test.go @@ -120,7 +120,7 @@ func TestDeleteResourcesValidate(t *testing.T) { } // Good case. - dr := DeleteResources{Disks: []string{"d0"}, Images: []string{"im0"}, Instances: []string{"in0"}} + dr := DeleteResources{Disks: []string{"d0"}, Images: []string{"im0", "projects/foo/global/images/" + testImage, "projects/foo/global/images/family/foo"}, Instances: []string{"in0"}} if err := dr.validate(ctx, s); err != nil { t.Errorf("validation should not have failed: %v", err) } diff --git a/daisy/test_common_test.go b/daisy/test_common_test.go index 3317fc033..ff9f2df26 100644 --- a/daisy/test_common_test.go +++ b/daisy/test_common_test.go @@ -161,8 +161,11 @@ func newTestGCEClient() (*daisyCompute.TestClient, error) { c.ListZonesFn = func(_ string, _ ...daisyCompute.ListCallOption) ([]*compute.Zone, error) { return []*compute.Zone{{Name: testZone}}, nil } - c.ListImagesFn = func(_ string, _ ...daisyCompute.ListCallOption) ([]*compute.Image, error) { - return []*compute.Image{{Name: testImage}}, nil + c.ListImagesFn = func(p string, _ ...daisyCompute.ListCallOption) ([]*compute.Image, error) { + if p == testProject { + return []*compute.Image{{Name: testImage}}, nil + } + return []*compute.Image{{Name: testImage, Deprecated: &compute.DeprecationStatus{State: "OBSOLETE"}}}, nil } c.ListDisksFn = func(p, z string, _ ...daisyCompute.ListCallOption) ([]*compute.Disk, error) { if p != testProject { @@ -200,6 +203,12 @@ func newTestGCEClient() (*daisyCompute.TestClient, error) { } return nil } + c.GetImageFromFamilyFn = func(_, f string) (*compute.Image, error) { + if f == testFamily { + return &compute.Image{Name: testImage}, nil + } + return &compute.Image{Name: testImage, Deprecated: &compute.DeprecationStatus{State: "OBSOLETE"}}, nil + } return c, err }