Skip to content

Commit

Permalink
Update HCP Packer build labels when re-running Packer on an incomplet…
Browse files Browse the repository at this point in the history 
…e build (#11584)

* Update HCP Packer build labels argument

Previously when running a partial build on multi-cloud build template it
was found that build labels were only being applied at the creation for
the partially executed build. Leaving all other completed builds with
no HCP Packer build labels. This updates how incomplete builds are
loaded from the registry and ensure that any defined
hcp_packer_registry.build_labels are assigned to the build before
starting an actual Packer build.

Related to: #11573

* Add test case for overwriting build labels

* Update tests to call CreateInitialBuild for non-existing builds

* Rename test case to TestBucket_PopulateIteration

* Fix data race in PopulateIteration against mock service

Before Change
```
WARNING: DATA RACE
Write at 0x00c0005421b0 by goroutine 47:
  github.com/hashicorp/packer/internal/registry.(*MockPackerClientService).PackerServiceCreateBuild()
      /Users/scrubbed/Development/packer/internal/registry/mock_service.go:173 +0x2b6
  github.com/hashicorp/packer/internal/registry.(*Client).CreateBuild()
      /Users/scrubbed/Development/packer/internal/registry/service.go:169 +0x592
  github.com/hashicorp/packer/internal/registry.(*Bucket).CreateInitialBuildForIteration()
      /Users/scrubbed/Development/packer/internal/registry/types.bucket.go:107 +0x204
  github.com/hashicorp/packer/internal/registry.(*Bucket).PopulateIteration.func1()
      /Users/scrubbed/Development/packer/internal/registry/types.bucket.go:368 +0x14e
  github.com/hashicorp/packer/internal/registry.(*Bucket).PopulateIteration·dwrap·1()
      /Users/scrubbed/Development/packer/internal/registry/types.bucket.go:376 +0x58

Previous write at 0x00c0005421b0 by goroutine 46:
  github.com/hashicorp/packer/internal/registry.(*MockPackerClientService).PackerServiceCreateBuild()
      /Users/scrubbed/Development/packer/internal/registry/mock_service.go:173 +0x2b6
  github.com/hashicorp/packer/internal/registry.(*Client).CreateBuild()
      /Users/scrubbed/Development/packer/internal/registry/service.go:169 +0x592
  github.com/hashicorp/packer/internal/registry.(*Bucket).CreateInitialBuildForIteration()
      /Users/scrubbed/Development/packer/internal/registry/types.bucket.go:107 +0x204
  github.com/hashicorp/packer/internal/registry.(*Bucket).PopulateIteration.func1()
      /Users/scrubbed/Development/packer/internal/registry/types.bucket.go:368 +0x14e
  github.com/hashicorp/packer/internal/registry.(*Bucket).PopulateIteration·dwrap·1()
      /Users/scrubbed/Development/packer/internal/registry/types.bucket.go:376 +0x58

Goroutine 47 (running) created at:
  github.com/hashicorp/packer/internal/registry.(*Bucket).PopulateIteration()
      /Users/scrubbed/Development/packer/internal/registry/types.bucket.go:362 +0x5c7
  github.com/hashicorp/packer/internal/registry.TestBucket_UpdateLabelsForBuild_withMultipleBuilds()
      /Users/scrubbed/Development/packer/internal/registry/types.bucket_test.go:179 +0xf7
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47

Goroutine 46 (finished) created at:
  github.com/hashicorp/packer/internal/registry.(*Bucket).PopulateIteration()
      /Users/scrubbed/Development/packer/internal/registry/types.bucket.go:362 +0x5c7
  github.com/hashicorp/packer/internal/registry.TestBucket_UpdateLabelsForBuild_withMultipleBuilds()
      /Users/scrubbed/Development/packer/internal/registry/types.bucket_test.go:179 +0xf7
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1259 +0x22f
  testing.(*T).Run·dwrap·21()
      /usr/local/go/src/testing/testing.go:1306 +0x47
==================
```

* Add methods for managing builds on an Iteration

* Update Mock Service to be a bit more concurrent

It is a simple set of changes to make it work for the current testing
use cases. If we need to we can move the Called fields to counters or
re-architect the mock.

This change also fixes a race condition when appending to the Slice of
errs when calling PopulateIteration.

* Update test case descriptions

* Apply suggestions from code review

Only append to errs if err is not nil

Co-authored-by: Sylvia Moss <moss@hashicorp.com>

* Fix linting issues

Co-authored-by: Sylvia Moss <moss@hashicorp.com>
  • Loading branch information
@nywilken @sylviamoss
nywilken and sylviamoss committed Feb 25, 2022
1 parent 0a7dbb6 commit dd525fb
Show file tree
Hide file tree
Showing 6 changed files with 485 additions and 269 deletions.
123 changes: 77 additions & 46 deletions internal/registry/mock_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@ import (
"google.golang.org/grpc/status"
)

//MockPackerClientService represents a basic mock of the Cloud Packer Service.
//Upon calling a service method a boolean is set to true to indicate that a method has been called.
//To skip the setting of these booleans set TrackCalledServiceMethods to false; defaults to true in NewMockPackerClientService().
type MockPackerClientService struct {
CreateBucketCalled, UpdateBucketCalled, BucketAlreadyExist bool
CreateIterationCalled, GetIterationCalled, IterationAlreadyExist, IterationCompleted bool
CreateBuildCalled, UpdateBuildCalled, ListBuildsCalled, BuildAlreadyDone bool
TrackCalledServiceMethods bool

// Mock Creates
CreateBucketResp *models.HashicorpCloudPackerCreateBucketResponse
Expand All @@ -25,46 +29,27 @@ type MockPackerClientService struct {
// Mock Gets
GetIterationResp *models.HashicorpCloudPackerGetIterationResponse

ExistingBuilds []string
ExistingBuilds []string
ExistingBuildLabels map[string]string

packerSvc.ClientService
}

//NewMockPackerClientService returns a basic mock of the Cloud Packer Service.
//Upon calling a service method a boolean is set to true to indicate that a method has been called.
//To skip the setting of these booleans set TrackCalledServiceMethods to false. By default it is true.
func NewMockPackerClientService() *MockPackerClientService {
m := MockPackerClientService{
ExistingBuilds: make([]string, 0),
}

m.CreateBucketResp = &models.HashicorpCloudPackerCreateBucketResponse{
Bucket: &models.HashicorpCloudPackerBucket{
ID: "bucket-id",
},
}

m.CreateIterationResp = &models.HashicorpCloudPackerCreateIterationResponse{
Iteration: &models.HashicorpCloudPackerIteration{
ID: "iteration-id",
},
}

m.CreateBuildResp = &models.HashicorpCloudPackerCreateBuildResponse{
Build: &models.HashicorpCloudPackerBuild{
PackerRunUUID: "test-uuid",
Status: models.HashicorpCloudPackerBuildStatusUNSET,
},
}

m.GetIterationResp = &models.HashicorpCloudPackerGetIterationResponse{
Iteration: &models.HashicorpCloudPackerIteration{
ID: "iteration-id",
Builds: make([]*models.HashicorpCloudPackerBuild, 0),
},
ExistingBuilds: make([]string, 0),
ExistingBuildLabels: make(map[string]string),
TrackCalledServiceMethods: true,
}

return &m
}

func (svc *MockPackerClientService) PackerServiceCreateBucket(params *packerSvc.PackerServiceCreateBucketParams, _ runtime.ClientAuthInfoWriter) (*packerSvc.PackerServiceCreateBucketOK, error) {

if svc.BucketAlreadyExist {
return nil, status.Error(codes.AlreadyExists, fmt.Sprintf("Code:%d %s", codes.AlreadyExists, codes.AlreadyExists.String()))
}
Expand All @@ -76,19 +61,27 @@ func (svc *MockPackerClientService) PackerServiceCreateBucket(params *packerSvc.
return nil, errors.New("No bucket slug was passed in")
}

svc.CreateBucketCalled = true
// This is set in NewMockPackerClientService()
svc.CreateBucketResp.Bucket.Slug = params.Body.BucketSlug
if svc.TrackCalledServiceMethods {
svc.CreateBucketCalled = true
}
payload := &models.HashicorpCloudPackerCreateBucketResponse{
Bucket: &models.HashicorpCloudPackerBucket{
ID: "bucket-id",
},
}
payload.Bucket.Slug = params.Body.BucketSlug

ok := &packerSvc.PackerServiceCreateBucketOK{
Payload: svc.CreateBucketResp,
Payload: payload,
}

return ok, nil
}

func (svc *MockPackerClientService) PackerServiceUpdateBucket(params *packerSvc.PackerServiceUpdateBucketParams, _ runtime.ClientAuthInfoWriter) (*packerSvc.PackerServiceUpdateBucketOK, error) {
svc.UpdateBucketCalled = true
if svc.TrackCalledServiceMethods {
svc.UpdateBucketCalled = true
}

return packerSvc.NewPackerServiceUpdateBucketOK(), nil
}
Expand All @@ -106,12 +99,20 @@ func (svc *MockPackerClientService) PackerServiceCreateIteration(params *packerS
return nil, errors.New("No valid Fingerprint was passed in")
}

svc.CreateIterationCalled = true
svc.CreateIterationResp.Iteration.BucketSlug = params.Body.BucketSlug
svc.CreateIterationResp.Iteration.Fingerprint = params.Body.Fingerprint
if svc.TrackCalledServiceMethods {
svc.CreateIterationCalled = true
}
payload := &models.HashicorpCloudPackerCreateIterationResponse{
Iteration: &models.HashicorpCloudPackerIteration{
ID: "iteration-id",
},
}

payload.Iteration.BucketSlug = params.Body.BucketSlug
payload.Iteration.Fingerprint = params.Body.Fingerprint

ok := &packerSvc.PackerServiceCreateIterationOK{
Payload: svc.CreateIterationResp,
Payload: payload,
}

return ok, nil
Expand All @@ -130,11 +131,21 @@ func (svc *MockPackerClientService) PackerServiceGetIteration(params *packerSvc.
return nil, errors.New("No valid Fingerprint was passed in")
}

svc.GetIterationCalled = true
if svc.TrackCalledServiceMethods {
svc.GetIterationCalled = true
}

payload := &models.HashicorpCloudPackerGetIterationResponse{
Iteration: &models.HashicorpCloudPackerIteration{
ID: "iteration-id",
Builds: make([]*models.HashicorpCloudPackerBuild, 0),
},
}

//
payload.Iteration.BucketSlug = params.BucketSlug
payload.Iteration.Fingerprint = *params.Fingerprint
ok := &packerSvc.PackerServiceGetIterationOK{
Payload: svc.GetIterationResp,
Payload: payload,
}

if svc.IterationCompleted {
Expand All @@ -147,6 +158,7 @@ func (svc *MockPackerClientService) PackerServiceGetIteration(params *packerSvc.
Images: []*models.HashicorpCloudPackerImage{
{ImageID: "image-id", Region: "somewhere"},
},
Labels: make(map[string]string),
})
}

Expand All @@ -166,13 +178,22 @@ func (svc *MockPackerClientService) PackerServiceCreateBuild(params *packerSvc.P
return nil, errors.New("No build componentType was passed in")
}

svc.CreateBuildCalled = true
if svc.TrackCalledServiceMethods {
svc.CreateBuildCalled = true
}

svc.CreateBuildResp.Build.ComponentType = params.Body.Build.ComponentType
svc.CreateBuildResp.Build.IterationID = params.IterationID
payload := &models.HashicorpCloudPackerCreateBuildResponse{
Build: &models.HashicorpCloudPackerBuild{
PackerRunUUID: "test-uuid",
Status: models.HashicorpCloudPackerBuildStatusUNSET,
},
}

payload.Build.ComponentType = params.Body.Build.ComponentType
payload.Build.IterationID = params.IterationID

ok := packerSvc.NewPackerServiceCreateBuildOK()
ok.Payload = svc.CreateBuildResp
ok.Payload = payload

return ok, nil
}
Expand All @@ -190,7 +211,10 @@ func (svc *MockPackerClientService) PackerServiceUpdateBuild(params *packerSvc.P
return nil, errors.New("No build status was passed in")
}

svc.UpdateBuildCalled = true
if svc.TrackCalledServiceMethods {
svc.UpdateBuildCalled = true
}

ok := packerSvc.NewPackerServiceUpdateBuildOK()
ok.Payload = &models.HashicorpCloudPackerUpdateBuildResponse{
Build: &models.HashicorpCloudPackerBuild{
Expand All @@ -204,18 +228,25 @@ func (svc *MockPackerClientService) PackerServiceListBuilds(params *packerSvc.Pa

status := models.HashicorpCloudPackerBuildStatusUNSET
images := make([]*models.HashicorpCloudPackerImage, 0)
labels := make(map[string]string)
if svc.BuildAlreadyDone {
status = models.HashicorpCloudPackerBuildStatusDONE
images = append(images, &models.HashicorpCloudPackerImage{ID: "image-id", Region: "somewhere"})
images = append(images, &models.HashicorpCloudPackerImage{ImageID: "image-id", Region: "somewhere"})
}

for k, v := range svc.ExistingBuildLabels {
labels[k] = v
}

builds := make([]*models.HashicorpCloudPackerBuild, 0, len(svc.ExistingBuilds))
for i, name := range svc.ExistingBuilds {
builds = append(builds, &models.HashicorpCloudPackerBuild{
ID: name + "--" + strconv.Itoa(i),
ComponentType: name,
CloudProvider: "mockProvider",
Status: status,
Images: images,
Labels: labels,
})
}

Expand Down
Loading

0 comments on commit dd525fb

Please sign in to comment.