Skip to content

Commit 82767ae

Browse files
sjbermansjberman
committed
Revert label change; use annotation if too long
1 parent f511b10 commit 82767ae

File tree

8 files changed

+53
-64
lines changed

8 files changed

+53
-64
lines changed

‎internal/controller/provisioner/handler.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,10 @@ func (h *eventHandler) HandleEventBatch(ctx context.Context, logger logr.Logger,
6666
*corev1.ConfigMap, *rbacv1.Role, *rbacv1.RoleBinding, *autoscalingv2.HorizontalPodAutoscaler:
6767
objLabels := labels.Set(obj.GetLabels())
6868
if h.labelSelector.Matches(objLabels) {
69-
gatewayName := obj.GetAnnotations()[controller.GatewayAnnotation]
69+
gatewayName := objLabels.Get(controller.GatewayLabel)
70+
if gatewayName == "" {
71+
gatewayName = obj.GetAnnotations()[controller.GatewayLabel]
72+
}
7073
gatewayNSName := types.NamespacedName{Namespace: obj.GetNamespace(), Name: gatewayName}
7174
if err := h.updateOrDeleteResources(ctx, logger, obj, gatewayNSName); err != nil {
7275
logger.Error(err, "error handling resource update")
@@ -75,7 +78,10 @@ func (h *eventHandler) HandleEventBatch(ctx context.Context, logger logr.Logger,
7578
case *corev1.Service:
7679
objLabels := labels.Set(obj.GetLabels())
7780
if h.labelSelector.Matches(objLabels) {
78-
gatewayName := obj.GetAnnotations()[controller.GatewayAnnotation]
81+
gatewayName := objLabels.Get(controller.GatewayLabel)
82+
if gatewayName == "" {
83+
gatewayName = obj.GetAnnotations()[controller.GatewayLabel]
84+
}
7985
gatewayNSName := types.NamespacedName{Namespace: obj.GetNamespace(), Name: gatewayName}
8086
if err := h.updateOrDeleteResources(ctx, logger, obj, gatewayNSName); err != nil {
8187
logger.Error(err, "error handling resource update")
@@ -90,7 +96,10 @@ func (h *eventHandler) HandleEventBatch(ctx context.Context, logger logr.Logger,
9096
case *corev1.Secret:
9197
objLabels := labels.Set(obj.GetLabels())
9298
if h.labelSelector.Matches(objLabels) {
93-
gatewayName := obj.GetAnnotations()[controller.GatewayAnnotation]
99+
gatewayName := objLabels.Get(controller.GatewayLabel)
100+
if gatewayName == "" {
101+
gatewayName = obj.GetAnnotations()[controller.GatewayLabel]
102+
}
94103
gatewayNSName := types.NamespacedName{Namespace: obj.GetNamespace(), Name: gatewayName}
95104
if err := h.updateOrDeleteResources(ctx, logger, obj, gatewayNSName); err != nil {
96105
logger.Error(err, "error handling resource update")

‎internal/controller/provisioner/handler_test.go‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ func TestHandleEventBatch_Upsert(t *testing.T) {
5151
Name: "gw-nginx",
5252
Namespace: "default",
5353
ResourceVersion: "1",
54-
Labels: map[string]string{"app": "nginx"},
55-
Annotations: map[string]string{controller.GatewayAnnotation: "gw"},
54+
Labels: map[string]string{"app": "nginx", controller.GatewayLabel: "gw"},
5655
},
5756
}
5857

@@ -61,8 +60,7 @@ func TestHandleEventBatch_Upsert(t *testing.T) {
6160
Name: "gw-nginx",
6261
Namespace: "default",
6362
ResourceVersion: "1",
64-
Labels: map[string]string{"app": "nginx"},
65-
Annotations: map[string]string{controller.GatewayAnnotation: "gw"},
63+
Labels: map[string]string{"app": "nginx", controller.GatewayLabel: "gw"},
6664
},
6765
}
6866

@@ -71,8 +69,7 @@ func TestHandleEventBatch_Upsert(t *testing.T) {
7169
Name: "gw-nginx-" + jwtTestSecretName,
7270
Namespace: "default",
7371
ResourceVersion: "1",
74-
Labels: map[string]string{"app": "nginx"},
75-
Annotations: map[string]string{controller.GatewayAnnotation: "gw"},
72+
Labels: map[string]string{"app": "nginx", controller.GatewayLabel: "gw"},
7673
},
7774
Data: map[string][]byte{
7875
"data": []byte("oldData"),
@@ -95,8 +92,7 @@ func TestHandleEventBatch_Upsert(t *testing.T) {
9592
Name: "gw-nginx-" + dockerTestSecretName,
9693
Namespace: "default",
9794
ResourceVersion: "1",
98-
Labels: map[string]string{"app": "nginx"},
99-
Annotations: map[string]string{controller.GatewayAnnotation: "gw"},
95+
Labels: map[string]string{"app": "nginx", controller.GatewayLabel: "gw"},
10096
},
10197
Data: map[string][]byte{
10298
"data": []byte("oldDockerData"),
@@ -249,10 +245,9 @@ func TestHandleEventBatch_Delete(t *testing.T) {
249245

250246
deployment := &appsv1.Deployment{
251247
ObjectMeta: metav1.ObjectMeta{
252-
Name: "gw-nginx",
253-
Namespace: "default",
254-
Labels: map[string]string{"app": "nginx"},
255-
Annotations: map[string]string{controller.GatewayAnnotation: "gw"},
248+
Name: "gw-nginx",
249+
Namespace: "default",
250+
Labels: map[string]string{"app": "nginx", controller.GatewayLabel: "gw"},
256251
},
257252
}
258253

@@ -266,10 +261,9 @@ func TestHandleEventBatch_Delete(t *testing.T) {
266261

267262
jwtSecret := &corev1.Secret{
268263
ObjectMeta: metav1.ObjectMeta{
269-
Name: "gw-nginx-" + jwtTestSecretName,
270-
Namespace: "default",
271-
Labels: map[string]string{"app": "nginx"},
272-
Annotations: map[string]string{controller.GatewayAnnotation: "gw"},
264+
Name: "gw-nginx-" + jwtTestSecretName,
265+
Namespace: "default",
266+
Labels: map[string]string{"app": "nginx", controller.GatewayLabel: "gw"},
273267
},
274268
}
275269

‎internal/controller/provisioner/objects.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,12 @@ func (p *NginxProvisioner) buildNginxResourceObjects(
9494

9595
labels := make(map[string]string)
9696
annotations := make(map[string]string)
97-
annotations[controller.GatewayAnnotation] = gateway.GetName()
97+
98+
if len(gateway.GetName()) > controller.MaxServiceNameLen {
99+
annotations[controller.GatewayLabel] = gateway.GetName()
100+
} else {
101+
selectorLabels[controller.GatewayLabel] = gateway.GetName()
102+
}
98103

99104
maps.Copy(labels, selectorLabels)
100105

‎internal/controller/provisioner/objects_test.go‎

Lines changed: 16 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,13 @@ func TestBuildNginxResourceObjects(t *testing.T) {
9191
}
9292

9393
expLabels := map[string]string{
94-
"label": "value",
95-
"app": "nginx",
96-
"app.kubernetes.io/name": "gw-nginx",
94+
"label": "value",
95+
"app": "nginx",
96+
"gateway.networking.k8s.io/gateway-name": "gw",
97+
"app.kubernetes.io/name": "gw-nginx",
9798
}
9899
expAnnotations := map[string]string{
99-
"annotation": "value",
100-
"gateway.networking.k8s.io/gateway-name": "gw",
100+
"annotation": "value",
101101
}
102102

103103
resourceName := "gw-nginx"
@@ -627,13 +627,13 @@ func TestBuildNginxResourceObjects_Plus(t *testing.T) {
627627
g.Expect(objects).To(HaveLen(9))
628628

629629
expLabels := map[string]string{
630-
"label": "value",
631-
"app": "nginx",
632-
"app.kubernetes.io/name": "gw-nginx",
630+
"label": "value",
631+
"app": "nginx",
632+
"gateway.networking.k8s.io/gateway-name": "gw",
633+
"app.kubernetes.io/name": "gw-nginx",
633634
}
634635
expAnnotations := map[string]string{
635-
"annotation": "value",
636-
"gateway.networking.k8s.io/gateway-name": "gw",
636+
"annotation": "value",
637637
}
638638

639639
secretObj := objects[1]
@@ -770,19 +770,16 @@ func TestBuildNginxResourceObjects_DockerSecrets(t *testing.T) {
770770
g.Expect(objects).To(HaveLen(9))
771771

772772
expLabels := map[string]string{
773-
"app": "nginx",
774-
"app.kubernetes.io/name": "gw-nginx",
775-
}
776-
expAnnotations := map[string]string{
773+
"app": "nginx",
777774
"gateway.networking.k8s.io/gateway-name": "gw",
775+
"app.kubernetes.io/name": "gw-nginx",
778776
}
779777

780778
secretObj := objects[0]
781779
secret, ok := secretObj.(*corev1.Secret)
782780
g.Expect(ok).To(BeTrue())
783781
g.Expect(secret.GetName()).To(Equal(controller.CreateNginxResourceName(resourceName, agentTLSTestSecretName)))
784782
g.Expect(secret.GetLabels()).To(Equal(expLabels))
785-
g.Expect(secret.GetAnnotations()).To(Equal(expAnnotations))
786783

787784
// the (docker-only) secret order in the object list is sorted by secret name
788785

@@ -791,21 +788,18 @@ func TestBuildNginxResourceObjects_DockerSecrets(t *testing.T) {
791788
g.Expect(ok).To(BeTrue())
792789
g.Expect(secret.GetName()).To(Equal(controller.CreateNginxResourceName(resourceName, dockerTestSecretName)))
793790
g.Expect(secret.GetLabels()).To(Equal(expLabels))
794-
g.Expect(secret.GetAnnotations()).To(Equal(expAnnotations))
795791

796792
registry1SecretObj := objects[2]
797793
secret, ok = registry1SecretObj.(*corev1.Secret)
798794
g.Expect(ok).To(BeTrue())
799795
g.Expect(secret.GetName()).To(Equal(controller.CreateNginxResourceName(resourceName, dockerSecretRegistry1Name)))
800796
g.Expect(secret.GetLabels()).To(Equal(expLabels))
801-
g.Expect(secret.GetAnnotations()).To(Equal(expAnnotations))
802797

803798
registry2SecretObj := objects[3]
804799
secret, ok = registry2SecretObj.(*corev1.Secret)
805800
g.Expect(ok).To(BeTrue())
806801
g.Expect(secret.GetName()).To(Equal(controller.CreateNginxResourceName(resourceName, dockerSecretRegistry2Name)))
807802
g.Expect(secret.GetLabels()).To(Equal(expLabels))
808-
g.Expect(secret.GetAnnotations()).To(Equal(expAnnotations))
809803

810804
depObj := objects[8]
811805
dep, ok := depObj.(*appsv1.Deployment)
@@ -889,18 +883,15 @@ func TestBuildNginxResourceObjects_DaemonSet(t *testing.T) {
889883
g.Expect(objects).To(HaveLen(6))
890884

891885
expLabels := map[string]string{
892-
"app": "nginx",
893-
"app.kubernetes.io/name": "gw-nginx",
894-
}
895-
expAnnotations := map[string]string{
886+
"app": "nginx",
896887
"gateway.networking.k8s.io/gateway-name": "gw",
888+
"app.kubernetes.io/name": "gw-nginx",
897889
}
898890

899891
dsObj := objects[5]
900892
ds, ok := dsObj.(*appsv1.DaemonSet)
901893
g.Expect(ok).To(BeTrue())
902894
g.Expect(ds.GetLabels()).To(Equal(expLabels))
903-
g.Expect(ds.GetAnnotations()).To(Equal(expAnnotations))
904895

905896
template := ds.Spec.Template
906897
g.Expect(template.GetAnnotations()).To(HaveKey("prometheus.io/scrape"))
@@ -956,24 +947,20 @@ func TestBuildNginxResourceObjects_OpenShift(t *testing.T) {
956947
g.Expect(objects).To(HaveLen(8))
957948

958949
expLabels := map[string]string{
959-
"app": "nginx",
960-
"app.kubernetes.io/name": "gw-nginx",
961-
}
962-
expAnnotations := map[string]string{
950+
"app": "nginx",
963951
"gateway.networking.k8s.io/gateway-name": "gw",
952+
"app.kubernetes.io/name": "gw-nginx",
964953
}
965954

966955
roleObj := objects[4]
967956
role, ok := roleObj.(*rbacv1.Role)
968957
g.Expect(ok).To(BeTrue())
969958
g.Expect(role.GetLabels()).To(Equal(expLabels))
970-
g.Expect(role.GetAnnotations()).To(Equal(expAnnotations))
971959

972960
roleBindingObj := objects[5]
973961
roleBinding, ok := roleBindingObj.(*rbacv1.RoleBinding)
974962
g.Expect(ok).To(BeTrue())
975963
g.Expect(roleBinding.GetLabels()).To(Equal(expLabels))
976-
g.Expect(roleBinding.GetAnnotations()).To(Equal(expAnnotations))
977964
}
978965

979966
func TestBuildNginxResourceObjects_DataplaneKeySecret(t *testing.T) {

‎internal/framework/controller/labels.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package controller
22

3-
// The following labels/annotations are added to each nginx resource created by the control plane.
3+
// The following labels are added to each nginx resource created by the control plane.
44
const (
5-
GatewayAnnotation = "gateway.networking.k8s.io/gateway-name"
5+
GatewayLabel = "gateway.networking.k8s.io/gateway-name"
66
AppNameLabel = "app.kubernetes.io/name"
77
AppInstanceLabel = "app.kubernetes.io/instance"
88
AppManagedByLabel = "app.kubernetes.io/managed-by"

‎internal/framework/controller/resource.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
const (
99
// inferencePoolServiceSuffix is the suffix of the headless Service name for an InferencePool.
1010
inferencePoolServiceSuffix = "pool-svc"
11-
maxServiceNameLen = 63
11+
MaxServiceNameLen = 63
1212
hashLen = 8
1313
)
1414

@@ -29,14 +29,14 @@ func CreateInferencePoolServiceName(name string) string {
2929
func truncateAndHashName(name string, suffix string) string {
3030
sep := "-"
3131
full := name + sep + suffix
32-
if len(full) <= maxServiceNameLen {
32+
if len(full) <= MaxServiceNameLen {
3333
return full
3434
}
3535

3636
// Always include the suffix, truncate name as needed
3737
hash := sha256.Sum256([]byte(full))
3838
hashStr := hex.EncodeToString(hash[:])[:hashLen]
39-
maxNameLen := maxServiceNameLen - (len(sep) * 2) - hashLen - len(suffix)
39+
maxNameLen := MaxServiceNameLen - (len(sep) * 2) - hashLen - len(suffix)
4040
truncName := name
4141
if len(name) > maxNameLen {
4242
truncName = name[:maxNameLen]

‎internal/framework/controller/resource_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func TestCreateNginxResourceName(t *testing.T) {
4242
g := NewWithT(t)
4343

4444
name := CreateNginxResourceName(test.prefix, test.suffix)
45-
g.Expect(len(name)).To(BeNumerically("<=", maxServiceNameLen))
45+
g.Expect(len(name)).To(BeNumerically("<=", MaxServiceNameLen))
4646
g.Expect(name).To(Equal(test.expected), "expected %q, got %q", test.expected, name)
4747
})
4848
}
@@ -79,7 +79,7 @@ func TestCreateInferencePoolServiceName(t *testing.T) {
7979
g := NewWithT(t)
8080

8181
serviceName := CreateInferencePoolServiceName(test.name)
82-
g.Expect(len(serviceName)).To(BeNumerically("<=", maxServiceNameLen))
82+
g.Expect(len(serviceName)).To(BeNumerically("<=", MaxServiceNameLen))
8383
g.Expect(serviceName).To(Equal(test.expected), "expected %q, got %q", test.expected, serviceName)
8484
})
8585
}

‎tests/framework/resourcemanager.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,18 +1021,12 @@ func (rm *ResourceManager) GetReadyNginxPodNames(
10211021
ctx,
10221022
&podList,
10231023
client.InNamespace(namespace),
1024+
client.HasLabels{"gateway.networking.k8s.io/gateway-name"},
10241025
); err != nil {
10251026
return false, fmt.Errorf("error getting list of NGINX Pods: %w", err)
10261027
}
10271028

1028-
var filteredPodList core.PodList
1029-
for _, pod := range podList.Items {
1030-
if _, ok := pod.Annotations["gateway.networking.k8s.io/gateway-name"]; ok {
1031-
filteredPodList.Items = append(filteredPodList.Items, pod)
1032-
}
1033-
}
1034-
1035-
nginxPodNames = getReadyPodNames(filteredPodList, opts...)
1029+
nginxPodNames = getReadyPodNames(podList, opts...)
10361030
return len(nginxPodNames) > 0, nil
10371031
},
10381032
)

0 commit comments

Comments
 (0)