diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 8a55b4bc83..8b8ee96267 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -62,6 +62,14 @@ | Add --service-account flag | https://github.com/knative/client/pull/401[#401] +| 🧽 +| Docs restructure +| https://github.com/knative/client/pull/421[#421] + +| 🎁 +| Add --annotation flag +| https://github.com/knative/client/pull/422[#422] + |=== ## v0.2.0 (2019-07-10) diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index 96bda931da..ee00c7a725 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -34,11 +34,15 @@ kn service create NAME --image IMAGE [flags] # (earlier configured resource requests and limits will be replaced with default) # (earlier configured environment variables will be cleared too if any) kn service create --force s1 --image dev.local/ns/image:v1 + + # Create a service with annotation + kn service create s1 --image dev.local/ns/image:v3 --annotation sidecar.istio.io/inject=false ``` ### Options ``` + --annotation stringArray Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). --async Create service and don't wait for it to become ready. --concurrency-limit int Hard Limit of concurrent requests to be processed by a single replica. --concurrency-target int Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given. diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md index d068115480..d01aa95e0b 100644 --- a/docs/cmd/kn_service_update.md +++ b/docs/cmd/kn_service_update.md @@ -38,6 +38,7 @@ kn service update NAME [flags] ### Options ``` + --annotation stringArray Service annotation to set. name=value; you may provide this flag any number of times to set multiple annotations. To unset, specify the annotation name followed by a "-" (e.g., name-). --async Update service and don't wait for it to become ready. --concurrency-limit int Hard Limit of concurrent requests to be processed by a single replica. --concurrency-target int Recommendation for when to scale up based on the concurrent number of incoming request. Defaults to --concurrency-limit when given. diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go index d3ae456806..70cdf2ca8e 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -41,6 +41,7 @@ type ConfigurationEditFlags struct { NamePrefix string RevisionName string ServiceAccountName string + Annotations []string // Preferences about how to do the action. LockToDigest bool @@ -110,6 +111,11 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) { // Don't mark as changing the revision. command.Flags().StringVar(&p.ServiceAccountName, "service-account", "", "Service account name to set. Empty service account name will result to clear the service account.") p.markFlagMakesRevision("service-account") + command.Flags().StringArrayVar(&p.Annotations, "annotation", []string{}, + "Service annotation to set. name=value; you may provide this flag "+ + "any number of times to set multiple annotations. "+ + "To unset, specify the annotation name followed by a \"-\" (e.g., name-).") + p.markFlagMakesRevision("annotation") } // AddUpdateFlags adds the flags specific to update. @@ -256,6 +262,24 @@ func (p *ConfigurationEditFlags) Apply( } } + if cmd.Flags().Changed("annotation") { + annotationsMap, err := util.MapFromArrayAllowingSingles(p.Annotations, "=") + if err != nil { + return errors.Wrap(err, "Invalid --annotation") + } + annotationsToRemove := []string{} + for key := range annotationsMap { + if strings.HasSuffix(key, "-") { + annotationsToRemove = append(annotationsToRemove, key[:len(key)-1]) + delete(annotationsMap, key) + } + } + err = servinglib.UpdateAnnotations(service, template, annotationsMap, annotationsToRemove) + if err != nil { + return err + } + } + if cmd.Flags().Changed("service-account") { err = servinglib.UpdateServiceAccountName(template, p.ServiceAccountName) if err != nil { diff --git a/pkg/kn/commands/service/create.go b/pkg/kn/commands/service/create.go index a04f7cd2e1..2cc010a630 100644 --- a/pkg/kn/commands/service/create.go +++ b/pkg/kn/commands/service/create.go @@ -33,14 +33,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command { - var editFlags ConfigurationEditFlags - var waitFlags commands.WaitFlags - - serviceCreateCommand := &cobra.Command{ - Use: "create NAME --image IMAGE", - Short: "Create a service.", - Example: ` +var create_example = ` # Create a service 'mysvc' using image at dev.local/ns/image:latest kn service create mysvc --image dev.local/ns/image:latest @@ -60,8 +53,19 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command { # Create or replace default resources of a service 's1' using --force flag # (earlier configured resource requests and limits will be replaced with default) # (earlier configured environment variables will be cleared too if any) - kn service create --force s1 --image dev.local/ns/image:v1`, + kn service create --force s1 --image dev.local/ns/image:v1 + + # Create a service with annotation + kn service create s1 --image dev.local/ns/image:v3 --annotation sidecar.istio.io/inject=false` +func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command { + var editFlags ConfigurationEditFlags + var waitFlags commands.WaitFlags + + serviceCreateCommand := &cobra.Command{ + Use: "create NAME --image IMAGE", + Short: "Create a service.", + Example: create_example, RunE: func(cmd *cobra.Command, args []string) (err error) { if len(args) != 1 { return errors.New("'service create' requires the service name given as single argument") diff --git a/pkg/kn/commands/service/service_update_mock_test.go b/pkg/kn/commands/service/service_update_mock_test.go index d9df60fbaf..804b6da57c 100644 --- a/pkg/kn/commands/service/service_update_mock_test.go +++ b/pkg/kn/commands/service/service_update_mock_test.go @@ -69,3 +69,64 @@ func TestServiceUpdateEnvMock(t *testing.T) { r.Validate() } + +func TestServiceUpdateAnnotationsMock(t *testing.T) { + client := knclient.NewMockKnClient(t) + svcName := "svc1" + newService := getService(svcName) + template, err := servinglib.RevisionTemplateOfService(newService) + assert.NilError(t, err) + template.Spec.GetContainer().Image = "gcr.io/foo/bar:baz" + newService.ObjectMeta.Annotations = map[string]string{ + "an1": "staysConstant", + "an2": "getsUpdated", + "an3": "getsRemoved", + } + template.ObjectMeta.Annotations = map[string]string{ + "an1": "staysConstant", + "an2": "getsUpdated", + "an3": "getsRemoved", + servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz", + } + + updatedService := getService(svcName) + template, err = servinglib.RevisionTemplateOfService(updatedService) + assert.NilError(t, err) + template.Spec.GetContainer().Image = "gcr.io/foo/bar:baz" + updatedService.ObjectMeta.Annotations = map[string]string{ + "an1": "staysConstant", + "an2": "isUpdated", + } + template.ObjectMeta.Annotations = map[string]string{ + "an1": "staysConstant", + "an2": "isUpdated", + servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz", + } + + r := client.Recorder() + r.GetService(svcName, nil, errors.NewNotFound(v1alpha1.Resource("service"), svcName)) + r.CreateService(newService, nil) + r.GetService(svcName, newService, nil) + r.UpdateService(updatedService, nil) + + output, err := executeServiceCommand(client, + "create", svcName, "--image", "gcr.io/foo/bar:baz", + "--annotation", "an1=staysConstant", + "--annotation", "an2=getsUpdated", + "--annotation", "an3=getsRemoved", + "--async", "--revision-name=", + ) + assert.NilError(t, err) + assert.Assert(t, util.ContainsAll(output, "created", svcName, "default")) + + output, err = executeServiceCommand(client, + "update", svcName, + "--annotation", "an2=isUpdated", + "--annotation", "an3-", + "--async", "--revision-name=", + ) + assert.NilError(t, err) + assert.Assert(t, util.ContainsAll(output, "updated", svcName, "default")) + + r.Validate() +} diff --git a/pkg/kn/commands/service/update.go b/pkg/kn/commands/service/update.go index 8922d8b64b..090c4f18b1 100644 --- a/pkg/kn/commands/service/update.go +++ b/pkg/kn/commands/service/update.go @@ -28,14 +28,7 @@ import ( "knative.dev/serving/pkg/apis/serving/v1alpha1" ) -func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command { - var editFlags ConfigurationEditFlags - var waitFlags commands.WaitFlags - var trafficFlags flags.Traffic - serviceUpdateCommand := &cobra.Command{ - Use: "update NAME [flags]", - Short: "Update a service.", - Example: ` +var update_example = ` # Updates a service 'svc' with new environment variables kn service update svc --env KEY1=VALUE1 --env KEY2=VALUE2 @@ -54,7 +47,16 @@ func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command { kn service update svc --untag testing --tag @latest=staging # Add tag 'test' to echo-v3 revision with 10% traffic and rest to latest ready revision of service - kn service update svc --tag echo-v3=test --traffic test=10,@latest=90`, + kn service update svc --tag echo-v3=test --traffic test=10,@latest=90` + +func NewServiceUpdateCommand(p *commands.KnParams) *cobra.Command { + var editFlags ConfigurationEditFlags + var waitFlags commands.WaitFlags + var trafficFlags flags.Traffic + serviceUpdateCommand := &cobra.Command{ + Use: "update NAME [flags]", + Short: "Update a service.", + Example: update_example, RunE: func(cmd *cobra.Command, args []string) (err error) { if len(args) != 1 { return errors.New("requires the service name.") diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go index ac9e0c449a..cb73ed9149 100644 --- a/pkg/serving/config_changes.go +++ b/pkg/serving/config_changes.go @@ -52,24 +52,17 @@ func UpdateEnvVars(template *servingv1alpha1.RevisionTemplateSpec, toUpdate map[ // UpdateMinScale updates min scale annotation func UpdateMinScale(template *servingv1alpha1.RevisionTemplateSpec, min int) error { - return UpdateAnnotation(template, autoscaling.MinScaleAnnotationKey, strconv.Itoa(min)) + return UpdateRevisionTemplateAnnotation(template, autoscaling.MinScaleAnnotationKey, strconv.Itoa(min)) } // UpdatMaxScale updates max scale annotation func UpdateMaxScale(template *servingv1alpha1.RevisionTemplateSpec, max int) error { - return UpdateAnnotation(template, autoscaling.MaxScaleAnnotationKey, strconv.Itoa(max)) + return UpdateRevisionTemplateAnnotation(template, autoscaling.MaxScaleAnnotationKey, strconv.Itoa(max)) } // UpdateConcurrencyTarget updates container concurrency annotation func UpdateConcurrencyTarget(template *servingv1alpha1.RevisionTemplateSpec, target int) error { - // TODO(toVersus): Remove the following validation once serving library is updated to v0.8.0 - // and just rely on ValidateAnnotations method. - if target < autoscaling.TargetMin { - return fmt.Errorf("invalid 'concurrency-target' value: must be an integer greater than 0: %s", - autoscaling.TargetAnnotationKey) - } - - return UpdateAnnotation(template, autoscaling.TargetAnnotationKey, strconv.Itoa(target)) + return UpdateRevisionTemplateAnnotation(template, autoscaling.TargetAnnotationKey, strconv.Itoa(target)) } // UpdateConcurrencyLimit updates container concurrency limit @@ -84,8 +77,9 @@ func UpdateConcurrencyLimit(template *servingv1alpha1.RevisionTemplateSpec, limi return nil } -// UpdateAnnotation updates (or adds) an annotation to the given service -func UpdateAnnotation(template *servingv1alpha1.RevisionTemplateSpec, annotation string, value string) error { +// UpdateRevisionTemplateAnnotation updates an annotation for the given Revision Template. +// Also validates the autoscaling annotation values +func UpdateRevisionTemplateAnnotation(template *servingv1alpha1.RevisionTemplateSpec, annotation string, value string) error { annoMap := template.Annotations if annoMap == nil { annoMap = make(map[string]string) @@ -245,6 +239,35 @@ func UpdateLabels(service *servingv1alpha1.Service, template *servingv1alpha1.Re return nil } +// UpdateAnnotations updates the annotations identically on a service and template. +// Does not overwrite the entire Annotations field, only makes the requested updates. +func UpdateAnnotations( + service *servingv1alpha1.Service, + template *servingv1alpha1.RevisionTemplateSpec, + toUpdate map[string]string, + toRemove []string) error { + + if service.ObjectMeta.Annotations == nil { + service.ObjectMeta.Annotations = make(map[string]string) + } + + if template.ObjectMeta.Annotations == nil { + template.ObjectMeta.Annotations = make(map[string]string) + } + + for key, value := range toUpdate { + service.ObjectMeta.Annotations[key] = value + template.ObjectMeta.Annotations[key] = value + } + + for _, key := range toRemove { + delete(service.ObjectMeta.Annotations, key) + delete(template.ObjectMeta.Annotations, key) + } + + return nil +} + // UpdateServiceAccountName updates the service account name used for the corresponding knative service func UpdateServiceAccountName(template *servingv1alpha1.RevisionTemplateSpec, serviceAccountName string) error { serviceAccountName = strings.TrimSpace(serviceAccountName) diff --git a/pkg/serving/config_changes_test.go b/pkg/serving/config_changes_test.go index 3f42e395f3..e1c0c70389 100644 --- a/pkg/serving/config_changes_test.go +++ b/pkg/serving/config_changes_test.go @@ -419,6 +419,73 @@ func TestUpdateServiceAccountName(t *testing.T) { assert.Equal(t, template.Spec.ServiceAccountName, "") } +func TestUpdateAnnotationsNew(t *testing.T) { + service, template, _ := getV1alpha1Service() + + annotations := map[string]string{ + "a": "foo", + "b": "bar", + } + err := UpdateAnnotations(service, template, annotations, []string{}) + assert.NilError(t, err) + + actual := service.ObjectMeta.Annotations + if !reflect.DeepEqual(annotations, actual) { + t.Fatalf("Service annotations did not match expected %v found %v", annotations, actual) + } + + actual = template.ObjectMeta.Annotations + if !reflect.DeepEqual(annotations, actual) { + t.Fatalf("Template annotations did not match expected %v found %v", annotations, actual) + } +} + +func TestUpdateAnnotationsExisting(t *testing.T) { + service, template, _ := getV1alpha1Service() + service.ObjectMeta.Annotations = map[string]string{"a": "foo", "b": "bar"} + template.ObjectMeta.Annotations = map[string]string{"a": "foo", "b": "bar"} + + annotations := map[string]string{ + "a": "notfoo", + "c": "bat", + "d": "", + } + err := UpdateAnnotations(service, template, annotations, []string{}) + assert.NilError(t, err) + expected := map[string]string{ + "a": "notfoo", + "b": "bar", + "c": "bat", + "d": "", + } + + actual := service.ObjectMeta.Annotations + assert.DeepEqual(t, expected, actual) + + actual = template.ObjectMeta.Annotations + assert.DeepEqual(t, expected, actual) +} + +func TestUpdateAnnotationsRemoveExisting(t *testing.T) { + service, template, _ := getV1alpha1Service() + service.ObjectMeta.Annotations = map[string]string{"a": "foo", "b": "bar"} + template.ObjectMeta.Annotations = map[string]string{"a": "foo", "b": "bar"} + + remove := []string{"b"} + err := UpdateAnnotations(service, template, map[string]string{}, remove) + assert.NilError(t, err) + expected := map[string]string{ + "a": "foo", + } + + actual := service.ObjectMeta.Annotations + assert.DeepEqual(t, expected, actual) + + actual = template.ObjectMeta.Annotations + assert.DeepEqual(t, expected, actual) +} + +// // ========================================================================================================= func getV1alpha1RevisionTemplateWithOldFields() (*servingv1alpha1.RevisionTemplateSpec, *corev1.Container) { diff --git a/test/e2e/service_options_test.go b/test/e2e/service_options_test.go index 040e43e261..2d3d813044 100644 --- a/test/e2e/service_options_test.go +++ b/test/e2e/service_options_test.go @@ -17,6 +17,7 @@ package e2e import ( + "fmt" "testing" "gotest.tools/assert" @@ -69,6 +70,14 @@ func TestServiceOptions(t *testing.T) { t.Run("delete service", func(t *testing.T) { test.serviceDelete(t, "svc2") }) + + t.Run("create, update and validate service with annotations", func(t *testing.T) { + test.serviceCreateWithOptions(t, "svc3", []string{"--annotation", "alpha=wolf", "--annotation", "brave=horse"}) + test.validateServiceAnnotations(t, "svc3", map[string]string{"alpha": "wolf", "brave": "horse"}) + test.serviceUpdate(t, "svc3", []string{"--annotation", "alpha=direwolf", "--annotation", "brave-"}) + test.validateServiceAnnotations(t, "svc3", map[string]string{"alpha": "direwolf", "brave": ""}) + test.serviceDelete(t, "svc3") + }) } func (test *e2eTest) serviceCreateWithOptions(t *testing.T, serviceName string, options []string) { @@ -142,3 +151,27 @@ func (test *e2eTest) validateServiceMaxScale(t *testing.T, serviceName, maxScale assert.Equal(t, maxScale, out) } } + +func (test *e2eTest) validateServiceAnnotations(t *testing.T, serviceName string, annotations map[string]string) { + metadataAnnotationsJsonpathFormat := "jsonpath={.metadata.annotations.%s}" + templateAnnotationsJsonpathFormat := "jsonpath={.spec.template.metadata.annotations.%s}" + oldTemplateAnnotationsJsonpathFormat := "jsonpath={.spec.runLatest.configuration.revisionTemplate.metadata.annotations.%s}" + + for k, v := range annotations { + out, err := test.kn.RunWithOpts([]string{"service", "describe", serviceName, "-o", fmt.Sprintf(metadataAnnotationsJsonpathFormat, k)}, runOpts{}) + assert.NilError(t, err) + assert.Equal(t, v, out) + + out, err = test.kn.RunWithOpts([]string{"service", "describe", serviceName, "-o", fmt.Sprintf(templateAnnotationsJsonpathFormat, k)}, runOpts{}) + assert.NilError(t, err) + if out != "" || v == "" { + assert.Equal(t, v, out) + } else { + // case where server returns fields like spec.runLatest.configuration.revisionTemplate.metadata.annotations + // TODO: Remove this case when `runLatest` field is deprecated altogether / v1beta1 + out, err := test.kn.RunWithOpts([]string{"service", "describe", serviceName, "-o", fmt.Sprintf(oldTemplateAnnotationsJsonpathFormat, k)}, runOpts{}) + assert.NilError(t, err) + assert.Equal(t, v, out) + } + } +}