Adjust image lookup logic to deal with more dynamic situations

Signed-off-by: Darren Shepherd <[email protected]>

pkg/apis/internal.acorn.io/v1/build.go

@@ -17,7 +17,7 @@ type ContainerImageBuilderSpec struct {
 
 func (in *ContainerImageBuilderSpec) Normalize() *ContainerImageBuilderSpec {
 	out := *in
-	if out.Image != "" {
+	if out.Image != "" && out.Build != nil && len(out.Build.ContextDirs) == 0 {
 		out.Build = nil
 	}
 	if len(in.Sidecars) > 0 {

pkg/appdefinition/appdefinition_test.go

@@ -1385,6 +1385,231 @@ secrets: {
 	}, appSpec.Secrets["opt"])
 }
 
+func TestImageDynamicallyChangedToUnavailable(t *testing.T) {
+	acornCue := `
+args: image: "foo"
+containers: c: image: args.image
+`
+	image := &v1.ImagesData{
+		Builds: []v1.BuildRecord{
+			{
+				ContainerBuild: &v1.ContainerImageBuilderSpec{
+					Image: "foo",
+				},
+				ImageKey: "c",
+			},
+		},
+		Containers: map[string]v1.ContainerData{
+			"c": {
+				Image: "foo-hash",
+			},
+		},
+	}
+
+	app, err := NewAppDefinition([]byte(acornCue))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = app.BuilderSpec()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	app = app.WithImageData(*image)
+
+	appSpec, err := app.AppSpec()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assert.Equal(t, "foo-hash", appSpec.Containers["c"].Image)
+
+	app, _, err = app.WithArgs(map[string]any{
+		"image": "not-foo",
+	}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = app.AppSpec()
+	assert.ErrorContains(t, err, "failed to find image for container [c] in Acornfile, you may need to define the image/build in the images section of the Acornfile")
+}
+
+func TestImageDynamicScale(t *testing.T) {
+	acornCue := `
+args: scale: 1
+for i in std.range(1,args.scale+1) {
+	containers: "a\(i)": build: "./foo"
+	acorns: "ac\(i)": build: "./afoo"
+	services: "sc\(i)": build: "./afoo"
+}
+`
+	image := &v1.ImagesData{
+		Builds: []v1.BuildRecord{
+			{
+				ContainerBuild: &v1.ContainerImageBuilderSpec{
+					Build: &v1.Build{
+						Context:    "./foo",
+						Dockerfile: "foo/Dockerfile",
+					},
+				},
+				ImageKey: "a1",
+			},
+			{
+				AcornBuild: &v1.AcornBuilderSpec{
+					Build: &v1.AcornBuild{
+						Context:   "./afoo",
+						Acornfile: "afoo/Acornfile",
+					},
+				},
+				ImageKey: "ac1",
+			},
+			{
+				AcornBuild: &v1.AcornBuilderSpec{
+					Build: &v1.AcornBuild{
+						Context:   "./afoo",
+						Acornfile: "afoo/Acornfile",
+					},
+				},
+				ImageKey: "sc1",
+			},
+		},
+		Containers: map[string]v1.ContainerData{
+			"a1": {
+				Image: "foo-hash",
+			},
+		},
+		Acorns: map[string]v1.ImageData{
+			"ac1": {
+				Image: "afoo-hash",
+			},
+		},
+	}
+
+	app, err := NewAppDefinition([]byte(acornCue))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = app.BuilderSpec()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	app = app.WithImageData(*image)
+	devApp, _, err := app.WithArgs(map[string]any{"scale": 2}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	appSpec, err := devApp.AppSpec()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assert.Equal(t, "foo-hash", appSpec.Containers["a1"].Image)
+	assert.Equal(t, "foo-hash", appSpec.Containers["a2"].Image)
+	assert.Equal(t, "afoo-hash", appSpec.Acorns["ac1"].Image)
+	assert.Equal(t, "afoo-hash", appSpec.Acorns["ac2"].Image)
+	assert.Equal(t, "afoo-hash", appSpec.Services["sc1"].Image)
+	assert.Equal(t, "afoo-hash", appSpec.Services["sc2"].Image)
+}
+
+func TestImageDynamicSwitch(t *testing.T) {
+	acornCue := `
+if args.dev {
+	acorns: a: image: "acorn-dev"
+	containers: c: image: "container-dev"
+}
+if !args.dev {
+	acorns: a: image: "acorn"
+	containers: c: image: "container"
+}
+
+images: aimage: image: "acorn-dev"
+images: cimage: image: "container-dev"
+`
+	image := &v1.ImagesData{
+		Builds: []v1.BuildRecord{
+			{
+				ContainerBuild: &v1.ContainerImageBuilderSpec{
+					Image: "container",
+				},
+				ImageKey: "c",
+			},
+			{
+				AcornBuild: &v1.AcornBuilderSpec{
+					Image: "acorn",
+				},
+				ImageKey: "a",
+			},
+			{
+				ImageBuild: &v1.ImageBuilderSpec{
+					Image: "acorn-dev",
+				},
+				ImageKey: "aimage",
+			},
+			{
+				ImageBuild: &v1.ImageBuilderSpec{
+					Image: "container-dev",
+				},
+				ImageKey: "cimage",
+			},
+		},
+		Containers: map[string]v1.ContainerData{
+			"c": {
+				Image: "container-hash",
+			},
+		},
+		Acorns: map[string]v1.ImageData{
+			"a": {
+				Image: "acorn-hash",
+			},
+		},
+		Images: map[string]v1.ImageData{
+			"aimage": {
+				Image: "acorn-dev-hash",
+			},
+			"cimage": {
+				Image: "container-dev-hash",
+			},
+		},
+	}
+
+	app, err := NewAppDefinition([]byte(acornCue))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = app.BuilderSpec()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	app = app.WithImageData(*image)
+	devApp, _, err := app.WithArgs(nil, []string{"devMode"})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	appSpec, err := devApp.AppSpec()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assert.Equal(t, "container-dev-hash", appSpec.Containers["c"].Image)
+	assert.Equal(t, "acorn-dev-hash", appSpec.Acorns["a"].Image)
+
+	appSpec, err = app.AppSpec()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assert.Equal(t, "container-hash", appSpec.Containers["c"].Image)
+	assert.Equal(t, "acorn-hash", appSpec.Acorns["a"].Image)
+}
+
 func TestImageDataOverride(t *testing.T) {
 	acornCue := `
 containers: db: image: "mariadb"

pkg/appdefinition/lookup.go

@@ -4,6 +4,7 @@ import (
 	"strings"
 
 	v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
+	"github.com/acorn-io/z"
 	"k8s.io/apimachinery/pkg/api/equality"
 )
 
@@ -40,8 +41,18 @@ func findImageInImageData(imageData v1.ImagesData, imageKey string) (string, boo
 	return "", false
 }
 
-func findContainerImage(imageData v1.ImagesData, containerBuild *v1.Build) (string, bool) {
+func findContainerImage(imageData v1.ImagesData, image string, containerBuild *v1.Build) (string, bool) {
 	if containerBuild == nil {
+		if image != "" {
+			for _, build := range imageData.Builds {
+				if build.ContainerBuild != nil && build.ContainerBuild.Image == image && build.ImageKey != "" {
+					return findImageInImageData(imageData, build.ImageKey)
+				}
+				if build.ImageBuild != nil && build.ImageBuild.Image == image && build.ImageKey != "" {
+					return findImageInImageData(imageData, build.ImageKey)
+				}
+			}
+		}
 		return "", false
 	}
 
@@ -68,18 +79,27 @@ func findContainerImage(imageData v1.ImagesData, containerBuild *v1.Build) (stri
 	return "", false
 }
 
-func findAcornImage(imageData v1.ImagesData, image string, acornBuild *v1.AcornBuild) (string, bool) {
+func isAutoUpgradePattern(image string) bool {
+	return strings.ContainsAny(image, "*#")
+}
+
+func findAcornImage(imageData v1.ImagesData, autoUpgrade *bool, image string, acornBuild *v1.AcornBuild) (string, bool) {
+	if isAutoUpgradePattern(image) || z.Dereference(autoUpgrade) {
+		return image, image != ""
+	}
+
 	if acornBuild == nil {
 		for _, build := range imageData.Builds {
-			if build.ImageKey == "" && build.AcornBuild != nil && build.AcornBuild.Image == image {
-				return image, true
+			if build.ImageKey != "" && build.AcornBuild != nil && build.AcornBuild.Image == image && !build.AcornBuild.AutoUpgrade {
+				return findImageInImageData(imageData, build.ImageKey)
 			}
-			if build.ImageKey != "" && build.AcornBuild != nil && build.AcornBuild.Image == image && build.AcornBuild.Build == nil && !build.AcornBuild.AutoUpgrade {
+			if build.ImageKey != "" && build.ImageBuild != nil && build.ImageBuild.Image == image {
 				return findImageInImageData(imageData, build.ImageKey)
 			}
 		}
 		return "", false
 	}
+
 	for _, build := range imageData.Builds {
 		var (
 			testBuild *v1.AcornBuild
@@ -106,12 +126,13 @@ func findAcornImage(imageData v1.ImagesData, image string, acornBuild *v1.AcornB
 	return "", false
 }
 
-func GetImageReferenceForServiceName(svcName string, appSpec *v1.AppSpec, imageData v1.ImagesData) (string, bool) {
+func GetImageReferenceForServiceName(svcName string, appSpec *v1.AppSpec, imageData v1.ImagesData) (result string, found bool) {
 	var (
 		parts         = strings.Split(svcName, ".")
 		containerName string
 		sidecarName   string
 	)
+
 	if len(parts) > 2 {
 		return "", false
 	} else if len(parts) == 2 {
@@ -120,47 +141,54 @@ func GetImageReferenceForServiceName(svcName string, appSpec *v1.AppSpec, imageD
 		containerName = svcName
 	}
 
-	// This logic is here to support where autoUpgrade is true but it wasn't true during the build.  So the build actually
-	// as an embedded image. But since autoUpgrade is on we want to fall back to the behavior of grabbing the external
-	// image.
-	if serviceDef, ok := appSpec.Services[svcName]; ok && serviceDef.AutoUpgrade != nil && *serviceDef.AutoUpgrade && serviceDef.Image != "" {
-		return serviceDef.Image, true
-	} else if acornDef, ok := appSpec.Acorns[svcName]; ok && acornDef.AutoUpgrade != nil && *acornDef.AutoUpgrade && acornDef.Image != "" {
-		return acornDef.Image, true
-	}
-
-	image, ok := findImageInImageData(imageData, svcName)
-	if ok {
-		return image, true
-	}
-
 	if serviceDef, ok := appSpec.Services[svcName]; ok {
-		return findAcornImage(imageData, serviceDef.Image, serviceDef.Build)
+		return findAcornImage(imageData, serviceDef.AutoUpgrade, serviceDef.Image, serviceDef.Build)
 	} else if acornDef, ok := appSpec.Acorns[svcName]; ok {
-		return findAcornImage(imageData, acornDef.Image, acornDef.Build)
+		return findAcornImage(imageData, acornDef.AutoUpgrade, acornDef.Image, acornDef.Build)
 	} else if containerDef, ok := appSpec.Containers[containerName]; ok {
 		if sidecarName != "" {
 			containerDef, ok = containerDef.Sidecars[sidecarName]
 			if !ok {
 				return "", false
 			}
 		}
-		return findContainerImage(imageData, containerDef.Build)
+		result, ok := findContainerImage(imageData, containerDef.Image, containerDef.Build)
+		// Only fall back to this check if there are no build records available, or this was a old build
+		// that didn't record build with a context dir properly
+		if !ok && oldBuggyBuild(containerDef, imageData) {
+			return findImageInImageData(imageData, svcName)
+		}
+		return result, ok
 	} else if jobDef, ok := appSpec.Jobs[containerName]; ok {
 		if sidecarName != "" {
 			jobDef, ok = jobDef.Sidecars[sidecarName]
 			if !ok {
 				return "", false
 			}
 		}
-		return findContainerImage(imageData, jobDef.Build)
+		result, ok := findContainerImage(imageData, jobDef.Image, jobDef.Build)
+		// Only fall back to this check if there are no build records available, or this was a old build
+		// that didn't record build with a context dir properly
+		if !ok && oldBuggyBuild(jobDef, imageData) {
+			return findImageInImageData(imageData, svcName)
+		}
+		return result, ok
 	} else if imageDef, ok := appSpec.Images[svcName]; ok {
 		if imageDef.Build != nil {
-			findContainerImage(imageData, imageDef.Build)
+			return findContainerImage(imageData, "", imageDef.Build)
 		} else if imageDef.AcornBuild != nil {
-			findContainerImage(imageData, imageDef.Build)
+			return findContainerImage(imageData, "", imageDef.Build)
+		} else {
+			return findImageInImageData(imageData, svcName)
 		}
 	}
 
 	return "", false
 }
+
+func oldBuggyBuild(con v1.Container, imageData v1.ImagesData) bool {
+	if len(imageData.Builds) == 0 {
+		return true
+	}
+	return con.Build != nil && len(con.Build.ContextDirs) > 0
+}