From 81c1d9ce038e4649ed144e0ad8793aba2144040a Mon Sep 17 00:00:00 2001 From: Shashwathi Date: Mon, 9 Mar 2020 14:02:28 -0700 Subject: [PATCH 1/6] Add option for adding labels only to service and/or template (#703) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add option for adding labels only to service and/or template In this PR we are introducing 2 additional flags for setting label for service(label-service) and revision(label-revision) only. Signed-off-by: Andrew Su * Update docs Signed-off-by: Andrew Su * Add changelog entry Signed-off-by: Andrew Su * Refactor UpdateLabels method to be more generic. * Refactored labels common code. Signed-off-by: Shash Reddy * Pass pointer refernce instead of copy to update labels Co-authored-by: Roland Huß Co-authored-by: Andrew Su --- CHANGELOG.adoc | 4 ++ docs/cmd/kn_service_create.md | 64 ++++++++--------- docs/cmd/kn_service_update.md | 68 ++++++++++--------- .../service/configuration_edit_flags.go | 47 +++++++++++-- pkg/serving/config_changes.go | 34 +++------- pkg/serving/config_changes_test.go | 56 ++++++++------- 6 files changed, 150 insertions(+), 123 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 74c2f0b38b..0d6981c67b 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -26,6 +26,10 @@ | Replaced `kn source cron` with `kn source ping`. `--schedule` is not mandatory anymore and defaults to "* * * * *" (every minute) | https://github.com/knative/client/issues/564[#564] +| ✨ +| Add option for adding labels only to service and/or template +| https://github.com/knative/client/issues/675[#675] + | ✨ | Update to Knative serving 0.13.0 and Knative eventing 0.13.1 | https://github.com/knative/client/issues/564[#564] diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index dd8ab5be0f..78ee934691 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -42,37 +42,39 @@ kn service create NAME --image IMAGE [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-). - --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) - --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. - -e, --env stringArray Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. To unset, specify the environment variable name followed by a "-" (e.g., NAME-). - --env-from stringArray Add environment variables from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret:). Example: --env-from cm:myconfigmap or --env-from secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --env-from cm:myconfigmap-. - --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; 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-). - --limits-cpu string The limits on the requested CPU (e.g., 1000m). - --limits-memory string The limits on the requested memory (e.g., 1024Mi). - --lock-to-digest 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) (default true) - --max-scale int Maximal number of replicas. - --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-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. - --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. - --requests-cpu string The requested CPU (e.g., 250m). - --requests-memory string The requested memory (e.g., 64Mi). - --revision-name string 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. Accepts golang templates, allowing {{.Service}} for the service name, {{.Generation}} for the generation, and {{.Random [n]}} for n random consonants. (default "{{.Service}}-{{.Random 5}}-{{.Generation}}") - --service-account string Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace. - --user int The user ID to run the container (e.g., 1001). - --volume stringArray Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-. - --wait-timeout int Seconds to wait before giving up on waiting for service to be ready. (default 600) + --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-). + --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) + --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. + -e, --env stringArray Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. To unset, specify the environment variable name followed by a "-" (e.g., NAME-). + --env-from stringArray Add environment variables from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret:). Example: --env-from cm:myconfigmap or --env-from secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --env-from cm:myconfigmap-. + --force Create service forcefully, replaces existing service if any. + -h, --help help for create + --image string Image to run. + -l, --label stringArray 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-). + --label-revision stringArray Revision label to set. 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-). This flag takes precedence over "label" flag. + --label-service stringArray Service label to set. 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-). This flag takes precedence over "label" flag. + --limits-cpu string The limits on the requested CPU (e.g., 1000m). + --limits-memory string The limits on the requested memory (e.g., 1024Mi). + --lock-to-digest 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) (default true) + --max-scale int Maximal number of replicas. + --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-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. + --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. + --requests-cpu string The requested CPU (e.g., 250m). + --requests-memory string The requested memory (e.g., 64Mi). + --revision-name string 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. Accepts golang templates, allowing {{.Service}} for the service name, {{.Generation}} for the generation, and {{.Random [n]}} for n random consonants. (default "{{.Service}}-{{.Random 5}}-{{.Generation}}") + --service-account string Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace. + --user int The user ID to run the container (e.g., 1001). + --volume stringArray Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-. + --wait-timeout int Seconds to wait before giving up on waiting for service to be ready. (default 600) ``` ### Options inherited from parent commands diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md index ed7fbb0ecc..0c8f7c052e 100644 --- a/docs/cmd/kn_service_update.md +++ b/docs/cmd/kn_service_update.md @@ -38,39 +38,41 @@ 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-). - --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) - --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. - -e, --env stringArray Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. To unset, specify the environment variable name followed by a "-" (e.g., NAME-). - --env-from stringArray Add environment variables from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret:). Example: --env-from cm:myconfigmap or --env-from secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --env-from cm:myconfigmap-. - -h, --help help for update - --image string Image to run. - -l, --label stringArray Service label to set. 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-). - --limits-cpu string The limits on the requested CPU (e.g., 1000m). - --limits-memory string The limits on the requested memory (e.g., 1024Mi). - --lock-to-digest 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) (default true) - --max-scale int Maximal number of replicas. - --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-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. - --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. - --requests-cpu string The requested CPU (e.g., 250m). - --requests-memory string The requested memory (e.g., 64Mi). - --revision-name string 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. Accepts golang templates, allowing {{.Service}} for the service name, {{.Generation}} for the generation, and {{.Random [n]}} for n random consonants. (default "{{.Service}}-{{.Random 5}}-{{.Generation}}") - --service-account string Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace. - --tag strings Set tag (format: --tag revisionRef=tagName) where revisionRef can be a revision or '@latest' string representing latest ready revision. This flag can be specified multiple times. - --traffic strings Set traffic distribution (format: --traffic revisionRef=percent) where revisionRef can be a revision or a tag or '@latest' string representing latest ready revision. This flag can be given multiple times with percent summing up to 100%. - --untag strings Untag revision (format: --untag tagName). This flag can be specified multiple times. - --user int The user ID to run the container (e.g., 1001). - --volume stringArray Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-. - --wait-timeout int Seconds to wait before giving up on waiting for service to be ready. (default 600) + --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-). + --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) + --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. + -e, --env stringArray Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables. To unset, specify the environment variable name followed by a "-" (e.g., NAME-). + --env-from stringArray Add environment variables from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret:). Example: --env-from cm:myconfigmap or --env-from secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --env-from cm:myconfigmap-. + -h, --help help for update + --image string Image to run. + -l, --label stringArray 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-). + --label-revision stringArray Revision label to set. 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-). This flag takes precedence over "label" flag. + --label-service stringArray Service label to set. 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-). This flag takes precedence over "label" flag. + --limits-cpu string The limits on the requested CPU (e.g., 1000m). + --limits-memory string The limits on the requested memory (e.g., 1024Mi). + --lock-to-digest 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) (default true) + --max-scale int Maximal number of replicas. + --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-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. + --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. + --requests-cpu string The requested CPU (e.g., 250m). + --requests-memory string The requested memory (e.g., 64Mi). + --revision-name string 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. Accepts golang templates, allowing {{.Service}} for the service name, {{.Generation}} for the generation, and {{.Random [n]}} for n random consonants. (default "{{.Service}}-{{.Random 5}}-{{.Generation}}") + --service-account string Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace. + --tag strings Set tag (format: --tag revisionRef=tagName) where revisionRef can be a revision or '@latest' string representing latest ready revision. This flag can be specified multiple times. + --traffic strings Set traffic distribution (format: --traffic revisionRef=percent) where revisionRef can be a revision or a tag or '@latest' string representing latest ready revision. This flag can be given multiple times with percent summing up to 100%. + --untag strings Untag revision (format: --untag tagName). This flag can be specified multiple times. + --user int The user ID to run the container (e.g., 1001). + --volume stringArray Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-. + --wait-timeout int Seconds to wait before giving up on waiting for service to be ready. (default 600) ``` ### Options inherited from parent commands diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go index 127a342a33..53c6836e80 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -20,8 +20,11 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/client/pkg/kn/flags" servinglib "knative.dev/client/pkg/serving" "knative.dev/client/pkg/util" @@ -47,6 +50,8 @@ type ConfigurationEditFlags struct { AutoscaleWindow string Port int32 Labels []string + LabelsService []string + LabelsRevision []string NamePrefix string RevisionName string ServiceAccountName string @@ -160,10 +165,22 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) { 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{}, - "Service label to set. name=value; you may provide this flag "+ + "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. "+ + "To unset, specify the label name followed by a \"-\" (e.g., name-). This flag takes "+ + "precedence over \"label\" flag.") + p.markFlagMakesRevision("label-service") + command.Flags().StringArrayVarP(&p.LabelsRevision, "label-revision", "", []string{}, + "Revision label to set. 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-). 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. "+ @@ -362,16 +379,20 @@ func (p *ConfigurationEditFlags) Apply( } } - if cmd.Flags().Changed("label") { - labelsMap, err := util.MapFromArrayAllowingSingles(p.Labels, "=") + if cmd.Flags().Changed("label") || cmd.Flags().Changed("label-service") || cmd.Flags().Changed("label-revision") { + labelsAllMap, err := util.MapFromArrayAllowingSingles(p.Labels, "=") if err != nil { return errors.Wrap(err, "Invalid --label") } - labelsToRemove := util.ParseMinusSuffix(labelsMap) - err = servinglib.UpdateLabels(service, template, labelsMap, labelsToRemove) + err = p.updateLabels(&service.ObjectMeta, p.LabelsService, labelsAllMap) if err != nil { - return err + return errors.Wrap(err, "Invalid --label-service") + } + + err = p.updateLabels(&template.ObjectMeta, p.LabelsRevision, labelsAllMap) + if err != nil { + return errors.Wrap(err, "Invalid --label-revision") } } @@ -406,6 +427,20 @@ func (p *ConfigurationEditFlags) Apply( return nil } +func (p *ConfigurationEditFlags) updateLabels(obj *metav1.ObjectMeta, flagLabels []string, labelsAllMap map[string]string) error { + labelFlagMap, err := util.MapFromArrayAllowingSingles(flagLabels, "=") + if err != nil { + return errors.Wrap(err, "Unable to parse label flags") + } + labelsMap := make(util.StringMap) + labelsMap.Merge(labelsAllMap) + labelsMap.Merge(labelFlagMap) + revisionLabelsToRemove := util.ParseMinusSuffix(labelsMap) + obj.Labels = servinglib.UpdateLabels(obj.Labels, labelsMap, revisionLabelsToRemove) + + return nil +} + func (p *ConfigurationEditFlags) computeResources(resourceFlags ResourceFlags) (corev1.ResourceList, error) { resourceList := corev1.ResourceList{} diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go index 7afad5c215..3cec6e7531 100644 --- a/pkg/serving/config_changes.go +++ b/pkg/serving/config_changes.go @@ -374,34 +374,20 @@ func UpdateResources(template *servingv1.RevisionTemplateSpec, requestsResourceL 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. -// 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 { - if service.ObjectMeta.Labels == nil { - service.ObjectMeta.Labels = make(map[string]string) +// UpdateLabels updates the labels by adding items from `add` then removing any items from `remove` +func UpdateLabels(labelsMap map[string]string, add map[string]string, remove []string) map[string]string { + if labelsMap == nil { + labelsMap = map[string]string{} } - if template.ObjectMeta.Labels == nil { - template.ObjectMeta.Labels = make(map[string]string) - } - for key, value := range toUpdate { - service.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, value := range add { + labelsMap[key] = value } - for _, key := range toRemove { - delete(service.ObjectMeta.Labels, key) - delete(template.ObjectMeta.Labels, key) + for _, key := range remove { + delete(labelsMap, key) } - return nil + + return labelsMap } // UpdateAnnotations updates the annotations identically on a service and template. diff --git a/pkg/serving/config_changes_test.go b/pkg/serving/config_changes_test.go index f17fd170f9..75879a58a5 100644 --- a/pkg/serving/config_changes_test.go +++ b/pkg/serving/config_changes_test.go @@ -335,27 +335,9 @@ func TestUpdateLabelsNew(t *testing.T) { "a": "foo", "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{}) - assert.NilError(t, err) + service.ObjectMeta.Labels = UpdateLabels(service.ObjectMeta.Labels, labels, []string{}) + template.ObjectMeta.Labels = UpdateLabels(template.ObjectMeta.Labels, labels, []string{}) actual := service.ObjectMeta.Labels if !reflect.DeepEqual(labels, actual) { @@ -363,8 +345,8 @@ func TestUpdateLabelsNew(t *testing.T) { } actual = template.ObjectMeta.Labels - if !reflect.DeepEqual(tLabels, actual) { - t.Fatalf("Template labels did not match expected %v found %v", tLabels, actual) + if !reflect.DeepEqual(labels, actual) { + t.Fatalf("Template labels did not match expected %v found %v", labels, actual) } } @@ -378,20 +360,35 @@ func TestUpdateLabelsExisting(t *testing.T) { "c": "bat", "d": "", } - err := UpdateLabels(service, template, labels, []string{}) - assert.NilError(t, err) - expected := map[string]string{ + tlabels := map[string]string{ + "a": "notfoo", + "c": "bat", + "d": "", + "r": "poo", + } + + service.ObjectMeta.Labels = UpdateLabels(service.ObjectMeta.Labels, labels, []string{}) + template.ObjectMeta.Labels = UpdateLabels(template.ObjectMeta.Labels, tlabels, []string{}) + + expectedServiceLabel := map[string]string{ "a": "notfoo", "b": "bar", "c": "bat", "d": "", } + expectedRevLabel := map[string]string{ + "a": "notfoo", + "b": "bar", + "c": "bat", + "d": "", + "r": "poo", + } actual := service.ObjectMeta.Labels - assert.DeepEqual(t, expected, actual) + assert.DeepEqual(t, expectedServiceLabel, actual) actual = template.ObjectMeta.Labels - assert.DeepEqual(t, expected, actual) + assert.DeepEqual(t, expectedRevLabel, actual) } func TestUpdateLabelsRemoveExisting(t *testing.T) { @@ -400,8 +397,9 @@ func TestUpdateLabelsRemoveExisting(t *testing.T) { template.ObjectMeta.Labels = map[string]string{"a": "foo", "b": "bar"} remove := []string{"b"} - err := UpdateLabels(service, template, map[string]string{}, remove) - assert.NilError(t, err) + service.ObjectMeta.Labels = UpdateLabels(service.ObjectMeta.Labels, map[string]string{}, remove) + template.ObjectMeta.Labels = UpdateLabels(template.ObjectMeta.Labels, map[string]string{}, remove) + expected := map[string]string{ "a": "foo", } From 29087e64ff5365d9c441e9a77abb74b8f110c6ae Mon Sep 17 00:00:00 2001 From: Ying Chun Guo Date: Tue, 10 Mar 2020 11:50:28 +0800 Subject: [PATCH 2/6] chore: fix a typo in pull request template (#720) --- .github/pull-request-template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/pull-request-template.md b/.github/pull-request-template.md index f45ccff0d9..3df8317096 100644 --- a/.github/pull-request-template.md +++ b/.github/pull-request-template.md @@ -1,5 +1,5 @@ -## Desciption +## Description From ba7e14c807a402a9ead5f66f76b61756ed1b7299 Mon Sep 17 00:00:00 2001 From: Navid Shaikh Date: Tue, 10 Mar 2020 13:11:28 +0530 Subject: [PATCH 3/6] feat(source): Add 'kn source list' (#666) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(source): Add 'kn source list' Fixes #480 - Add 'kn source list' listing the available sources COs - Use dynamic client to - find out available source types - find the COs in given namespace for each source type - Add --type flag to filter source list based on source type - Add unit tests - Add e2e tests - Add CHANGELOG entry * Update group for ApiServerSource in tests * Add kn duck source type for holding common eventing source fileds - Common infromation from sources are extracted into a struct for source list command output - Printer function picks the information from this struct object and doesn't know the details about extracting this info from multiple types of sources received - Group and Version for list is set to empty string and Kind to 'List', if printed a source list with multiple types of sources in machine readable format - Group, Version and Kind for list are kept intact if printed a source list with same type of source objects in machine readable format * Update go.sum * Add WithType builder for listing source - Separate the flags definition - WithType builder can be re-used to filter the source types - Add unit tests * Update ApiServer source group to apiserversources.sources.knative.dev * Fix typos and align Co-authored-by: Roland Huß --- CHANGELOG.adoc | 4 + docs/cmd/kn_source.md | 1 + docs/cmd/kn_source_list.md | 51 +++++ go.mod | 1 - pkg/dynamic/client.go | 59 ++++++ pkg/dynamic/client_test.go | 134 ++++++++++--- pkg/dynamic/lib.go | 125 +++++++++++++ pkg/dynamic/lib_test.go | 79 ++++++++ pkg/kn/commands/flags/listfilters.go | 32 ++++ pkg/kn/commands/flags/listfilters_test.go | 29 +++ .../commands/source/duck/multisourcelist.go | 177 ++++++++++++++++++ .../commands/source/human_readable_flags.go | 68 ++++++- pkg/kn/commands/source/list.go | 91 +++++++++ pkg/kn/commands/source/list_test.go | 145 ++++++++++++++ pkg/kn/commands/source/list_types.go | 3 +- pkg/kn/commands/source/list_types_test.go | 83 -------- pkg/kn/commands/source/source.go | 3 +- pkg/util/compare.go | 10 + pkg/util/compare_test.go | 10 + test/e2e/source_apiserver_test.go | 11 ++ ...list_types_test.go => source_list_test.go} | 27 +++ .../magiconair/properties/assert/assert.go | 90 --------- vendor/modules.txt | 1 - 23 files changed, 1020 insertions(+), 214 deletions(-) create mode 100644 docs/cmd/kn_source_list.md create mode 100644 pkg/dynamic/lib.go create mode 100644 pkg/dynamic/lib_test.go create mode 100644 pkg/kn/commands/flags/listfilters.go create mode 100644 pkg/kn/commands/flags/listfilters_test.go create mode 100644 pkg/kn/commands/source/duck/multisourcelist.go create mode 100644 pkg/kn/commands/source/list.go create mode 100644 pkg/kn/commands/source/list_test.go delete mode 100644 pkg/kn/commands/source/list_types_test.go rename test/e2e/{source_list_types_test.go => source_list_test.go} (66%) delete mode 100644 vendor/github.com/magiconair/properties/assert/assert.go diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 0d6981c67b..bd470c1f15 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -18,6 +18,10 @@ |=== | | Description | PR +| 🎁 +| Add `kn source list` +| https://github.com/knative/client/pull/666[#666] + | 🎁 | Add JSON/YAML output format for version command | https://github.com/knative/client/pull/709[#709] diff --git a/docs/cmd/kn_source.md b/docs/cmd/kn_source.md index cfb73fb8ca..5872fe46f3 100644 --- a/docs/cmd/kn_source.md +++ b/docs/cmd/kn_source.md @@ -29,6 +29,7 @@ kn source [flags] * [kn](kn.md) - Knative client * [kn source apiserver](kn_source_apiserver.md) - Kubernetes API Server Event Source command group * [kn source binding](kn_source_binding.md) - Sink binding command group +* [kn source list](kn_source_list.md) - List available sources * [kn source list-types](kn_source_list-types.md) - List available source types * [kn source ping](kn_source_ping.md) - Ping source command group diff --git a/docs/cmd/kn_source_list.md b/docs/cmd/kn_source_list.md new file mode 100644 index 0000000000..b9bfd5572b --- /dev/null +++ b/docs/cmd/kn_source_list.md @@ -0,0 +1,51 @@ +## kn source list + +List available sources + +### Synopsis + +List available sources + +``` +kn source list [flags] +``` + +### Examples + +``` + + # List available eventing sources + kn source list + + # List PingSource type sources + kn source list --type=PingSource + + # List PingSource and ApiServerSource types sources + kn source list --type=PingSource --type=apiserversource +``` + +### Options + +``` + -A, --all-namespaces If present, list the requested object(s) across all namespaces. Namespace in current context is ignored even if specified with --namespace. + --allow-missing-template-keys If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true) + -h, --help help for list + -n, --namespace string Specify the namespace to operate in. + --no-headers When using the default output format, don't print headers (default: print headers). + -o, --output string Output format. One of: json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-file. + --template string Template string or path to template file to use when -o=go-template, -o=go-template-file. The template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview]. + -t, --type strings Filter list on given source type. This flag can be given multiple times. +``` + +### Options inherited from parent commands + +``` + --config string kn config file (default is ~/.config/kn/config.yaml) + --kubeconfig string kubectl config file (default is ~/.kube/config) + --log-http log http traffic +``` + +### SEE ALSO + +* [kn source](kn_source.md) - Event source command group + diff --git a/go.mod b/go.mod index b165d83f66..7941a6655c 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,6 @@ require ( contrib.go.opencensus.io/exporter/prometheus v0.1.0 // indirect contrib.go.opencensus.io/exporter/stackdriver v0.13.0 // indirect github.com/google/go-containerregistry v0.0.0-20200304201134-fcc8ea80e26f // indirect - github.com/magiconair/properties v1.8.0 github.com/mitchellh/go-homedir v1.1.0 github.com/openzipkin/zipkin-go v0.2.2 // indirect github.com/pkg/errors v0.8.1 diff --git a/pkg/dynamic/client.go b/pkg/dynamic/client.go index 35d5ca52bc..f5ca6f4126 100644 --- a/pkg/dynamic/client.go +++ b/pkg/dynamic/client.go @@ -20,6 +20,8 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/dynamic" + + "knative.dev/client/pkg/util" ) const ( @@ -43,6 +45,9 @@ type KnDynamicClient interface { // ListSourceCRDs returns list of eventing sources CRDs ListSourcesTypes() (*unstructured.UnstructuredList, error) + // ListSources returns list of available source objects + ListSources(types ...WithType) (*unstructured.UnstructuredList, error) + // RawClient returns the raw dynamic client interface RawClient() dynamic.Interface } @@ -94,3 +99,57 @@ func (c *knDynamicClient) ListSourcesTypes() (*unstructured.UnstructuredList, er func (c knDynamicClient) RawClient() dynamic.Interface { return c.client } + +// ListSources returns list of available sources objects +// Provide the list of source types as for example: WithTypes("pingsource", "apiserversource"...) to list +// only given types of source objects +func (c *knDynamicClient) ListSources(types ...WithType) (*unstructured.UnstructuredList, error) { + var ( + sourceList unstructured.UnstructuredList + options metav1.ListOptions + numberOfsourceTypesFound int + ) + sourceTypes, err := c.ListSourcesTypes() + if err != nil { + return nil, err + } + namespace := c.Namespace() + filters := WithTypes(types).List() + // For each source type available, find out each source types objects + for _, source := range sourceTypes.Items { + // find source kind before hand to fail early + sourceKind, err := kindFromUnstructured(&source) + if err != nil { + return nil, err + } + + if len(filters) > 0 && !util.SliceContainsIgnoreCase(filters, sourceKind) { + continue + } + + // find source's GVR from unstructured source type object + gvr, err := gvrFromUnstructured(&source) + if err != nil { + return nil, err + } + + // list objects of source type with this GVR + sList, err := c.client.Resource(gvr).Namespace(namespace).List(options) + if err != nil { + return nil, err + } + + if len(sList.Items) > 0 { + // keep a track if we found source objects of different types + numberOfsourceTypesFound++ + sourceList.Items = append(sourceList.Items, sList.Items...) + sourceList.SetGroupVersionKind(sList.GetObjectKind().GroupVersionKind()) + } + } + // Clear the Group and Version for list if there are multiple types of source objects found + // Keep the source's GVK if there is only one type of source objects found or requested via --type filter + if numberOfsourceTypesFound > 1 { + sourceList.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "", Kind: "List"}) + } + return &sourceList, nil +} diff --git a/pkg/dynamic/client_test.go b/pkg/dynamic/client_test.go index 077508f954..55f7b1ece1 100644 --- a/pkg/dynamic/client_test.go +++ b/pkg/dynamic/client_test.go @@ -15,9 +15,10 @@ package dynamic import ( + "strings" "testing" - "github.com/magiconair/properties/assert" + "gotest.tools/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" @@ -26,45 +27,29 @@ import ( dynamicfake "k8s.io/client-go/dynamic/fake" eventingv1alpha1 "knative.dev/eventing/pkg/apis/eventing/v1alpha1" servingv1 "knative.dev/serving/pkg/apis/serving/v1" -) -const testNamespace = "testns" + "knative.dev/client/pkg/util" +) -func newUnstructured(name string) *unstructured.Unstructured { - return &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": crdGroup + "/" + crdVersion, - "kind": crdKind, - "metadata": map[string]interface{}{ - "namespace": testNamespace, - "name": name, - "labels": map[string]interface{}{ - sourcesLabelKey: sourcesLabelValue, - }, - }, - }, - } -} +const testNamespace = "current" func TestNamespace(t *testing.T) { - client := createFakeKnDynamicClient(testNamespace, newUnstructured("foo")) + client := createFakeKnDynamicClient(testNamespace, newSourceCRDObj("foo")) assert.Equal(t, client.Namespace(), testNamespace) } func TestListCRDs(t *testing.T) { client := createFakeKnDynamicClient( testNamespace, - newUnstructured("foo"), - newUnstructured("bar"), + newSourceCRDObj("foo"), + newSourceCRDObj("bar"), ) + assert.Check(t, client.RawClient() != nil) t.Run("List CRDs with match", func(t *testing.T) { options := metav1.ListOptions{} uList, err := client.ListCRDs(options) - if err != nil { - t.Fatal(err) - } - + assert.NilError(t, err) assert.Equal(t, len(uList.Items), 2) }) @@ -84,8 +69,8 @@ func TestListCRDs(t *testing.T) { func TestListSourceTypes(t *testing.T) { client := createFakeKnDynamicClient( testNamespace, - newUnstructured("foo"), - newUnstructured("bar"), + newSourceCRDObj("foo"), + newSourceCRDObj("bar"), ) t.Run("List source types", func(t *testing.T) { @@ -100,12 +85,107 @@ func TestListSourceTypes(t *testing.T) { }) } +func TestListSources(t *testing.T) { + t.Run("No GVRs set", func(t *testing.T) { + obj := newSourceCRDObj("foo") + client := createFakeKnDynamicClient(testNamespace, obj) + assert.Check(t, client.RawClient() != nil) + _, err := client.ListSources() + assert.Check(t, err != nil) + assert.Check(t, util.ContainsAll(err.Error(), "can't", "find", "source", "kind", "CRD")) + }) + + t.Run("source list empty", func(t *testing.T) { + client := createFakeKnDynamicClient(testNamespace, + newSourceCRDObjWithSpec("pingsources", "sources.knative.dev", "v1alpha1", "PingSource"), + ) + sources, err := client.ListSources() + assert.NilError(t, err) + assert.Equal(t, len(sources.Items), 0) + }) + + t.Run("source list non empty", func(t *testing.T) { + client := createFakeKnDynamicClient(testNamespace, + newSourceCRDObjWithSpec("pingsources", "sources.knative.dev", "v1alpha1", "PingSource"), + newSourceCRDObjWithSpec("apiserversources", "sources.knative.dev", "v1alpha1", "ApiServerSource"), + newSourceCRDObjWithSpec("cronjobsources", "sources.knative.dev", "v1alpha1", "CronJobSource"), + newSourceUnstructuredObj("p1", "sources.knative.dev/v1alpha1", "PingSource"), + newSourceUnstructuredObj("a1", "sources.knative.dev/v1alpha1", "ApiServerSource"), + newSourceUnstructuredObj("c1", "sources.knative.dev/v1alpha1", "CronJobSource"), + ) + sources, err := client.ListSources(WithTypeFilter("pingsource"), WithTypeFilter("ApiServerSource")) + assert.NilError(t, err) + assert.Equal(t, len(sources.Items), 2) + }) +} + // createFakeKnDynamicClient gives you a dynamic client for testing containing the given objects. // See also the one in the fake package. Duplicated here to avoid a dependency loop. func createFakeKnDynamicClient(testNamespace string, objects ...runtime.Object) KnDynamicClient { scheme := runtime.NewScheme() scheme.AddKnownTypeWithName(schema.GroupVersionKind{Group: "serving.knative.dev", Version: "v1alpha1", Kind: "Service"}, &servingv1.Service{}) scheme.AddKnownTypeWithName(schema.GroupVersionKind{Group: "eventing.knative.dev", Version: "v1alpha1", Kind: "Broker"}, &eventingv1alpha1.Broker{}) + client := dynamicfake.NewSimpleDynamicClient(scheme, objects...) return NewKnDynamicClient(client, testNamespace) } + +func newSourceCRDObj(name string) *unstructured.Unstructured { + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": crdGroup + "/" + crdVersion, + "kind": crdKind, + "metadata": map[string]interface{}{ + "namespace": testNamespace, + "name": name, + }, + }, + } + obj.SetLabels(labels.Set{sourcesLabelKey: sourcesLabelValue}) + return obj +} + +func newSourceCRDObjWithSpec(name, group, version, kind string) *unstructured.Unstructured { + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": crdGroup + "/" + crdVersion, + "kind": crdKind, + "metadata": map[string]interface{}{ + "namespace": testNamespace, + "name": name, + }, + }, + } + + obj.Object["spec"] = map[string]interface{}{ + "group": group, + "version": version, + "names": map[string]interface{}{ + "kind": kind, + "plural": strings.ToLower(kind) + "s", + }, + } + obj.SetLabels(labels.Set{sourcesLabelKey: sourcesLabelValue}) + return obj +} + +func newSourceUnstructuredObj(name, apiVersion, kind string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": apiVersion, + "kind": kind, + "metadata": map[string]interface{}{ + "namespace": "current", + "name": name, + }, + "spec": map[string]interface{}{ + "sink": map[string]interface{}{ + "ref": map[string]interface{}{ + "kind": "Service", + "name": "foo", + }, + }, + }, + }, + } +} diff --git a/pkg/dynamic/lib.go b/pkg/dynamic/lib.go new file mode 100644 index 0000000000..06b677567d --- /dev/null +++ b/pkg/dynamic/lib.go @@ -0,0 +1,125 @@ +// 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 dynamic + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// gvrFromUnstructured takes a unstructured object of CRD type and finds GVR from its spec +func gvrFromUnstructured(u *unstructured.Unstructured) (gvr schema.GroupVersionResource, err error) { + group, err := groupFromUnstructured(u) + if err != nil { + return gvr, err + } + + version, err := versionFromUnstructured(u) + if err != nil { + return gvr, err + } + + resource, err := resourceFromUnstructured(u) + if err != nil { + return gvr, err + } + + return schema.GroupVersionResource{ + Group: group, + Version: version, + Resource: resource, + }, nil +} + +func groupFromUnstructured(u *unstructured.Unstructured) (string, error) { + content := u.UnstructuredContent() + group, found, err := unstructured.NestedString(content, "spec", "group") + if err != nil || !found { + return "", fmt.Errorf("can't find group for source GVR: %v", err) + } + return group, nil +} + +func versionFromUnstructured(u *unstructured.Unstructured) (version string, err error) { + content := u.UnstructuredContent() + versions, found, err := unstructured.NestedSlice(content, "spec", "versions") + if err != nil || !found || len(versions) == 0 { + // fallback to .spec.version + version, found, err = unstructured.NestedString(content, "spec", "version") + if err != nil || !found { + return version, fmt.Errorf("can't find version for source GVR: %v", err) + } + } else { + for _, v := range versions { + if vmap, ok := v.(map[string]interface{}); ok { + // find the version name which is being served + if vmap["served"] == true { + version = vmap["name"].(string) + break + } + } + } + } + // if we could find the version at all + if version == "" { + err = fmt.Errorf("can't find version for source GVR") + } + return version, err +} + +func resourceFromUnstructured(u *unstructured.Unstructured) (string, error) { + content := u.UnstructuredContent() + resource, found, err := unstructured.NestedString(content, "spec", "names", "plural") + if err != nil || !found { + return "", fmt.Errorf("can't find resource for source GVR: %v", err) + } + return resource, nil +} + +func kindFromUnstructured(u *unstructured.Unstructured) (string, error) { + content := u.UnstructuredContent() + kind, found, err := unstructured.NestedString(content, "spec", "names", "kind") + if !found || err != nil { + return "", fmt.Errorf("can't find source kind from source CRD: %v", err) + } + return kind, nil +} + +// TypesFilter for keeping list of sources types to filter upo +type TypesFilter []string + +// WithType function for easy filtering on source types +type WithType func(filters *TypesFilter) + +// WithTypes for recording the source type filtering function WithType +type WithTypes []WithType + +// WithTypeFilter can be used to filter based on source type name +func WithTypeFilter(name string) WithType { + return func(filters *TypesFilter) { + *filters = append(*filters, name) + } +} + +// List returns the source type name list recorded via WithTypeFilter +func (types WithTypes) List() []string { + var stypes TypesFilter + for _, f := range types { + f(&stypes) + } + return stypes +} diff --git a/pkg/dynamic/lib_test.go b/pkg/dynamic/lib_test.go new file mode 100644 index 0000000000..7ed81c1d37 --- /dev/null +++ b/pkg/dynamic/lib_test.go @@ -0,0 +1,79 @@ +// Copyright © 2020 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 dynamic + +import ( + "testing" + + "gotest.tools/assert" + + "knative.dev/client/pkg/util" +) + +func TestKindFromUnstructured(t *testing.T) { + kind, err := kindFromUnstructured( + newSourceCRDObjWithSpec("pingsources", "sources.knative.dev", "v1alpha1", "PingSource"), + ) + assert.NilError(t, err) + assert.Equal(t, kind, "PingSource") + _, err = kindFromUnstructured(newSourceCRDObj("foo")) + assert.Check(t, err != nil) +} + +func TestGVRFromUnstructured(t *testing.T) { + obj := newSourceCRDObj("foo") + + obj.Object["spec"] = map[string]interface{}{} + _, err := gvrFromUnstructured(obj) + assert.Check(t, err != nil) + assert.Check(t, util.ContainsAll(err.Error(), "can't", "find", "group")) + + obj.Object["spec"] = map[string]interface{}{ + "group": "sources.knative.dev", + } + _, err = gvrFromUnstructured(obj) + assert.Check(t, err != nil) + assert.Check(t, util.ContainsAll(err.Error(), "can't", "find", "version")) + + // with deprecated CRD field spec version + obj.Object["spec"] = map[string]interface{}{ + "group": "sources.knative.dev", + "version": "v1alpha1", + } + _, err = gvrFromUnstructured(obj) + assert.Check(t, err != nil) + assert.Check(t, util.ContainsAll(err.Error(), "can't", "find", "resource")) + + // with CRD field spec versions + obj.Object["spec"] = map[string]interface{}{ + "group": "sources.knative.dev", + "versions": []interface{}{ + map[string]interface{}{"name": "v1alpha1", "served": true}, + }, + } + _, err = gvrFromUnstructured(obj) + assert.Check(t, err != nil) + assert.Check(t, util.ContainsAll(err.Error(), "can't", "find", "resource")) + + obj.Object["spec"] = map[string]interface{}{ + "group": "sources.knative.dev", + "versions": []interface{}{ + map[string]interface{}{}, + }, + } + _, err = gvrFromUnstructured(obj) + assert.Check(t, err != nil) + assert.Check(t, util.ContainsAll(err.Error(), "can't", "find", "version")) +} diff --git a/pkg/kn/commands/flags/listfilters.go b/pkg/kn/commands/flags/listfilters.go new file mode 100644 index 0000000000..f2a00b3e0b --- /dev/null +++ b/pkg/kn/commands/flags/listfilters.go @@ -0,0 +1,32 @@ +// Copyright © 2020 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 flags + +import ( + "fmt" + + "github.com/spf13/cobra" +) + +// SourceTypeFilters defines flags used for kn source list to filter sources on types +type SourceTypeFilters struct { + Filters []string +} + +// Add attaches the SourceTypeFilters flags to given command +func (s *SourceTypeFilters) Add(cmd *cobra.Command, what string) { + usage := fmt.Sprintf("Filter list on given %s. This flag can be given multiple times.", what) + cmd.Flags().StringSliceVarP(&s.Filters, "type", "t", nil, usage) +} diff --git a/pkg/kn/commands/flags/listfilters_test.go b/pkg/kn/commands/flags/listfilters_test.go new file mode 100644 index 0000000000..a51e828f54 --- /dev/null +++ b/pkg/kn/commands/flags/listfilters_test.go @@ -0,0 +1,29 @@ +// Copyright © 2020 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 flags + +import ( + "testing" + + "github.com/spf13/cobra" + "gotest.tools/assert" +) + +func TestSourceListFlags(t *testing.T) { + filters := &SourceTypeFilters{} + cmd := &cobra.Command{} + filters.Add(cmd, "foo") + assert.Check(t, cmd.Flag("type") != nil) +} diff --git a/pkg/kn/commands/source/duck/multisourcelist.go b/pkg/kn/commands/source/duck/multisourcelist.go new file mode 100644 index 0000000000..dd02f908a9 --- /dev/null +++ b/pkg/kn/commands/source/duck/multisourcelist.go @@ -0,0 +1,177 @@ +// Copyright © 2020 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 duck + +import ( + "fmt" + "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + eventinglegacy "knative.dev/eventing/pkg/apis/legacysources/v1alpha1" + sourcesv1alpha1 "knative.dev/eventing/pkg/apis/sources/v1alpha1" + pkgduck "knative.dev/pkg/apis/duck" + pkgduckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" + + "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/kn/commands/flags" +) + +// Source struct holds common properties between different eventing sources +// which we want to print for commands like 'kn source list'. +// The properties held in this struct is meant for simple access by human readable +// printer function +type Source struct { + metav1.TypeMeta + // Name of the created source object + Name string + // Namespace of object, used for printing with namespace for example: 'kn source list -A' + Namespace string + // Kind of the source object created + SourceKind string + // Resource this source object represent + Resource string + // Sink configured for this source object + Sink string + // String representation if source is ready + Ready string +} + +// SourceList for holding list of Source type objects +type SourceList struct { + metav1.TypeMeta + Items []Source +} + +const DSListKind = "List" + +// GetNamespace returns the namespace of the Source, used for printing +// sources with namespace for commands like 'kn source list -A' +func (s *Source) GetNamespace() string { return s.Namespace } + +// DeepCopyObject noop method to satisfy Object interface +func (s *Source) DeepCopyObject() runtime.Object { return s } + +// DeepCopyObject noop method to satisfy Object interface +func (s *SourceList) DeepCopyObject() runtime.Object { return s } + +// toSource transforms eventing source object received as Unstructured object +// into Source object +func toSource(u *unstructured.Unstructured) Source { + ds := Source{} + ds.Name = u.GetName() + ds.Namespace = u.GetNamespace() + ds.SourceKind = u.GetKind() + ds.Resource = getSourceTypeName(u) + ds.Sink = findSink(u) + ds.Ready = isReady(u) + // set empty GVK + ds.APIVersion, ds.Kind = schema.GroupVersionKind{}.ToAPIVersionAndKind() + return ds +} + +// ToSourceList transforms list of eventing sources objects received as +// UnstructuredList object into SourceList object +func ToSourceList(uList *unstructured.UnstructuredList) *SourceList { + dsl := SourceList{Items: []Source{}} + //dsl.Items = make(Source, 0, len(uList.Items)) + for _, u := range uList.Items { + dsl.Items = append(dsl.Items, toSource(&u)) + } + // set empty group, version and non empty kind + dsl.APIVersion, dsl.Kind = schema.GroupVersion{}.WithKind(DSListKind).ToAPIVersionAndKind() + return &dsl +} + +func getSourceTypeName(source *unstructured.Unstructured) string { + return fmt.Sprintf("%s%s.%s", + strings.ToLower(source.GetKind()), + "s", + strings.Split(source.GetAPIVersion(), "/")[0], + ) +} + +func findSink(source *unstructured.Unstructured) string { + switch source.GetKind() { + case "ApiServerSource": + var apiSource eventinglegacy.ApiServerSource + if err := pkgduck.FromUnstructured(source, &apiSource); err == nil { + return sinkToString(apiSource.Spec.Sink) + } + case "CronJobSource": + var cronSource eventinglegacy.CronJobSource + if err := pkgduck.FromUnstructured(source, &cronSource); err == nil { + return sinkToString(cronSource.Spec.Sink) + } + case "SinkBinding": + var binding sourcesv1alpha1.SinkBinding + if err := pkgduck.FromUnstructured(source, &binding); err == nil { + return flags.SinkToString(binding.Spec.Sink) + } + case "PingSource": + var pingSource sourcesv1alpha1.PingSource + if err := pkgduck.FromUnstructured(source, &pingSource); err == nil { + return flags.SinkToString(*pingSource.Spec.Sink) + } + } + // TODO: Find out how to find sink in untyped sources + return "" +} + +func isReady(source *unstructured.Unstructured) string { + switch source.GetKind() { + case "ApiServerSource": + var tSource eventinglegacy.ApiServerSource + if err := pkgduck.FromUnstructured(source, &tSource); err == nil { + return commands.ReadyCondition(tSource.Status.Conditions) + } + case "CronJobSource": + var tSource eventinglegacy.CronJobSource + if err := pkgduck.FromUnstructured(source, &tSource); err == nil { + return commands.ReadyCondition(tSource.Status.Conditions) + } + case "SinkBinding": + var tSource eventinglegacy.SinkBinding + if err := pkgduck.FromUnstructured(source, &tSource); err == nil { + return commands.ReadyCondition(tSource.Status.Conditions) + } + case "PingSource": + var tSource sourcesv1alpha1.PingSource + if err := pkgduck.FromUnstructured(source, &tSource); err == nil { + return commands.ReadyCondition(tSource.Status.Conditions) + } + } + // TODO: Find out how to find ready conditions for untyped sources + return "" +} + +// temporary sinkToString for deprecated sources +func sinkToString(sink *pkgduckv1beta1.Destination) string { + if sink != nil { + if sink.Ref != nil { + if sink.Ref.Kind == "Service" { + return fmt.Sprintf("svc:%s", sink.Ref.Name) + } + return fmt.Sprintf("%s:%s", strings.ToLower(sink.Ref.Kind), sink.Ref.Name) + } + + if sink.URI != nil { + return sink.URI.String() + } + } + return "" +} diff --git a/pkg/kn/commands/source/human_readable_flags.go b/pkg/kn/commands/source/human_readable_flags.go index 539704f493..ba85125a19 100644 --- a/pkg/kn/commands/source/human_readable_flags.go +++ b/pkg/kn/commands/source/human_readable_flags.go @@ -22,7 +22,8 @@ import ( metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/runtime" - hprinters "knative.dev/client/pkg/printers" + clientduck "knative.dev/client/pkg/kn/commands/source/duck" + "knative.dev/client/pkg/printers" ) var sourceTypeDescription = map[string]string{ @@ -31,12 +32,8 @@ var sourceTypeDescription = map[string]string{ "PingSource": "Send periodically ping events to a sink", } -func getSourceTypeDescription(kind string) string { - return sourceTypeDescription[kind] -} - -// ListTypesHandlers handles printing human readable table for `kn source list-types` command's output -func ListTypesHandlers(h hprinters.PrintHandler) { +// ListTypesHandlers handles printing human readable table for `kn source list-types` +func ListTypesHandlers(h printers.PrintHandler) { sourceTypesColumnDefinitions := []metav1beta1.TableColumnDefinition{ {Name: "Type", Type: "string", Description: "Kind / Type of the source type", Priority: 1}, {Name: "Name", Type: "string", Description: "Name of the source type", Priority: 1}, @@ -46,8 +43,21 @@ func ListTypesHandlers(h hprinters.PrintHandler) { h.TableHandler(sourceTypesColumnDefinitions, printSourceTypesList) } +// ListHandlers handles printing human readable table for `kn source list` +func ListHandlers(h printers.PrintHandler) { + sourceListColumnDefinitions := []metav1beta1.TableColumnDefinition{ + {Name: "Name", Type: "string", Description: "Name of the created source", Priority: 1}, + {Name: "Type", Type: "string", Description: "Type of the source", Priority: 1}, + {Name: "Resource", Type: "string", Description: "Source type name", Priority: 1}, + {Name: "Sink", Type: "string", Description: "Sink of the source", Priority: 1}, + {Name: "Ready", Type: "string", Description: "Ready condition status", Priority: 1}, + } + h.TableHandler(sourceListColumnDefinitions, printSource) + h.TableHandler(sourceListColumnDefinitions, printSourceList) +} + // printSourceTypes populates a single row of source types list table -func printSourceTypes(sourceType unstructured.Unstructured, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) { +func printSourceTypes(sourceType unstructured.Unstructured, options printers.PrintOptions) ([]metav1beta1.TableRow, error) { name := sourceType.GetName() content := sourceType.UnstructuredContent() kind, found, err := unstructured.NestedString(content, "spec", "names", "kind") @@ -62,12 +72,12 @@ func printSourceTypes(sourceType unstructured.Unstructured, options hprinters.Pr row := metav1beta1.TableRow{ Object: runtime.RawExtension{Object: &sourceType}, } - row.Cells = append(row.Cells, kind, name, getSourceTypeDescription(kind)) + row.Cells = append(row.Cells, kind, name, sourceTypeDescription[kind]) return []metav1beta1.TableRow{row}, nil } // printSourceTypesList populates the source types list table rows -func printSourceTypesList(sourceTypesList *unstructured.UnstructuredList, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) { +func printSourceTypesList(sourceTypesList *unstructured.UnstructuredList, options printers.PrintOptions) ([]metav1beta1.TableRow, error) { rows := make([]metav1beta1.TableRow, 0, len(sourceTypesList.Items)) sort.SliceStable(sourceTypesList.Items, func(i, j int) bool { @@ -83,3 +93,41 @@ func printSourceTypesList(sourceTypesList *unstructured.UnstructuredList, option } return rows, nil } + +// printSource populates a single row of source list table +func printSource(source *clientduck.Source, options printers.PrintOptions) ([]metav1beta1.TableRow, error) { + row := metav1beta1.TableRow{ + Object: runtime.RawExtension{Object: source}, + } + + if options.AllNamespaces { + row.Cells = append(row.Cells, source.GetNamespace()) + } + + row.Cells = append(row.Cells, + source.Name, + source.SourceKind, + source.Resource, + source.Sink, + source.Ready, + ) + return []metav1beta1.TableRow{row}, nil +} + +// printSourceList populates the source list table rows +func printSourceList(sourceList *clientduck.SourceList, options printers.PrintOptions) ([]metav1beta1.TableRow, error) { + rows := make([]metav1beta1.TableRow, 0, len(sourceList.Items)) + + sort.SliceStable(sourceList.Items, func(i, j int) bool { + return sourceList.Items[i].Name < sourceList.Items[j].Name + }) + for _, source := range sourceList.Items { + row, err := printSource(&source, options) + if err != nil { + return nil, err + } + + rows = append(rows, row...) + } + return rows, nil +} diff --git a/pkg/kn/commands/source/list.go b/pkg/kn/commands/source/list.go new file mode 100644 index 0000000000..943143d451 --- /dev/null +++ b/pkg/kn/commands/source/list.go @@ -0,0 +1,91 @@ +// Copyright © 2020 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 source + +import ( + "fmt" + + "github.com/spf13/cobra" + + "knative.dev/client/pkg/dynamic" + "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/kn/commands/flags" + "knative.dev/client/pkg/kn/commands/source/duck" +) + +var listExample = ` + # List available eventing sources + kn source list + + # List PingSource type sources + kn source list --type=PingSource + + # List PingSource and ApiServerSource types sources + kn source list --type=PingSource --type=apiserversource` + +// NewListCommand defines and processes `kn source list` +func NewListCommand(p *commands.KnParams) *cobra.Command { + filterFlags := &flags.SourceTypeFilters{} + listFlags := flags.NewListPrintFlags(ListHandlers) + listCommand := &cobra.Command{ + Use: "list", + Short: "List available sources", + Example: listExample, + RunE: func(cmd *cobra.Command, args []string) error { + namespace, err := p.GetNamespace(cmd) + if err != nil { + return err + } + dynamicClient, err := p.NewDynamicClient(namespace) + if err != nil { + return err + } + var filters dynamic.WithTypes + for _, filter := range filterFlags.Filters { + filters = append(filters, dynamic.WithTypeFilter(filter)) + } + sourceList, err := dynamicClient.ListSources(filters...) + if err != nil { + return err + } + if len(sourceList.Items) == 0 { + fmt.Fprintf(cmd.OutOrStdout(), "No sources found in %s namespace.\n", namespace) + return nil + } + // empty namespace indicates all namespaces flag is specified + if namespace == "" { + listFlags.EnsureWithNamespace() + } + printer, err := listFlags.ToPrinter() + if err != nil { + return nil + } + if listFlags.GenericPrintFlags.OutputFlagSpecified() { + return printer.PrintObj(sourceList, cmd.OutOrStdout()) + } + // Convert the source list to DuckSourceList only if human readable table printing requested + sourceDuckList := duck.ToSourceList(sourceList) + err = printer.PrintObj(sourceDuckList, cmd.OutOrStdout()) + if err != nil { + return err + } + return nil + }, + } + commands.AddNamespaceFlags(listCommand.Flags(), true) + listFlags.AddFlags(listCommand) + filterFlags.Add(listCommand, "source type") + return listCommand +} diff --git a/pkg/kn/commands/source/list_test.go b/pkg/kn/commands/source/list_test.go new file mode 100644 index 0000000000..30c15a84f4 --- /dev/null +++ b/pkg/kn/commands/source/list_test.go @@ -0,0 +1,145 @@ +// 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 source + +import ( + "strings" + "testing" + + "gotest.tools/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + + "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/util" +) + +const ( + crdGroup = "apiextensions.k8s.io" + crdVersion = "v1beta1" + crdKind = "CustomResourceDefinition" + crdKinds = "customresourcedefinitions" + sourcesLabelKey = "duck.knative.dev/source" + sourcesLabelValue = "true" + testNamespace = "current" +) + +// sourceFakeCmd takes cmd to be executed using dynamic client +// pass the objects to be registered to dynamic client +func sourceFakeCmd(args []string, objects ...runtime.Object) (output []string, err error) { + knParams := &commands.KnParams{} + cmd, _, buf := commands.CreateDynamicTestKnCommand(NewSourceCommand(knParams), knParams, objects...) + cmd.SetArgs(args) + err = cmd.Execute() + if err != nil { + return + } + output = strings.Split(buf.String(), "\n") + return +} + +func TestSourceListTypes(t *testing.T) { + output, err := sourceFakeCmd([]string{"source", "list-types"}, + newSourceCRDObjWithSpec("pingsources", "sources.knative.dev", "v1alpha1", "PingSource"), + newSourceCRDObjWithSpec("apiserversources", "sources.knative.dev", "v1alpha1", "ApiServerSource"), + ) + assert.NilError(t, err) + assert.Check(t, util.ContainsAll(output[0], "TYPE", "NAME", "DESCRIPTION")) + assert.Check(t, util.ContainsAll(output[1], "ApiServerSource", "apiserversources")) + assert.Check(t, util.ContainsAll(output[2], "PingSource", "pingsources")) +} + +func TestSourceListTypesNoHeaders(t *testing.T) { + output, err := sourceFakeCmd([]string{"source", "list-types", "--no-headers"}, + newSourceCRDObjWithSpec("pingsources", "sources.knative.dev", "v1alpha1", "PingSource"), + ) + assert.NilError(t, err) + assert.Check(t, util.ContainsNone(output[0], "TYPE", "NAME", "DESCRIPTION")) + assert.Check(t, util.ContainsAll(output[0], "PingSource")) +} + +func TestSourceList(t *testing.T) { + output, err := sourceFakeCmd([]string{"source", "list"}, + newSourceCRDObjWithSpec("pingsources", "sources.knative.dev", "v1alpha1", "PingSource"), + newSourceCRDObjWithSpec("sinkbindings", "sources.knative.dev", "v1alpha1", "SinkBinding"), + newSourceCRDObjWithSpec("apiserversources", "sources.knative.dev", "v1alpha1", "ApiServerSource"), + newSourceCRDObjWithSpec("cronjobsources", "sources.eventing.knative.dev", "v1alpha1", "CronJobSource"), + newSourceUnstructuredObj("p1", "sources.knative.dev/v1alpha1", "PingSource"), + newSourceUnstructuredObj("s1", "sources.knative.dev/v1alpha1", "SinkBinding"), + newSourceUnstructuredObj("a1", "sources.knative.dev/v1alpha1", "ApiServerSource"), + newSourceUnstructuredObj("c1", "sources.eventing.knative.dev/v1alpha1", "CronJobSource"), + ) + assert.NilError(t, err) + assert.Check(t, util.ContainsAll(output[0], "NAME", "TYPE", "RESOURCE", "SINK", "READY")) + assert.Check(t, util.ContainsAll(output[1], "a1", "ApiServerSource", "apiserversources.sources.knative.dev", "svc:foo", "")) + assert.Check(t, util.ContainsAll(output[2], "c1", "CronJobSource", "cronjobsources.sources.eventing.knative.dev", "svc:foo", "")) + assert.Check(t, util.ContainsAll(output[3], "p1", "PingSource", "pingsources.sources.knative.dev", "svc:foo", "")) + assert.Check(t, util.ContainsAll(output[4], "s1", "SinkBinding", "sinkbindings.sources.knative.dev", "svc:foo", "")) +} + +func TestSourceListNoHeaders(t *testing.T) { + output, err := sourceFakeCmd([]string{"source", "list", "--no-headers"}, + newSourceCRDObjWithSpec("pingsources", "sources.knative.dev", "v1alpha1", "PingSource"), + newSourceUnstructuredObj("p1", "sources.knative.dev/v1alpha1", "PingSource"), + ) + assert.NilError(t, err) + assert.Check(t, util.ContainsNone(output[0], "NAME", "TYPE", "RESOURCE", "SINK", "READY")) + assert.Check(t, util.ContainsAll(output[0], "p1")) +} + +func newSourceCRDObjWithSpec(name, group, version, kind string) *unstructured.Unstructured { + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": crdGroup + "/" + crdVersion, + "kind": crdKind, + "metadata": map[string]interface{}{ + "namespace": testNamespace, + "name": name, + }, + }, + } + obj.Object["spec"] = map[string]interface{}{ + "group": group, + "version": version, + "names": map[string]interface{}{ + "kind": kind, + "plural": strings.ToLower(kind) + "s", + }, + } + obj.SetLabels(labels.Set{sourcesLabelKey: sourcesLabelValue}) + return obj +} + +func newSourceUnstructuredObj(name, apiVersion, kind string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": apiVersion, + "kind": kind, + "metadata": map[string]interface{}{ + "namespace": "current", + "name": name, + }, + "spec": map[string]interface{}{ + "sink": map[string]interface{}{ + "ref": map[string]interface{}{ + "kind": "Service", + "name": "foo", + }, + }, + }, + }, + } +} diff --git a/pkg/kn/commands/source/list_types.go b/pkg/kn/commands/source/list_types.go index 38c7c82e31..c10074f0b7 100644 --- a/pkg/kn/commands/source/list_types.go +++ b/pkg/kn/commands/source/list_types.go @@ -18,11 +18,12 @@ import ( "fmt" "github.com/spf13/cobra" + "knative.dev/client/pkg/kn/commands" "knative.dev/client/pkg/kn/commands/flags" ) -// NewListTypesCommand defines and processes `kn source list-types` command operations +// NewListTypesCommand defines and processes `kn source list-types` func NewListTypesCommand(p *commands.KnParams) *cobra.Command { listTypesFlags := flags.NewListPrintFlags(ListTypesHandlers) listTypesCommand := &cobra.Command{ diff --git a/pkg/kn/commands/source/list_types_test.go b/pkg/kn/commands/source/list_types_test.go deleted file mode 100644 index ef55c4730d..0000000000 --- a/pkg/kn/commands/source/list_types_test.go +++ /dev/null @@ -1,83 +0,0 @@ -// 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 source - -import ( - "gotest.tools/assert" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "knative.dev/client/pkg/kn/commands" - "knative.dev/client/pkg/util" - - "strings" - "testing" -) - -func newUnstructured(name string) *unstructured.Unstructured { - return &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "apiextensions.k8s.io/v1beta1", - "kind": "CustomResourceDefinition", - "metadata": map[string]interface{}{ - "namespace": "current", - "name": name, - "labels": map[string]interface{}{ - "duck.knative.dev/source": "true", - }, - }, - }, - } -} - -func newUnstructuredWithSpecNames(name string, value map[string]interface{}) *unstructured.Unstructured { - u := newUnstructured(name) - u.Object["spec"] = map[string]interface{}{"names": value} - return u -} - -func fakeListTypes(args []string, objects ...runtime.Object) (output []string, err error) { - knParams := &commands.KnParams{} - // not using the fake dynamic client returned here - cmd, _, buf := commands.CreateDynamicTestKnCommand(NewSourceCommand(knParams), knParams, objects...) - - cmd.SetArgs(args) - err = cmd.Execute() - if err != nil { - return - } - - output = strings.Split(buf.String(), "\n") - return -} - -func TestSourceListTypes(t *testing.T) { - output, err := fakeListTypes([]string{"source", "list-types"}, - newUnstructuredWithSpecNames("foo.in", map[string]interface{}{"kind": "foo"}), - newUnstructuredWithSpecNames("bar.in", map[string]interface{}{"kind": "bar"}), - ) - assert.NilError(t, err) - assert.Check(t, util.ContainsAll(output[0], "TYPE", "NAME", "DESCRIPTION")) - assert.Check(t, util.ContainsAll(output[1], "bar", "bar.in")) - assert.Check(t, util.ContainsAll(output[2], "foo", "foo.in")) -} - -func TestSourceListTypesNoHeaders(t *testing.T) { - output, err := fakeListTypes([]string{"source", "list-types", "--no-headers"}, - newUnstructuredWithSpecNames("foo.in", map[string]interface{}{"kind": "foo"}), - newUnstructuredWithSpecNames("bar.in", map[string]interface{}{"kind": "bar"}), - ) - assert.NilError(t, err) - assert.Check(t, util.ContainsNone(output[0], "TYPE", "NAME", "DESCRIPTION")) -} diff --git a/pkg/kn/commands/source/source.go b/pkg/kn/commands/source/source.go index d90f709c01..0e08a56ec8 100644 --- a/pkg/kn/commands/source/source.go +++ b/pkg/kn/commands/source/source.go @@ -28,8 +28,9 @@ func NewSourceCommand(p *commands.KnParams) *cobra.Command { Use: "source", Short: "Event source command group", } - sourceCmd.AddCommand(apiserver.NewAPIServerCommand(p)) sourceCmd.AddCommand(NewListTypesCommand(p)) + sourceCmd.AddCommand(NewListCommand(p)) + sourceCmd.AddCommand(apiserver.NewAPIServerCommand(p)) sourceCmd.AddCommand(ping.NewPingCommand(p)) sourceCmd.AddCommand(binding.NewBindingCommand(p)) return sourceCmd diff --git a/pkg/util/compare.go b/pkg/util/compare.go index 252cc21737..0d28395fa1 100644 --- a/pkg/util/compare.go +++ b/pkg/util/compare.go @@ -73,3 +73,13 @@ func ContainsNone(target string, substrings ...string) cmp.Comparison { return cmp.ResultSuccess } } + +// SliceContainsIgnoreCase checks (case insensitive) if given target string is present in slice +func SliceContainsIgnoreCase(slice []string, target string) bool { + for _, each := range slice { + if strings.EqualFold(target, each) { + return true + } + } + return false +} diff --git a/pkg/util/compare_test.go b/pkg/util/compare_test.go index 04c1f49496..5555283121 100644 --- a/pkg/util/compare_test.go +++ b/pkg/util/compare_test.go @@ -20,6 +20,7 @@ import ( "strings" "testing" + "gotest.tools/assert" "gotest.tools/assert/cmp" ) @@ -146,3 +147,12 @@ func TestContainsNone(t *testing.T) { } } } + +func TestSliceContainsIgnoreCase(t *testing.T) { + assert.Equal(t, + SliceContainsIgnoreCase([]string{"FOO", "bar"}, "foo"), + true) + assert.Equal(t, + SliceContainsIgnoreCase([]string{"BAR", "bar"}, "foo"), + false) +} diff --git a/test/e2e/source_apiserver_test.go b/test/e2e/source_apiserver_test.go index 2a943e77fa..cf94091f1e 100644 --- a/test/e2e/source_apiserver_test.go +++ b/test/e2e/source_apiserver_test.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" "gotest.tools/assert" + "knative.dev/client/pkg/util" ) @@ -55,6 +56,16 @@ func TestSourceApiServer(t *testing.T) { test.apiServerSourceCreate(t, r, "testapisource0", "Event:v1:true", "testsa", "svc:testsvc0") test.apiServerSourceCreate(t, r, "testapisource1", "Event:v1", "testsa", "svc:testsvc0") + t.Log("list sources") + output := test.sourceList(t, r) + assert.Check(t, util.ContainsAll(output, "NAME", "TYPE", "RESOURCE", "SINK", "READY")) + assert.Check(t, util.ContainsAll(output, "testapisource0", "ApiServerSource", "apiserversources.sources.knative.dev", "svc:testsvc0")) + assert.Check(t, util.ContainsAll(output, "testapisource1", "ApiServerSource", "apiserversources.sources.knative.dev", "svc:testsvc0")) + + t.Log("list sources in YAML format") + output = test.sourceList(t, r, "-oyaml") + assert.Check(t, util.ContainsAll(output, "testapisource1", "ApiServerSource", "Service", "testsvc0")) + t.Log("delete apiserver sources") test.apiServerSourceDelete(t, r, "testapisource0") test.apiServerSourceDelete(t, r, "testapisource1") diff --git a/test/e2e/source_list_types_test.go b/test/e2e/source_list_test.go similarity index 66% rename from test/e2e/source_list_types_test.go rename to test/e2e/source_list_test.go index 5a4fd8cf03..faceaa1b15 100644 --- a/test/e2e/source_list_types_test.go +++ b/test/e2e/source_list_test.go @@ -21,6 +21,7 @@ import ( "testing" "gotest.tools/assert" + "knative.dev/client/pkg/util" ) @@ -45,9 +46,35 @@ func TestSourceListTypes(t *testing.T) { assert.Check(t, util.ContainsAll(output, "apiextensions.k8s.io/v1beta1", "CustomResourceDefinition", "Ping", "ApiServer")) } +func TestSourceList(t *testing.T) { + t.Parallel() + test, err := NewE2eTest() + assert.NilError(t, err) + defer func() { + assert.NilError(t, test.Teardown()) + }() + + r := NewKnRunResultCollector(t) + defer r.DumpIfFailed() + + t.Log("List sources empty case") + output := test.sourceList(t, r) + assert.Check(t, util.ContainsAll(output, "No", "sources", "found", "namespace")) + assert.Check(t, util.ContainsNone(output, "NAME", "TYPE", "RESOURCE", "SINK", "READY")) + + // non empty list case is tested in test/e2e/source_apiserver_test.go where source setup is present +} + func (test *e2eTest) sourceListTypes(t *testing.T, r *KnRunResultCollector, args ...string) string { cmd := append([]string{"source", "list-types"}, args...) out := test.kn.Run(cmd...) r.AssertNoError(out) return out.Stdout } + +func (test *e2eTest) sourceList(t *testing.T, r *KnRunResultCollector, args ...string) string { + cmd := append([]string{"source", "list"}, args...) + out := test.kn.Run(cmd...) + r.AssertNoError(out) + return out.Stdout +} diff --git a/vendor/github.com/magiconair/properties/assert/assert.go b/vendor/github.com/magiconair/properties/assert/assert.go deleted file mode 100644 index d0f2704670..0000000000 --- a/vendor/github.com/magiconair/properties/assert/assert.go +++ /dev/null @@ -1,90 +0,0 @@ -// Copyright 2018 Frank Schroeder. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// Package assert provides helper functions for testing. -package assert - -import ( - "fmt" - "path/filepath" - "reflect" - "regexp" - "runtime" - "strings" - "testing" -) - -// skip defines the default call depth -const skip = 2 - -// Equal asserts that got and want are equal as defined by -// reflect.DeepEqual. The test fails with msg if they are not equal. -func Equal(t *testing.T, got, want interface{}, msg ...string) { - if x := equal(2, got, want, msg...); x != "" { - fmt.Println(x) - t.Fail() - } -} - -func equal(skip int, got, want interface{}, msg ...string) string { - if !reflect.DeepEqual(got, want) { - return fail(skip, "got %v want %v %s", got, want, strings.Join(msg, " ")) - } - return "" -} - -// Panic asserts that function fn() panics. -// It assumes that recover() either returns a string or -// an error and fails if the message does not match -// the regular expression in 'matches'. -func Panic(t *testing.T, fn func(), matches string) { - if x := doesPanic(2, fn, matches); x != "" { - fmt.Println(x) - t.Fail() - } -} - -func doesPanic(skip int, fn func(), expr string) (err string) { - defer func() { - r := recover() - if r == nil { - err = fail(skip, "did not panic") - return - } - var v string - switch r.(type) { - case error: - v = r.(error).Error() - case string: - v = r.(string) - } - err = matches(skip, v, expr) - }() - fn() - return "" -} - -// Matches asserts that a value matches a given regular expression. -func Matches(t *testing.T, value, expr string) { - if x := matches(2, value, expr); x != "" { - fmt.Println(x) - t.Fail() - } -} - -func matches(skip int, value, expr string) string { - ok, err := regexp.MatchString(expr, value) - if err != nil { - return fail(skip, "invalid pattern %q. %s", expr, err) - } - if !ok { - return fail(skip, "got %s which does not match %s", value, expr) - } - return "" -} - -func fail(skip int, format string, args ...interface{}) string { - _, file, line, _ := runtime.Caller(skip) - return fmt.Sprintf("\t%s:%d: %s\n", filepath.Base(file), line, fmt.Sprintf(format, args...)) -} diff --git a/vendor/modules.txt b/vendor/modules.txt index d47c3a756b..79ab33800a 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -78,7 +78,6 @@ github.com/json-iterator/go github.com/liggitt/tabwriter # github.com/magiconair/properties v1.8.0 github.com/magiconair/properties -github.com/magiconair/properties/assert # github.com/mailru/easyjson v0.7.0 github.com/mailru/easyjson/buffer github.com/mailru/easyjson/jlexer From 5c794d3b0427aaa268ac0f29195b9715fdab0464 Mon Sep 17 00:00:00 2001 From: Murugappan Chetty Date: Tue, 10 Mar 2020 02:53:28 -0700 Subject: [PATCH 4/6] add "kn service export" (#653) (#669) * add kn export (#653) * add kn export (#653) * review comments for #669 * add kn export command * add kn export command * add kn export command * add kn export command * add changelog for pr 679 * add changelog * review comments for pr #669 * review comments for pr #669 * review comments for kn export --- CHANGELOG.adoc | 4 + docs/cmd/kn_service.md | 1 + docs/cmd/kn_service_export.md | 45 ++++ pkg/kn/commands/service/export.go | 230 ++++++++++++++++++ pkg/kn/commands/service/export_test.go | 322 +++++++++++++++++++++++++ pkg/kn/commands/service/service.go | 1 + 6 files changed, 603 insertions(+) create mode 100644 docs/cmd/kn_service_export.md create mode 100644 pkg/kn/commands/service/export.go create mode 100644 pkg/kn/commands/service/export_test.go diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index bd470c1f15..27191a42e9 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -81,6 +81,10 @@ | 🎁 | Add `--user` flag for specifying the user id to run the container | https://github.com/knative/client/pull/679[#679] + +| 🎁 +| add kn service export command for exporting a service +| https://github.com/knative/client/pull/669[#669] |=== ## v0.12.0 (2020-01-29) diff --git a/docs/cmd/kn_service.md b/docs/cmd/kn_service.md index 28b0741e6b..ca289a08cd 100644 --- a/docs/cmd/kn_service.md +++ b/docs/cmd/kn_service.md @@ -30,6 +30,7 @@ kn service [flags] * [kn service create](kn_service_create.md) - Create a service. * [kn service delete](kn_service_delete.md) - Delete a service. * [kn service describe](kn_service_describe.md) - Show details of a service +* [kn service export](kn_service_export.md) - export a service * [kn service list](kn_service_list.md) - List available services. * [kn service update](kn_service_update.md) - Update a service. diff --git a/docs/cmd/kn_service_export.md b/docs/cmd/kn_service_export.md new file mode 100644 index 0000000000..f9491301d8 --- /dev/null +++ b/docs/cmd/kn_service_export.md @@ -0,0 +1,45 @@ +## kn service export + +export a service + +### Synopsis + +export a service + +``` +kn service export NAME [flags] +``` + +### Examples + +``` + + # Export a service in yaml format + kn service export foo -n bar -o yaml + # Export a service in json format + kn service export foo -n bar -o json +``` + +### Options + +``` + --allow-missing-template-keys If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true) + -h, --help help for export + -r, --history Export all active revisions + -n, --namespace string Specify the namespace to operate in. + -o, --output string Output format. One of: json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-file. + --template string Template string or path to template file to use when -o=go-template, -o=go-template-file. The template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview]. +``` + +### Options inherited from parent commands + +``` + --config string kn config file (default is ~/.config/kn/config.yaml) + --kubeconfig string kubectl config file (default is ~/.kube/config) + --log-http log http traffic +``` + +### SEE ALSO + +* [kn service](kn_service.md) - Service command group + diff --git a/pkg/kn/commands/service/export.go b/pkg/kn/commands/service/export.go new file mode 100644 index 0000000000..a2cf1aa726 --- /dev/null +++ b/pkg/kn/commands/service/export.go @@ -0,0 +1,230 @@ +// Copyright © 2020 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 service + +import ( + "errors" + "fmt" + + "sort" + "strconv" + + "github.com/spf13/cobra" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/cli-runtime/pkg/genericclioptions" + + "knative.dev/client/pkg/kn/commands" + clientservingv1 "knative.dev/client/pkg/serving/v1" + "knative.dev/serving/pkg/apis/serving" + servingv1 "knative.dev/serving/pkg/apis/serving/v1" +) + +// NewServiceExportCommand returns a new command for exporting a service. +func NewServiceExportCommand(p *commands.KnParams) *cobra.Command { + + // For machine readable output + machineReadablePrintFlags := genericclioptions.NewPrintFlags("") + + command := &cobra.Command{ + Use: "export NAME", + Short: "export a service", + Example: ` + # Export a service in yaml format + kn service export foo -n bar -o yaml + # Export a service in json format + kn service export foo -n bar -o json`, + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) != 1 { + return errors.New("'kn service export' requires name of the service as single argument") + } + if !machineReadablePrintFlags.OutputFlagSpecified() { + return errors.New("'kn service export' requires output format") + } + serviceName := args[0] + + namespace, err := p.GetNamespace(cmd) + if err != nil { + return err + } + + client, err := p.NewServingClient(namespace) + if err != nil { + return err + } + + service, err := client.GetService(serviceName) + if err != nil { + return err + } + + history, err := cmd.Flags().GetBool("history") + if err != nil { + return err + } + + printer, err := machineReadablePrintFlags.ToPrinter() + if err != nil { + return err + } + + if history { + if svcList, err := exportServicewithActiveRevisions(service, client); err != nil { + return err + } else { + return printer.PrintObj(svcList, cmd.OutOrStdout()) + } + } + return printer.PrintObj(exportService(service), cmd.OutOrStdout()) + }, + } + flags := command.Flags() + commands.AddNamespaceFlags(flags, false) + flags.BoolP("history", "r", false, "Export all active revisions") + machineReadablePrintFlags.AddFlags(command) + return command +} + +func exportService(latestSvc *servingv1.Service) *servingv1.Service { + + exportedSvc := servingv1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: latestSvc.ObjectMeta.Name, + Labels: latestSvc.ObjectMeta.Labels, + }, + TypeMeta: latestSvc.TypeMeta, + } + + exportedSvc.Spec.Template = servingv1.RevisionTemplateSpec{ + Spec: latestSvc.Spec.ConfigurationSpec.Template.Spec, + ObjectMeta: latestSvc.Spec.ConfigurationSpec.Template.ObjectMeta, + } + + return &exportedSvc +} + +func constructServicefromRevision(latestSvc *servingv1.Service, revision servingv1.Revision) servingv1.Service { + + exportedSvc := servingv1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: latestSvc.ObjectMeta.Name, + Labels: latestSvc.ObjectMeta.Labels, + }, + TypeMeta: latestSvc.TypeMeta, + } + + exportedSvc.Spec.Template = servingv1.RevisionTemplateSpec{ + Spec: revision.Spec, + ObjectMeta: latestSvc.Spec.ConfigurationSpec.Template.ObjectMeta, + } + + exportedSvc.Spec.ConfigurationSpec.Template.ObjectMeta.Name = revision.ObjectMeta.Name + + return exportedSvc +} + +func exportServicewithActiveRevisions(latestSvc *servingv1.Service, client clientservingv1.KnServingClient) (*servingv1.ServiceList, error) { + var exportedSvcItems []servingv1.Service + + //get revisions to export from traffic + revsMap := getRevisionstoExport(latestSvc) + + var params []clientservingv1.ListConfig + params = append(params, clientservingv1.WithService(latestSvc.ObjectMeta.Name)) + + // Query for list with filters + revisionList, err := client.ListRevisions(params...) + if err != nil { + return nil, err + } + if len(revisionList.Items) == 0 { + return nil, fmt.Errorf("No revisions found for the service %s", latestSvc.ObjectMeta.Name) + } + + // sort revisions to main the order of generations + sortRevisions(revisionList) + + for _, revision := range revisionList.Items { + //construct service only for active revisions + if revsMap[revision.ObjectMeta.Name] { + exportedSvcItems = append(exportedSvcItems, constructServicefromRevision(latestSvc, revision)) + } + } + + //set traffic in the latest revision + exportedSvcItems[len(exportedSvcItems)-1] = setTrafficSplit(latestSvc, exportedSvcItems[len(exportedSvcItems)-1]) + + typeMeta := metav1.TypeMeta{ + APIVersion: "v1", + Kind: "List", + } + exportedSvcList := &servingv1.ServiceList{ + TypeMeta: typeMeta, + Items: exportedSvcItems, + } + + return exportedSvcList, nil +} + +func setTrafficSplit(latestSvc *servingv1.Service, exportedSvc servingv1.Service) servingv1.Service { + + exportedSvc.Spec.RouteSpec = latestSvc.Spec.RouteSpec + + return exportedSvc +} + +func getRevisionstoExport(latestSvc *servingv1.Service) map[string]bool { + trafficList := latestSvc.Spec.RouteSpec.Traffic + revsMap := make(map[string]bool) + + for _, traffic := range trafficList { + if traffic.RevisionName == "" { + revsMap[latestSvc.Spec.ConfigurationSpec.Template.ObjectMeta.Name] = true + } else { + revsMap[traffic.RevisionName] = true + } + } + return revsMap +} + +// sortRevisions sorts revisions by generation and name (in this order) +func sortRevisions(revisionList *servingv1.RevisionList) { + // sort revisionList by configuration generation key + sort.SliceStable(revisionList.Items, revisionListSortFunc(revisionList)) +} + +// revisionListSortFunc sorts by generation and name +func revisionListSortFunc(revisionList *servingv1.RevisionList) func(i int, j int) bool { + return func(i, j int) bool { + a := revisionList.Items[i] + b := revisionList.Items[j] + + // By Generation + // Convert configuration generation key from string to int for avoiding string comparison. + agen, err := strconv.Atoi(a.Labels[serving.ConfigurationGenerationLabelKey]) + if err != nil { + return a.Name > b.Name + } + bgen, err := strconv.Atoi(b.Labels[serving.ConfigurationGenerationLabelKey]) + if err != nil { + return a.Name > b.Name + } + + if agen != bgen { + return agen < bgen + } + return a.Name > b.Name + } +} diff --git a/pkg/kn/commands/service/export_test.go b/pkg/kn/commands/service/export_test.go new file mode 100644 index 0000000000..6b9be103a2 --- /dev/null +++ b/pkg/kn/commands/service/export_test.go @@ -0,0 +1,322 @@ +// Copyright © 2020 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 service + +import ( + "encoding/json" + "fmt" + "testing" + + "gotest.tools/assert" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + servinglib "knative.dev/client/pkg/serving" + knclient "knative.dev/client/pkg/serving/v1" + "knative.dev/client/pkg/util/mock" + "knative.dev/pkg/ptr" + apiserving "knative.dev/serving/pkg/apis/serving" + servingv1 "knative.dev/serving/pkg/apis/serving/v1" + "sigs.k8s.io/yaml" +) + +func TestServiceExport(t *testing.T) { + var svcs []*servingv1.Service + typeMeta := metav1.TypeMeta{ + Kind: "service", + APIVersion: "serving.knative.dev/v1", + } + + // case 1 - plain svc + plainService := getService("foo") + svcs = append(svcs, plainService) + + // case 2 - svc with env variables + envSvc := getService("foo") + envVars := []v1.EnvVar{ + {Name: "a", Value: "mouse"}, + {Name: "b", Value: "cookie"}, + {Name: "empty", Value: ""}, + } + template := &envSvc.Spec.Template + template.Spec.Containers[0].Env = envVars + template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" + template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} + svcs = append(svcs, envSvc) + + //case 3 - svc with labels + labelService := getService("foo") + expected := map[string]string{ + "a": "mouse", + "b": "cookie", + "empty": "", + } + labelService.Labels = expected + labelService.Spec.Template.Annotations = map[string]string{ + servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz", + } + template = &labelService.Spec.Template + template.ObjectMeta.Labels = expected + template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" + svcs = append(svcs, labelService) + + //case 4 - config map + CMservice := getService("foo") + template = &CMservice.Spec.Template + template.Spec.Containers[0].EnvFrom = []v1.EnvFromSource{ + { + ConfigMapRef: &v1.ConfigMapEnvSource{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "config-map-name", + }, + }, + }, + } + template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" + template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} + svcs = append(svcs, CMservice) + + //case 5 - volume mount and secrets + Volservice := getService("foo") + template = &Volservice.Spec.Template + template.Spec.Volumes = []v1.Volume{ + { + Name: "volume-name", + VolumeSource: v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: "secret-name", + }, + }, + }, + } + + template.Spec.Containers[0].VolumeMounts = []v1.VolumeMount{ + { + Name: "volume-name", + MountPath: "/mount/path", + ReadOnly: true, + }, + } + svcs = append(svcs, Volservice) + + template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" + template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} + + for _, svc := range svcs { + svc.TypeMeta = typeMeta + callServiceExportTest(t, svc) + } + +} + +func callServiceExportTest(t *testing.T, expectedService *servingv1.Service) { + // New mock client + client := knclient.NewMockKnServiceClient(t) + + // Recording: + r := client.Recorder() + + r.GetService(expectedService.ObjectMeta.Name, expectedService, nil) + + output, err := executeServiceCommand(client, "export", expectedService.ObjectMeta.Name, "-o", "yaml") + + assert.NilError(t, err) + + expectedService.ObjectMeta.Namespace = "" + + expSvcYaml, err := yaml.Marshal(expectedService) + + assert.NilError(t, err) + + assert.Equal(t, string(expSvcYaml), output) + + // Validate that all recorded API methods have been called + r.Validate() +} + +func TestServiceExportwithMultipleRevisions(t *testing.T) { + //case 1 = 2 revisions with traffic split + trafficSplitService := createServiceTwoRevsionsWithTraffic("foo", true) + + multiRevs := createTestRevisionList("rev", "foo") + + callServiceExportHistoryTest(t, trafficSplitService, multiRevs) + + //case 2 - same revisions no traffic split + noTrafficSplitService := createServiceTwoRevsionsWithTraffic("foo", false) + + callServiceExportHistoryTest(t, noTrafficSplitService, multiRevs) +} + +func callServiceExportHistoryTest(t *testing.T, expectedService *servingv1.Service, revs *servingv1.RevisionList) { + // New mock client + client := knclient.NewMockKnServiceClient(t) + + // Recording: + r := client.Recorder() + + r.GetService(expectedService.ObjectMeta.Name, expectedService, nil) + + r.ListRevisions(mock.Any(), revs, nil) + + output, err := executeServiceCommand(client, "export", expectedService.ObjectMeta.Name, "-r", "-o", "json") + + assert.NilError(t, err) + + actSvcList := servingv1.ServiceList{} + + json.Unmarshal([]byte(output), &actSvcList) + + for i, actSvc := range actSvcList.Items { + var checkTraffic bool + if i == (len(actSvcList.Items) - 1) { + checkTraffic = true + } + validateServiceWithRevisionHistory(t, expectedService, revs, actSvc, checkTraffic) + } + + // Validate that all recorded API methods have been called + r.Validate() +} + +func validateServiceWithRevisionHistory(t *testing.T, expectedsvc *servingv1.Service, expectedRevList *servingv1.RevisionList, actualSvc servingv1.Service, checkTraffic bool) { + var expectedRev servingv1.Revision + var routeSpec servingv1.RouteSpec + for _, rev := range expectedRevList.Items { + if actualSvc.Spec.ConfigurationSpec.Template.ObjectMeta.Name == rev.ObjectMeta.Name { + expectedRev = rev + break + } + } + expectedsvc.Spec.ConfigurationSpec.Template.ObjectMeta.Name = expectedRev.ObjectMeta.Name + expectedsvc.Spec.Template.Spec = expectedRev.Spec + + stripExpectedSvcVariables(expectedsvc) + + if !checkTraffic { + routeSpec = expectedsvc.Spec.RouteSpec + expectedsvc.Spec.RouteSpec = servingv1.RouteSpec{} + } + assert.DeepEqual(t, expectedsvc, &actualSvc) + + expectedsvc.Spec.RouteSpec = routeSpec +} + +func TestServiceExportError(t *testing.T) { + // New mock client + client := knclient.NewMockKnServiceClient(t) + + expectedService := getService("foo") + + _, err := executeServiceCommand(client, "export", expectedService.ObjectMeta.Name) + + assert.Error(t, err, "'kn service export' requires output format") +} + +func createTestRevisionList(revision string, service string) *servingv1.RevisionList { + labels1 := make(map[string]string) + labels1[apiserving.ConfigurationGenerationLabelKey] = "1" + labels1[apiserving.ServiceLabelKey] = service + + labels2 := make(map[string]string) + labels2[apiserving.ConfigurationGenerationLabelKey] = "2" + labels2[apiserving.ServiceLabelKey] = service + + rev1 := servingv1.Revision{ + TypeMeta: metav1.TypeMeta{ + Kind: "Revision", + APIVersion: "serving.knative.dev/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-%s-%d", service, revision, 1), + Namespace: "default", + Generation: int64(1), + Labels: labels1, + }, + Spec: servingv1.RevisionSpec{ + PodSpec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: "gcr.io/test/image:v1", + Env: []v1.EnvVar{ + {Name: "env1", Value: "eval1"}, + {Name: "env2", Value: "eval2"}, + }, + EnvFrom: []v1.EnvFromSource{ + {ConfigMapRef: &v1.ConfigMapEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: "test1"}}}, + {ConfigMapRef: &v1.ConfigMapEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: "test2"}}}, + }, + Ports: []v1.ContainerPort{ + {ContainerPort: 8080}, + }, + }, + }, + }, + }, + } + + rev2 := rev1 + + rev2.Spec.PodSpec.Containers[0].Image = "gcr.io/test/image:v2" + rev2.ObjectMeta.Labels = labels2 + rev2.ObjectMeta.Generation = int64(2) + rev2.ObjectMeta.Name = fmt.Sprintf("%s-%s-%d", service, revision, 2) + + typeMeta := metav1.TypeMeta{ + APIVersion: "v1", + Kind: "List", + } + + return &servingv1.RevisionList{ + TypeMeta: typeMeta, + Items: []servingv1.Revision{rev1, rev2}, + } +} + +func createServiceTwoRevsionsWithTraffic(svc string, trafficSplit bool) *servingv1.Service { + expectedService := createTestService(svc, []string{svc + "-rev-1", svc + "-rev-2"}, goodConditions()) + expectedService.Status.Traffic[0].LatestRevision = ptr.Bool(true) + expectedService.Status.Traffic[0].Tag = "latest" + expectedService.Status.Traffic[1].Tag = "current" + + if trafficSplit { + trafficList := []servingv1.TrafficTarget{ + { + RevisionName: "foo-rev-1", + Percent: ptr.Int64(int64(50)), + }, { + RevisionName: "foo-rev-2", + Percent: ptr.Int64(int64(50)), + }} + expectedService.Spec.RouteSpec = servingv1.RouteSpec{Traffic: trafficList} + } else { + trafficList := []servingv1.TrafficTarget{ + { + RevisionName: "foo-rev-2", + Percent: ptr.Int64(int64(50)), + }} + expectedService.Spec.RouteSpec = servingv1.RouteSpec{Traffic: trafficList} + } + + return &expectedService +} + +func stripExpectedSvcVariables(expectedsvc *servingv1.Service) { + expectedsvc.ObjectMeta.Namespace = "" + expectedsvc.Spec.Template.Spec.Containers[0].Resources = v1.ResourceRequirements{} + expectedsvc.Status = servingv1.ServiceStatus{} + expectedsvc.ObjectMeta.Annotations = nil + expectedsvc.ObjectMeta.CreationTimestamp = metav1.Time{} +} diff --git a/pkg/kn/commands/service/service.go b/pkg/kn/commands/service/service.go index a9eb58734c..b6f357e646 100644 --- a/pkg/kn/commands/service/service.go +++ b/pkg/kn/commands/service/service.go @@ -41,6 +41,7 @@ func NewServiceCommand(p *commands.KnParams) *cobra.Command { serviceCmd.AddCommand(NewServiceCreateCommand(p)) serviceCmd.AddCommand(NewServiceDeleteCommand(p)) serviceCmd.AddCommand(NewServiceUpdateCommand(p)) + serviceCmd.AddCommand(NewServiceExportCommand(p)) return serviceCmd } From 74e43b88065807c28ae888bc93681ea769ad1f33 Mon Sep 17 00:00:00 2001 From: Navid Shaikh Date: Tue, 10 Mar 2020 16:04:29 +0530 Subject: [PATCH 5/6] Report missing kubeconfig or error connecting to cluster (#725) Fixes #315 - Improve error reporting if - kubeconfig file is missing or - there is no route to host - i/o timeout --- CHANGELOG.adoc | 4 ++++ pkg/errors/errors.go | 15 ++++++++++-- pkg/errors/errors_test.go | 8 +++---- pkg/errors/factory.go | 44 +++++++++++++++++++++-------------- pkg/errors/factory_test.go | 42 +++++++++++++++++++++++++++++++-- pkg/kn/commands/types.go | 5 ++-- pkg/kn/commands/types_test.go | 6 ++--- 7 files changed, 94 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 27191a42e9..46bee13d32 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -18,6 +18,10 @@ |=== | | Description | PR +| 🐛 +| Improve reporting for missing kubeconfig and error connecting to the cluster +| https://github.com/knative/client/pull/725[#725] + | 🎁 | Add `kn source list` | https://github.com/knative/client/pull/666[#666] diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 99eab62934..c9a0cd5454 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -22,7 +22,18 @@ import ( func newInvalidCRD(apiGroup string) *KNError { parts := strings.Split(apiGroup, ".") name := parts[0] - msg := fmt.Sprintf("no Knative %s API found on the backend. Please verify the installation.", name) - + msg := fmt.Sprintf("no Knative %s API found on the backend, please verify the installation", name) return NewKNError(msg) } + +func newNoRouteToHost(errString string) error { + parts := strings.SplitAfter(errString, "dial tcp") + if len(parts) == 2 { + return NewKNError(fmt.Sprintf("error connecting to the cluster, please verify connection at: %s", strings.Trim(parts[1], " "))) + } + return NewKNError(fmt.Sprintf("error connecting to the cluster: %s", errString)) +} + +func newNoKubeConfig(errString string) error { + return NewKNError("no kubeconfig has been provided, please use a valid configuration to connect to the cluster") +} diff --git a/pkg/errors/errors_test.go b/pkg/errors/errors_test.go index 016b5ef2b4..6e3df075b5 100644 --- a/pkg/errors/errors_test.go +++ b/pkg/errors/errors_test.go @@ -22,12 +22,12 @@ import ( func TestNewInvalidCRD(t *testing.T) { err := newInvalidCRD("serving.knative.dev") - assert.Error(t, err, "no Knative serving API found on the backend. Please verify the installation.") + assert.Error(t, err, "no Knative serving API found on the backend, please verify the installation") - err = newInvalidCRD("serving") - assert.Error(t, err, "no Knative serving API found on the backend. Please verify the installation.") + err = newInvalidCRD("eventing") + assert.Error(t, err, "no Knative eventing API found on the backend, please verify the installation") err = newInvalidCRD("") - assert.Error(t, err, "no Knative API found on the backend. Please verify the installation.") + assert.Error(t, err, "no Knative API found on the backend, please verify the installation") } diff --git a/pkg/errors/factory.go b/pkg/errors/factory.go index 38e602db7c..d0b473856f 100644 --- a/pkg/errors/factory.go +++ b/pkg/errors/factory.go @@ -27,29 +27,39 @@ func isCRDError(status api_errors.APIStatus) bool { return true } } - return false } +func isNoRouteToHostError(err error) bool { + return strings.Contains(err.Error(), "no route to host") || strings.Contains(err.Error(), "i/o timeout") +} + +func isEmptyConfigError(err error) bool { + return strings.Contains(err.Error(), "no configuration has been provided") +} + //Retrieves a custom error struct based on the original error APIStatus struct //Returns the original error struct in case it can't identify the kind of APIStatus error func GetError(err error) error { - apiStatus, ok := err.(api_errors.APIStatus) - if !ok { - return err - } - - if apiStatus.Status().Details == nil { + switch { + case isEmptyConfigError(err): + return newNoKubeConfig(err.Error()) + case isNoRouteToHostError(err): + return newNoRouteToHost(err.Error()) + default: + apiStatus, ok := err.(api_errors.APIStatus) + if !ok { + return err + } + if apiStatus.Status().Details == nil { + return err + } + var knerr *KNError + if isCRDError(apiStatus) { + knerr = newInvalidCRD(apiStatus.Status().Details.Group) + knerr.Status = apiStatus + return knerr + } return err } - - var knerr *KNError - - if isCRDError(apiStatus) { - knerr = newInvalidCRD(apiStatus.Status().Details.Group) - knerr.Status = apiStatus - return knerr - } - - return err } diff --git a/pkg/errors/factory_test.go b/pkg/errors/factory_test.go index bce45f5582..9c94b94952 100644 --- a/pkg/errors/factory_test.go +++ b/pkg/errors/factory_test.go @@ -15,6 +15,7 @@ package errors import ( + "errors" "testing" "gotest.tools/assert" @@ -23,7 +24,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ) -func TestBuild(t *testing.T) { +func TestKnErrorsStatusErrors(t *testing.T) { cases := []struct { Name string Schema schema.GroupResource @@ -47,7 +48,7 @@ func TestBuild(t *testing.T) { } return statusError }, - ExpectedMsg: "no Knative serving API found on the backend. Please verify the installation.", + ExpectedMsg: "no Knative serving API found on the backend, please verify the installation", Validate: func(t *testing.T, err error, msg string) { assert.Error(t, err, msg) }, @@ -92,3 +93,40 @@ func TestBuild(t *testing.T) { }) } } + +func TestKnErrors(t *testing.T) { + cases := []struct { + Name string + Error error + ExpectedMsg string + }{ + { + Name: "no kubeconfig provided", + Error: errors.New("invalid configuration: no configuration has been provided"), + ExpectedMsg: "no kubeconfig has been provided, please use a valid configuration to connect to the cluster", + }, + { + Name: "i/o timeout", + Error: errors.New("Get https://api.example.com:27435/apis/foo/bar: dial tcp 192.168.1.1:27435: i/o timeout"), + ExpectedMsg: "error connecting to the cluster, please verify connection at: 192.168.1.1:27435: i/o timeout", + }, + { + Name: "no route to host", + Error: errors.New("Get https://192.168.39.141:8443/apis/foo/bar: dial tcp 192.168.39.141:8443: connect: no route to host"), + ExpectedMsg: "error connecting to the cluster, please verify connection at: 192.168.39.141:8443: connect: no route to host", + }, + { + Name: "no route to host without dial tcp string", + Error: errors.New("no route to host 192.168.1.1"), + ExpectedMsg: "error connecting to the cluster: no route to host 192.168.1.1", + }, + } + for _, tc := range cases { + tc := tc + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + err := GetError(tc.Error) + assert.Error(t, err, tc.ExpectedMsg) + }) + } +} diff --git a/pkg/kn/commands/types.go b/pkg/kn/commands/types.go index 5b30c5349f..b7b7235203 100644 --- a/pkg/kn/commands/types.go +++ b/pkg/kn/commands/types.go @@ -31,6 +31,7 @@ import ( "knative.dev/client/pkg/util" clientdynamic "knative.dev/client/pkg/dynamic" + knerrors "knative.dev/client/pkg/errors" clienteventingv1alpha1 "knative.dev/client/pkg/eventing/v1alpha1" clientservingv1 "knative.dev/client/pkg/serving/v1" ) @@ -145,13 +146,13 @@ func (params *KnParams) RestConfig() (*rest.Config, error) { if params.ClientConfig == nil { params.ClientConfig, err = params.GetClientConfig() if err != nil { - return nil, err + return nil, knerrors.GetError(err) } } config, err := params.ClientConfig.ClientConfig() if err != nil { - return nil, err + return nil, knerrors.GetError(err) } if params.LogHTTP { // TODO: When we update to the newer version of client-go, replace with diff --git a/pkg/kn/commands/types_test.go b/pkg/kn/commands/types_test.go index 58c14db27a..e83a52f427 100644 --- a/pkg/kn/commands/types_test.go +++ b/pkg/kn/commands/types_test.go @@ -61,7 +61,7 @@ func TestPrepareConfig(t *testing.T) { for i, tc := range []configTestCase{ { clientcmd.NewDefaultClientConfig(clientcmdapi.Config{}, &clientcmd.ConfigOverrides{}), - "no configuration has been provided", + "no kubeconfig has been provided, please use a valid configuration to connect to the cluster", false, }, { @@ -151,7 +151,7 @@ func TestNewSourcesClient(t *testing.T) { for i, tc := range []configTestCase{ { clientcmd.NewDefaultClientConfig(clientcmdapi.Config{}, &clientcmd.ConfigOverrides{}), - "no configuration has been provided", + "no kubeconfig has been provided, please use a valid configuration to connect to the cluster", false, }, { @@ -202,7 +202,7 @@ func TestNewDynamicClient(t *testing.T) { for i, tc := range []configTestCase{ { clientcmd.NewDefaultClientConfig(clientcmdapi.Config{}, &clientcmd.ConfigOverrides{}), - "no configuration has been provided", + "no kubeconfig has been provided, please use a valid configuration to connect to the cluster", false, }, { From a0f13548a6cbefc24cbbde25f3b01efd1095ab49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Tue, 10 Mar 2020 18:04:28 +0100 Subject: [PATCH 6/6] chore(service export): Rename `--history` to `--with-revisions` (#729) --- docs/cmd/kn_service_export.md | 2 +- pkg/kn/commands/service/export.go | 22 +++++++++++----------- pkg/kn/commands/service/export_test.go | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/cmd/kn_service_export.md b/docs/cmd/kn_service_export.md index f9491301d8..5c2b726699 100644 --- a/docs/cmd/kn_service_export.md +++ b/docs/cmd/kn_service_export.md @@ -25,10 +25,10 @@ kn service export NAME [flags] ``` --allow-missing-template-keys If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true) -h, --help help for export - -r, --history Export all active revisions -n, --namespace string Specify the namespace to operate in. -o, --output string Output format. One of: json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-file. --template string Template string or path to template file to use when -o=go-template, -o=go-template-file. The template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview]. + --with-revisions Export all routed revisions (experimental) ``` ### Options inherited from parent commands diff --git a/pkg/kn/commands/service/export.go b/pkg/kn/commands/service/export.go index a2cf1aa726..1b34abb92f 100644 --- a/pkg/kn/commands/service/export.go +++ b/pkg/kn/commands/service/export.go @@ -70,7 +70,7 @@ func NewServiceExportCommand(p *commands.KnParams) *cobra.Command { return err } - history, err := cmd.Flags().GetBool("history") + withRevisions, err := cmd.Flags().GetBool("with-revisions") if err != nil { return err } @@ -80,8 +80,8 @@ func NewServiceExportCommand(p *commands.KnParams) *cobra.Command { return err } - if history { - if svcList, err := exportServicewithActiveRevisions(service, client); err != nil { + if withRevisions { + if svcList, err := exportServiceWithActiveRevisions(service, client); err != nil { return err } else { return printer.PrintObj(svcList, cmd.OutOrStdout()) @@ -92,7 +92,7 @@ func NewServiceExportCommand(p *commands.KnParams) *cobra.Command { } flags := command.Flags() commands.AddNamespaceFlags(flags, false) - flags.BoolP("history", "r", false, "Export all active revisions") + flags.Bool("with-revisions", false, "Export all routed revisions (experimental)") machineReadablePrintFlags.AddFlags(command) return command } @@ -135,22 +135,19 @@ func constructServicefromRevision(latestSvc *servingv1.Service, revision serving return exportedSvc } -func exportServicewithActiveRevisions(latestSvc *servingv1.Service, client clientservingv1.KnServingClient) (*servingv1.ServiceList, error) { +func exportServiceWithActiveRevisions(latestSvc *servingv1.Service, client clientservingv1.KnServingClient) (*servingv1.ServiceList, error) { var exportedSvcItems []servingv1.Service //get revisions to export from traffic revsMap := getRevisionstoExport(latestSvc) - var params []clientservingv1.ListConfig - params = append(params, clientservingv1.WithService(latestSvc.ObjectMeta.Name)) - // Query for list with filters - revisionList, err := client.ListRevisions(params...) + revisionList, err := client.ListRevisions(clientservingv1.WithService(latestSvc.ObjectMeta.Name)) if err != nil { return nil, err } if len(revisionList.Items) == 0 { - return nil, fmt.Errorf("No revisions found for the service %s", latestSvc.ObjectMeta.Name) + return nil, fmt.Errorf("no revisions found for the service %s", latestSvc.ObjectMeta.Name) } // sort revisions to main the order of generations @@ -163,6 +160,10 @@ func exportServicewithActiveRevisions(latestSvc *servingv1.Service, client clien } } + if len(exportedSvcItems) == 0 { + return nil, fmt.Errorf("no revisions found for service %s", latestSvc.ObjectMeta.Name) + } + //set traffic in the latest revision exportedSvcItems[len(exportedSvcItems)-1] = setTrafficSplit(latestSvc, exportedSvcItems[len(exportedSvcItems)-1]) @@ -181,7 +182,6 @@ func exportServicewithActiveRevisions(latestSvc *servingv1.Service, client clien func setTrafficSplit(latestSvc *servingv1.Service, exportedSvc servingv1.Service) servingv1.Service { exportedSvc.Spec.RouteSpec = latestSvc.Spec.RouteSpec - return exportedSvc } diff --git a/pkg/kn/commands/service/export_test.go b/pkg/kn/commands/service/export_test.go index 6b9be103a2..59662281e5 100644 --- a/pkg/kn/commands/service/export_test.go +++ b/pkg/kn/commands/service/export_test.go @@ -171,7 +171,7 @@ func callServiceExportHistoryTest(t *testing.T, expectedService *servingv1.Servi r.ListRevisions(mock.Any(), revs, nil) - output, err := executeServiceCommand(client, "export", expectedService.ObjectMeta.Name, "-r", "-o", "json") + output, err := executeServiceCommand(client, "export", expectedService.ObjectMeta.Name, "--with-revisions", "-o", "json") assert.NilError(t, err)