fix: Check manifest images against the runtime class

internal/cmd/compute/deploy/deploy.go

@@ -139,10 +139,13 @@ publishes the workload. Later deploys keep it on until --http2=false.
 
 Use --runtime-class to choose the tier the workload's Instances run in, such as
 general-purpose or unikernel. A new workload without --runtime-class runs in
-the platform default. A workload keeps its runtime class for its whole life:
-omitting --runtime-class on an existing workload keeps its class, and naming a
-different one is refused. To change tiers, destroy the workload and deploy it
-again, or deploy under a new name.
+the class the platform marks as its default, which the plan names before
+anything is written. A unikernel class boots only Compute images built with
+'datumctl compute build'; run a standard container image with
+--runtime-class=general-purpose. A workload keeps its runtime class for its
+whole life: omitting --runtime-class on an existing workload keeps its class,
+and naming a different one is refused. To change tiers, destroy the workload
+and deploy it again, or deploy under a new name.
 
 Use --network to choose the network a new workload's Instances attach to. A
 new workload without --network attaches to the network named "default". If the
@@ -226,7 +229,7 @@ routine image bump changes nothing else. Remove one explicitly with a trailing
 	cmd.Flags().StringVar(&opts.build, "build", "", "Build and push the image from this directory before deploying (default \".\" if given with no value)")
 	cmd.Flags().Lookup("build").NoOptDefVal = "."
 	cmd.Flags().StringVar(&opts.instanceType, "instance-type", "datumcloud-d1-standard-2", "Instance type (e.g. datumcloud-d1-standard-2)")
-	cmd.Flags().StringVar(&opts.runtimeClass, "runtime-class", "", "Runtime class the workload's Instances run in (e.g. general-purpose, unikernel); defaults to the platform default and cannot be changed after the workload is created")
+	cmd.Flags().StringVar(&opts.runtimeClass, "runtime-class", "", "Runtime class the workload's Instances run in (e.g. general-purpose, unikernel); defaults to the class the platform marks as default, which the plan names, and cannot be changed after the workload is created")
 	cmd.Flags().StringVar(&opts.network, "network", "", "Network a new workload's Instances attach to (default \"default\"); cannot be changed after the workload is created")
 	cmd.Flags().StringSliceVar(&opts.locations, "location", nil, "One or more locations to deploy to (e.g. us-east-1,eu-west-1)")
 	cmd.Flags().StringVar(&opts.locationSelector, "location-selector", "", "Select every location whose topology matches a label selector (e.g. 'topology.datum.net/city-code=DFW' or 'topology.datum.net/region in (us-east-1,eu-west-1)')")
@@ -610,8 +613,8 @@ func deployFromFlags(cmd *cobra.Command, workloadName string, opts *options) err
 
 	fmt.Fprintln(out, planLine(`Placement "default"`,
 		fmt.Sprintf("%s, %s", describePlacementLocations(placement), util.FormatScaleSettings(scaleSettings))))
-	if runtimeClass != "" {
-		fmt.Fprintln(out, planLine("Runtime class", runtimeClass))
+	if class := planRuntimeClass(ctx, c, runtimeClass, creating); class != "" {
+		fmt.Fprintln(out, planLine("Runtime class", class))
 	}
 	fmt.Fprintln(out, planLine("Network", strings.Join(networks, ", ")))
 	for _, line := range planConfigLines(config) {
@@ -1170,6 +1173,14 @@ func deployFromFile(cmd *cobra.Command, opts *options) error {
 		}
 	}
 
+	var current *computev1alpha.Workload
+	if !creating {
+		current = &existing
+	}
+	if err := checkManifestImages(ctx, c, &workload, current, build.ImagePlatforms); err != nil {
+		return err
+	}
+
 	var diffLines []string
 	if !creating {
 		diffLines = manifestDiff(existing, workload)

internal/cmd/compute/deploy/image_preflight.go

@@ -28,6 +28,19 @@ type imagePlatformsFunc func(ctx context.Context, image string) ([]string, error
 // the client cannot list, a class it does not list, or an image the
 // registry will not show from this machine all let the deploy proceed.
 func checkImageRuntime(ctx context.Context, c client.Client, image, class string, platforms imagePlatformsFunc) error {
+	return checkImageRuntimeIn(ctx, c, image, class, platforms, flagClassHint)
+}
+
+// classHint tells the user how to select a runtime class where they deploy.
+type classHint func(class string) string
+
+func flagClassHint(class string) string { return "--runtime-class=" + class }
+
+func manifestClassHint(class string) string {
+	return "spec.template.spec.runtime.class: " + class + " in the manifest"
+}
+
+func checkImageRuntimeIn(ctx context.Context, c client.Client, image, class string, platforms imagePlatformsFunc, hint classHint) error {
 	var classes computev1alpha.RuntimeClassList
 	if err := c.List(ctx, &classes); err != nil {
 		return nil
@@ -52,20 +65,57 @@ func checkImageRuntime(ctx context.Context, c client.Client, image, class string
 			"  datumctl compute deploy ... --image <ref>\n",
 			image, strings.Join(got, ", "), selected.Name)
 		if alt := classWithBoundary(classes.Items, func(b string) bool { return b != isolationUnikernel }); alt != "" {
-			msg += fmt.Sprintf("or run it as a container with --runtime-class=%s", alt)
+			msg += "or run it as a container with " + hint(alt)
 		}
 		return fmt.Errorf("%s", strings.TrimSpace(msg))
 	case !unikernel && !hasContainer:
 		msg := fmt.Sprintf("image %q is a Compute image (%s), which the %q runtime class cannot run as a container.",
 			image, strings.Join(got, ", "), selected.Name)
 		if alt := classWithBoundary(classes.Items, func(b string) bool { return b == isolationUnikernel }); alt != "" {
-			msg += fmt.Sprintf("\n\nDeploy it with --runtime-class=%s", alt)
+			msg += "\n\nDeploy it with " + hint(alt)
 		}
 		return fmt.Errorf("%s", msg)
 	}
 	return nil
 }
 
+// checkManifestImages runs checkImageRuntime on every container a manifest
+// declares, in the class the workload will run in: the manifest's, else the
+// existing workload's, else the catalog default.
+func checkManifestImages(ctx context.Context, c client.Client, manifest, existing *computev1alpha.Workload, platforms imagePlatformsFunc) error {
+	sandbox := manifest.Spec.Template.Spec.Runtime.Sandbox
+	if sandbox == nil {
+		return nil
+	}
+	class := manifest.Spec.Template.Spec.Runtime.Class
+	if class == "" && existing != nil {
+		class = existing.Spec.Template.Spec.Runtime.Class
+	}
+	for _, container := range sandbox.Containers {
+		if err := checkImageRuntimeIn(ctx, c, container.Image, class, platforms, manifestClassHint); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// planRuntimeClass names the class a deploy writes for its plan. A new
+// workload with no class gets the catalog default, so the user sees the tier
+// before applying. An existing workload with no class predates runtime
+// classes and is left unnamed.
+func planRuntimeClass(ctx context.Context, c client.Client, class string, creating bool) string {
+	if class != "" || !creating {
+		return class
+	}
+	var classes computev1alpha.RuntimeClassList
+	if err := c.List(ctx, &classes); err == nil {
+		if selected := selectRuntimeClass(classes.Items, ""); selected != nil {
+			return selected.Name + " (platform default)"
+		}
+	}
+	return "platform default"
+}
+
 // selectRuntimeClass returns the class named, or the catalog default when
 // none is, the way admission fills in an empty class.
 func selectRuntimeClass(classes []computev1alpha.RuntimeClass, name string) *computev1alpha.RuntimeClass {

internal/cmd/compute/deploy/image_preflight_test.go

@@ -5,6 +5,7 @@ package deploy
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strings"
 	"testing"
 
@@ -130,3 +131,129 @@ func TestCheckImageRuntimeWithoutACatalog(t *testing.T) {
 		t.Fatalf("a missing catalog must not block the deploy: %v", err)
 	}
 }
+
+func manifestWorkload(class string, images ...string) *computev1alpha.Workload {
+	w := &computev1alpha.Workload{ObjectMeta: metav1.ObjectMeta{Name: testWorkload}}
+	w.Spec.Template.Spec.Runtime.Class = class
+	sandbox := &computev1alpha.SandboxRuntime{}
+	for i, image := range images {
+		sandbox.Containers = append(sandbox.Containers, computev1alpha.SandboxContainer{Name: fmt.Sprintf("c%d", i), Image: image})
+	}
+	w.Spec.Template.Spec.Runtime.Sandbox = sandbox
+	return w
+}
+
+func TestCheckManifestImages(t *testing.T) {
+	const (
+		containerImage = "ghcr.io/acme/api:1"
+		computeImage   = "ghcr.io/acme/api-compute:1"
+	)
+	catalog := []client.Object{
+		runtimeClass(classGeneralPurpose, "virtual-machine", false),
+		runtimeClass(classUnikernel, "unikernel", true),
+	}
+	lookup := func(_ context.Context, image string) ([]string, error) {
+		if image == computeImage {
+			return []string{platformCompute}, nil
+		}
+		return []string{platformLinux}, nil
+	}
+
+	tests := []struct {
+		name     string
+		manifest *computev1alpha.Workload
+		existing *computev1alpha.Workload
+		wantErr  []string
+	}{
+		{
+			name:     "container image with no class is checked against the default",
+			manifest: manifestWorkload("", containerImage),
+			wantErr: []string{
+				containerImage, "container image", classUnikernel,
+				"datumctl compute build --push --output <ref>",
+				"spec.template.spec.runtime.class: " + classGeneralPurpose,
+			},
+		},
+		{
+			name:     "container image in the unikernel class is refused",
+			manifest: manifestWorkload(classUnikernel, containerImage),
+			wantErr:  []string{containerImage, classUnikernel},
+		},
+		{
+			name:     "every container is checked",
+			manifest: manifestWorkload(classUnikernel, computeImage, containerImage),
+			wantErr:  []string{containerImage, classUnikernel},
+		},
+		{
+			name:     "an update with no class is checked against the workload's class",
+			manifest: manifestWorkload("", computeImage),
+			existing: manifestWorkload(classGeneralPurpose, containerImage),
+			wantErr:  []string{computeImage, "Compute image", classGeneralPurpose, "spec.template.spec.runtime.class: " + classUnikernel},
+		},
+		{
+			name:     "Compute image in the unikernel class proceeds",
+			manifest: manifestWorkload(classUnikernel, computeImage),
+		},
+		{
+			name:     "Compute image with no class proceeds on a unikernel default",
+			manifest: manifestWorkload("", computeImage),
+		},
+		{
+			name:     "container image in the general-purpose class proceeds",
+			manifest: manifestWorkload(classGeneralPurpose, containerImage),
+		},
+		{
+			name:     "a manifest without a sandbox is left to admission",
+			manifest: &computev1alpha.Workload{ObjectMeta: metav1.ObjectMeta{Name: "vm"}},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			objs := make([]client.Object, 0, len(catalog))
+			for _, c := range catalog {
+				objs = append(objs, c.DeepCopyObject().(client.Object))
+			}
+			err := checkManifestImages(context.Background(), newFakeClient(t, objs...), tt.manifest, tt.existing, lookup)
+			if len(tt.wantErr) == 0 {
+				if err != nil {
+					t.Fatalf("unexpected error: %v", err)
+				}
+				return
+			}
+			if err == nil {
+				t.Fatal("expected an error")
+			}
+			for _, want := range tt.wantErr {
+				if !strings.Contains(err.Error(), want) {
+					t.Errorf("error %q does not mention %q", err, want)
+				}
+			}
+		})
+	}
+}
+
+func TestPlanRuntimeClass(t *testing.T) {
+	catalog := newFakeClient(t,
+		runtimeClass(classGeneralPurpose, "virtual-machine", false),
+		runtimeClass(classUnikernel, "unikernel", true),
+	)
+	tests := []struct {
+		name     string
+		kube     client.Client
+		class    string
+		existing bool
+		want     string
+	}{
+		{name: "a named class is shown as is", kube: catalog, class: classGeneralPurpose, want: classGeneralPurpose},
+		{name: "no class names the catalog default", kube: catalog, want: classUnikernel + " (platform default)"},
+		{name: "no class and no catalog says so", kube: newFakeClient(t), want: "platform default"},
+		{name: "an existing workload with no class is left unnamed", kube: catalog, existing: true, want: ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := planRuntimeClass(context.Background(), tt.kube, tt.class, !tt.existing); got != tt.want {
+				t.Errorf("planRuntimeClass() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}