fix(staged-dockerfile): panics on Healthcheck and Maintainer instruct…

…ions and no duplicates in images-sets 1. Panics occured in Healthcheck and Maintainer instructions because of nil param instead of corresponding backend-instruction. 2. Duplicates was in images-sets when several dockerfile-stage instructions refer to the same stage. refs #2215 Signed-off-by: Timofey Kirillov <timofey.kirillov@flant.com>
werf · Oct 26, 2022 · 2a55266 · 2a55266
1 parent f0cde50
commit 2a55266
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 20 deletions.
diff --git a/pkg/build/image/dockerfile.go b/pkg/build/image/dockerfile.go
@@ -65,36 +65,43 @@ func mapDockerfileToImagesSets(ctx context.Context, cfg *dockerfile.Dockerfile,
 	}
 
 	queue := []struct {
-		Stage *dockerfile.DockerfileStage
-		Level int
+		WerfImageName string
+		Stage         *dockerfile.DockerfileStage
+		Level         int
 	}{
-		{Stage: targetStage, Level: 0},
+		{WerfImageName: dockerfileImageConfig.Name, Stage: targetStage, Level: 0},
 	}
 
-	appendQueue := func(stage *dockerfile.DockerfileStage, level int) {
+	appendQueue := func(werfImageName string, stage *dockerfile.DockerfileStage, level int) {
 		queue = append(queue, struct {
-			Stage *dockerfile.DockerfileStage
-			Level int
-		}{Stage: stage, Level: level})
+			WerfImageName string
+			Stage         *dockerfile.DockerfileStage
+			Level         int
+		}{WerfImageName: werfImageName, Stage: stage, Level: level})
 	}
 
 	for len(queue) > 0 {
 		item := queue[0]
 		queue = queue[1:]
 
-		appendImageToCurrentSet := func(img *Image) {
+		appendImageToCurrentSet := func(newImg *Image) {
 			if item.Level == len(ret) {
 				ret = append([][]*Image{nil}, ret...)
 			}
-			ret[len(ret)-item.Level-1] = append(ret[len(ret)-item.Level-1], img)
+			for _, img := range ret[len(ret)-item.Level-1] {
+				if img.Name == newImg.Name {
+					return
+				}
+			}
+			ret[len(ret)-item.Level-1] = append(ret[len(ret)-item.Level-1], newImg)
 		}
 
 		stg := item.Stage
 
 		var img *Image
 		var err error
 		if baseStg := cfg.FindStage(stg.BaseName); baseStg != nil {
-			img, err = NewImage(ctx, dockerfileImageConfig.Name, StageAsBaseImage, ImageOptions{
+			img, err = NewImage(ctx, item.WerfImageName, StageAsBaseImage, ImageOptions{
 				IsDockerfileImage:     true,
 				DockerfileImageConfig: dockerfileImageConfig,
 				CommonImageOptions:    opts,
@@ -104,9 +111,9 @@ func mapDockerfileToImagesSets(ctx context.Context, cfg *dockerfile.Dockerfile,
 				return nil, fmt.Errorf("unable to map stage %s to werf image %q: %w", stg.LogName(), dockerfileImageConfig.Name, err)
 			}
 
-			appendQueue(baseStg, item.Level+1)
+			appendQueue(baseStg.WerfImageName(), baseStg, item.Level+1)
 		} else {
-			img, err = NewImage(ctx, dockerfileImageConfig.Name, ImageFromRegistryAsBaseImage, ImageOptions{
+			img, err = NewImage(ctx, item.WerfImageName, ImageFromRegistryAsBaseImage, ImageOptions{
 				IsDockerfileImage:     true,
 				DockerfileImageConfig: dockerfileImageConfig,
 				CommonImageOptions:    opts,
@@ -130,7 +137,8 @@ func mapDockerfileToImagesSets(ctx context.Context, cfg *dockerfile.Dockerfile,
 			var stg stage.Interface
 			switch typedInstr := any(instr).(type) {
 			case *dockerfile.DockerfileStageInstruction[*dockerfile_instruction.Arg]:
-				// TODO: implement
+				// TODO(staged-dockerfile): support build-args at this level or dockerfile pkg level (?)
+				continue
 			case *dockerfile.DockerfileStageInstruction[*dockerfile_instruction.Add]:
 				stg = stage_instruction.NewAdd(stageName, typedInstr, dockerfileImageConfig.Dependencies, !isFirstStage, baseStageOptions)
 			case *dockerfile.DockerfileStageInstruction[*dockerfile_instruction.Cmd]:
@@ -170,7 +178,7 @@ func mapDockerfileToImagesSets(ctx context.Context, cfg *dockerfile.Dockerfile,
 			img.stages = append(img.stages, stg)
 
 			for _, dep := range instr.GetDependenciesByStageRef() {
-				appendQueue(dep, item.Level+1)
+				appendQueue(dep.WerfImageName(), dep, item.Level+1)
 			}
 		}
 

diff --git a/pkg/build/stage/instruction/healthcheck.go b/pkg/build/stage/instruction/healthcheck.go
@@ -7,6 +7,7 @@ import (
 	"github.com/werf/werf/pkg/build/stage"
 	"github.com/werf/werf/pkg/config"
 	"github.com/werf/werf/pkg/container_backend"
+	backend_instruction "github.com/werf/werf/pkg/container_backend/instruction"
 	"github.com/werf/werf/pkg/dockerfile"
 	dockerfile_instruction "github.com/werf/werf/pkg/dockerfile/instruction"
 	"github.com/werf/werf/pkg/util"
@@ -17,8 +18,7 @@ type Healthcheck struct {
 }
 
 func NewHealthcheck(name stage.StageName, i *dockerfile.DockerfileStageInstruction[*dockerfile_instruction.Healthcheck], dependencies []*config.Dependency, hasPrevStage bool, opts *stage.BaseStageOptions) *Healthcheck {
-	// FIXME(staged-dockerfile): construct backend instruction
-	return &Healthcheck{Base: NewBase(name, i, nil, dependencies, hasPrevStage, opts)}
+	return &Healthcheck{Base: NewBase(name, i, backend_instruction.NewHealthcheck(*i.Data), dependencies, hasPrevStage, opts)}
 }
 
 func (stage *Healthcheck) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) {

diff --git a/pkg/build/stage/instruction/maintainer.go b/pkg/build/stage/instruction/maintainer.go
@@ -6,6 +6,7 @@ import (
 	"github.com/werf/werf/pkg/build/stage"
 	"github.com/werf/werf/pkg/config"
 	"github.com/werf/werf/pkg/container_backend"
+	backend_instruction "github.com/werf/werf/pkg/container_backend/instruction"
 	"github.com/werf/werf/pkg/dockerfile"
 	dockerfile_instruction "github.com/werf/werf/pkg/dockerfile/instruction"
 	"github.com/werf/werf/pkg/util"
@@ -16,8 +17,7 @@ type Maintainer struct {
 }
 
 func NewMaintainer(name stage.StageName, i *dockerfile.DockerfileStageInstruction[*dockerfile_instruction.Maintainer], dependencies []*config.Dependency, hasPrevStage bool, opts *stage.BaseStageOptions) *Maintainer {
-	// FIXME(staged-dockerfile): no Maintainer instruction
-	return &Maintainer{Base: NewBase(name, i, nil, dependencies, hasPrevStage, opts)}
+	return &Maintainer{Base: NewBase(name, i, backend_instruction.NewMaintainer(*i.Data), dependencies, hasPrevStage, opts)}
 }
 
 func (stage *Maintainer) GetDependencies(ctx context.Context, c stage.Conveyor, cb container_backend.ContainerBackend, prevImage, prevBuiltImage *stage.StageImage, buildContextArchive container_backend.BuildContextArchiver) (string, error) {

diff --git a/pkg/dockerfile/dockerfile_stage.go b/pkg/dockerfile/dockerfile_stage.go
@@ -35,9 +35,9 @@ func (stage *DockerfileStage) AppendDependencyStage(dep *DockerfileStage) {
 
 func (stage *DockerfileStage) WerfImageName() string {
 	if stage.HasStageName() {
-		return fmt.Sprintf("dockerfile-stage-%s", stage.StageName)
+		return fmt.Sprintf("stage/%s", stage.StageName)
 	} else {
-		return fmt.Sprintf("dockerfile-stage-%d", stage.Index)
+		return fmt.Sprintf("stage/%d", stage.Index)
 	}
 }
 

diff --git a/pkg/dockerfile/instruction.go b/pkg/dockerfile/instruction.go
@@ -30,6 +30,7 @@ func (i *DockerfileStageInstruction[T]) SetDependencyByStageRef(ref string, dep
 		if d.Index != dep.Index {
 			panic(fmt.Sprintf("already set instruction dependency %q to stage %d named %q, cannot replace with stage %d named %q, please report a bug", ref, d.Index, d.StageName, dep.Index, dep.StageName))
 		}
+		return
 	}
 	i.DependenciesByStageRef[ref] = dep
 }