update pod labels when claiming

api/v1alpha1/sandbox_types.go

@@ -24,8 +24,13 @@ import (
 
 const (
 	// SandboxHashWithoutImageAndResources represents the key of sandbox hash without image and resources.
+	// Deprecated, use SandboxHashImmutablePart instead
 	SandboxHashWithoutImageAndResources = "sandbox.agents.kruise.io/hash-without-image-resources"
 
+	// SandboxHashImmutablePart represents the key of sandbox hash than exclude immutable part of sandbox
+	// e.g. metadata, image and resources
+	SandboxHashImmutablePart = "sandbox.agents.kruise.io/hash-immutable-part"
+
 	// PodLabelTemplateHash is pod template hash
 	PodLabelTemplateHash = "pod-template-hash"
 

pkg/controller/sandbox/core/common_control_test.go

@@ -1163,7 +1163,7 @@ func TestCommonControl_handleInplaceUpdateSandbox(t *testing.T) {
 			Name:      "test-sandbox",
 			Namespace: "default",
 			Annotations: map[string]string{
-				agentsv1alpha1.SandboxHashWithoutImageAndResources: "different-hash",
+				agentsv1alpha1.SandboxHashImmutablePart: "different-hash",
 			},
 		},
 		Spec: agentsv1alpha1.SandboxSpec{
@@ -1210,7 +1210,7 @@ func TestCommonControl_handleInplaceUpdateSandbox(t *testing.T) {
 			Name:      "test-sandbox",
 			Namespace: "default",
 			Annotations: map[string]string{
-				agentsv1alpha1.SandboxHashWithoutImageAndResources: "same-hash",
+				agentsv1alpha1.SandboxHashImmutablePart: "same-hash",
 			},
 		},
 		Spec: agentsv1alpha1.SandboxSpec{

pkg/controller/sandbox/core/common_inplace_update_handler.go

@@ -43,18 +43,19 @@ func handleInPlaceUpdateCommon(
 ) (bool, error) {
 	logger := handler.GetLogger(ctx, box)
 
-	_, hashWithoutImageAndResource := HashSandbox(box)
+	_, hashImmutablePart := HashSandbox(box)
+
 	// old Pod do not include Labels[pod-template-hash] and do not support inplace update.
 	// Check if inplace update is supported
 	if pod.Labels[agentsv1alpha1.PodLabelTemplateHash] == "" {
 		return true, nil
 		// todo, update inplaceupdate condition
-	} else if box.Annotations[agentsv1alpha1.SandboxHashWithoutImageAndResources] != hashWithoutImageAndResource {
-		logger.Info("sandbox hash-without-image-resources changed, and does not permit in-place upgrades",
-			"old hash", box.Annotations[agentsv1alpha1.SandboxHashWithoutImageAndResources],
-			"new hash", hashWithoutImageAndResource)
+	} else if box.Annotations[agentsv1alpha1.SandboxHashImmutablePart] != hashImmutablePart {
+		logger.Info("sandbox hash-immutable-part changed, and does not permit in-place upgrades",
+			"old hash", box.Annotations[agentsv1alpha1.SandboxHashImmutablePart],
+			"new hash", hashImmutablePart)
 		handler.GetRecorder().Eventf(box, corev1.EventTypeWarning, "InplaceUpdateForbidden",
-			"InplaceUpdate only support image, resources")
+			"InplaceUpdate only support image, resources, metadata")
 		return true, nil
 	}
 

pkg/controller/sandbox/core/common_inplace_update_handler_test.go

@@ -106,7 +106,7 @@ func TestHandleInPlaceUpdateCommon(t *testing.T) {
 			box: &agentsv1alpha1.Sandbox{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
-						agentsv1alpha1.SandboxHashWithoutImageAndResources: "new-hash", // Mismatch with Pod label
+						agentsv1alpha1.SandboxHashImmutablePart: "new-hash", // Mismatch with Pod label
 					},
 				},
 				Spec: agentsv1alpha1.SandboxSpec{
@@ -151,7 +151,7 @@ func TestHandleInPlaceUpdateCommon(t *testing.T) {
 			box: &agentsv1alpha1.Sandbox{
 				ObjectMeta: metav1.ObjectMeta{
 					Annotations: map[string]string{
-						agentsv1alpha1.SandboxHashWithoutImageAndResources: "test-revision",
+						agentsv1alpha1.SandboxHashImmutablePart: "test-revision",
 					},
 				},
 				Spec: agentsv1alpha1.SandboxSpec{
@@ -237,7 +237,7 @@ func TestHandleInPlaceUpdateCommon_WithUpdateInProgress(t *testing.T) {
 			Name:      "test-sandbox",
 			Namespace: "default",
 			Annotations: map[string]string{
-				agentsv1alpha1.SandboxHashWithoutImageAndResources: "current-hash",
+				agentsv1alpha1.SandboxHashImmutablePart: "current-hash",
 			},
 		},
 		Spec: agentsv1alpha1.SandboxSpec{
@@ -301,7 +301,7 @@ func TestHandleInPlaceUpdateCommon_InitialState(t *testing.T) {
 			Name:      "test-sandbox",
 			Namespace: "default",
 			Annotations: map[string]string{
-				agentsv1alpha1.SandboxHashWithoutImageAndResources: "current-hash",
+				agentsv1alpha1.SandboxHashImmutablePart: "current-hash",
 			},
 		},
 		Spec: agentsv1alpha1.SandboxSpec{

pkg/controller/sandbox/core/util.go

@@ -51,6 +51,8 @@ func HashSandbox(box *agentsv1alpha1.Sandbox) (string, string) {
 
 	// hash using sandbox.spec.template without image and resources
 	tempClone := box.Spec.Template.DeepCopy()
+	tempClone.Labels = nil
+	tempClone.Annotations = nil
 	for i := range tempClone.Spec.Containers {
 		container := &tempClone.Spec.Containers[i]
 		container.Image = ""
@@ -62,8 +64,8 @@ func HashSandbox(box *agentsv1alpha1.Sandbox) (string, string) {
 		container.Resources = corev1.ResourceRequirements{}
 	}
 	by, _ = json.Marshal(*tempClone)
-	hashWithoutImageResources := utils.HashData(by)
-	return hash, hashWithoutImageResources
+	hashImmutablePart := utils.HashData(by)
+	return hash, hashImmutablePart
 }
 
 // GeneratePVCName generates a persistent volume claim name from template name and sandbox name

pkg/controller/sandbox/core/util_test.go

@@ -155,6 +155,26 @@ func TestHashSandbox(t *testing.T) {
 			},
 			validateDifferentHashes: false, // Both hashes should be the same when no containers have images/resources
 		},
+		{
+			name: "sandbox with labels in template but empty containers",
+			sandbox: &agentsv1alpha1.Sandbox{
+				Spec: agentsv1alpha1.SandboxSpec{
+					EmbeddedSandboxTemplate: agentsv1alpha1.EmbeddedSandboxTemplate{
+						Template: &corev1.PodTemplateSpec{
+							ObjectMeta: metav1.ObjectMeta{
+								Labels: map[string]string{
+									"app": "test",
+								},
+							},
+							Spec: corev1.PodSpec{
+								Containers: []corev1.Container{},
+							},
+						},
+					},
+				},
+			},
+			validateDifferentHashes: true, // Both hashes should be the same when no containers have images/resources
+		},
 		{
 			name: "sandbox with volumes and other fields",
 			sandbox: &agentsv1alpha1.Sandbox{

pkg/controller/sandbox/sandbox_controller.go

@@ -264,8 +264,8 @@ func (r *SandboxReconciler) addSandboxFinalizerAndHash(ctx context.Context, box
 	if originObj.Annotations == nil {
 		originObj.Annotations = make(map[string]string)
 	}
-	_, hashWithoutImageAndResource := core.HashSandbox(box)
-	originObj.Annotations[agentsv1alpha1.SandboxHashWithoutImageAndResources] = hashWithoutImageAndResource
+	_, hashImmutablePart := core.HashSandbox(box)
+	originObj.Annotations[agentsv1alpha1.SandboxHashImmutablePart] = hashImmutablePart
 	if err := client.IgnoreNotFound(r.Patch(ctx, originObj, patch)); err != nil {
 		logger.Error(err, "failed to patch sandbox finalizer and hash")
 		return nil, fmt.Errorf("failed to patch finalizer: %w", err)

pkg/controller/sandbox/sandbox_controller_test.go

@@ -1790,7 +1790,7 @@ func TestSandboxReconciler_AddSandboxFinalizerAndHash(t *testing.T) {
 				if result.Annotations == nil {
 					t.Fatalf("Annotations should not be nil")
 				}
-				if result.Annotations[agentsv1alpha1.SandboxHashWithoutImageAndResources] == "" {
+				if result.Annotations[agentsv1alpha1.SandboxHashImmutablePart] == "" {
 					t.Errorf("Hash annotation should be set")
 				}
 			},
@@ -1891,7 +1891,7 @@ func TestSandboxReconciler_AddSandboxFinalizerAndHash(t *testing.T) {
 					t.Errorf("Annotations map should be created")
 				}
 				// Check hash annotation
-				if result.Annotations[agentsv1alpha1.SandboxHashWithoutImageAndResources] == "" {
+				if result.Annotations[agentsv1alpha1.SandboxHashImmutablePart] == "" {
 					t.Errorf("Hash annotation should be set")
 				}
 			},
@@ -1929,7 +1929,7 @@ func TestSandboxReconciler_AddSandboxFinalizerAndHash(t *testing.T) {
 					t.Errorf("Existing annotation should be preserved")
 				}
 				// Check hash annotation is added
-				if result.Annotations[agentsv1alpha1.SandboxHashWithoutImageAndResources] == "" {
+				if result.Annotations[agentsv1alpha1.SandboxHashImmutablePart] == "" {
 					t.Errorf("Hash annotation should be added")
 				}
 			},
@@ -1994,7 +1994,7 @@ func TestSandboxReconciler_AddSandboxFinalizerAndHash(t *testing.T) {
 				if tt.expectHashAnnotation {
 					if updatedSandbox.Annotations == nil {
 						t.Errorf("Annotations should not be nil in persisted sandbox")
-					} else if updatedSandbox.Annotations[agentsv1alpha1.SandboxHashWithoutImageAndResources] == "" {
+					} else if updatedSandbox.Annotations[agentsv1alpha1.SandboxHashImmutablePart] == "" {
 						t.Errorf("Hash annotation should be set in persisted sandbox")
 					}
 				}

pkg/controller/sandboxclaim/core/common_control.go

@@ -242,7 +242,19 @@ func (c *commonControl) buildClaimOptions(ctx context.Context, claim *agentsv1al
 		User:     string(claim.UID), // Use UID to ensure uniqueness across claim recreations
 		Template: sandboxSet.Name,
 		Modifier: func(sbx infra.Sandbox) {
-			// 1. apply labels
+			// propagate annotations to sandbox
+			if len(claim.Spec.Annotations) > 0 {
+				annotations := sbx.GetAnnotations()
+				if annotations == nil {
+					annotations = make(map[string]string)
+				}
+				for k, v := range claim.Spec.Annotations {
+					annotations[k] = v
+				}
+				sbx.SetAnnotations(annotations)
+			}
+
+			// propagate labels to sandbox
 			labels := sbx.GetLabels()
 			if labels == nil {
 				labels = make(map[string]string)
@@ -254,19 +266,18 @@ func (c *commonControl) buildClaimOptions(ctx context.Context, claim *agentsv1al
 			}
 			sbx.SetLabels(labels)
 
-			// 2. apply annotations
-			if len(claim.Spec.Annotations) > 0 {
-				annotations := sbx.GetAnnotations()
-				if annotations == nil {
-					annotations = make(map[string]string)
-				}
-				for k, v := range claim.Spec.Annotations {
-					annotations[k] = v
-				}
-				sbx.SetAnnotations(annotations)
+			// propagate annotations to podtemplate
+			labels = sbx.GetPodLabels()
+			if labels == nil {
+				labels = make(map[string]string)
+			}
+
+			for k, v := range claim.Spec.Labels {
+				labels[k] = v
 			}
+			sbx.SetPodLabels(labels)
 
-			// 3. apply shutdownTime
+			// apply shutdownTime
 			if claim.Spec.ShutdownTime != nil {
 				sbx.SetTimeout(infra.TimeoutOptions{
 					ShutdownTime: claim.Spec.ShutdownTime.Time,

pkg/servers/e2b/create.go

@@ -254,6 +254,7 @@ func (sc *Controller) basicSandboxCreateModifier(ctx context.Context, sbx infra.
 	sbx.SetTimeout(timeoutOptions)
 	log.Info("timeout options calculated", "options", timeoutOptions)
 
+	// propagate annotations to sandbox
 	annotations := sbx.GetAnnotations()
 	if annotations == nil {
 		annotations = make(map[string]string)
@@ -263,7 +264,18 @@ func (sc *Controller) basicSandboxCreateModifier(ctx context.Context, sbx infra.
 	}
 	sbx.SetAnnotations(annotations)
 
-	labels := sbx.GetPodLabels()
+	// propagate labels to sandbox
+	labels := sbx.GetLabels()
+	if labels == nil {
+		labels = make(map[string]string)
+	}
+	for k, v := range request.Extensions.Labels {
+		labels[k] = v
+	}
+	sbx.SetLabels(labels)
+
+	// propagate annotations to podtemplate
+	labels = sbx.GetPodLabels()
 	if labels == nil {
 		labels = make(map[string]string)
 	}

pkg/utils/inplaceupdate/inplace_update.go

@@ -19,7 +19,6 @@ package inplaceupdate
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 
 	agentsapiv1alpha1 "github.com/openkruise/agents/api/v1alpha1"
 	"github.com/openkruise/agents/pkg/utils"
@@ -101,6 +100,9 @@ func DefaultGeneratePatchBodyFunc(opts InPlaceUpdateOptions) string {
 		UpdateTimestamp:       metav1.Now(),
 		LastContainerStatuses: map[string]InPlaceUpdateContainerStatus{},
 	}
+	labels := map[string]string{
+		agentsapiv1alpha1.PodLabelTemplateHash: revision,
+	}
 	// container.name -> container
 	originContainers := map[string]corev1.Container{}
 	for i := range box.Spec.Template.Spec.Containers {
@@ -113,7 +115,19 @@ func DefaultGeneratePatchBodyFunc(opts InPlaceUpdateOptions) string {
 		originStatus[status.Name] = status.ImageID
 	}
 
-	patchSpec := corev1.PodSpec{}
+	if box.Spec.Template != nil {
+		for k, v := range box.Spec.Template.Labels {
+			if pod.Labels[k] != v {
+				labels[k] = v
+			}
+		}
+	}
+
+	patch := corev1.Pod{
+		Spec: corev1.PodSpec{
+			Containers: []corev1.Container{},
+		},
+	}
 	for i := range pod.Spec.Containers {
 		container := pod.Spec.Containers[i]
 		origin, ok := originContainers[container.Name]
@@ -127,24 +141,19 @@ func DefaultGeneratePatchBodyFunc(opts InPlaceUpdateOptions) string {
 			Name:  container.Name,
 			Image: origin.Image,
 		}
-		patchSpec.Containers = append(patchSpec.Containers, patchContainer)
+		patch.Spec.Containers = append(patch.Spec.Containers, patchContainer)
 		state.UpdateImages = true
 		imageId := originStatus[container.Name]
 		state.LastContainerStatuses[container.Name] = InPlaceUpdateContainerStatus{
 			ImageID: imageId,
 		}
 	}
-	if len(patchSpec.Containers) == 0 {
-		return ""
-	}
-
 	annotations := map[string]string{
 		PodAnnotationInPlaceUpdateStateKey: utils.DumpJson(state),
 	}
-	labels := map[string]string{
-		agentsapiv1alpha1.PodLabelTemplateHash: revision,
-	}
-	return fmt.Sprintf(`{"metadata":{"annotations":%s,"labels":%s},"spec":%s}`, utils.DumpJson(annotations), utils.DumpJson(labels), utils.DumpJson(patchSpec))
+	patch.Labels = labels
+	patch.Annotations = annotations
+	return utils.DumpJson(patch)
 }
 
 func (c *InPlaceUpdateControl) Update(ctx context.Context, opts InPlaceUpdateOptions) (bool, error) {
@@ -158,7 +167,7 @@ func (c *InPlaceUpdateControl) Update(ctx context.Context, opts InPlaceUpdateOpt
 
 	clone := pod.DeepCopy()
 	if err := c.Patch(ctx, clone, client.RawPatch(types.StrategicMergePatchType, []byte(patchBody))); err != nil {
-		logger.Error(err, "inplace update pod failed")
+		logger.Error(err, "inplace update pod failed", "body", patchBody)
 		return false, err
 	}
 	logger.Info("inplace update pod success", "revision", revision, "patchBody", patchBody)