Update ActorTemplate's container spec to mirror k8s semantic for command and args

cmd/ateapi/internal/controlapi/workload_spec.go

@@ -59,6 +59,7 @@ func workloadSpecFromActorTemplate(actorTemplate *atev1alpha1.ActorTemplate) *at
 			Name:    ctr.Name,
 			Image:   ctr.Image,
 			Command: ctr.Command,
+			Args:    ctr.Args,
 			Readyz:  toAteletReadyz(ctr.Readyz),
 		}
 		for _, mount := range ctr.VolumeMounts {

cmd/ateapi/internal/controlapi/workload_spec_test.go

@@ -168,6 +168,30 @@ func TestWorkloadSpecFromActorTemplate(t *testing.T) {
 				Containers: []*ateletpb.Container{{Name: "main", Image: "main"}},
 			},
 		},
+		{
+			name: "maps command and args",
+			template: &atev1alpha1.ActorTemplate{
+				ObjectMeta: metav1.ObjectMeta{Name: "tmpl1", Namespace: "agent-ns"},
+				Spec: atev1alpha1.ActorTemplateSpec{
+					Containers: []atev1alpha1.Container{
+						{
+							Name:    "main",
+							Image:   "main",
+							Command: []string{"/entrypoint"},
+							Args:    []string{"--foo", "--bar"},
+						},
+					},
+				},
+			},
+			want: &ateletpb.WorkloadSpec{
+				Containers: []*ateletpb.Container{{
+					Name:    "main",
+					Image:   "main",
+					Command: []string{"/entrypoint"},
+					Args:    []string{"--foo", "--bar"},
+				}},
+			},
+		},
 	}
 
 	for _, tt := range tests {

cmd/atelet/main.go

@@ -244,7 +244,7 @@ func (s *AteomHerder) Run(ctx context.Context, req *ateletpb.RunRequest) (*atele
 	if err := s.prepareOCIBundles(ctx, actorUID, actorName,
 		req.GetSpec(), req.GetTargetAteomUid(),
 	); err != nil {
-		return nil, err
+		return nil, ateerrors.CrashIfReason(ctx, err, ateerrors.ReasonInvalidContainerConfig)
 	}
 
 	client, err := s.dialAteom(ctx, req.GetTargetAteomUid())
@@ -534,7 +534,7 @@ func (s *AteomHerder) Restore(ctx context.Context, req *ateletpb.RestoreRequest)
 		}
 		t := time.Now()
 		if err := s.prepareOCIBundles(gctx, actorUID, actorName, req.GetSpec(), req.GetTargetAteomUid()); err != nil {
-			return ateerrors.CrashIfReason(ctx, err, ateerrors.ReasonTerminalFileSystemError)
+			return ateerrors.CrashIfReason(ctx, err, ateerrors.ReasonTerminalFileSystemError, ateerrors.ReasonInvalidContainerConfig)
 		}
 		dBundles = time.Since(t)
 		return nil
@@ -700,6 +700,7 @@ func (s *AteomHerder) prepareOCIBundles(
 			spec.GetPauseImage(),
 			[]string{"/pause"},
 			nil,
+			nil,
 			annotations,
 			ateompath.AteomNetNSPath(targetAteomUid),
 			"", // pause is sandbox infra; it gets no actor identity mount.
@@ -731,6 +732,7 @@ func (s *AteomHerder) prepareOCIBundles(
 				ctr.GetName(),
 				ctr.GetImage(),
 				ctr.GetCommand(),
+				ctr.GetArgs(),
 				envs,
 				map[string]string{
 					"io.kubernetes.cri.container-type": "container",

cmd/atelet/oci.go

@@ -30,6 +30,7 @@ import (
 	"strings"
 
 	"github.com/agent-substrate/substrate/cmd/atelet/internal/memorypullcache"
+	"github.com/agent-substrate/substrate/internal/ateerrors"
 	"github.com/agent-substrate/substrate/internal/ateompath"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/opencontainers/runtime-spec/specs-go"
@@ -56,7 +57,7 @@ const (
 	ActorIDFileName = "actor-id"
 )
 
-func prepareOCIDirectory(ctx context.Context, pullCache *memorypullcache.MemoryPullCache, actorUID, containerName, ref string, args []string, env []string, annotations map[string]string, netns string, identityDir string, durableDirVolumeMounts []*ateletpb.VolumeMount) error {
+func prepareOCIDirectory(ctx context.Context, pullCache *memorypullcache.MemoryPullCache, actorUID, containerName, ref string, command, args []string, env []string, annotations map[string]string, netns string, identityDir string, durableDirVolumeMounts []*ateletpb.VolumeMount) error {
 	tracer := otel.Tracer("prepareOCIDirectory")
 
 	ctx, span := tracer.Start(ctx, "prepareOCIDirectory")
@@ -80,6 +81,14 @@ func prepareOCIDirectory(ctx context.Context, pullCache *memorypullcache.MemoryP
 	}
 	defer tarData.Close()
 
+	// Argv and env need only the image config, so resolve them before the
+	// untar to fail fast on an invalid container config.
+	resolvedArgs, err := resolveProcessArgs(imageCfg, command, args)
+	if err != nil {
+		return fmt.Errorf("while resolving process args for container %q: %w", containerName, err)
+	}
+	resolvedEnv := resolveActorEnv(imageCfg, env)
+
 	if err := untar(ctx, tarData, rootPath); err != nil {
 		return fmt.Errorf("in untar: %w", err)
 	}
@@ -93,7 +102,7 @@ func prepareOCIDirectory(ctx context.Context, pullCache *memorypullcache.MemoryP
 		}
 	}
 
-	ociSpec := buildActorOCISpec(actorUID, imageCfg, args, env, annotations, netns, identityDir, durableDirVolumeMounts)
+	ociSpec := buildActorOCISpec(actorUID, resolvedArgs, resolvedEnv, annotations, netns, identityDir, durableDirVolumeMounts)
 	ociSpecBytes, err := json.MarshalIndent(ociSpec, "", "  ")
 	if err != nil {
 		return fmt.Errorf("while marshaling OCI spec: %w", err)
@@ -106,10 +115,16 @@ func prepareOCIDirectory(ctx context.Context, pullCache *memorypullcache.MemoryP
 	return nil
 }
 
-// mergeActorEnv merges the ActorTemplate env and the image's ENV, with the template taking precedence.
-// duplicated keys are removed in favor of the following precedence template env > image env.
-// default PATH stands in for an image config with no env
-func mergeActorEnv(imageEnv, templateEnv []string) []string {
+// resolveActorEnv computes the final container environment from the image's ENV
+// and the ActorTemplate env, with the template taking precedence. Duplicate keys
+// are removed in favor of template env > image env, and a default PATH stands in
+// when neither source sets one.
+func resolveActorEnv(imageCfg *v1.Config, templateEnv []string) []string {
+	var imageEnv []string
+	if imageCfg != nil {
+		imageEnv = imageCfg.Env
+	}
+
 	seen := make(map[string]struct{})
 	var out []string
 	add := func(entries ...string) {
@@ -132,17 +147,39 @@ func mergeActorEnv(imageEnv, templateEnv []string) []string {
 	return out
 }
 
-// buildActorOCISpec assembles the OCI runtime spec for an actor container.
-// When identityDir is non-empty it adds a read-only bind mount of that host
-// directory at IdentityMountPath so the actor can read its own ID (see
-// IdentityMountPath for why this is a bind mount rather than env vars).
-func buildActorOCISpec(actorUID string, imageCfg *v1.Config, args []string, env []string, annotations map[string]string, netns string, identityDir string, durableDirVolumeMounts []*ateletpb.VolumeMount) *specs.Spec {
-	var imageEnv []string
+// resolveProcessArgs computes the final process argv for a container,
+// following Kubernetes Pod semantics: setting command overrides both the
+// image's ENTRYPOINT and its CMD (CMD is dropped, not appended), while
+// setting only args overrides just the image's CMD.
+func resolveProcessArgs(imageCfg *v1.Config, command, args []string) ([]string, error) {
+	var entrypoint, cmd []string
 	if imageCfg != nil {
-		imageEnv = imageCfg.Env
+		entrypoint = imageCfg.Entrypoint
+		cmd = imageCfg.Cmd
+	}
+	if len(command) > 0 {
+		entrypoint = command
+		cmd = nil
+	}
+	if len(args) > 0 {
+		cmd = args
+	}
+
+	argv := make([]string, 0, len(entrypoint)+len(cmd))
+	argv = append(argv, entrypoint...)
+	argv = append(argv, cmd...)
+	if len(argv) == 0 {
+		return nil, fmt.Errorf("%w: no command specified: image defines neither ENTRYPOINT nor CMD and the container sets neither command nor args", ateerrors.ReasonInvalidContainerConfig)
 	}
-	envVars := mergeActorEnv(imageEnv, env)
+	return argv, nil
+}
 
+// buildActorOCISpec assembles the OCI runtime spec for an actor container from
+// already-resolved args and env (see resolveProcessArgs and resolveActorEnv).
+// When identityDir is non-empty it adds a read-only bind mount of that host
+// directory at IdentityMountPath so the actor can read its own ID (see
+// IdentityMountPath for why this is a bind mount rather than env vars).
+func buildActorOCISpec(actorUID string, args []string, env []string, annotations map[string]string, netns string, identityDir string, durableDirVolumeMounts []*ateletpb.VolumeMount) *specs.Spec {
 	mounts := []specs.Mount{
 		{
 			Destination: "/proc",
@@ -188,7 +225,7 @@ func buildActorOCISpec(actorUID string, imageCfg *v1.Config, args []string, env
 				GID: 0,
 			},
 			Args: args,
-			Env:  envVars,
+			Env:  env,
 			Cwd:  "/",
 			Capabilities: &specs.LinuxCapabilities{
 				Bounding: []string{

cmd/atelet/oci_test.go

@@ -18,14 +18,17 @@ import (
 	"archive/tar"
 	"bytes"
 	"context"
+	"errors"
 	"os"
 	"path/filepath"
 	"slices"
 	"strings"
 	"testing"
 
+	"github.com/agent-substrate/substrate/internal/ateerrors"
 	"github.com/agent-substrate/substrate/internal/ateompath"
 	"github.com/agent-substrate/substrate/internal/proto/ateletpb"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
 )
 
 type tarEntry struct {
@@ -88,7 +91,6 @@ func runUntar(t *testing.T, entries []tarEntry) (string, error) {
 func TestBuildActorOCISpec_IdentityMount(t *testing.T) {
 	spec := buildActorOCISpec(
 		"actor_uid",
-		nil,
 		[]string{"/app"},
 		[]string{"FOO=bar"},
 		map[string]string{"k": "v"},
@@ -117,57 +119,143 @@ func TestBuildActorOCISpec_IdentityMount(t *testing.T) {
 	}
 }
 
-func TestMergeActorEnv(t *testing.T) {
+func TestResolveActorEnv(t *testing.T) {
 	defaultPath := "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
 
 	tests := []struct {
 		name        string
-		imageEnv    []string
+		image       *v1.Config
 		templateEnv []string
 		want        []string
 	}{
 		{
 			name:        "template overrides image by key",
-			imageEnv:    []string{"FOO=image"},
+			image:       &v1.Config{Env: []string{"FOO=image"}},
 			templateEnv: []string{"FOO=template"},
 			want:        []string{"FOO=template", defaultPath},
 		},
 		{
 			name:        "default PATH applies when neither sets it",
-			imageEnv:    []string{"FOO=image"},
+			image:       &v1.Config{Env: []string{"FOO=image"}},
 			templateEnv: []string{"BAR=template"},
 			want:        []string{"BAR=template", "FOO=image", defaultPath},
 		},
 		{
-			name:     "image PATH overrides default",
-			imageEnv: []string{"PATH=/image/bin"},
-			want:     []string{"PATH=/image/bin"},
+			name:  "image PATH overrides default",
+			image: &v1.Config{Env: []string{"PATH=/image/bin"}},
+			want:  []string{"PATH=/image/bin"},
 		},
 		{
 			name:        "template PATH overrides default",
+			image:       &v1.Config{},
 			templateEnv: []string{"PATH=/template/bin"},
 			want:        []string{"PATH=/template/bin"},
 		},
 		{
-			name:     "blank and keyless entries are dropped",
-			imageEnv: []string{"", "=novalue"},
-			want:     []string{defaultPath},
+			name:  "blank and keyless entries are dropped",
+			image: &v1.Config{Env: []string{"", "=novalue"}},
+			want:  []string{defaultPath},
+		},
+		{
+			name:        "nil image config uses template env and default PATH",
+			image:       nil,
+			templateEnv: []string{"FOO=template"},
+			want:        []string{"FOO=template", defaultPath},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := resolveActorEnv(tc.image, tc.templateEnv)
+			if !slices.Equal(got, tc.want) {
+				t.Errorf("resolveActorEnv(%v, %v) =\n  %v\nwant:\n  %v", tc.image, tc.templateEnv, got, tc.want)
+			}
+		})
+	}
+}
+
+func TestResolveProcessArgs(t *testing.T) {
+	cfg := func(entrypoint, cmd []string) *v1.Config {
+		return &v1.Config{Entrypoint: entrypoint, Cmd: cmd}
+	}
+
+	tests := []struct {
+		name    string
+		image   *v1.Config
+		command []string
+		args    []string
+		want    []string
+		wantErr bool
+	}{
+		{
+			name:  "image ENTRYPOINT+CMD used when neither is overridden",
+			image: cfg([]string{"/app"}, []string{"--serve"}),
+			want:  []string{"/app", "--serve"},
+		},
+		{
+			name:  "args override CMD, image ENTRYPOINT kept",
+			image: cfg([]string{"/init", "/wrapper.sh"}, nil),
+			args:  []string{"serve"},
+			want:  []string{"/init", "/wrapper.sh", "serve"},
+		},
+		{
+			name:    "command overrides both ENTRYPOINT and CMD",
+			image:   cfg([]string{"/app"}, []string{"--serve"}),
+			command: []string{"/other"},
+			want:    []string{"/other"},
+		},
+		{
+			name:    "command and args override both",
+			image:   cfg([]string{"/app"}, []string{"--serve"}),
+			command: []string{"/other"},
+			args:    []string{"--flag"},
+			want:    []string{"/other", "--flag"},
+		},
+		{
+			name:  "image ENTRYPOINT only, no CMD",
+			image: cfg([]string{"/ko-app/counter"}, nil),
+			want:  []string{"/ko-app/counter"},
+		},
+		{
+			name:    "no image config, command supplies argv",
+			image:   nil,
+			command: []string{"/pause"},
+			want:    []string{"/pause"},
+		},
+		{
+			name:    "empty argv is an error",
+			image:   cfg(nil, nil),
+			wantErr: true,
+		},
+		{
+			name:    "nil image and no overrides is an error",
+			image:   nil,
+			wantErr: true,
 		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			got := mergeActorEnv(tc.imageEnv, tc.templateEnv)
+			got, err := resolveProcessArgs(tc.image, tc.command, tc.args)
+			if (err != nil) != tc.wantErr {
+				t.Fatalf("resolveProcessArgs(%v, %v, %v) err = %v, wantErr %v", tc.image, tc.command, tc.args, err, tc.wantErr)
+			}
+			if err != nil {
+				if !errors.Is(err, ateerrors.ReasonInvalidContainerConfig) {
+					t.Errorf("empty-argv error must carry ReasonInvalidContainerConfig, got: %v", err)
+				}
+				return
+			}
 			if !slices.Equal(got, tc.want) {
-				t.Errorf("mergeActorEnv(%v, %v) =\n  %v\nwant:\n  %v", tc.imageEnv, tc.templateEnv, got, tc.want)
+				t.Errorf("resolveProcessArgs(%v, %v, %v) = %v, want %v", tc.image, tc.command, tc.args, got, tc.want)
 			}
 		})
 	}
 }
 
 // Without an identity dir (the pause container), no identity mount appears.
 func TestBuildActorOCISpec_NoIdentityMountForPause(t *testing.T) {
-	bare := buildActorOCISpec("actor_uid", nil, []string{"/pause"}, nil, nil, "/run/netns/x", "", nil)
+	bare := buildActorOCISpec("actor_uid", []string{"/pause"}, nil, nil, "/run/netns/x", "", nil)
 	for _, m := range bare.Mounts {
 		if m.Destination == IdentityMountPath {
 			t.Errorf("identity mount must be absent when identityDir is empty")
@@ -185,7 +273,7 @@ func TestBuildActorOCISpec_DurableDirVolumeMounts(t *testing.T) {
 	}
 	spec := buildActorOCISpec(
 		actorUID,
-		nil, []string{"/app"}, nil, nil,
+		[]string{"/app"}, nil, nil,
 		"/run/netns/x",
 		"",
 		durableDirs,

demos/agent-secret/agent-secret.yaml.tmpl

@@ -66,7 +66,6 @@ spec:
   containers:
   - name: agent-secret
     image: ko://github.com/agent-substrate/substrate/demos/agent-secret
-    command: ["/ko-app/agent-secret"]
     env:
     - name: PORT
       value: "80"

demos/counter/counter-microvm.yaml.tmpl

@@ -115,7 +115,6 @@ spec:
   containers:
   - name: counter
     image: ko://github.com/agent-substrate/substrate/demos/counter
-    command: ["/ko-app/counter"]
     readyz:
       httpGet:
         path: /readyz