Skip to content

Commit 063d073

Browse files
committed
updated profile controller tests to use setNamespaceLabelsAndServiceMesh
Signed-off-by: madmecodes <[email protected]>
1 parent c5a9989 commit 063d073

File tree

2 files changed

+26
-24
lines changed

2 files changed

+26
-24
lines changed

components/profile-controller/controllers/profile_controller.go

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -838,28 +838,6 @@ func (r *ProfileReconciler) readDefaultLabelsFromFile(path string) map[string]st
838838
return labels
839839
}
840840

841-
// setNamespaceLabels applies the provided labels to a namespace, preserving existing labels
842-
// This function is kept separate for testing purposes
843-
func setNamespaceLabels(ns *corev1.Namespace, newLabels map[string]string) {
844-
if ns.Labels == nil {
845-
ns.Labels = make(map[string]string)
846-
}
847-
848-
for k, v := range newLabels {
849-
_, ok := ns.Labels[k]
850-
if len(v) == 0 {
851-
// When there is an empty value, k should be removed.
852-
if ok {
853-
delete(ns.Labels, k)
854-
}
855-
} else {
856-
if !ok {
857-
// Add label if not exist, otherwise skipping update.
858-
ns.Labels[k] = v
859-
}
860-
}
861-
}
862-
}
863841

864842
// createWaypoint creates a waypoint proxy in the profile namespace for ambient mode
865843
func (r *ProfileReconciler) createWaypoint(profileIns *profilev1.Profile) error {

components/profile-controller/controllers/profile_controller_test.go

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
profilev1 "github.com/kubeflow/kubeflow/components/profile-controller/api/v1"
1010
"github.com/stretchr/testify/assert"
1111
corev1 "k8s.io/api/core/v1"
12+
rbacv1 "k8s.io/api/rbac/v1"
1213
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1314
"k8s.io/apimachinery/pkg/runtime"
1415
ctrl "sigs.k8s.io/controller-runtime"
@@ -22,6 +23,25 @@ type namespaceLabelSuite struct {
2223

2324
func TestEnforceNamespaceLabelsFromConfig(t *testing.T) {
2425
name := "test-namespace"
26+
27+
// Create a minimal ProfileReconciler for testing
28+
reconciler := &ProfileReconciler{
29+
ServiceMeshMode: "istio-sidecar", // Test sidecar mode
30+
}
31+
32+
// Create a minimal profile for testing
33+
profile := &profilev1.Profile{
34+
ObjectMeta: metav1.ObjectMeta{
35+
Name: name,
36+
},
37+
Spec: profilev1.ProfileSpec{
38+
Owner: rbacv1.Subject{
39+
Kind: "User",
40+
Name: "test-user",
41+
},
42+
},
43+
}
44+
2545
tests := []namespaceLabelSuite{
2646
namespaceLabelSuite{
2747
corev1.Namespace{
@@ -42,6 +62,7 @@ func TestEnforceNamespaceLabelsFromConfig(t *testing.T) {
4262
"serving.kubeflow.org/inferenceservice": "enabled",
4363
"pipelines.kubeflow.org/enabled": "true",
4464
"app.kubernetes.io/part-of": "kubeflow-profile",
65+
"istio-injection": "enabled", // Added by service mesh logic
4566
},
4667
Name: name,
4768
},
@@ -68,9 +89,10 @@ func TestEnforceNamespaceLabelsFromConfig(t *testing.T) {
6889
Labels: map[string]string{
6990
"user-name": "Jim",
7091
"katib.kubeflow.org/metrics-collector-injection": "enabled",
71-
"serving.kubeflow.org/inferenceservice": "disabled",
92+
"serving.kubeflow.org/inferenceservice": "disabled", // Existing label preserved
7293
"pipelines.kubeflow.org/enabled": "true",
7394
"app.kubernetes.io/part-of": "kubeflow-profile",
95+
"istio-injection": "enabled", // Added by service mesh logic
7496
},
7597
Name: name,
7698
},
@@ -101,14 +123,16 @@ func TestEnforceNamespaceLabelsFromConfig(t *testing.T) {
101123
"serving.kubeflow.org/inferenceservice": "enabled",
102124
"pipelines.kubeflow.org/enabled": "true",
103125
"app.kubernetes.io/part-of": "kubeflow-profile",
126+
"istio-injection": "enabled", // Added by service mesh logic
127+
// "removal-label" should be removed due to empty value
104128
},
105129
Name: name,
106130
},
107131
},
108132
},
109133
}
110134
for _, test := range tests {
111-
setNamespaceLabels(&test.current, test.labels)
135+
reconciler.setNamespaceLabelsAndServiceMesh(&test.current, profile, test.labels)
112136
if !reflect.DeepEqual(&test.expected, &test.current) {
113137
t.Errorf("Expect:\n%v; Output:\n%v", &test.expected, &test.current)
114138
}

0 commit comments

Comments
 (0)