From 32f89015fef72926605cfdd5fe783cb0a20d75ca Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Mon, 26 Feb 2024 16:56:23 -0500 Subject: [PATCH] hcp: fix hcp artifact extraction method With Packer 1.10.1 we started warning when a build failed to complete because of a potential incompatibility with the builder being used. This led to cases in which the build failed for other reasons, and Packer would still warn of potential incompatibilities, even if the builder was in effect HCP compatible. We attempted to fix this issue by introducing a new error type, and checks when we read the artifacts linked to a build, however this loop would fail when any one of the artifacts is not compatible with HCP Packer, leading to false failures. To avoid this problem, we log incompatibilities to the verbose logger, and only signal the problem if all the artifacts could not be used to upload data to HCP Packer, in which case it's almost certain that if the build succeeded and no artifacts are registered to the build, that all the components used are not compatible with HCP, and should be reported as such to users. --- internal/hcp/registry/types.bucket.go | 24 ++-- internal/hcp/registry/types.bucket_test.go | 125 +++++++++++++++++++++ 2 files changed, 141 insertions(+), 8 deletions(-) diff --git a/internal/hcp/registry/types.bucket.go b/internal/hcp/registry/types.bucket.go index 6b92e6ce1..6eda3b484 100644 --- a/internal/hcp/registry/types.bucket.go +++ b/internal/hcp/registry/types.bucket.go @@ -643,26 +643,34 @@ func (bucket *Bucket) completeBuild( state := art.State(packerSDKRegistry.ArtifactStateURI) if state == nil { - return packerSDKArtifacts, &NotAHCPArtifactError{ - fmt.Errorf("The HCP artifact returned by the builder is nil, this is likely because the builder does not support HCP Packer."), - } + log.Printf("[WARN] - artifact %q returned a nil value for the HCP state, ignoring", art.BuilderId()) + continue } err = decoder.Decode(state) if err != nil { - return packerSDKArtifacts, &NotAHCPArtifactError{ - fmt.Errorf("Failed to obtain HCP Packer compliant artifact: %s", err), - } + log.Printf("[WARN] - artifact %q failed to be decoded to an HCP artifact, this is probably because it is not compatible: %s", art.BuilderId(), err) + continue } - log.Printf("[TRACE] updating artifacts for build %q", buildName) err = bucket.UpdateArtifactForBuild(buildName, sdkImages...) - if err != nil { return packerSDKArtifacts, fmt.Errorf("failed to add artifact for %q: %s", buildName, err) } } + build, err := bucket.Version.Build(buildName) + if err != nil { + return packerSDKArtifacts, fmt.Errorf( + "failed to get build %q from version being built. This is a Packer bug.", + buildName) + } + if len(build.Artifacts) == 0 { + return packerSDKArtifacts, &NotAHCPArtifactError{ + fmt.Errorf("No HCP Packer-compatible artifacts were found for the build"), + } + } + parErr := bucket.markBuildComplete(ctx, buildName) if parErr != nil { return packerSDKArtifacts, fmt.Errorf( diff --git a/internal/hcp/registry/types.bucket_test.go b/internal/hcp/registry/types.bucket_test.go index 57b18d6a9..1b50fb887 100644 --- a/internal/hcp/registry/types.bucket_test.go +++ b/internal/hcp/registry/types.bucket_test.go @@ -5,10 +5,15 @@ package registry import ( "context" + "reflect" "strconv" + "sync" "testing" "github.com/google/go-cmp/cmp" + "github.com/hashicorp/hcp-sdk-go/clients/cloud-packer-service/stable/2023-01-01/models" + "github.com/hashicorp/packer-plugin-sdk/packer" + "github.com/hashicorp/packer-plugin-sdk/packer/registry/image" "github.com/hashicorp/packer/hcl2template" hcpPackerAPI "github.com/hashicorp/packer/internal/hcp/api" ) @@ -385,3 +390,123 @@ func TestReadFromHCLBuildBlock(t *testing.T) { }) } } + +func TestCompleteBuild(t *testing.T) { + hcpArtifact := &packer.MockArtifact{ + BuilderIdValue: "builder.test", + FilesValue: []string{"file.one"}, + IdValue: "Test", + StateValues: map[string]interface{}{ + "builder.test": "OK", + image.ArtifactStateURI: &image.Image{ + ImageID: "hcp-test", + ProviderName: "none", + ProviderRegion: "none", + Labels: map[string]string{}, + SourceImageID: "", + }, + }, + DestroyCalled: false, + StringValue: "", + } + nonHCPArtifact := &packer.MockArtifact{ + BuilderIdValue: "builder.test", + FilesValue: []string{"file.one"}, + IdValue: "Test", + StateValues: map[string]interface{}{ + "builder.test": "OK", + }, + DestroyCalled: false, + StringValue: "", + } + + testCases := []struct { + name string + artifactsToUse []packer.Artifact + expectError bool + wantNotHCPErr bool + }{ + { + "OK - one artifact compatible with HCP", + []packer.Artifact{ + hcpArtifact, + }, + false, false, + }, + { + "Fail - no artifacts", + []packer.Artifact{}, + true, false, + }, + { + "Fail - only non HCP compatible artifacts", + []packer.Artifact{ + nonHCPArtifact, + }, + true, true, + }, + { + "OK - one hcp artifact, one non hcp artifact (order matters)", + []packer.Artifact{ + hcpArtifact, + nonHCPArtifact, + }, + false, false, + }, + { + "OK - one non hcp artifact, one hcp artifact (order matters)", + []packer.Artifact{ + nonHCPArtifact, + hcpArtifact, + }, + false, false, + }, + } + mockCli := &hcpPackerAPI.Client{ + Packer: hcpPackerAPI.NewMockPackerClientService(), + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + dummyBucket := &Bucket{ + Name: "test-bucket", + Description: "test", + Destination: "none", + RunningBuilds: map[string]chan struct{}{ + // Need buffer with 1 cap so we can signal end of + // heartbeats in test, otherwise it'll block + "test-build": make(chan struct{}, 1), + }, + Version: &Version{ + ID: "noneID", + Fingerprint: "TestFingerprint", + RunUUID: "testuuid", + builds: sync.Map{}, + }, + client: mockCli, + } + + dummyBucket.Version.StoreBuild("test-build", &Build{ + ID: "test-build", + Platform: "none", + ComponentType: "none", + RunUUID: "testuuid", + Artifacts: make(map[string]image.Image), + Status: models.HashicorpCloudPacker20230101BuildStatusBUILDRUNNING, + }) + + _, err := dummyBucket.completeBuild(context.Background(), "test-build", tt.artifactsToUse, nil) + if err != nil != tt.expectError { + t.Errorf("expected %t error; got %t", tt.expectError, err != nil) + t.Logf("error was: %s", err) + } + + if err != nil && tt.wantNotHCPErr { + _, ok := err.(*NotAHCPArtifactError) + if !ok { + t.Errorf("expected a NotAHCPArtifactError, got a %q", reflect.TypeOf(err).String()) + } + } + }) + } +}