Add a serviceUID label to ksvc (and implicitly to config) (#10539)

This will allow people to find all pods for a service w/o running into the problem of finding pods associated with an old ksvc that just happens to share the same name. With this the user can search for pods by the ksvc UID via the new label. Rather than having the code go and lookup the ksvc from the revision, I decided to just add this label to the config as well. If people would prefer to not have this extra label on the config (and do it via a ksvc lookup) I can look into that change. Signed-off-by: Doug Davis <[email protected]>
knative · Jan 16, 2021 · 3177cc7 · 3177cc7
1 parent 59e35cd
commit 3177cc7
Show file tree

Hide file tree

Showing 9 changed files with 67 additions and 10 deletions.
diff --git a/pkg/apis/serving/register.go b/pkg/apis/serving/register.go
@@ -77,6 +77,14 @@ const (
 	// its unique identifier
 	RevisionUID = GroupName + "/revisionUID"
 
+	// ConfigUIDLabelKey is the label key attached to a pod to reference its
+	// Knative Configuration by its unique UID
+	ConfigUIDLabelKey = GroupName + "/configUID"
+
+	// ServiceUIDLabelKey is the label key attached to a pod to reference its
+	// Knative Service by its unique UID
+	ServiceUIDLabelKey = GroupName + "/serviceUID"
+
 	// ServiceLabelKey is the label key attached to a Route and Configuration indicating by
 	// which Service they are created.
 	ServiceLabelKey = GroupName + "/service"

diff --git a/pkg/apis/serving/v1/configuration_validation.go b/pkg/apis/serving/v1/configuration_validation.go
@@ -65,8 +65,10 @@ func (cs *ConfigurationSpec) Validate(ctx context.Context) *apis.FieldError {
 func (c *Configuration) validateLabels() (errs *apis.FieldError) {
 	for key, val := range c.GetLabels() {
 		switch key {
-		case serving.RouteLabelKey:
-			// Known valid labels.
+		case serving.RouteLabelKey,
+			serving.ConfigUIDLabelKey,
+			serving.ServiceUIDLabelKey:
+			// Known valid labels - so just skip them
 		case serving.ServiceLabelKey:
 			errs = errs.Also(verifyLabelOwnerRef(val, serving.ServiceLabelKey, "Service", c.GetOwnerReferences()))
 		default:

diff --git a/pkg/apis/serving/v1/revision_validation.go b/pkg/apis/serving/v1/revision_validation.go
@@ -128,6 +128,8 @@ func (r *Revision) ValidateLabels() (errs *apis.FieldError) {
 		case serving.RoutingStateLabelKey,
 			serving.RouteLabelKey,
 			serving.ServiceLabelKey,
+			serving.ConfigUIDLabelKey,
+			serving.ServiceUIDLabelKey,
 			serving.ConfigurationGenerationLabelKey:
 			// Known valid labels.
 		case serving.ConfigurationLabelKey:

diff --git a/pkg/reconciler/configuration/resources/revision.go b/pkg/reconciler/configuration/resources/revision.go
@@ -64,6 +64,8 @@ func updateRevisionLabels(rev, config metav1.Object) {
 		serving.ConfigurationLabelKey,
 		serving.ServiceLabelKey,
 		serving.ConfigurationGenerationLabelKey,
+		serving.ConfigUIDLabelKey,
+		serving.ServiceUIDLabelKey,
 	} {
 		labels[key] = RevisionLabelValueForKey(key, config)
 	}
@@ -101,6 +103,10 @@ func RevisionLabelValueForKey(key string, config metav1.Object) string {
 		return config.GetLabels()[serving.ServiceLabelKey]
 	case serving.ConfigurationGenerationLabelKey:
 		return fmt.Sprint(config.GetGeneration())
+	case serving.ConfigUIDLabelKey:
+		return string(config.GetUID())
+	case serving.ServiceUIDLabelKey:
+		return config.GetLabels()[serving.ServiceUIDLabelKey]
 	}
 	return ""
 }
diff --git a/pkg/reconciler/configuration/resources/revision_test.go b/pkg/reconciler/configuration/resources/revision_test.go
@@ -49,6 +49,10 @@ func TestMakeRevisions(t *testing.T) {
 				Namespace:  "no",
 				Name:       "build",
 				Generation: 10,
+				Labels: map[string]string{
+					serving.ServiceUIDLabelKey: "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
+				},
+				UID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 			},
 			Spec: v1.ConfigurationSpec{
 				Template: v1.RevisionTemplateSpec{
@@ -72,12 +76,15 @@ func TestMakeRevisions(t *testing.T) {
 					Name:               "build",
 					Controller:         ptr.Bool(true),
 					BlockOwnerDeletion: ptr.Bool(true),
+					UID:                "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 				}},
 				Labels: map[string]string{
-					serving.RoutingStateLabelKey:            "pending",
 					serving.ConfigurationLabelKey:           "build",
 					serving.ConfigurationGenerationLabelKey: "10",
+					serving.ConfigUIDLabelKey:               "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
+					serving.RoutingStateLabelKey:            "pending",
 					serving.ServiceLabelKey:                 "",
+					serving.ServiceUIDLabelKey:              "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
 				},
 				Annotations: map[string]string{
 					serving.RoutingStateModifiedAnnotationKey: v1.RoutingStateModifiedString(clock),
@@ -98,6 +105,10 @@ func TestMakeRevisions(t *testing.T) {
 				Namespace:  "with",
 				Name:       "labels",
 				Generation: 100,
+				Labels: map[string]string{
+					serving.ServiceUIDLabelKey: "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
+				},
+				UID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 			},
 			Spec: v1.ConfigurationSpec{
 				Template: v1.RevisionTemplateSpec{
@@ -130,12 +141,15 @@ func TestMakeRevisions(t *testing.T) {
 					Name:               "labels",
 					Controller:         ptr.Bool(true),
 					BlockOwnerDeletion: ptr.Bool(true),
+					UID:                "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 				}},
 				Labels: map[string]string{
 					serving.ConfigurationLabelKey:           "labels",
 					serving.ConfigurationGenerationLabelKey: "100",
-					serving.ServiceLabelKey:                 "",
+					serving.ConfigUIDLabelKey:               "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 					serving.RoutingStateLabelKey:            "pending",
+					serving.ServiceLabelKey:                 "",
+					serving.ServiceUIDLabelKey:              "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
 					"foo":                                   "bar",
 					"baz":                                   "blah",
 				},
@@ -155,6 +169,10 @@ func TestMakeRevisions(t *testing.T) {
 				Namespace:  "with",
 				Name:       "annotations",
 				Generation: 100,
+				Labels: map[string]string{
+					serving.ServiceUIDLabelKey: "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
+				},
+				UID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 			},
 			Spec: v1.ConfigurationSpec{
 				Template: v1.RevisionTemplateSpec{
@@ -184,12 +202,15 @@ func TestMakeRevisions(t *testing.T) {
 					Name:               "annotations",
 					Controller:         ptr.Bool(true),
 					BlockOwnerDeletion: ptr.Bool(true),
+					UID:                "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 				}},
 				Labels: map[string]string{
 					serving.ConfigurationLabelKey:           "annotations",
 					serving.ConfigurationGenerationLabelKey: "100",
-					serving.ServiceLabelKey:                 "",
+					serving.ConfigUIDLabelKey:               "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 					serving.RoutingStateLabelKey:            "pending",
+					serving.ServiceLabelKey:                 "",
+					serving.ServiceUIDLabelKey:              "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
 				},
 				Annotations: map[string]string{
 					"foo": "bar",
@@ -217,7 +238,11 @@ func TestMakeRevisions(t *testing.T) {
 					"serving.knative.dev/lastModifier": "someone",
 					serving.RoutesAnnotationKey:        "route",
 				},
+				Labels: map[string]string{
+					serving.ServiceUIDLabelKey: "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
+				},
 				Generation: 10,
+				UID:        "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 			},
 			Spec: v1.ConfigurationSpec{
 				Template: v1.RevisionTemplateSpec{
@@ -246,12 +271,15 @@ func TestMakeRevisions(t *testing.T) {
 					Name:               "config",
 					Controller:         ptr.Bool(true),
 					BlockOwnerDeletion: ptr.Bool(true),
+					UID:                "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 				}},
 				Labels: map[string]string{
 					serving.ConfigurationLabelKey:           "config",
 					serving.ConfigurationGenerationLabelKey: "10",
-					serving.ServiceLabelKey:                 "",
+					serving.ConfigUIDLabelKey:               "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 					serving.RoutingStateLabelKey:            "active",
+					serving.ServiceLabelKey:                 "",
+					serving.ServiceUIDLabelKey:              "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
 				},
 			},
 			Spec: v1.RevisionSpec{
@@ -272,7 +300,11 @@ func TestMakeRevisions(t *testing.T) {
 					"serving.knative.dev/creator":      "admin",
 					"serving.knative.dev/lastModifier": "someone",
 				},
+				Labels: map[string]string{
+					serving.ServiceUIDLabelKey: "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
+				},
 				Generation: 10,
+				UID:        "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 			},
 			Spec: v1.ConfigurationSpec{
 				Template: v1.RevisionTemplateSpec{
@@ -308,12 +340,15 @@ func TestMakeRevisions(t *testing.T) {
 					Name:               "config",
 					Controller:         ptr.Bool(true),
 					BlockOwnerDeletion: ptr.Bool(true),
+					UID:                "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 				}},
 				Labels: map[string]string{
 					serving.ConfigurationLabelKey:           "config",
 					serving.ConfigurationGenerationLabelKey: "10",
-					serving.ServiceLabelKey:                 "",
+					serving.ConfigUIDLabelKey:               "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
 					serving.RoutingStateLabelKey:            "pending",
+					serving.ServiceLabelKey:                 "",
+					serving.ServiceUIDLabelKey:              "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
 				},
 			},
 			Spec: v1.RevisionSpec{

diff --git a/pkg/reconciler/service/resources/configuration.go b/pkg/reconciler/service/resources/configuration.go
@@ -36,7 +36,10 @@ func MakeConfiguration(service *v1.Service) *v1.Configuration {
 
 // MakeConfigurationFromExisting creates a Configuration from a Service object given an existing Configuration.
 func MakeConfigurationFromExisting(service *v1.Service, existing *v1.Configuration) *v1.Configuration {
-	labels := map[string]string{serving.ServiceLabelKey: service.Name}
+	labels := map[string]string{
+		serving.ServiceLabelKey:    service.Name,
+		serving.ServiceUIDLabelKey: string(service.ObjectMeta.UID),
+	}
 	anns := kmeta.FilterMap(service.GetAnnotations(), func(key string) bool {
 		return key == corev1.LastAppliedConfigAnnotation
 	})

diff --git a/pkg/reconciler/service/resources/configuration_test.go b/pkg/reconciler/service/resources/configuration_test.go
@@ -41,7 +41,7 @@ func TestConfigurationSpec(t *testing.T) {
 	}
 	expectOwnerReferencesSetCorrectly(t, c.OwnerReferences)
 
-	if got, want := c.Labels, map[string]string{serving.ServiceLabelKey: testServiceName}; !cmp.Equal(got, want) {
+	if got, want := c.Labels, map[string]string{serving.ServiceLabelKey: testServiceName, serving.ServiceUIDLabelKey: "cccccccc-cccc-cccc-cccc-cccccccccccc"}; !cmp.Equal(got, want) {
 		t.Errorf("Labels mismatch: diff(-want,+got):\n%s", cmp.Diff(want, got))
 	}
 	if got, want := c.Annotations, map[string]string{

diff --git a/pkg/reconciler/service/resources/shared_test.go b/pkg/reconciler/service/resources/shared_test.go
@@ -49,7 +49,7 @@ func expectOwnerReferencesSetCorrectly(t *testing.T, ownerRefs []metav1.OwnerRef
 		Kind:       "Service",
 		Name:       testServiceName,
 	}}
-	if diff := cmp.Diff(expectedRefs, ownerRefs, cmpopts.IgnoreFields(expectedRefs[0], "Controller", "BlockOwnerDeletion")); diff != "" {
+	if diff := cmp.Diff(expectedRefs, ownerRefs, cmpopts.IgnoreFields(expectedRefs[0], "Controller", "BlockOwnerDeletion", "UID")); diff != "" {
 		t.Error("Unexpected service owner refs diff (-want +got):", diff)
 	}
 }

diff --git a/pkg/testing/v1/service.go b/pkg/testing/v1/service.go
@@ -42,6 +42,7 @@ func Service(name, namespace string, so ...ServiceOption) *v1.Service {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
 			Namespace: namespace,
+			UID:       "cccccccc-cccc-cccc-cccc-cccccccccccc",
 		},
 	}
 	for _, opt := range so {