update labels when claiming

Signed-off-by: furykerry <shouchen.zz@alibaba-inc.com>

Author: furykerry

api/v1alpha1/annotations.go

@@ -10,7 +10,8 @@ const (
 // E2B annotations
 
 const (
-	E2BPrefix = "e2b." + InternalPrefix
+	E2BPrefix      = "e2b." + InternalPrefix
+	E2BLabelPrefix = "label:"
 
 	AnnotationEnvdAccessToken = E2BPrefix + "envd-access-token"
 	AnnotationEnvdURL         = E2BPrefix + "envd-url"

pkg/controller/sandbox/core/common_inplace_update_handler.go

@@ -43,13 +43,18 @@ func handleInPlaceUpdateCommon(
 ) (bool, error) {
 	logger := handler.GetLogger(ctx, box)
 
-	_, hashWithoutImageAndResource := HashSandbox(box)
+	_, hashWithoutImageResourceMetadata, hashWithoutImageAndResource := 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 {
+	} else if box.Annotations[agentsv1alpha1.SandboxHashWithoutImageAndResources] != hashWithoutImageAndResource &&
+		box.Annotations[agentsv1alpha1.SandboxHashWithoutImageAndResources] != hashWithoutImageResourceMetadata {
+		// SandboxHashWithoutImageAndResources computation is switching
+		// from  hashWithoutImageAndResource to hashWithoutImageResourceMetadata
+		//TODO: drop hashWithoutImageAndResource comparison once we're sure no
+		// sandboxes with old hashWithoutImageAndResource exists
 		logger.Info("sandbox hash-without-image-resources changed, and does not permit in-place upgrades",
 			"old hash", box.Annotations[agentsv1alpha1.SandboxHashWithoutImageAndResources],
 			"new hash", hashWithoutImageAndResource)

pkg/controller/sandbox/core/util.go

@@ -36,7 +36,7 @@ import (
 )
 
 // HashSandbox calculates the hash value using sandbox.spec.template
-func HashSandbox(box *agentsv1alpha1.Sandbox) (string, string) {
+func HashSandbox(box *agentsv1alpha1.Sandbox) (string, string, string) {
 	// hash using sandbox.spec.template
 	by, _ := json.Marshal(*box.Spec.Template)
 	hash := utils.HashData(by)
@@ -55,7 +55,13 @@ func HashSandbox(box *agentsv1alpha1.Sandbox) (string, string) {
 	}
 	by, _ = json.Marshal(*tempClone)
 	hashWithoutImageResources := utils.HashData(by)
-	return hash, hashWithoutImageResources
+
+	// hash using sandbox.spec.template without image, resources and metadata
+	tempClone.ObjectMeta.Annotations = map[string]string{}
+	tempClone.ObjectMeta.Labels = map[string]string{}
+	by, _ = json.Marshal(*tempClone)
+	hashWithoutImageResourcesMetadata := utils.HashData(by)
+	return hash, hashWithoutImageResources, hashWithoutImageResourcesMetadata
 }
 
 // GeneratePVCName generates a persistent volume claim name from template name and sandbox name

pkg/controller/sandbox/core/util_test.go

@@ -39,6 +39,7 @@ func TestHashSandbox(t *testing.T) {
 		expectedHash                      string
 		expectedHashWithoutImageResources string
 		validateDifferentHashes           bool
+		validateDifferentMetadataHashes   bool
 	}{
 		{
 			name: "basic sandbox with containers",
@@ -165,11 +166,6 @@ func TestHashSandbox(t *testing.T) {
 				Spec: agentsv1alpha1.SandboxSpec{
 					EmbeddedSandboxTemplate: agentsv1alpha1.EmbeddedSandboxTemplate{
 						Template: &corev1.PodTemplateSpec{
-							ObjectMeta: metav1.ObjectMeta{
-								Labels: map[string]string{
-									"app": "test",
-								},
-							},
 							Spec: corev1.PodSpec{
 								Containers: []corev1.Container{
 									{
@@ -195,11 +191,37 @@ func TestHashSandbox(t *testing.T) {
 			},
 			validateDifferentHashes: true,
 		},
+		{
+			name: "sandbox with metadata",
+			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{
+									{
+										Name:  "test-container",
+										Image: "nginx:latest",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			validateDifferentHashes:         true,
+			validateDifferentMetadataHashes: true,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			hash, hashWithoutImageResources := HashSandbox(tt.sandbox)
+			hash, hashWithoutImageResources, hashWithoutImageResourcesMetadata := HashSandbox(tt.sandbox)
 
 			// Verify both hashes are not empty
 			if hash == "" {
@@ -210,12 +232,18 @@ func TestHashSandbox(t *testing.T) {
 			}
 
 			// Verify consistency - same input should always produce same output
-			hash2, hashWithoutImageResources2 := HashSandbox(tt.sandbox)
+			hash2, hashWithoutImageResources2, hashWithoutImageResourcesMetadata2 := HashSandbox(tt.sandbox)
 			if hash != hash2 {
 				t.Errorf("HashSandbox() is not consistent for hash: got %s, want %s", hash, hash2)
 			}
 			if hashWithoutImageResources != hashWithoutImageResources2 {
-				t.Errorf("HashSandbox() is not consistent for hashWithoutImageResources: got %s, want %s", hashWithoutImageResources, hashWithoutImageResources2)
+				t.Errorf("HashSandbox() is not consistent for hashWithoutImageResources: got %s, want %s",
+					hashWithoutImageResources, hashWithoutImageResources2)
+			}
+
+			if hashWithoutImageResourcesMetadata != hashWithoutImageResourcesMetadata2 {
+				t.Errorf("HashSandbox() is not consistent for hashWithoutImageResourcesMetadata: got %s, want %s",
+					hashWithoutImageResourcesMetadata, hashWithoutImageResourcesMetadata2)
 			}
 
 			// Validate that hashes have expected format (from HashData function)
@@ -228,10 +256,16 @@ func TestHashSandbox(t *testing.T) {
 				if hash == hashWithoutImageResources {
 					t.Errorf("Expected different hashes when image/resources are present, but got same: %s", hash)
 				}
-			} else {
-				if hash != hashWithoutImageResources {
-					t.Errorf("Expected same hashes when no image/resources differences, but got different: %s vs %s", hash, hashWithoutImageResources)
+			} else if hash != hashWithoutImageResources {
+				t.Errorf("Expected same hashes when no image/resources differences, but got different: %s vs %s", hash, hashWithoutImageResources)
+			}
+
+			if tt.validateDifferentMetadataHashes {
+				if hashWithoutImageResources == hashWithoutImageResourcesMetadata {
+					t.Errorf("Expected different hashes when sandbox metadata are present, but got same: %s", hashWithoutImageResources)
 				}
+			} else if hashWithoutImageResourcesMetadata != hashWithoutImageResources {
+				t.Errorf("Expected same hashes when no metadata differences, but got different: %s vs %s", hash, hashWithoutImageResources)
 			}
 		})
 	}
@@ -273,8 +307,8 @@ func TestHashSandboxWithDifferentImages(t *testing.T) {
 		},
 	}
 
-	hash1, hashWithoutImageResources1 := HashSandbox(sandbox1)
-	hash2, hashWithoutImageResources2 := HashSandbox(sandbox2)
+	hash1, hashWithoutImageResources1, _ := HashSandbox(sandbox1)
+	hash2, hashWithoutImageResources2, _ := HashSandbox(sandbox2)
 
 	// Full hashes should be different because images are different
 	if hash1 == hash2 {
@@ -288,6 +322,69 @@ func TestHashSandboxWithDifferentImages(t *testing.T) {
 	}
 }
 
+func TestHashSandboxWithDifferentHash(t *testing.T) {
+	// Test that changing only image results in different full hash but same hash without image/resources
+	sandbox1 := &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{
+							{
+								Name:  "test-container",
+								Image: "nginx:1.19", // Different image
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	sandbox2 := &agentsv1alpha1.Sandbox{
+		Spec: agentsv1alpha1.SandboxSpec{
+			EmbeddedSandboxTemplate: agentsv1alpha1.EmbeddedSandboxTemplate{
+				Template: &corev1.PodTemplateSpec{
+					Spec: corev1.PodSpec{
+						Containers: []corev1.Container{
+							{
+								Name:  "test-container",
+								Image: "nginx:1.20", // Different image
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	hash1, hashWithoutImageResources1, hashWithoutImageResourcesMetadata1 := HashSandbox(sandbox1)
+	hash2, hashWithoutImageResources2, hashWithoutImageResourcesMetadata2 := HashSandbox(sandbox2)
+
+	// Full hashes should be different because images are different
+	if hash1 == hash2 {
+		t.Errorf("Expected different full hashes for different images, but got same: %s", hash1)
+	}
+
+	// Hashes without images/resources should be the different
+	if hashWithoutImageResources1 == hashWithoutImageResources2 {
+		t.Errorf("Expected different hashes without images/resources, but got same: %s",
+			hashWithoutImageResources1)
+	}
+
+	// Hashes without images/resources/metadata should be the same
+	if hashWithoutImageResourcesMetadata1 != hashWithoutImageResourcesMetadata2 {
+		t.Errorf("Expected same hashes without images/resources/metadata, but got different:"+
+			"%s vs %s",
+			hashWithoutImageResourcesMetadata1, hashWithoutImageResourcesMetadata2)
+	}
+}
+
 func TestHashSandboxWithDifferentResources(t *testing.T) {
 	// Test that changing only resources results in different full hash but same hash without image/resources
 	sandbox1 := &agentsv1alpha1.Sandbox{
@@ -336,8 +433,8 @@ func TestHashSandboxWithDifferentResources(t *testing.T) {
 		},
 	}
 
-	hash1, hashWithoutImageResources1 := HashSandbox(sandbox1)
-	hash2, hashWithoutImageResources2 := HashSandbox(sandbox2)
+	hash1, hashWithoutImageResources1, _ := HashSandbox(sandbox1)
+	hash2, hashWithoutImageResources2, _ := HashSandbox(sandbox2)
 
 	// Full hashes should be different because resources are different
 	if hash1 == hash2 {

pkg/controller/sandbox/sandbox_controller.go

@@ -252,8 +252,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
+	_, _, hashNotUpgradable := core.HashSandbox(box)
+	originObj.Annotations[agentsv1alpha1.SandboxHashWithoutImageAndResources] = hashNotUpgradable
 	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)
@@ -286,7 +286,7 @@ func calculateStatus(args core.EnsureFuncArgs) (*agentsv1alpha1.SandboxStatus, b
 	pod, box, newStatus := args.Pod, args.Box, args.NewStatus
 	logger := logf.FromContext(context.TODO()).WithValues("sandbox", klog.KObj(box))
 
-	hash, _ := core.HashSandbox(box)
+	hash, _, _ := core.HashSandbox(box)
 	newStatus.ObservedGeneration = box.Generation
 	newStatus.UpdateRevision = hash
 	if newStatus.Phase == "" {

pkg/sandbox-manager/infra/sandboxcr/claim.go

@@ -9,6 +9,7 @@ import (
 	"io"
 	"math/rand/v2"
 	"net/http"
+	"strings"
 	"sync"
 	"time"
 
@@ -423,6 +424,17 @@ func modifyPickedSandbox(sbx *Sandbox, lockType infra.LockType, opts infra.Claim
 	labels[v1alpha1.LabelSandboxIsClaimed] = v1alpha1.True
 	sbx.SetLabels(labels)
 
+	// propagate labels to podtemplates
+	for k, v := range labels {
+		if strings.HasPrefix(k, v1alpha1.InternalPrefix) {
+			continue
+		}
+		if sbx.Spec.Template.Labels == nil {
+			sbx.Spec.Template.Labels = make(map[string]string, len(labels))
+		}
+		sbx.Spec.Template.Labels[k] = v
+	}
+
 	annotations := sbx.GetAnnotations()
 	if annotations == nil {
 		annotations = make(map[string]string, 1)

pkg/servers/e2b/create.go

@@ -257,6 +257,12 @@ func (sc *Controller) basicSandboxCreateModifier(ctx context.Context, sbx infra.
 		annotations[k] = v
 	}
 	sbx.SetAnnotations(annotations)
+
+	labels := sbx.GetLabels()
+	for k, v := range request.Extensions.Labels {
+		labels[k] = v
+	}
+	sbx.SetLabels(labels)
 }
 
 func (sc *Controller) csiMountOptionsConfigRecord(ctx context.Context, sbx infra.Sandbox, request models.NewSandboxRequest) {

pkg/servers/e2b/models/extensions.go

@@ -5,9 +5,11 @@ import (
 	"fmt"
 	"net/http"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/distribution/reference"
+	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/utils/ptr"
 
 	"github.com/openkruise/agents/api/v1alpha1"
@@ -74,6 +76,34 @@ func (r *NewSandboxRequest) parseCommonExtensions() error {
 	if r.Extensions.WaitReadySeconds, err = r.parseAndRemoveIntExtension(ExtensionKeyWaitReadyTimeout); err != nil {
 		return err
 	}
+
+	if err = r.parseExtensionLabels(); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (r *NewSandboxRequest) parseExtensionLabels() error {
+	for k, v := range r.Metadata {
+		key := strings.TrimPrefix(k, v1alpha1.E2BLabelPrefix)
+		if key == k {
+			// not a label
+			continue
+		}
+		if r.Extensions.Labels == nil {
+			r.Extensions.Labels = make(map[string]string)
+		}
+		if len(validation.IsQualifiedName(key)) != 0 {
+			return fmt.Errorf("invalid label name [%s]", key)
+		}
+
+		if len(validation.IsValidLabelValue(v)) != 0 {
+			return fmt.Errorf("invalid label value [%s]", v)
+		}
+
+		r.Extensions.Labels[key] = v
+		delete(r.Metadata, k)
+	}
 	return nil
 }