Add session type annotation.

apis/browser/v1/selenosis_keys.go

@@ -1,11 +1,13 @@
 package v1
 
 var (
-	SelenosisOptionsAnnotationKey = "selenosis.io/options"
-	SelenosisOwnerLabelKey        = "selenosis.io/owner"
-	BrowserLabelKey               = "selenosis.io/browser"
-	BrowserNameLabelKey           = "selenosis.io/browser.name"
-	BrowserVersionLabelKey        = "selenosis.io/browser.version"
-	SelenosisHubLabelKey          = "selenosis.io/hub.hostname"
-	SelenosisServiceLabelKey      = "selenosis.io/service.hostname"
+	SelenosisOptionsAnnotationKey     = "selenosis.io/options"
+	SelenosisSessionTypeAnnotationKey = "selenosis.io/session.type"
+
+	SelenosisOwnerLabelKey   = "selenosis.io/owner"
+	BrowserLabelKey          = "selenosis.io/browser"
+	BrowserNameLabelKey      = "selenosis.io/browser.name"
+	BrowserVersionLabelKey   = "selenosis.io/browser.version"
+	SelenosisHubLabelKey     = "selenosis.io/hub.hostname"
+	SelenosisServiceLabelKey = "selenosis.io/service.hostname"
 )

controllers/browser/browser_reconciler.go

@@ -34,8 +34,8 @@ const (
 	mediumRetry = time.Second * 10
 	quickCheck  = time.Second * 15
 
-	browserContainerName = "browser"
-	sidecarContainerName = "seleniferous"
+	BrowserContainerName = "browser"
+	SidecarContainerName = "seleniferous"
 )
 
 func jitter(base time.Duration) time.Duration {
@@ -54,7 +54,6 @@ type ReconcilerConfig struct {
 }
 
 type SelenosisOptions struct {
-	Labels     map[string]string          `json:"labels,omitempty"`
 	Containers map[string]ContainerOption `json:"containers,omitempty"`
 }
 
@@ -525,7 +524,7 @@ func (r *BrowserReconciler) updateBrowserStatus(ctx context.Context, browser *br
 	// Check for critical container termination
 	for _, containerStatus := range pod.Status.ContainerStatuses {
 		// Check if it's a critical container and if it has terminated state
-		if (containerStatus.Name == browserContainerName || containerStatus.Name == sidecarContainerName) &&
+		if (containerStatus.Name == BrowserContainerName || containerStatus.Name == SidecarContainerName) &&
 			containerStatus.State.Terminated != nil {
 
 			if browser.Status.Phase != corev1.PodFailed {
@@ -785,7 +784,7 @@ func buildBrowserPod(browser *browserv1.Browser, cfg *configv1.BrowserVersionCon
 
 	// Base container
 	browserContainer := corev1.Container{
-		Name:  browserContainerName,
+		Name:  BrowserContainerName,
 		Image: cfg.Image,
 	}
 
@@ -995,15 +994,6 @@ func applySelenosisOptions(pod *corev1.Pod, opts *SelenosisOptions) {
 			pod.Spec.Containers[i].Env = mergeEnvVars(pod.Spec.Containers[i].Env, option.Env)
 		}
 	}
-
-	if opts.Labels != nil {
-		if pod.Labels == nil {
-			pod.Labels = map[string]string{}
-		}
-		for k, v := range opts.Labels {
-			pod.Labels[k] = v
-		}
-	}
 }
 
 func getContainerPorts(containerName string, pod *corev1.Pod) []browserv1.ContainerPort {

controllers/browser/browser_reconciler_test.go

@@ -253,15 +253,15 @@ func TestParseSelenosisOptionsValidJSON(t *testing.T) {
 	if err != nil {
 		t.Fatalf("expected no error, got %v", err)
 	}
-	if opts == nil || opts.Labels["a"] != "b" {
-		t.Fatalf("expected labels to be parsed")
+	if opts == nil {
+		t.Fatal("expected options to be parsed")
 	}
 	if opts.Containers["browser"].Env["X"] != "1" {
 		t.Fatalf("expected container env to be parsed")
 	}
 }
 
-func TestApplySelenosisOptionsMergesEnvAndLabels(t *testing.T) {
+func TestApplySelenosisOptionsMergesEnv(t *testing.T) {
 	pod := &corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Labels: map[string]string{"existing": "1"},
@@ -280,16 +280,18 @@ func TestApplySelenosisOptionsMergesEnvAndLabels(t *testing.T) {
 		},
 	}
 	opts := &SelenosisOptions{
-		Labels: map[string]string{"from": "options"},
 		Containers: map[string]ContainerOption{
 			"browser": {Env: map[string]string{"B": "override", "C": "new"}},
 		},
 	}
 
 	applySelenosisOptions(pod, opts)
 
-	if pod.Labels["existing"] != "1" || pod.Labels["from"] != "options" {
-		t.Fatalf("expected labels to be merged, got %+v", pod.Labels)
+	if pod.Labels["existing"] != "1" {
+		t.Fatalf("expected existing labels to survive, got %+v", pod.Labels)
+	}
+	if _, ok := pod.Labels["from"]; ok {
+		t.Fatalf("options must not set pod labels any more, got %+v", pod.Labels)
 	}
 
 	env := pod.Spec.Containers[0].Env
@@ -554,7 +556,7 @@ func TestUpdateBrowserStatusCriticalContainer(t *testing.T) {
 			Phase: corev1.PodRunning,
 			ContainerStatuses: []corev1.ContainerStatus{
 				{
-					Name: browserContainerName,
+					Name: BrowserContainerName,
 					State: corev1.ContainerState{
 						Terminated: &corev1.ContainerStateTerminated{
 							ExitCode: 1,
@@ -2477,7 +2479,7 @@ func TestUpdateBrowserStatusCriticalSidecar(t *testing.T) {
 		Status: corev1.PodStatus{
 			ContainerStatuses: []corev1.ContainerStatus{
 				{
-					Name: sidecarContainerName,
+					Name: SidecarContainerName,
 					State: corev1.ContainerState{
 						Terminated: &corev1.ContainerStateTerminated{ExitCode: 1},
 					},
@@ -2550,7 +2552,7 @@ func TestUpdateBrowserStatusCriticalAlreadyFailed(t *testing.T) {
 		Status: corev1.PodStatus{
 			ContainerStatuses: []corev1.ContainerStatus{
 				{
-					Name: browserContainerName,
+					Name: BrowserContainerName,
 					State: corev1.ContainerState{
 						Terminated: &corev1.ContainerStateTerminated{ExitCode: 1},
 					},
@@ -3507,14 +3509,14 @@ func TestContainerStateEqualWaiting(t *testing.T) {
 	}
 }
 
-func TestApplySelenosisOptionsNilLabels(t *testing.T) {
+func TestApplySelenosisOptionsNilInputs(t *testing.T) {
 	pod := &corev1.Pod{}
-	opts := &SelenosisOptions{
-		Labels: map[string]string{"env": "test"},
-	}
-	applySelenosisOptions(pod, opts)
-	if pod.Labels["env"] != "test" {
-		t.Fatalf("expected label env=test, got %v", pod.Labels)
+
+	applySelenosisOptions(pod, nil)
+	applySelenosisOptions(nil, &SelenosisOptions{})
+
+	if pod.Labels != nil {
+		t.Fatalf("expected no labels, got %v", pod.Labels)
 	}
 }
 
@@ -3814,3 +3816,129 @@ func TestPodStatusMatchesBrowser(t *testing.T) {
 		})
 	}
 }
+
+func TestBuildBrowserPodConfigAnnotationsWinOverBrowser(t *testing.T) {
+	configAnnotations := map[string]string{"shared": "from-config", "only-config": "1"}
+	cfg := &configv1.BrowserVersionConfigSpec{
+		Image:       "browser",
+		Annotations: &configAnnotations,
+	}
+	brw := &browserv1.Browser{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "b1",
+			Namespace:   "ns",
+			Annotations: map[string]string{"shared": "from-browser", "only-browser": "1"},
+		},
+	}
+
+	pod := buildBrowserPod(brw, cfg, nil)
+
+	if pod.Annotations["shared"] != "from-config" {
+		t.Fatalf("shared annotation = %q, want from-config", pod.Annotations["shared"])
+	}
+	if pod.Annotations["only-browser"] != "1" {
+		t.Fatalf("browser-only annotation must survive, got %+v", pod.Annotations)
+	}
+	if pod.Annotations["only-config"] != "1" {
+		t.Fatalf("config-only annotation must be applied, got %+v", pod.Annotations)
+	}
+}
+
+func TestBuildBrowserPodConfigLabelsWinOverBrowser(t *testing.T) {
+	configLabels := map[string]string{"shared": "from-config", "only-config": "1"}
+	cfg := &configv1.BrowserVersionConfigSpec{
+		Image:  "browser",
+		Labels: &configLabels,
+	}
+	brw := &browserv1.Browser{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "b1",
+			Namespace: "ns",
+			Labels:    map[string]string{"shared": "from-browser", "only-browser": "1"},
+		},
+	}
+
+	pod := buildBrowserPod(brw, cfg, nil)
+
+	if pod.Labels["shared"] != "from-config" {
+		t.Fatalf("shared label = %q, want from-config", pod.Labels["shared"])
+	}
+	if pod.Labels["only-browser"] != "1" {
+		t.Fatalf("browser-only label must survive, got %+v", pod.Labels)
+	}
+	if pod.Labels["only-config"] != "1" {
+		t.Fatalf("config-only label must be applied, got %+v", pod.Labels)
+	}
+}
+
+func TestBuildBrowserPodOptionsAnnotationNotCopiedToPod(t *testing.T) {
+	cfg := &configv1.BrowserVersionConfigSpec{Image: "browser"}
+	brw := &browserv1.Browser{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "b1",
+			Namespace: "ns",
+			Annotations: map[string]string{
+				browserv1.SelenosisOptionsAnnotationKey: `{"containers":{"browser":{"env":{"X":"1"}}}}`,
+				"startedManually":                       "true",
+			},
+		},
+	}
+
+	pod := buildBrowserPod(brw, cfg, nil)
+
+	if _, ok := pod.Annotations[browserv1.SelenosisOptionsAnnotationKey]; ok {
+		t.Fatalf("%s must not be copied to the pod, got %+v", browserv1.SelenosisOptionsAnnotationKey, pod.Annotations)
+	}
+	if pod.Annotations["startedManually"] != "true" {
+		t.Fatalf("other annotations must be copied, got %+v", pod.Annotations)
+	}
+}
+
+func TestBuildBrowserPodSessionTypeIsAnnotationOnly(t *testing.T) {
+	tests := []struct {
+		name           string
+		labels         map[string]string
+		annotations    map[string]string
+		wantAnnotation string
+		wantLabel      string
+	}{
+		{
+			name:           "annotation is copied to pod annotation only",
+			annotations:    map[string]string{browserv1.SelenosisSessionTypeAnnotationKey: "playwright"},
+			wantAnnotation: "playwright",
+		},
+		{
+			name:      "legacy label is not promoted to pod annotation",
+			labels:    map[string]string{browserv1.SelenosisSessionTypeAnnotationKey: "selenium"},
+			wantLabel: "selenium",
+		},
+		{
+			name: "absent session type sets nothing",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := &configv1.BrowserVersionConfigSpec{Image: "browser"}
+			brw := &browserv1.Browser{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "b1",
+					Namespace:   "ns",
+					Labels:      tt.labels,
+					Annotations: tt.annotations,
+				},
+			}
+
+			pod := buildBrowserPod(brw, cfg, nil)
+
+			gotAnnotation, hasAnnotation := pod.Annotations[browserv1.SelenosisSessionTypeAnnotationKey]
+			if gotAnnotation != tt.wantAnnotation || hasAnnotation != (tt.wantAnnotation != "") {
+				t.Fatalf("pod annotation %s = %q (present=%v), want %q", browserv1.SelenosisSessionTypeAnnotationKey, gotAnnotation, hasAnnotation, tt.wantAnnotation)
+			}
+			gotLabel, hasLabel := pod.Labels[browserv1.SelenosisSessionTypeAnnotationKey]
+			if gotLabel != tt.wantLabel || hasLabel != (tt.wantLabel != "") {
+				t.Fatalf("pod label %s = %q (present=%v), want %q", browserv1.SelenosisSessionTypeAnnotationKey, gotLabel, hasLabel, tt.wantLabel)
+			}
+		})
+	}
+}