From 522660cc8ac218660462ac107133b3b643b358c0 Mon Sep 17 00:00:00 2001 From: Michael Costello Date: Fri, 6 Nov 2026 14:18:21 -0500 Subject: [PATCH] fix: Check manifest images against the runtime class A plain container image deployed into the unikernel runtime class never starts, and the Instance reports that the image could not be pulled anonymously. The flag deploy refuses that mismatch up front, but a manifest deploy wrote the workload without checking, and neither the help nor the plan said which class a workload with no class lands in. A manifest deploy now checks every container image against the class the workload will run in: the manifest's, else the existing workload's, else the catalog default. A refusal tells the user to set the class in the manifest, since -f does not accept --runtime-class. The plan of a new workload now names its runtime class, resolving an empty class to the catalog default, and the help says the default comes from the platform and that unikernel boots only Compute images. The default is catalog data, so the help points at the plan rather than naming a class that could go stale. The Instance message itself comes from the runtime provider and is tracked there. Fixes #619 Related to datum-cloud/unikraft-provider#257 Co-Authored-By: Claude Opus 5.5 --- internal/cmd/compute/deploy/deploy.go | 25 +++- .../cmd/compute/deploy/image_preflight.go | 54 +++++++- .../compute/deploy/image_preflight_test.go | 127 ++++++++++++++++++ 3 files changed, 197 insertions(+), 9 deletions(-) diff --git a/internal/cmd/compute/deploy/deploy.go b/internal/cmd/compute/deploy/deploy.go index 6dd33bc3..3e2dea5d 100644 --- a/internal/cmd/compute/deploy/deploy.go +++ b/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) diff --git a/internal/cmd/compute/deploy/image_preflight.go b/internal/cmd/compute/deploy/image_preflight.go index 9f112a92..d8202468 100644 --- a/internal/cmd/compute/deploy/image_preflight.go +++ b/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 \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 { diff --git a/internal/cmd/compute/deploy/image_preflight_test.go b/internal/cmd/compute/deploy/image_preflight_test.go index 7bbf1e56..6298bf45 100644 --- a/internal/cmd/compute/deploy/image_preflight_test.go +++ b/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 ", + "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) + } + }) + } +}