diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index b0fdb78b5..776a07acc 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -26,6 +26,10 @@ | Add `kn source list` | https://github.com/knative/client/pull/666[#666] +| 🎁 +| Add --cluster-local and --no-cluster-local flags +| https://github.com/knative/client/pull/629[#629] + | 🎁 | Add JSON/YAML output format for version command | https://github.com/knative/client/pull/709[#709] diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index 78ee93469..e5d4a9f8f 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -37,6 +37,9 @@ kn service create NAME --image IMAGE [flags] # Create a service with annotation kn service create s1 --image dev.local/ns/image:v3 --annotation sidecar.istio.io/inject=false + + # Create a private service (that is a service with no external endpoint) + kn service create s1 --image dev.local/ns/image:v3 --cluster-local ``` ### Options @@ -46,6 +49,7 @@ kn service create NAME --image IMAGE [flags] --arg stringArray Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times. --async DEPRECATED: please use --no-wait instead. Create service and don't wait for it to be ready. --autoscale-window string Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s) + --cluster-local Specify that the service be private. (--no-cluster-local will make the service publicly available --cmd string Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments. --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. @@ -64,6 +68,7 @@ kn service create NAME --image IMAGE [flags] --min-scale int Minimal number of replicas. --mount stringArray Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. When a configmap or a secret is specified, a corresponding volume is automatically generated. You can use this flag multiple times. For unmounting a directory, append "-", e.g. --mount /mydir-, which also removes any auto-generated volume. -n, --namespace string Specify the namespace to operate in. + --no-cluster-local Do not specify that the service be private. (--no-cluster-local will make the service publicly available (default true) --no-lock-to-digest Do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision) --no-wait Create service and don't wait for it to be ready. -p, --port int32 The port where application listens on. diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md index 0c8f7c052..6597f9e74 100644 --- a/docs/cmd/kn_service_update.md +++ b/docs/cmd/kn_service_update.md @@ -42,6 +42,7 @@ kn service update NAME [flags] --arg stringArray Add argument to the container command. Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. You can use this flag multiple times. --async DEPRECATED: please use --no-wait instead. Update service and don't wait for it to be ready. --autoscale-window string Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s) + --cluster-local Specify that the service be private. (--no-cluster-local will make the service publicly available --cmd string Specify command to be used as entrypoint instead of default one. Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments. --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. @@ -59,6 +60,7 @@ kn service update NAME [flags] --min-scale int Minimal number of replicas. --mount stringArray Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. When a configmap or a secret is specified, a corresponding volume is automatically generated. You can use this flag multiple times. For unmounting a directory, append "-", e.g. --mount /mydir-, which also removes any auto-generated volume. -n, --namespace string Specify the namespace to operate in. + --no-cluster-local Do not specify that the service be private. (--no-cluster-local will make the service publicly available (default true) --no-lock-to-digest Do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision) --no-wait Update service and don't wait for it to be ready. -p, --port int32 The port where application listens on. diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go index 53c6836e8..799118002 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -28,6 +28,7 @@ import ( "knative.dev/client/pkg/kn/flags" servinglib "knative.dev/client/pkg/serving" "knative.dev/client/pkg/util" + "knative.dev/serving/pkg/apis/serving" servingv1 "knative.dev/serving/pkg/apis/serving/v1" ) @@ -57,6 +58,7 @@ type ConfigurationEditFlags struct { ServiceAccountName string ImagePullSecrets string Annotations []string + ClusterLocal bool User int64 // Preferences about how to do the action. @@ -134,6 +136,7 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) { "Specify command to be used as entrypoint instead of default one. "+ "Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments.") p.markFlagMakesRevision("cmd") + command.Flags().StringArrayVarP(&p.Arg, "arg", "", []string{}, "Add argument to the container command. "+ "Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. "+ @@ -142,33 +145,50 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) { command.Flags().StringVar(&p.RequestsFlags.CPU, "requests-cpu", "", "The requested CPU (e.g., 250m).") p.markFlagMakesRevision("requests-cpu") + command.Flags().StringVar(&p.RequestsFlags.Memory, "requests-memory", "", "The requested memory (e.g., 64Mi).") p.markFlagMakesRevision("requests-memory") + command.Flags().StringVar(&p.LimitsFlags.CPU, "limits-cpu", "", "The limits on the requested CPU (e.g., 1000m).") p.markFlagMakesRevision("limits-cpu") + command.Flags().StringVar(&p.LimitsFlags.Memory, "limits-memory", "", "The limits on the requested memory (e.g., 1024Mi).") p.markFlagMakesRevision("limits-memory") + command.Flags().IntVar(&p.MinScale, "min-scale", 0, "Minimal number of replicas.") p.markFlagMakesRevision("min-scale") + command.Flags().IntVar(&p.MaxScale, "max-scale", 0, "Maximal number of replicas.") p.markFlagMakesRevision("max-scale") + command.Flags().StringVar(&p.AutoscaleWindow, "autoscale-window", "", "Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)") p.markFlagMakesRevision("autoscale-window") + + flags.AddBothBoolFlagsUnhidden(command.Flags(), &p.ClusterLocal, "cluster-local", "", false, + "Specify that the service be private. (--no-cluster-local will make the service publicly available") + //TODO: Need to also not change revision when already set (solution to issue #646) + p.markFlagMakesRevision("cluster-local") + p.markFlagMakesRevision("no-cluster-local") + 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.") p.markFlagMakesRevision("concurrency-target") + command.Flags().IntVar(&p.ConcurrencyLimit, "concurrency-limit", 0, "Hard Limit of concurrent requests to be processed by a single replica.") p.markFlagMakesRevision("concurrency-limit") + command.Flags().Int32VarP(&p.Port, "port", "p", 0, "The port where application listens on.") p.markFlagMakesRevision("port") + command.Flags().StringArrayVarP(&p.Labels, "label", "l", []string{}, "Labels to set for both Service and Revision. name=value; you may provide this flag "+ "any number of times to set multiple labels. "+ "To unset, specify the label name followed by a \"-\" (e.g., name-).") p.markFlagMakesRevision("label") + command.Flags().StringArrayVarP(&p.LabelsService, "label-service", "", []string{}, "Service label to set. name=value; you may provide this flag "+ "any number of times to set multiple labels. "+ @@ -181,6 +201,7 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) { "To unset, specify the label name followed by a \"-\" (e.g., name-). This flag takes "+ "precedence over \"label\" flag.") p.markFlagMakesRevision("label-revision") + command.Flags().StringVar(&p.RevisionName, "revision-name", "{{.Service}}-{{.Random 5}}-{{.Generation}}", "The revision name to set. Must start with the service name and a dash as a prefix. "+ "Empty revision name will result in the server generating a name for the revision. "+ @@ -192,16 +213,19 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) { "Keep the running image for the service constant when not explicitly specifying "+ "the image. (--no-lock-to-digest pulls the image tag afresh with each new revision)") // Don't mark as changing the revision. + command.Flags().StringVar(&p.ServiceAccountName, "service-account", "", "Service account name to set. An empty argument (\"\") clears the service account. The referenced service account must exist in the service's namespace.") 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") + command.Flags().StringVar(&p.ImagePullSecrets, "pull-secret", "", @@ -288,6 +312,7 @@ func (p *ConfigurationEditFlags) Apply( if p.AnyMutation(cmd) { template.Name = name } + imageSet := false if cmd.Flags().Changed("image") { err = servinglib.UpdateImage(template, p.Image.String()) @@ -379,6 +404,16 @@ func (p *ConfigurationEditFlags) Apply( } } + if cmd.Flags().Changed("cluster-local") || cmd.Flags().Changed("no-cluster-local") { + if p.ClusterLocal { + labels := servinglib.UpdateLabels(service.ObjectMeta.Labels, map[string]string{serving.VisibilityLabelKey: serving.VisibilityClusterLocal}, []string{}) + service.ObjectMeta.Labels = labels // In case service.ObjectMeta.Labels was nil + } else { + labels := servinglib.UpdateLabels(service.ObjectMeta.Labels, map[string]string{}, []string{serving.VisibilityLabelKey}) + service.ObjectMeta.Labels = labels // In case service.ObjectMeta.Labels was nil + } + } + if cmd.Flags().Changed("label") || cmd.Flags().Changed("label-service") || cmd.Flags().Changed("label-revision") { labelsAllMap, err := util.MapFromArrayAllowingSingles(p.Labels, "=") if err != nil { diff --git a/pkg/kn/commands/service/create.go b/pkg/kn/commands/service/create.go index 6667cd95f..33900354d 100644 --- a/pkg/kn/commands/service/create.go +++ b/pkg/kn/commands/service/create.go @@ -56,7 +56,10 @@ var create_example = ` 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` + kn service create s1 --image dev.local/ns/image:v3 --annotation sidecar.istio.io/inject=false + + # Create a private service (that is a service with no external endpoint) + kn service create s1 --image dev.local/ns/image:v3 --cluster-local` func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command { var editFlags ConfigurationEditFlags diff --git a/pkg/kn/commands/service/create_test.go b/pkg/kn/commands/service/create_test.go index 6eb522acd..0ade9c327 100644 --- a/pkg/kn/commands/service/create_test.go +++ b/pkg/kn/commands/service/create_test.go @@ -22,7 +22,6 @@ import ( "testing" "gotest.tools/assert" - api_errors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/watch" @@ -36,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/runtime" clienttesting "k8s.io/client-go/testing" + "knative.dev/serving/pkg/apis/serving" servingv1 "knative.dev/serving/pkg/apis/serving/v1" ) @@ -379,9 +379,6 @@ func TestServiceCreateMaxMinScale(t *testing.T) { } template := &created.Spec.Template - if err != nil { - t.Fatal(err) - } actualAnnos := template.Annotations expectedAnnos := []string{ @@ -545,3 +542,24 @@ func TestServiceCreateWithServiceAccountName(t *testing.T) { t.Fatalf("wrong service account name:%v", template.Spec.ServiceAccountName) } } + +func TestServiceCreateWithClusterLocal(t *testing.T) { + action, created, _, err := fakeServiceCreate([]string{ + "service", "create", "foo", "--image", "gcr.io/foo/bar:baz", + "--cluster-local"}, false) + + if err != nil { + t.Fatal(err) + } else if !action.Matches("create", "services") { + t.Fatalf("Bad action %v", action) + } + + labels := created.ObjectMeta.Labels + + labelValue, present := labels[serving.VisibilityLabelKey] + assert.Assert(t, present) + + if labelValue != serving.VisibilityClusterLocal { + t.Fatalf("Incorrect VisibilityClusterLocal value '%s'", labelValue) + } +} diff --git a/pkg/kn/commands/service/update_test.go b/pkg/kn/commands/service/update_test.go index a04fd0e9a..bb48aad86 100644 --- a/pkg/kn/commands/service/update_test.go +++ b/pkg/kn/commands/service/update_test.go @@ -35,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" clienttesting "k8s.io/client-go/testing" + "knative.dev/serving/pkg/apis/serving" servingv1 "knative.dev/serving/pkg/apis/serving/v1" ) @@ -648,6 +649,74 @@ func TestServiceUpdateLabelExisting(t *testing.T) { assert.DeepEqual(t, expected, actual) } +func TestServiceUpdateNoClusterLocal(t *testing.T) { + original := newEmptyService() + original.ObjectMeta.Labels = map[string]string{serving.VisibilityLabelKey: serving.VisibilityClusterLocal} + originalTemplate := &original.Spec.Template + originalTemplate.ObjectMeta.Labels = map[string]string{serving.VisibilityLabelKey: serving.VisibilityClusterLocal} + + action, updated, _, err := fakeServiceUpdate(original, []string{ + "service", "update", "foo", "--no-cluster-local", "--no-wait"}) + + if err != nil { + t.Fatal(err) + } else if !action.Matches("update", "services") { + t.Fatalf("Bad action %v", action) + } + + expected := map[string]string{} + actual := updated.ObjectMeta.Labels + assert.DeepEqual(t, expected, actual) +} + +//TODO: add check for template name not changing when issue #646 solution is merged +func TestServiceUpdateNoClusterLocalOnPublicService(t *testing.T) { + original := newEmptyService() + original.ObjectMeta.Labels = map[string]string{} + + action, updated, _, err := fakeServiceUpdate(original, []string{ + "service", "update", "foo", "--no-cluster-local", "--no-wait"}) + + if err != nil { + t.Fatal(err) + } else if !action.Matches("update", "services") { + t.Fatalf("Bad action %v", action) + } + + expected := map[string]string{} + actual := updated.ObjectMeta.Labels + assert.DeepEqual(t, expected, actual) +} + +//TODO: add check for template name not changing when issue #646 solution is merged +func TestServiceUpdateNoClusterLocalOnPrivateService(t *testing.T) { + original := newEmptyService() + original.ObjectMeta.Labels = map[string]string{serving.VisibilityLabelKey: serving.VisibilityClusterLocal} + originalTemplate := &original.Spec.Template + originalTemplate.ObjectMeta.Labels = map[string]string{serving.VisibilityLabelKey: serving.VisibilityClusterLocal} + + action, updated, _, err := fakeServiceUpdate(original, []string{ + "service", "update", "foo", "--cluster-local", "--no-wait"}) + + if err != nil { + t.Fatal(err) + } else if !action.Matches("update", "services") { + t.Fatalf("Bad action %v", action) + } + + expected := map[string]string{serving.VisibilityLabelKey: serving.VisibilityClusterLocal} + actual := updated.ObjectMeta.Labels + assert.DeepEqual(t, expected, actual) + + newTemplate := updated.Spec.Template + if err != nil { + t.Fatal(err) + } + + actual = newTemplate.ObjectMeta.Labels + assert.DeepEqual(t, expected, actual) +} + func newEmptyService() *servingv1.Service { ret := &servingv1.Service{ TypeMeta: metav1.TypeMeta{ diff --git a/test/e2e/service_test.go b/test/e2e/service_test.go index ed0c8d070..8ae807876 100644 --- a/test/e2e/service_test.go +++ b/test/e2e/service_test.go @@ -23,8 +23,8 @@ import ( "testing" "gotest.tools/assert" - "knative.dev/client/pkg/util" + "knative.dev/serving/pkg/apis/serving" ) func TestService(t *testing.T) { @@ -38,9 +38,10 @@ func TestService(t *testing.T) { r := NewKnRunResultCollector(t) defer r.DumpIfFailed() - t.Log("create hello service duplicate and get service already exists error") + t.Log("create hello service, delete, and try to create duplicate and get service already exists error") test.serviceCreate(t, r, "hello") - test.serviceCreateDuplicate(t, r, "hello") + test.serviceCreatePrivate(t, r, "hello-private") + test.serviceCreateDuplicate(t, r, "hello-private") t.Log("return valid info about hello service with print flags") test.serviceDescribeWithPrintFlags(t, r, "hello") @@ -51,18 +52,49 @@ func TestService(t *testing.T) { t.Log("delete two services with a service nonexistent") test.serviceCreate(t, r, "hello") - test.serviceMultipleDelete(t, r, "hello", "bla123") + + t.Log("create service private and make public") + test.serviceCreatePrivateUpdatePublic(t, r, "hello-private-public") +} + +func (test *e2eTest) serviceCreatePrivate(t *testing.T, r *KnRunResultCollector, serviceName string) { + out := test.kn.Run("service", "create", serviceName, + "--image", KnDefaultTestImage, "--cluster-local") + r.AssertNoError(out) + assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, "service", serviceName, "creating", "namespace", test.kn.namespace, "ready")) + + out = test.kn.Run("service", "describe", serviceName, "--verbose") + r.AssertNoError(out) + assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, serving.VisibilityLabelKey, serving.VisibilityClusterLocal)) +} + +func (test *e2eTest) serviceCreatePrivateUpdatePublic(t *testing.T, r *KnRunResultCollector, serviceName string) { + out := test.kn.Run("service", "create", serviceName, + "--image", KnDefaultTestImage, "--cluster-local") + r.AssertNoError(out) + assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, "service", serviceName, "creating", "namespace", test.kn.namespace, "ready")) + + out = test.kn.Run("service", "describe", serviceName, "--verbose") + r.AssertNoError(out) + assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, serving.VisibilityLabelKey, serving.VisibilityClusterLocal)) + + out = test.kn.Run("service", "update", serviceName, + "--image", KnDefaultTestImage, "--no-cluster-local") + r.AssertNoError(out) + assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, "service", serviceName, "updated", "namespace", test.kn.namespace, "ready")) + + out = test.kn.Run("service", "describe", serviceName, "--verbose") + r.AssertNoError(out) + assert.Check(t, util.ContainsNone(out.Stdout, serving.VisibilityLabelKey, serving.VisibilityClusterLocal)) } func (test *e2eTest) serviceCreateDuplicate(t *testing.T, r *KnRunResultCollector, serviceName string) { out := test.kn.Run("service", "list", serviceName) - out.ErrorExpected = true r.AssertNoError(out) assert.Check(t, strings.Contains(out.Stdout, serviceName), "The service does not exist yet") out = test.kn.Run("service", "create", serviceName, "--image", KnDefaultTestImage) - out.ErrorExpected = true r.AssertError(out) assert.Check(t, util.ContainsAll(out.Stderr, "the service already exists")) }