fix(template): probe envd readiness before persisting envd capability

CubeMaster/pkg/templatecenter/store.go

@@ -482,6 +482,7 @@ func createTemplateReplicasOnNodes(ctx context.Context, templateID string, req *
 			replicas = append(replicas, replica)
 			envdVersions = append(envdVersions, nodeEnvdVersion{
 				NodeID:  target.ID(),
+				Ready:   replica.Status == ReplicaStatusReady,
 				Version: envdVersion,
 			})
 			lock.Unlock()
@@ -495,49 +496,58 @@ func createTemplateReplicasOnNodes(ctx context.Context, templateID string, req *
 		}()
 	}
 	wg.Wait()
-	// Converge per-node envd versions into a single template value and persist it
-	// once to the definition annotation (idempotent; covers create and redo).
-	if envdVersion := convergeEnvdVersion(ctx, envdVersions); envdVersion != "" {
-		if err := persistTemplateEnvdVersion(ctx, templateID, envdVersion); err != nil {
-			log.G(ctx).Warnf("persist template envd version fail, template=%s err=%v", templateID, err)
-		}
+	// Converge per-node envd versions into a single template value and reconcile
+	// the definition annotation once (idempotent; covers create and redo).
+	if err := reconcileTemplateEnvdVersion(ctx, templateID, convergeEnvdVersion(ctx, envdVersions)); err != nil {
+		log.G(ctx).Warnf("reconcile template envd version fail, template=%s err=%v", templateID, err)
 	}
 	return replicas, persistErr
 }
 
 type nodeEnvdVersion struct {
 	NodeID  string
+	Ready   bool
 	Version string
 }
 
-// convergeEnvdVersion picks a single template-level envd version from the
-// per-node collection results: the first valid semver wins, and any divergence
-// across nodes is logged but not treated as an error.
+// convergeEnvdVersion only returns a template-level envd version when every
+// READY replica reported a valid and consistent value. Failed/non-ready replicas
+// do not participate in this capability contract.
 func convergeEnvdVersion(ctx context.Context, versions []nodeEnvdVersion) string {
 	chosen := ""
+	readyCount := 0
 	for _, item := range versions {
+		if !item.Ready {
+			continue
+		}
+		readyCount++
 		v := sanitizeEnvdVersion(item.Version)
 		if v == "" {
-			continue
+			log.G(ctx).Warnf("envd version missing on ready node=%s; suppressing template capability annotation", item.NodeID)
+			return ""
 		}
 		if chosen == "" {
 			chosen = v
 			continue
 		}
 		if v != chosen {
-			log.G(ctx).Warnf("envd version mismatch across nodes: keeping=%s saw=%s node=%s", chosen, v, item.NodeID)
+			log.G(ctx).Warnf("envd version mismatch across ready nodes: keeping=%s saw=%s node=%s; suppressing template capability annotation", chosen, v, item.NodeID)
+			return ""
 		}
 	}
+	if readyCount == 0 {
+		return ""
+	}
 	return chosen
 }
 
-// persistTemplateEnvdVersion writes the converged envd version into the template
-// definition's request_json annotation exactly once (read-modify-write), then
-// invalidates the cached request so new sandboxes inherit the annotation. It is
-// idempotent: a no-op when the annotation already holds the same value.
-func persistTemplateEnvdVersion(ctx context.Context, templateID, version string) error {
+// reconcileTemplateEnvdVersion aligns the stored template envd annotation with
+// the latest convergence result. A non-empty version is persisted; an empty
+// version clears any previously persisted annotation so redo cannot leave a
+// stale capability signal behind.
+func reconcileTemplateEnvdVersion(ctx context.Context, templateID, version string) error {
 	version = sanitizeEnvdVersion(version)
-	if templateID == "" || version == "" {
+	if templateID == "" {
 		return nil
 	}
 	// Serialize the request_json read-modify-write against concurrent template
@@ -554,10 +564,17 @@ func persistTemplateEnvdVersion(ctx context.Context, templateID, version string)
 		if req.Annotations == nil {
 			req.Annotations = make(map[string]string)
 		}
-		if req.Annotations[constants.CubeAnnotationComponentEnvdVersion] == version {
+		current := strings.TrimSpace(req.Annotations[constants.CubeAnnotationComponentEnvdVersion])
+		if version == "" {
+			if current == "" {
+				return nil
+			}
+			delete(req.Annotations, constants.CubeAnnotationComponentEnvdVersion)
+		} else if current == version {
 			return nil
+		} else {
+			req.Annotations[constants.CubeAnnotationComponentEnvdVersion] = version
 		}
-		req.Annotations[constants.CubeAnnotationComponentEnvdVersion] = version
 		payload, err := json.Marshal(req)
 		if err != nil {
 			return err
@@ -572,6 +589,10 @@ func persistTemplateEnvdVersion(ctx context.Context, templateID, version string)
 	})
 }
 
+func persistTemplateEnvdVersion(ctx context.Context, templateID, version string) error {
+	return reconcileTemplateEnvdVersion(ctx, templateID, version)
+}
+
 func createReplicaOnNode(ctx context.Context, target *node.Node, req *sandboxtypes.CreateCubeSandboxReq, opts replicaRunOptions) (ReplicaStatus, string) {
 	replica := ReplicaStatus{
 		NodeID:          target.ID(),

CubeMaster/pkg/templatecenter/store_test.go

@@ -62,14 +62,175 @@ func TestNormalizeStoredTemplateRequestStripsPhysicalAnnotations(t *testing.T) {
 	assert.Empty(t, out.SnapshotDir)
 }
 
-func TestConvergeEnvdVersionUsesNodeCollectionResults(t *testing.T) {
-	got := convergeEnvdVersion(context.Background(), []nodeEnvdVersion{
-		{NodeID: "node-a", Version: ""},
-		{NodeID: "node-b", Version: "envd version 0.5.11"},
-		{NodeID: "node-c", Version: "0.6.0"},
+func TestConvergeEnvdVersionRequiresAllReadyReplicasToAgree(t *testing.T) {
+	tests := []struct {
+		name     string
+		versions []nodeEnvdVersion
+		want     string
+	}{
+		{
+			name: "all ready replicas report same version",
+			versions: []nodeEnvdVersion{
+				{NodeID: "node-a", Ready: true, Version: "envd version 0.5.11"},
+				{NodeID: "node-b", Ready: true, Version: "0.5.11"},
+				{NodeID: "node-c", Ready: false, Version: ""},
+			},
+			want: "0.5.11",
+		},
+		{
+			name: "missing version on ready replica suppresses annotation",
+			versions: []nodeEnvdVersion{
+				{NodeID: "node-a", Ready: true, Version: "0.5.11"},
+				{NodeID: "node-b", Ready: true, Version: ""},
+			},
+			want: "",
+		},
+		{
+			name: "version mismatch across ready replicas suppresses annotation",
+			versions: []nodeEnvdVersion{
+				{NodeID: "node-a", Ready: true, Version: "0.5.11"},
+				{NodeID: "node-b", Ready: true, Version: "0.6.0"},
+			},
+			want: "",
+		},
+		{
+			name: "failed replicas do not participate",
+			versions: []nodeEnvdVersion{
+				{NodeID: "node-a", Ready: false, Version: ""},
+				{NodeID: "node-b", Ready: true, Version: "0.5.11"},
+			},
+			want: "0.5.11",
+		},
+		{
+			name: "no ready replicas means no annotation",
+			versions: []nodeEnvdVersion{
+				{NodeID: "node-a", Ready: false, Version: "0.5.11"},
+				{NodeID: "node-b", Ready: false, Version: ""},
+			},
+			want: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := convergeEnvdVersion(context.Background(), tt.versions)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func TestCreateTemplateReplicasOnNodesClearsEnvdAnnotationWhenReadyReplicaHasNoEnvdVersion(t *testing.T) {
+	patches := gomonkey.NewPatches()
+	defer patches.Reset()
+
+	req := &sandboxtypes.CreateCubeSandboxReq{InstanceType: "cubebox"}
+	targets := []*node.Node{{InsID: "node-a", IP: "10.0.0.1", Healthy: true}}
+
+	patches.ApplyFunc(createReplicaOnNode, func(ctx context.Context, target *node.Node, req *sandboxtypes.CreateCubeSandboxReq, opts replicaRunOptions) (ReplicaStatus, string) {
+		return ReplicaStatus{
+			NodeID:       target.ID(),
+			NodeIP:       target.IP,
+			InstanceType: req.InstanceType,
+			Status:       ReplicaStatusReady,
+			Phase:        ReplicaPhaseReady,
+		}, ""
+	})
+	patches.ApplyFunc(UpsertReplica, func(ctx context.Context, templateID, instanceType string, replica ReplicaStatus) error {
+		return nil
+	})
+
+	reconcileCalled := false
+	reconciledVersion := "unexpected"
+	patches.ApplyFunc(reconcileTemplateEnvdVersion, func(ctx context.Context, templateID, version string) error {
+		reconcileCalled = true
+		reconciledVersion = version
+		return nil
+	})
+
+	replicas, err := createTemplateReplicasOnNodes(context.Background(), "tpl-no-envd-annotation", req, targets, replicaRunOptions{})
+	require.NoError(t, err)
+	require.Len(t, replicas, 1)
+	assert.Equal(t, ReplicaStatusReady, replicas[0].Status)
+	assert.True(t, reconcileCalled, "template envd annotation should still be reconciled when convergence is suppressed")
+	assert.Equal(t, "", reconciledVersion, "missing envd version from a READY replica must clear any stale template annotation")
+}
+
+func TestCreateTemplateReplicasOnNodesPersistsConvergedEnvdVersion(t *testing.T) {
+	patches := gomonkey.NewPatches()
+	defer patches.Reset()
+
+	req := &sandboxtypes.CreateCubeSandboxReq{InstanceType: "cubebox"}
+	targets := []*node.Node{
+		{InsID: "node-a", IP: "10.0.0.1", Healthy: true},
+		{InsID: "node-b", IP: "10.0.0.2", Healthy: true},
+	}
+
+	patches.ApplyFunc(createReplicaOnNode, func(ctx context.Context, target *node.Node, req *sandboxtypes.CreateCubeSandboxReq, opts replicaRunOptions) (ReplicaStatus, string) {
+		return ReplicaStatus{
+			NodeID:       target.ID(),
+			NodeIP:       target.IP,
+			InstanceType: req.InstanceType,
+			Status:       ReplicaStatusReady,
+			Phase:        ReplicaPhaseReady,
+		}, "0.5.11"
+	})
+	patches.ApplyFunc(UpsertReplica, func(ctx context.Context, templateID, instanceType string, replica ReplicaStatus) error {
+		return nil
+	})
+
+	var (
+		reconciledTemplateID string
+		reconciledVersion    string
+	)
+	patches.ApplyFunc(reconcileTemplateEnvdVersion, func(ctx context.Context, templateID, version string) error {
+		reconciledTemplateID = templateID
+		reconciledVersion = version
+		return nil
+	})
+
+	replicas, err := createTemplateReplicasOnNodes(context.Background(), "tpl-envd-annotation", req, targets, replicaRunOptions{})
+	require.NoError(t, err)
+	require.Len(t, replicas, 2)
+	assert.Equal(t, "tpl-envd-annotation", reconciledTemplateID)
+	assert.Equal(t, "0.5.11", reconciledVersion)
+}
+
+func TestReconcileTemplateEnvdVersionClearsStaleAnnotation(t *testing.T) {
+	patches := gomonkey.NewPatches()
+	defer patches.Reset()
+
+	const templateID = "tpl-stale-envd"
+	def := &models.TemplateDefinition{
+		TemplateID: templateID,
+		RequestJSON: `{
+			"annotations": {
+				"` + constants.CubeAnnotationComponentEnvdVersion + `": "0.5.11",
+				"keep": "value"
+			}
+		}`,
+	}
+
+	patches.ApplyFunc(withTemplateWriteLock, func(templateID string, fn func() error) error {
+		return fn()
+	})
+	patches.ApplyFunc(GetDefinition, func(ctx context.Context, gotTemplateID string) (*models.TemplateDefinition, error) {
+		assert.Equal(t, templateID, gotTemplateID)
+		return def, nil
+	})
+
+	var updatedRequestJSON string
+	patches.ApplyFunc(updateDefinitionFields, func(ctx context.Context, gotTemplateID string, values map[string]any) error {
+		assert.Equal(t, templateID, gotTemplateID)
+		payload, ok := values["request_json"].(string)
+		require.True(t, ok)
+		updatedRequestJSON = payload
+		return nil
 	})
 
-	assert.Equal(t, "0.5.11", got)
+	require.NoError(t, reconcileTemplateEnvdVersion(context.Background(), templateID, ""))
+	assert.NotEmpty(t, updatedRequestJSON)
+	assert.NotContains(t, updatedRequestJSON, constants.CubeAnnotationComponentEnvdVersion)
+	assert.Contains(t, updatedRequestJSON, `"keep":"value"`)
 }
 
 func TestResolveTemplateNodesFiltersRequestedHealthyNodes(t *testing.T) {

Cubelet/services/cubebox/appsnapshot.go

@@ -166,6 +166,7 @@ func (s *service) AppSnapshot(ctx context.Context, req *cubebox.AppSnapshotReque
 	rsp.SandboxID = sandboxID
 	stepLog = stepLog.WithFields(CubeLog.Fields{"sandboxID": sandboxID})
 	stepLog.Infof("Cubebox created successfully: %s", sandboxID)
+	envdVersion := s.collectReadyEnvdVersion(ctx, sandboxID)
 
 	snapshotSuccess := false
 	temporaryCubeboxDestroyed := false
@@ -276,10 +277,6 @@ func (s *service) AppSnapshot(ctx context.Context, req *cubebox.AppSnapshotReque
 		return rsp, nil
 	}
 
-	// collectEnvdVersion uses containerd Exec, which must run before
-	// cube-runtime marks the guest as app-snapshotting and disables exec.
-	envdVersion := s.collectEnvdVersion(ctx, sandboxID)
-
 	stepLog.Info("Step 4: Executing cube-runtime snapshot...")
 	// AppSnapshot builds a brand-new template from a fresh sandbox: there is
 	// no base memory blob to overlay onto, so we always ask for a full memory