From e758891878cf350b38ff84b562255d25e11776d5 Mon Sep 17 00:00:00 2001 From: sylviamoss Date: Fri, 25 Sep 2020 16:53:59 +0200 Subject: [PATCH 1/6] make shell-local post-processor return copy of previous artifact --- builder/docker/artifact_import.go | 6 ++-- post-processor/docker-push/post-processor.go | 6 ++-- post-processor/shell-local/artifact.go | 34 ++++++++++++++++++++ post-processor/shell-local/post-processor.go | 12 +++++++ 4 files changed, 50 insertions(+), 8 deletions(-) create mode 100644 post-processor/shell-local/artifact.go diff --git a/builder/docker/artifact_import.go b/builder/docker/artifact_import.go index c626a9df3..b6d0b3a5b 100644 --- a/builder/docker/artifact_import.go +++ b/builder/docker/artifact_import.go @@ -34,12 +34,10 @@ func (a *ImportArtifact) String() string { if tags == nil { return fmt.Sprintf("Imported Docker image: %s", a.Id()) } - cast := tags.([]interface{}) + cast := tags.([]string) names := []string{} for _, name := range cast { - if n, ok := name.(string); ok { - names = append(names, n) - } + names = append(names, name) } return fmt.Sprintf("Imported Docker image: %s with tags %s", a.Id(), strings.Join(names, " ")) diff --git a/post-processor/docker-push/post-processor.go b/post-processor/docker-push/post-processor.go index 979ea2896..440ce2989 100644 --- a/post-processor/docker-push/post-processor.go +++ b/post-processor/docker-push/post-processor.go @@ -106,11 +106,9 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact names := []string{artifact.Id()} tags := artifact.State("docker_tags") if tags != nil { - cast := tags.([]interface{}) + cast := tags.([]string) for _, name := range cast { - if n, ok := name.(string); ok { - names = append(names, n) - } + names = append(names, name) } } diff --git a/post-processor/shell-local/artifact.go b/post-processor/shell-local/artifact.go new file mode 100644 index 000000000..f8dee119a --- /dev/null +++ b/post-processor/shell-local/artifact.go @@ -0,0 +1,34 @@ +package shell_local + +type Artifact struct { + builderId string + stringVal string + destroy func() error + files []string + id string + state func(name string) interface{} +} + +func (a *Artifact) BuilderId() string { + return a.builderId +} + +func (a *Artifact) Files() []string { + return a.files +} + +func (a *Artifact) Id() string { + return a.id +} + +func (a *Artifact) String() string { + return a.stringVal +} + +func (a *Artifact) State(name string) interface{} { + return a.state(name) +} + +func (a *Artifact) Destroy() error { + return a.destroy() +} diff --git a/post-processor/shell-local/post-processor.go b/post-processor/shell-local/post-processor.go index a17ce67e3..78984bd55 100644 --- a/post-processor/shell-local/post-processor.go +++ b/post-processor/shell-local/post-processor.go @@ -54,6 +54,18 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact return nil, false, false, retErr } + // Return a "copy" of the artifact to keep the previous artifact data + // If we don't this, the data will be lost in packer/rpc/post_processor.go + // when a new artifact is created from the client. + artifact = &Artifact{ + builderId: artifact.BuilderId(), + stringVal: artifact.String(), + destroy: artifact.Destroy, + files: artifact.Files(), + id: artifact.Id(), + state: artifact.State, + } + // Force shell-local pp to keep the input artifact, because otherwise we'll // lose it instead of being able to pass it through. If you want to delete // the input artifact for a shell local pp, use the artifice pp to create a From aa9c162c607b90a95335d37ac1f99bddbbc8b827 Mon Sep 17 00:00:00 2001 From: sylviamoss Date: Mon, 28 Sep 2020 10:52:28 +0200 Subject: [PATCH 2/6] improve docker_tags artifact state read --- builder/docker/artifact_import.go | 15 +++++---------- post-processor/docker-push/post-processor.go | 9 +++------ 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/builder/docker/artifact_import.go b/builder/docker/artifact_import.go index b6d0b3a5b..b5f71751b 100644 --- a/builder/docker/artifact_import.go +++ b/builder/docker/artifact_import.go @@ -30,17 +30,12 @@ func (a *ImportArtifact) Id() string { } func (a *ImportArtifact) String() string { - tags := a.StateData["docker_tags"] - if tags == nil { - return fmt.Sprintf("Imported Docker image: %s", a.Id()) + tags, _ := a.StateData["docker_tags"].([]string) + if len(tags) > 0 { + return fmt.Sprintf("Imported Docker image: %s with tags %s", + a.Id(), strings.Join(tags, " ")) } - cast := tags.([]string) - names := []string{} - for _, name := range cast { - names = append(names, name) - } - return fmt.Sprintf("Imported Docker image: %s with tags %s", - a.Id(), strings.Join(names, " ")) + return fmt.Sprintf("Imported Docker image: %s", a.Id()) } func (a *ImportArtifact) State(name string) interface{} { diff --git a/post-processor/docker-push/post-processor.go b/post-processor/docker-push/post-processor.go index 440ce2989..5c12e85f3 100644 --- a/post-processor/docker-push/post-processor.go +++ b/post-processor/docker-push/post-processor.go @@ -104,12 +104,9 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact } names := []string{artifact.Id()} - tags := artifact.State("docker_tags") - if tags != nil { - cast := tags.([]string) - for _, name := range cast { - names = append(names, name) - } + tags, _ := artifact.State("docker_tags").([]string) + if len(tags) > 0 { + names = append(names, tags...) } // Get the name. From 49bc7665c119e784bb73d637639ee6ec1c7e8924 Mon Sep 17 00:00:00 2001 From: sylviamoss Date: Mon, 28 Sep 2020 11:18:24 +0200 Subject: [PATCH 3/6] fix docker push tags cast --- post-processor/docker-push/post-processor.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/post-processor/docker-push/post-processor.go b/post-processor/docker-push/post-processor.go index 5c12e85f3..831ba1a7c 100644 --- a/post-processor/docker-push/post-processor.go +++ b/post-processor/docker-push/post-processor.go @@ -104,9 +104,11 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact } names := []string{artifact.Id()} - tags, _ := artifact.State("docker_tags").([]string) - if len(tags) > 0 { - names = append(names, tags...) + tags, _ := artifact.State("docker_tags").([]interface{}) + for _, tag := range tags { + if name, ok := tag.(string); ok { + names = append(names, name) + } } // Get the name. From c8874c938221fc2d4371b3a7bda998adea4aa5cc Mon Sep 17 00:00:00 2001 From: sylviamoss Date: Tue, 29 Sep 2020 14:25:15 +0200 Subject: [PATCH 4/6] improve docker_tag cast to avoid failures --- builder/docker/artifact_import.go | 12 +++++++++++- post-processor/docker-push/post-processor.go | 18 ++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/builder/docker/artifact_import.go b/builder/docker/artifact_import.go index b5f71751b..a53c0a52b 100644 --- a/builder/docker/artifact_import.go +++ b/builder/docker/artifact_import.go @@ -30,7 +30,17 @@ func (a *ImportArtifact) Id() string { } func (a *ImportArtifact) String() string { - tags, _ := a.StateData["docker_tags"].([]string) + var tags []string + switch t := a.StateData["docker_tags"].(type) { + case []string: + tags = t + case []interface{}: + for _, name := range t { + if n, ok := name.(string); ok { + tags = append(tags, n) + } + } + } if len(tags) > 0 { return fmt.Sprintf("Imported Docker image: %s with tags %s", a.Id(), strings.Join(tags, " ")) diff --git a/post-processor/docker-push/post-processor.go b/post-processor/docker-push/post-processor.go index 831ba1a7c..97ffb8627 100644 --- a/post-processor/docker-push/post-processor.go +++ b/post-processor/docker-push/post-processor.go @@ -5,7 +5,6 @@ package dockerpush import ( "context" "fmt" - "github.com/hashicorp/hcl/v2/hcldec" "github.com/hashicorp/packer/builder/docker" "github.com/hashicorp/packer/common" @@ -103,14 +102,21 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact }() } - names := []string{artifact.Id()} - tags, _ := artifact.State("docker_tags").([]interface{}) - for _, tag := range tags { - if name, ok := tag.(string); ok { - names = append(names, name) + var tags []string + switch t := artifact.State("docker_tags").(type) { + case []string: + tags = t + case []interface{}: + for _, name := range t { + if n, ok := name.(string); ok { + tags = append(tags, n) + } } } + names := []string{artifact.Id()} + names = append(names, tags...) + // Get the name. for _, name := range names { ui.Message("Pushing: " + name) From 8f9001b5312d522c1b4845623dd24910dbaff53c Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Fri, 2 Oct 2020 11:35:20 +0200 Subject: [PATCH 5/6] uncopy --- post-processor/shell-local/artifact.go | 34 -------------------- post-processor/shell-local/post-processor.go | 12 ------- 2 files changed, 46 deletions(-) delete mode 100644 post-processor/shell-local/artifact.go diff --git a/post-processor/shell-local/artifact.go b/post-processor/shell-local/artifact.go deleted file mode 100644 index f8dee119a..000000000 --- a/post-processor/shell-local/artifact.go +++ /dev/null @@ -1,34 +0,0 @@ -package shell_local - -type Artifact struct { - builderId string - stringVal string - destroy func() error - files []string - id string - state func(name string) interface{} -} - -func (a *Artifact) BuilderId() string { - return a.builderId -} - -func (a *Artifact) Files() []string { - return a.files -} - -func (a *Artifact) Id() string { - return a.id -} - -func (a *Artifact) String() string { - return a.stringVal -} - -func (a *Artifact) State(name string) interface{} { - return a.state(name) -} - -func (a *Artifact) Destroy() error { - return a.destroy() -} diff --git a/post-processor/shell-local/post-processor.go b/post-processor/shell-local/post-processor.go index 78984bd55..a17ce67e3 100644 --- a/post-processor/shell-local/post-processor.go +++ b/post-processor/shell-local/post-processor.go @@ -54,18 +54,6 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact return nil, false, false, retErr } - // Return a "copy" of the artifact to keep the previous artifact data - // If we don't this, the data will be lost in packer/rpc/post_processor.go - // when a new artifact is created from the client. - artifact = &Artifact{ - builderId: artifact.BuilderId(), - stringVal: artifact.String(), - destroy: artifact.Destroy, - files: artifact.Files(), - id: artifact.Id(), - state: artifact.State, - } - // Force shell-local pp to keep the input artifact, because otherwise we'll // lose it instead of being able to pass it through. If you want to delete // the input artifact for a shell local pp, use the artifice pp to create a From 9f971733711aaf0895a8bb445161090f52d49557 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 5 Oct 2020 14:27:55 +0200 Subject: [PATCH 6/6] PostProcessorServer.PostProcess: don't close the artifact we are serving Sometimes, the artifact returned by PostProcess is the artifact from client.Artifact() and in that case we don't want to close client; otherwise the outcome is sort of undetermined. See #9995 for a good test file. fix #9995 --- packer/rpc/post_processor.go | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/packer/rpc/post_processor.go b/packer/rpc/post_processor.go index 4b36f2dd4..00d492a0a 100644 --- a/packer/rpc/post_processor.go +++ b/packer/rpc/post_processor.go @@ -99,28 +99,42 @@ func (p *PostProcessorServer) PostProcess(streamId uint32, reply *PostProcessorP if err != nil { return NewBasicError(err) } - defer client.Close() if p.context == nil { p.context, p.contextCancel = context.WithCancel(context.Background()) } - streamId = 0 - artifactResult, keep, forceOverride, err := p.p.PostProcess(p.context, client.Ui(), client.Artifact()) - if err == nil && artifactResult != nil { - streamId = p.mux.NextId() - server := newServerWithMux(p.mux, streamId) - server.RegisterArtifact(artifactResult) - go server.Serve() - } - + artifact := client.Artifact() + artifactResult, keep, forceOverride, err := p.p.PostProcess(p.context, client.Ui(), artifact) *reply = PostProcessorProcessResponse{ Err: NewBasicError(err), Keep: keep, ForceOverride: forceOverride, - StreamId: streamId, + StreamId: 0, + } + if err != nil { + log.Printf("error: %v", err) + client.Close() + return nil } + if artifactResult != artifact { + // Sometimes, the artifact returned by PostProcess is the artifact from + // client.Artifact() and in that case we don't want to close client; + // otherwise the outcome is sort of undetermined. See [GH-9995] for a + // good test file. + defer client.Close() + } + + if artifactResult != nil { + streamId = p.mux.NextId() + reply.StreamId = streamId + server := newServerWithMux(p.mux, streamId) + if err := server.RegisterArtifact(artifactResult); err != nil { + return err + } + go server.Serve() + } return nil }