Prevent service-specific labels from being put on the revision template (#672)

See: https://github.com/knative/serving/pull/6865

Signed-off-by: Doug Davis <dug@us.ibm.com>
This commit is contained in:
Doug Davis 2020-02-17 11:58:09 -05:00 committed by GitHub
parent a220a88534
commit 83b926b802
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 32 additions and 3 deletions

View File

@ -362,6 +362,12 @@ func UpdateResources(template *servingv1.RevisionTemplateSpec, requestsResourceL
return nil return nil
} }
// ServiceOnlyLabels should only appear on the Service and NOT on the
// Revision template
var ServiceOnlyLabels = map[string]struct{}{
serving.GroupName + "/visibility": {},
}
// UpdateLabels updates the labels identically on a service and template. // UpdateLabels updates the labels identically on a service and template.
// Does not overwrite the entire Labels field, only makes the requested updates // Does not overwrite the entire Labels field, only makes the requested updates
func UpdateLabels(service *servingv1.Service, template *servingv1.RevisionTemplateSpec, toUpdate map[string]string, toRemove []string) error { func UpdateLabels(service *servingv1.Service, template *servingv1.RevisionTemplateSpec, toUpdate map[string]string, toRemove []string) error {
@ -373,7 +379,11 @@ func UpdateLabels(service *servingv1.Service, template *servingv1.RevisionTempla
} }
for key, value := range toUpdate { for key, value := range toUpdate {
service.ObjectMeta.Labels[key] = value service.ObjectMeta.Labels[key] = value
template.ObjectMeta.Labels[key] = value
// Only add it to the template if it's not in our ServiceOnly list
if _, ok := ServiceOnlyLabels[key]; !ok {
template.ObjectMeta.Labels[key] = value
}
} }
for _, key := range toRemove { for _, key := range toRemove {
delete(service.ObjectMeta.Labels, key) delete(service.ObjectMeta.Labels, key)

View File

@ -331,6 +331,25 @@ func TestUpdateLabelsNew(t *testing.T) {
"a": "foo", "a": "foo",
"b": "bar", "b": "bar",
} }
tLabels := labels // revision template labels
// Only test service-specific labels if we have any
if len(ServiceOnlyLabels) != 0 {
// Make a copy of the expected labels so we can modify the original
// list w/o changing what's expected for the revsion template
tLabels = map[string]string{}
for k, v := range labels {
tLabels[k] = v
}
// Just add a random value from the list to make sure it doesn't show
// up in the revision template
for k := range ServiceOnlyLabels {
labels[k] = "testing"
break
}
}
err := UpdateLabels(service, template, labels, []string{}) err := UpdateLabels(service, template, labels, []string{})
assert.NilError(t, err) assert.NilError(t, err)
@ -340,8 +359,8 @@ func TestUpdateLabelsNew(t *testing.T) {
} }
actual = template.ObjectMeta.Labels actual = template.ObjectMeta.Labels
if !reflect.DeepEqual(labels, actual) { if !reflect.DeepEqual(tLabels, actual) {
t.Fatalf("Template labels did not match expected %v found %v", labels, actual) t.Fatalf("Template labels did not match expected %v found %v", tLabels, actual)
} }
} }