Service and Revision labels

docs/cmd/kn_service_create.md

@@ -46,6 +46,7 @@ kn service create NAME --image IMAGE [flags]
       --force                    Create service forcefully, replaces existing service if any.
   -h, --help                     help for create
       --image string             Image to run.
+  -l, --label stringArray        Service label to set. NAME=value; provide an empty string for the value to remove a label. You may provide this flag any number of times to set multiple labels.
       --limits-cpu string        The limits on the requested CPU (e.g., 1000m).
       --limits-memory string     The limits on the requested memory (e.g., 1024Mi).
       --max-scale int            Maximal number of replicas.

docs/cmd/kn_service_update.md

@@ -33,6 +33,7 @@ kn service update NAME [flags]
   -e, --env stringArray          Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables.
   -h, --help                     help for update
       --image string             Image to run.
+  -l, --label stringArray        Service label to set. NAME=value; provide an empty string for the value to remove a label. You may provide this flag any number of times to set multiple labels.
       --limits-cpu string        The limits on the requested CPU (e.g., 1000m).
       --limits-memory string     The limits on the requested memory (e.g., 1024Mi).
       --max-scale int            Maximal number of replicas.

go.mod

@@ -25,7 +25,7 @@ require (
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/modern-go/reflect2 v1.0.1 // indirect
 	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
-	github.com/pkg/errors v0.8.1 // indirect
+	github.com/pkg/errors v0.8.1
 	github.com/spf13/cobra v0.0.3
 	github.com/spf13/pflag v1.0.3
 	github.com/spf13/viper v1.3.1

pkg/kn/commands/parsing_helper.go

@@ -0,0 +1,34 @@
+// Copyright © 2019 The Knative Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package commands
+
+import (
+	"fmt"
+	"strings"
+)
+
+// MapFromArray takes an array of strings where each item is a (key, value) pair
+// separated by a delimiter and returns a map where keys are mapped to their respsective values.
+func MapFromArray(arr []string, delimiter string) (map[string]string, error) {
+	returnMap := map[string]string{}
+	for _, pairStr := range arr {
+		pairSlice := strings.SplitN(pairStr, delimiter, 2)
+		if len(pairSlice) <= 1 {
+			return nil, fmt.Errorf("Argument requires a value that contains the %q character; got %q", delimiter, pairStr)
+		}
+		returnMap[pairSlice[0]] = pairSlice[1]
+	}
+	return returnMap, nil
+}

pkg/kn/commands/parsing_helper_test.go

@@ -0,0 +1,49 @@
+// Copyright © 2018 The Knative Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package commands
+
+import (
+	"reflect"
+	"testing"
+
+	"gotest.tools/assert"
+)
+
+func TestMapFromArray(t *testing.T) {
+	testMapFromArray(t, []string{"good=value"}, "=", map[string]string{"good": "value"})
+	testMapFromArray(t, []string{"multi=value", "other=value"}, "=", map[string]string{"multi": "value", "other": "value"})
+	testMapFromArray(t, []string{"over|write", "over|written"}, "|", map[string]string{"over": "written"})
+	testMapFromArray(t, []string{"only,split,once"}, ",", map[string]string{"only": "split,once"})
+}
+
+func testMapFromArray(t *testing.T, input []string, delimiter string, expected map[string]string) {
+	actual, err := MapFromArray(input, delimiter)
+	assert.NilError(t, err)
+	if !reflect.DeepEqual(expected, actual) {
+		t.Fatalf("Map did not match expected: %s\nFound: %s", expected, actual)
+	}
+}
+
+func TestMapFromArrayNoDelimiter(t *testing.T) {
+	input := []string{"good=value", "badvalue"}
+	_, err := MapFromArray(input, "=")
+	assert.ErrorContains(t, err, "Argument requires")
+}
+
+func TestMapFromArrayEmptyValue(t *testing.T) {
+	input := []string{""}
+	_, err := MapFromArray(input, "=")
+	assert.ErrorContains(t, err, "Argument requires")
+}

pkg/kn/commands/service/configuration_edit_flags.go

@@ -15,11 +15,10 @@
 package service
 
 import (
-	"fmt"
-	"strings"
-
+	commands "github.com/knative/client/pkg/kn/commands"
 	servinglib "github.com/knative/client/pkg/serving"
 	servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
+	errors "github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -35,6 +34,7 @@ type ConfigurationEditFlags struct {
 	ConcurrencyTarget          int
 	ConcurrencyLimit           int
 	Port                       int32
+	Labels                     []string
 }
 
 type ResourceFlags struct {
@@ -56,6 +56,7 @@ func (p *ConfigurationEditFlags) AddUpdateFlags(command *cobra.Command) {
 	command.Flags().IntVar(&p.ConcurrencyTarget, "concurrency-target", 0, "Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given.")
 	command.Flags().IntVar(&p.ConcurrencyLimit, "concurrency-limit", 0, "Hard Limit of concurrent requests to be processed by a single replica.")
 	command.Flags().Int32VarP(&p.Port, "port", "p", 0, "The port where application listens on.")
+	command.Flags().StringArrayVarP(&p.Labels, "label", "l", []string{}, "Service label to set. NAME=value; provide an empty string for the value to remove a label. You may provide this flag any number of times to set multiple labels.")
 }
 
 func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) {
@@ -71,18 +72,15 @@ func (p *ConfigurationEditFlags) Apply(service *servingv1alpha1.Service, cmd *co
 		return err
 	}
 
-	envMap := map[string]string{}
-	for _, pairStr := range p.Env {
-		pairSlice := strings.SplitN(pairStr, "=", 2)
-		if len(pairSlice) <= 1 {
-			return fmt.Errorf(
-				"--env argument requires a value that contains the '=' character; got %s",
-				pairStr)
+	if cmd.Flags().Changed("env") {
+		envMap, err := commands.MapFromArray(p.Env, "=")
+		if err != nil {
+			return errors.Wrap(err, "Invalid --env")
+		}
+		err = servinglib.UpdateEnvVars(template, envMap)
+		if err != nil {
+			return err
 		}
-		envMap[pairSlice[0]] = pairSlice[1]
-	}
-	if err := servinglib.UpdateEnvVars(template, envMap); err != nil {
-		return err
 	}
 
 	if cmd.Flags().Changed("image") {
@@ -139,6 +137,17 @@ func (p *ConfigurationEditFlags) Apply(service *servingv1alpha1.Service, cmd *co
 		}
 	}
 
+	if cmd.Flags().Changed("label") {
+		labelMap, err := commands.MapFromArray(p.Labels, "=")
+		if err != nil {
+			return errors.Wrap(err, "Invalid --label")
+		}
+		err = servinglib.UpdateServiceLabels(service, labelMap)
+		if err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 

pkg/kn/commands/service/service_create_test.go

@@ -467,3 +467,30 @@ func TestServiceCreateEnvForce(t *testing.T) {
 		t.Fatalf("wrong output: %s", output)
 	}
 }
+
+func TestServiceCreateLabel(t *testing.T) {
+	action, created, _, err := fakeServiceCreate([]string{
+		"service", "create", "foo", "--image", "gcr.io/foo/bar:baz", "-l", "a=mouse", "--label", "b=cookie", "--async"}, false, false)
+
+	if err != nil {
+		t.Fatal(err)
+	} else if !action.Matches("create", "services") {
+		t.Fatalf("Bad action %v", action)
+	}
+
+	expectedLabels := map[string]string{
+		"a": "mouse",
+		"b": "cookie"}
+	actualLabels := created.ObjectMeta.Labels
+
+	template, err := servinglib.RevisionTemplateOfService(created)
+	if err != nil {
+		t.Fatal(err)
+	} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" {
+		t.Fatalf("wrong image set: %v", template.Spec.DeprecatedContainer.Image)
+	} else if !reflect.DeepEqual(
+		actualLabels,
+		expectedLabels) {
+		t.Fatalf("wrong labels %v", created.ObjectMeta.Labels)
+	}
+}

pkg/kn/commands/service/service_update_test.go

@@ -374,6 +374,51 @@ func TestServiceUpdateRequestsLimitsCPU_and_Memory(t *testing.T) {
 	}
 }
 
+func TestServiceUpdateLabelWhenEmpty(t *testing.T) {
+	original := newEmptyService()
+
+	action, updated, _, err := fakeServiceUpdate(original, []string{
+		"service", "update", "foo", "-l", "a=mouse", "--label", "b=cookie", "--async"}, false)
+
+	if err != nil {
+		t.Fatal(err)
+	} else if !action.Matches("update", "services") {
+		t.Fatalf("Bad action %v", action)
+	}
+
+	expectedLabels := map[string]string{
+		"a": "mouse",
+		"b": "cookie"}
+	actualLabels := updated.ObjectMeta.Labels
+
+	if !reflect.DeepEqual(actualLabels, expectedLabels) {
+		t.Fatalf("wrong labels %v", actualLabels)
+	}
+}
+
+func TestServiceUpdateLabelExisting(t *testing.T) {
+	original := newEmptyService()
+	original.ObjectMeta.Labels = map[string]string{"already": "here", "tobe": "removed"}
+
+	action, updated, _, err := fakeServiceUpdate(original, []string{
+		"service", "update", "foo", "-l", "already=gone", "--label=tobe=", "--label", "b=cookie", "--async"}, false)
+
+	if err != nil {
+		t.Fatal(err)
+	} else if !action.Matches("update", "services") {
+		t.Fatalf("Bad action %v", action)
+	}
+
+	expectedLabels := map[string]string{
+		"already": "gone",
+		"b":       "cookie"}
+	actualLabels := updated.ObjectMeta.Labels
+
+	if !reflect.DeepEqual(actualLabels, expectedLabels) {
+		t.Fatalf("wrong labels %v", actualLabels)
+	}
+}
+
 func newEmptyService() *v1alpha1.Service {
 	return &v1alpha1.Service{
 		TypeMeta: metav1.TypeMeta{

pkg/serving/config_changes.go

@@ -152,6 +152,22 @@ func UpdateResources(template *servingv1alpha1.RevisionTemplateSpec, requestsRes
 	return nil
 }
 
+// UpdateServiceLabels updates the labels on a service
+func UpdateServiceLabels(service *servingv1alpha1.Service, vars map[string]string) error {
+	if service.ObjectMeta.Labels == nil {
+		service.ObjectMeta.Labels = make(map[string]string)
+	}
+	for key, value := range vars {
+		// Delete the label if passed an empty string, otherwise set or update it
+		if value == "" {
+			delete(service.ObjectMeta.Labels, key)
+		} else {
+			service.ObjectMeta.Labels[key] = value
+		}
+	}
+	return nil
+}
+
 // =======================================================================================
 
 func updateEnvVarsFromMap(env []corev1.EnvVar, vars map[string]string) []corev1.EnvVar {

pkg/serving/config_changes_test.go

@@ -26,6 +26,7 @@ import (
 
 	servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestUpdateAutoscalingAnnotations(t *testing.T) {
@@ -270,6 +271,53 @@ func testUpdateEnvVarsBoth(t *testing.T, template *servingv1alpha1.RevisionTempl
 	}
 }
 
+func TestUpdateLabelsNew(t *testing.T) {
+	service := getV1alpha1Service()
+	labels := map[string]string{
+		"a": "foo",
+		"b": "bar",
+	}
+	err := UpdateServiceLabels(service, labels)
+	assert.NilError(t, err)
+	actual := service.ObjectMeta.Labels
+	if !reflect.DeepEqual(labels, actual) {
+		t.Fatalf("Labels did not match expected %v found %v", labels, actual)
+	}
+}
+
+func TestUpdateLabelsExisting(t *testing.T) {
+	service := getV1alpha1Service()
+	service.ObjectMeta.Labels = map[string]string{"a": "foo"}
+	labels := map[string]string{
+		"a": "notfood",
+		"b": "bar",
+	}
+	err := UpdateServiceLabels(service, labels)
+	assert.NilError(t, err)
+	actual := service.ObjectMeta.Labels
+	if !reflect.DeepEqual(labels, actual) {
+		t.Fatalf("Labels did not match expected %v found %v", labels, actual)
+	}
+}
+
+func TestUpdateLabelsRemoveExisting(t *testing.T) {
+	service := getV1alpha1Service()
+	service.ObjectMeta.Labels = map[string]string{"a": "foo", "b": "bar"}
+	labels := map[string]string{
+		"a": "foo",
+		"b": "",
+	}
+	err := UpdateServiceLabels(service, labels)
+	assert.NilError(t, err)
+	expected := map[string]string{
+		"a": "foo",
+	}
+	actual := service.ObjectMeta.Labels
+	if !reflect.DeepEqual(expected, actual) {
+		t.Fatalf("Labels did not match expected %v found %v", expected, actual)
+	}
+}
+
 // =========================================================================================================
 
 func getV1alpha1RevisionTemplateWithOldFields() (*servingv1alpha1.RevisionTemplateSpec, *corev1.Container) {
@@ -296,6 +344,28 @@ func getV1alpha1Config() (*servingv1alpha1.RevisionTemplateSpec, *corev1.Contain
 	return template, &containers[0]
 }
 
+func getV1alpha1Service() *servingv1alpha1.Service {
+	template, _ := getV1alpha1RevisionTemplateWithOldFields()
+	service := &servingv1alpha1.Service{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "Service",
+			APIVersion: "knative.dev/v1alph1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "foo",
+			Namespace: "default",
+		},
+		Spec: servingv1alpha1.ServiceSpec{
+			DeprecatedRunLatest: &servingv1alpha1.RunLatestType{
+				Configuration: servingv1alpha1.ConfigurationSpec{
+					DeprecatedRevisionTemplate: template,
+				},
+			},
+		},
+	}
+	return service
+}
+
 func assertNoV1alpha1Old(t *testing.T, template *servingv1alpha1.RevisionTemplateSpec) {
 	if template.Spec.DeprecatedContainer != nil {
 		t.Error("Assuming only new v1alpha1 fields but found spec.container")