From 4874b9a384bca8aff1a07579cdf7956d10fd1a63 Mon Sep 17 00:00:00 2001 From: Naomi Seyfer Date: Tue, 5 Nov 2019 17:43:02 -0800 Subject: [PATCH] Human-readable revision describe (#475) * Revision describe rebaseable * Fix up for rebase * Some tests * moretests * Forgot part of the refactor! * Fix unit tests * Change e2e tests and respond to feedback --- docs/cmd/kn_revision_describe.md | 3 +- pkg/kn/commands/describe.go | 44 ++- pkg/kn/commands/describe_test.go | 10 +- pkg/kn/commands/revision/describe.go | 248 ++++++++++++++++- pkg/kn/commands/revision/describe_test.go | 98 ++++++- pkg/kn/commands/service/describe.go | 321 +++------------------- pkg/printers/prefixwriter.go | 4 +- pkg/serving/revision_template.go | 55 ++++ pkg/serving/revision_template_test.go | 73 +++++ test/e2e/basic_workflow_test.go | 8 +- vendor/modules.txt | 2 +- 11 files changed, 540 insertions(+), 326 deletions(-) create mode 100644 pkg/serving/revision_template_test.go diff --git a/docs/cmd/kn_revision_describe.md b/docs/cmd/kn_revision_describe.md index 75c21cbb0..acef275f6 100644 --- a/docs/cmd/kn_revision_describe.md +++ b/docs/cmd/kn_revision_describe.md @@ -16,8 +16,9 @@ kn revision describe 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 describe -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. (default "yaml") + -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]. + -v, --verbose More output. ``` ### Options inherited from parent commands diff --git a/pkg/kn/commands/describe.go b/pkg/kn/commands/describe.go index ca4b03d27..476cf724c 100644 --- a/pkg/kn/commands/describe.go +++ b/pkg/kn/commands/describe.go @@ -34,8 +34,8 @@ const TruncateAt = 100 func WriteMetadata(dw printers.PrefixWriter, m *metav1.ObjectMeta, printDetails bool) { dw.WriteAttribute("Name", m.Name) dw.WriteAttribute("Namespace", m.Namespace) - WriteMapDesc(dw, m.Labels, l("Labels"), "", printDetails) - WriteMapDesc(dw, m.Annotations, l("Annotations"), "", printDetails) + WriteMapDesc(dw, m.Labels, "Labels", printDetails) + WriteMapDesc(dw, m.Annotations, "Annotations", printDetails) dw.WriteAttribute("Age", Age(m.CreationTimestamp.Time)) } @@ -52,7 +52,7 @@ func keyIsBoring(k string) bool { // Write a map either compact in a single line (possibly truncated) or, if printDetails is set, // over multiple line, one line per key-value pair. The output is sorted by keys. -func WriteMapDesc(dw printers.PrefixWriter, m map[string]string, label string, labelPrefix string, details bool) { +func WriteMapDesc(dw printers.PrefixWriter, m map[string]string, label string, details bool) { if len(m) == 0 { return } @@ -69,16 +69,17 @@ func WriteMapDesc(dw printers.PrefixWriter, m map[string]string, label string, l sort.Strings(keys) if details { - l := labelPrefix + label - - for _, key := range keys { + for i, key := range keys { + l := "" + if i == 0 { + l = printers.Label(label) + } dw.WriteColsLn(l, key+"="+m[key]) - l = labelPrefix } return } - dw.WriteColsLn(label, joinAndTruncate(keys, m, TruncateAt-len(label)-2)) + dw.WriteColsLn(printers.Label(label), joinAndTruncate(keys, m, TruncateAt-len(label)-2)) } func Age(t time.Time) string { @@ -180,9 +181,30 @@ func WriteConditions(dw printers.PrefixWriter, conditions []apis.Condition, prin } } -// Format label (extracted so that color could be added more easily to all labels) -func l(label string) string { - return label + ":" +// Writer a slice compact (printDetails == false) in one line, or over multiple line +// with key-value line-by-line (printDetails == true) +func WriteSliceDesc(dw printers.PrefixWriter, s []string, label string, printDetails bool) { + + if len(s) == 0 { + return + } + + if printDetails { + for i, value := range s { + if i == 0 { + dw.WriteColsLn(printers.Label(label), value) + } else { + dw.WriteColsLn("", value) + } + } + return + } + + joined := strings.Join(s, ", ") + if len(joined) > TruncateAt { + joined = joined[:TruncateAt-4] + " ..." + } + dw.WriteAttribute(label, joined) } // Join to key=value pair, comma separated, and truncate if longer than a limit diff --git a/pkg/kn/commands/describe_test.go b/pkg/kn/commands/describe_test.go index 7b5d79d91..056391ade 100644 --- a/pkg/kn/commands/describe_test.go +++ b/pkg/kn/commands/describe_test.go @@ -36,15 +36,15 @@ var testMap = map[string]string{ func TestWriteMapDesc(t *testing.T) { buf := &bytes.Buffer{} dw := printers.NewBarePrefixWriter(buf) - WriteMapDesc(dw, testMap, "eggs", "", false) - assert.Equal(t, buf.String(), "eggs\ta=b, c=d, foo=bar\n") + WriteMapDesc(dw, testMap, "eggs", false) + assert.Equal(t, buf.String(), "eggs:\ta=b, c=d, foo=bar\n") } func TestWriteMapDescDetailed(t *testing.T) { buf := &bytes.Buffer{} dw := printers.NewBarePrefixWriter(buf) - WriteMapDesc(dw, testMap, "eggs", "", true) - assert.Equal(t, buf.String(), "eggs\ta=b\n\tc=d\n\tfoo=bar\n\tserving.knative.dev/funky=chicken\n") + WriteMapDesc(dw, testMap, "eggs", true) + assert.Equal(t, buf.String(), "eggs:\ta=b\n\tc=d\n\tfoo=bar\n\tserving.knative.dev/funky=chicken\n") } func TestWriteMapTruncated(t *testing.T) { @@ -55,7 +55,7 @@ func TestWriteMapTruncated(t *testing.T) { for i := 0; i < 1000; i++ { items[strconv.Itoa(i)] = strconv.Itoa(i + 1) } - WriteMapDesc(dw, items, "eggs", "", false) + WriteMapDesc(dw, items, "eggs", false) assert.Assert(t, len(strings.TrimSpace(buf.String())) <= TruncateAt) } diff --git a/pkg/kn/commands/revision/describe.go b/pkg/kn/commands/revision/describe.go index 03efa66c1..bf83b34e0 100644 --- a/pkg/kn/commands/revision/describe.go +++ b/pkg/kn/commands/revision/describe.go @@ -16,15 +16,31 @@ package revision import ( "errors" + "fmt" + "io" + "regexp" + "strconv" + "strings" "github.com/spf13/cobra" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/cli-runtime/pkg/genericclioptions" "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/printers" + clientserving "knative.dev/client/pkg/serving" + servingserving "knative.dev/serving/pkg/apis/serving" + "knative.dev/serving/pkg/apis/serving/v1alpha1" ) +// Matching image digest +var imageDigestRegexp = regexp.MustCompile(`(?i)sha256:([0-9a-f]{64})`) + func NewRevisionDescribeCommand(p *commands.KnParams) *cobra.Command { - revisionDescribePrintFlags := genericclioptions.NewPrintFlags("").WithDefaultOutput("yaml") - revisionDescribeCmd := &cobra.Command{ + + // For machine readable output + machineReadablePrintFlags := genericclioptions.NewPrintFlags("") + + command := &cobra.Command{ Use: "describe NAME", Short: "Describe revisions.", RunE: func(cmd *cobra.Command, args []string) error { @@ -46,19 +62,229 @@ func NewRevisionDescribeCommand(p *commands.KnParams) *cobra.Command { return err } - printer, err := revisionDescribePrintFlags.ToPrinter() + if machineReadablePrintFlags.OutputFlagSpecified() { + printer, err := machineReadablePrintFlags.ToPrinter() + if err != nil { + return err + } + return printer.PrintObj(revision, cmd.OutOrStdout()) + } + printDetails, err := cmd.Flags().GetBool("verbose") if err != nil { return err } - - err = printer.PrintObj(revision, cmd.OutOrStdout()) - if err != nil { - return err + var service *v1alpha1.Service + serviceName, ok := revision.Labels[servingserving.ServiceLabelKey] + if printDetails && ok { + service, err = client.GetService(serviceName) + if err != nil { + return err + } } - return nil + // Do the human-readable printing thing. + return describe(cmd.OutOrStdout(), revision, service, printDetails) }, } - commands.AddNamespaceFlags(revisionDescribeCmd.Flags(), false) - revisionDescribePrintFlags.AddFlags(revisionDescribeCmd) - return revisionDescribeCmd + flags := command.Flags() + commands.AddNamespaceFlags(flags, false) + machineReadablePrintFlags.AddFlags(command) + flags.BoolP("verbose", "v", false, "More output.") + return command +} + +func describe(w io.Writer, revision *v1alpha1.Revision, service *v1alpha1.Service, printDetails bool) error { + dw := printers.NewPrefixWriter(w) + commands.WriteMetadata(dw, &revision.ObjectMeta, printDetails) + WriteImage(dw, revision) + WritePort(dw, revision) + WriteEnv(dw, revision, printDetails) + WriteScale(dw, revision) + WriteConcurrencyOptions(dw, revision) + WriteResources(dw, revision) + serviceName, ok := revision.Labels[servingserving.ServiceLabelKey] + if ok { + serviceSection := dw.WriteAttribute("Service", serviceName) + if printDetails { + serviceSection.WriteAttribute("Configuration Generation", revision.Labels[servingserving.ConfigurationGenerationLabelKey]) + serviceSection.WriteAttribute("Latest Created", strconv.FormatBool(revision.Name == service.Status.LatestCreatedRevisionName)) + serviceSection.WriteAttribute("Latest Ready", strconv.FormatBool(revision.Name == service.Status.LatestReadyRevisionName)) + percent, tags := trafficForRevision(revision.Name, service) + if percent != 0 { + serviceSection.WriteAttribute("Traffic", strconv.FormatInt(int64(percent), 10)+"%") + } + if len(tags) > 0 { + commands.WriteSliceDesc(serviceSection, tags, "Tags", printDetails) + } + } + + } + dw.WriteLine() + commands.WriteConditions(dw, revision.Status.Conditions, printDetails) + if err := dw.Flush(); err != nil { + return err + } + return nil +} + +func WriteConcurrencyOptions(dw printers.PrefixWriter, revision *v1alpha1.Revision) { + target := clientserving.ConcurrencyTarget(&revision.ObjectMeta) + limit := revision.Spec.ContainerConcurrency + if target != nil || limit != nil && *limit != 0 { + section := dw.WriteAttribute("Concurrency", "") + if limit != nil && *limit != 0 { + section.WriteAttribute("Limit", strconv.FormatInt(int64(*limit), 10)) + } + if target != nil { + section.WriteAttribute("Target", strconv.Itoa(*target)) + } + } +} + +// Write the image attribute (with +func WriteImage(dw printers.PrefixWriter, revision *v1alpha1.Revision) { + c, err := clientserving.ContainerOfRevisionSpec(&revision.Spec) + if err != nil { + dw.WriteAttribute("Image", "Unknown") + return + } + image := c.Image + // Check if the user image is likely a more user-friendly description + pinnedDesc := "at" + userImage := clientserving.UserImage(&revision.ObjectMeta) + imageDigest := revision.Status.ImageDigest + if userImage != "" && imageDigest != "" { + var parts []string + if strings.Contains(image, "@") { + parts = strings.Split(image, "@") + } else { + parts = strings.Split(image, ":") + } + // Check if the user image refers to the same thing. + if strings.HasPrefix(userImage, parts[0]) { + pinnedDesc = "pinned to" + image = userImage + } + } + if imageDigest != "" { + image = fmt.Sprintf("%s (%s %s)", image, pinnedDesc, shortenDigest(imageDigest)) + } + dw.WriteAttribute("Image", image) +} + +func WritePort(dw printers.PrefixWriter, revision *v1alpha1.Revision) { + port := clientserving.Port(&revision.Spec) + if port != nil { + dw.WriteAttribute("Port", strconv.FormatInt(int64(*port), 10)) + } +} + +func WriteEnv(dw printers.PrefixWriter, revision *v1alpha1.Revision, printDetails bool) { + env := stringifyEnv(revision) + if env != nil { + commands.WriteSliceDesc(dw, env, "Env", printDetails) + } +} + +func WriteScale(dw printers.PrefixWriter, revision *v1alpha1.Revision) { + // Scale spec if given + scale, err := clientserving.ScalingInfo(&revision.ObjectMeta) + if err != nil { + dw.WriteAttribute("Scale", fmt.Sprintf("Misformatted: %v", err)) + } + if scale != nil && (scale.Max != nil || scale.Min != nil) { + dw.WriteAttribute("Scale", formatScale(scale.Min, scale.Max)) + } +} + +func WriteResources(dw printers.PrefixWriter, r *v1alpha1.Revision) { + c, err := clientserving.ContainerOfRevisionSpec(&r.Spec) + if err != nil { + return + } + requests := c.Resources.Requests + limits := c.Resources.Limits + writeResourcesHelper(dw, "Memory", requests.Memory(), limits.Memory()) + writeResourcesHelper(dw, "CPU", requests.Cpu(), limits.Cpu()) +} + +// Write request ... limits or only one of them +func writeResourcesHelper(dw printers.PrefixWriter, label string, request *resource.Quantity, limit *resource.Quantity) { + value := "" + if !request.IsZero() && !limit.IsZero() { + value = request.String() + " ... " + limit.String() + } else if !request.IsZero() { + value = request.String() + } else if !limit.IsZero() { + value = limit.String() + } + + if value == "" { + return + } + + dw.WriteAttribute(label, value) +} + +// Extract pure sha sum and shorten to 8 digits, +// as the digest should to be user consumable. Use the resource via `kn service get` +// to get to the full sha +func shortenDigest(digest string) string { + match := imageDigestRegexp.FindStringSubmatch(digest) + if len(match) > 1 { + return string(match[1][:6]) + } + return digest +} + +// Format scale in the format "min ... max" with max = ∞ if not set +func formatScale(minScale *int, maxScale *int) string { + ret := "0" + if minScale != nil { + ret = strconv.Itoa(*minScale) + } + + ret += " ... " + + if maxScale != nil { + ret += strconv.Itoa(*maxScale) + } else { + ret += "∞" + } + return ret +} + +func stringifyEnv(revision *v1alpha1.Revision) []string { + container, err := clientserving.ContainerOfRevisionSpec(&revision.Spec) + if err != nil { + return nil + } + + envVars := make([]string, 0, len(container.Env)) + for _, env := range container.Env { + value := env.Value + if env.ValueFrom != nil { + value = "[ref]" + } + envVars = append(envVars, fmt.Sprintf("%s=%s", env.Name, value)) + } + return envVars +} + +func trafficForRevision(name string, service *v1alpha1.Service) (int64, []string) { + if len(service.Status.Traffic) == 0 { + return 0, nil + } + var percent int64 + tags := []string{} + for _, target := range service.Status.Traffic { + if target.RevisionName == name { + if target.Percent != nil { + percent += *target.Percent + } + if target.Tag != "" { + tags = append(tags, target.Tag) + } + } + } + return percent, tags } diff --git a/pkg/kn/commands/revision/describe_test.go b/pkg/kn/commands/revision/describe_test.go index e1ed97b68..883ba0acc 100644 --- a/pkg/kn/commands/revision/describe_test.go +++ b/pkg/kn/commands/revision/describe_test.go @@ -16,18 +16,30 @@ package revision import ( "encoding/json" + "fmt" "testing" + "time" - corev1 "k8s.io/api/core/v1" + "gotest.tools/assert" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" client_testing "k8s.io/client-go/testing" "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/util" + "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" + api_serving "knative.dev/serving/pkg/apis/serving" + servingv1 "knative.dev/serving/pkg/apis/serving/v1" "knative.dev/serving/pkg/apis/serving/v1alpha1" "sigs.k8s.io/yaml" ) +const ( + imageDigest = "sha256:1234567890123456789012345678901234567890123456789012345678901234" +) + func fakeRevision(args []string, response *v1alpha1.Revision) (action client_testing.Action, output string, err error) { knParams := &commands.KnParams{} cmd, fakeServing, buf := commands.CreateTestKnCommand(NewRevisionCommand(knParams), knParams) @@ -60,7 +72,7 @@ func TestDescribeRevisionYaml(t *testing.T) { Namespace: "default", }, Spec: v1alpha1.RevisionSpec{ - DeprecatedContainer: &corev1.Container{ + DeprecatedContainer: &v1.Container{ Name: "some-container", Image: "knative/test:latest", }, @@ -70,7 +82,7 @@ func TestDescribeRevisionYaml(t *testing.T) { }, } - action, data, err := fakeRevision([]string{"revision", "describe", "test-rev"}, &expectedRevision) + action, data, err := fakeRevision([]string{"revision", "describe", "test-rev", "-o", "yaml"}, &expectedRevision) if err != nil { t.Fatal(err) } @@ -96,3 +108,83 @@ func TestDescribeRevisionYaml(t *testing.T) { t.Fatal("mismatched objects") } } + +func TestDescribeRevisionBasic(t *testing.T) { + expectedRevision := createTestRevision("test-rev", 3) + + action, data, err := fakeRevision([]string{"revision", "describe", "test-rev"}, &expectedRevision) + if err != nil { + t.Fatal(err) + } + + if action == nil { + t.Fatal("No action") + } else if !action.Matches("get", "revisions") { + t.Fatalf("Bad action %v", action) + } + + assert.Assert(t, util.ContainsAll(data, "Image:", "gcr.io/test/image", "++ Ready", "Port:", "8080")) +} + +func createTestRevision(revision string, gen int64) v1alpha1.Revision { + labels := make(map[string]string) + labels[api_serving.ConfigurationGenerationLabelKey] = fmt.Sprintf("%d", gen) + + return v1alpha1.Revision{ + TypeMeta: metav1.TypeMeta{ + Kind: "Revision", + APIVersion: "knative.dev/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: revision, + Namespace: "default", + Generation: 1, + Labels: labels, + Annotations: make(map[string]string), + CreationTimestamp: metav1.Time{Time: time.Now()}, + }, + Spec: v1alpha1.RevisionSpec{ + RevisionSpec: servingv1.RevisionSpec{ + PodSpec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: "gcr.io/test/image", + Env: []v1.EnvVar{ + {Name: "env1", Value: "eval1"}, + {Name: "env2", Value: "eval2"}, + }, + Ports: []v1.ContainerPort{ + {ContainerPort: 8080}, + }, + }, + }, + }, + }, + }, + Status: v1alpha1.RevisionStatus{ + ImageDigest: "gcr.io/test/image@" + imageDigest, + Status: duckv1.Status{ + Conditions: goodConditions(), + }, + }, + } +} + +func goodConditions() duckv1.Conditions { + ret := make(duckv1.Conditions, 0) + ret = append(ret, apis.Condition{ + Type: apis.ConditionReady, + Status: v1.ConditionTrue, + LastTransitionTime: apis.VolatileTime{ + Inner: metav1.Time{Time: time.Now()}, + }, + }) + ret = append(ret, apis.Condition{ + Type: v1alpha1.ServiceConditionRoutesReady, + Status: v1.ConditionTrue, + LastTransitionTime: apis.VolatileTime{ + Inner: metav1.Time{Time: time.Now()}, + }, + }) + return ret +} diff --git a/pkg/kn/commands/service/describe.go b/pkg/kn/commands/service/describe.go index 6b7571517..ef2ce4024 100644 --- a/pkg/kn/commands/service/describe.go +++ b/pkg/kn/commands/service/describe.go @@ -18,27 +18,22 @@ import ( "errors" "fmt" "io" - "regexp" "sort" "strconv" - "strings" - "time" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/cli-runtime/pkg/genericclioptions" - "knative.dev/serving/pkg/apis/autoscaling" - "knative.dev/serving/pkg/apis/serving" - + "knative.dev/client/pkg/kn/commands/revision" "knative.dev/client/pkg/printers" - client_serving "knative.dev/client/pkg/serving" serving_kn_v1alpha1 "knative.dev/client/pkg/serving/v1alpha1" - - "knative.dev/serving/pkg/apis/serving/v1alpha1" + "knative.dev/serving/pkg/apis/serving" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" "knative.dev/client/pkg/kn/commands" + "knative.dev/pkg/apis" + "knative.dev/serving/pkg/apis/serving/v1alpha1" ) // Command for printing out a description of a service, meant to be consumed by humans @@ -48,48 +43,20 @@ import ( // Whether to print extended information var printDetails bool -// Matching image digest -var imageDigestRegexp = regexp.MustCompile(`(?i)sha256:([0-9a-f]{64})`) - // View object for collecting revision related information in the context // of a Service. These are plain data types which can be directly used // for printing out type revisionDesc struct { - name string - configuration string - configurationGeneration int - creationTimestamp time.Time + revision *v1alpha1.Revision // traffic stuff percent int64 tag string latestTraffic *bool - // basic revision stuff - logURL string - timeoutSeconds *int64 - - image string - userImage string - imageDigest string - env []string - port *int32 - - // concurrency options - maxScale *int - minScale *int - concurrencyTarget *int - concurrencyLimit *int64 - - // resource options - requestsMemory string - requestsCPU string - limitsMemory string - limitsCPU string + configurationGeneration int // status info - ready corev1.ConditionStatus - reason string latestCreated bool latestReady bool } @@ -211,44 +178,31 @@ func writeRevisions(dw printers.PrefixWriter, revisions []*revisionDesc, printDe revSection := dw.WriteAttribute("Revisions", "") dw.Flush() for _, revisionDesc := range revisions { - section := revSection.WriteColsLn(formatBullet(revisionDesc.percent, revisionDesc.ready), revisionHeader(revisionDesc)) - if revisionDesc.ready == v1.ConditionFalse { - section.WriteAttribute("Error", revisionDesc.reason) + ready := apis.Condition{ + Type: apis.ConditionReady, + Status: v1.ConditionUnknown, } - section.WriteAttribute("Image", getImageDesc(revisionDesc)) + for _, cond := range revisionDesc.revision.Status.Conditions { + if cond.Type == apis.ConditionReady { + ready = cond + break + } + } + section := revSection.WriteColsLn(formatBullet(revisionDesc.percent, ready.Status), revisionHeader(revisionDesc)) + if ready.Status == v1.ConditionFalse { + section.WriteAttribute("Error", ready.Reason) + } + revision.WriteImage(section, revisionDesc.revision) if printDetails { - if revisionDesc.port != nil { - section.WriteAttribute("Port", strconv.FormatInt(int64(*revisionDesc.port), 10)) - } - writeSliceDesc(section, revisionDesc.env, l("Env"), "") - - // Scale spec if given - if revisionDesc.maxScale != nil || revisionDesc.minScale != nil { - section.WriteAttribute("Scale", formatScale(revisionDesc.minScale, revisionDesc.maxScale)) - } - - // Concurrency specs if given - if revisionDesc.concurrencyLimit != nil || revisionDesc.concurrencyTarget != nil { - writeConcurrencyOptions(section, revisionDesc) - } - - // Resources if given - writeResources(section, "Memory", revisionDesc.requestsMemory, revisionDesc.limitsMemory) - writeResources(section, "CPU", revisionDesc.requestsCPU, revisionDesc.limitsCPU) + revision.WritePort(section, revisionDesc.revision) + revision.WriteEnv(section, revisionDesc.revision, printDetails) + revision.WriteScale(section, revisionDesc.revision) + revision.WriteConcurrencyOptions(section, revisionDesc.revision) + revision.WriteResources(section, revisionDesc.revision) } } } -func writeConcurrencyOptions(dw printers.PrefixWriter, desc *revisionDesc) { - section := dw.WriteAttribute("Concurrency", "") - if desc.concurrencyLimit != nil { - section.WriteAttribute("Limit", strconv.FormatInt(*desc.concurrencyLimit, 10)) - } - if desc.concurrencyTarget != nil { - section.WriteAttribute("Target", strconv.Itoa(*desc.concurrencyTarget)) - } -} - // ====================================================================================== // Helper functions @@ -257,32 +211,15 @@ func l(label string) string { return label + ":" } -// Format scale in the format "min ... max" with max = ∞ if not set -func formatScale(minScale *int, maxScale *int) string { - ret := "0" - if minScale != nil { - ret = strconv.Itoa(*minScale) - } - - ret += " ... " - - if maxScale != nil { - ret += strconv.Itoa(*maxScale) - } else { - ret += "∞" - } - return ret -} - // Format the revision name along with its generation. Use colors if enabled. func revisionHeader(desc *revisionDesc) string { - header := desc.name + header := desc.revision.Name if desc.latestTraffic != nil && *desc.latestTraffic { - header = fmt.Sprintf("@latest (%s)", desc.name) + header = fmt.Sprintf("@latest (%s)", desc.revision.Name) } else if desc.latestReady { - header = desc.name + " (current @latest)" + header = desc.revision.Name + " (current @latest)" } else if desc.latestCreated { - header = desc.name + " (latest created)" + header = desc.revision.Name + " (latest created)" } if desc.tag != "" { header = fmt.Sprintf("%s #%s", header, desc.tag) @@ -290,79 +227,7 @@ func revisionHeader(desc *revisionDesc) string { return header + " " + "[" + strconv.Itoa(desc.configurationGeneration) + "]" + " " + - "(" + commands.Age(desc.creationTimestamp) + ")" -} - -// Return either image name with tag or together with its resolved digest -func getImageDesc(desc *revisionDesc) string { - image := desc.image - // Check if the user image is likely a more user-friendly description - pinnedDesc := "at" - if desc.userImage != "" && desc.imageDigest != "" { - parts := strings.Split(image, "@") - // Check if the user image refers to the same thing. - if strings.HasPrefix(desc.userImage, parts[0]) { - pinnedDesc = "pinned to" - image = desc.userImage - } - } - if desc.imageDigest != "" { - return fmt.Sprintf("%s (%s %s)", image, pinnedDesc, shortenDigest(desc.imageDigest)) - } - return image -} - -// Extract pure sha sum and shorten to 8 digits, -// as the digest should to be user consumable. Use the resource via `kn service get` -// to get to the full sha -func shortenDigest(digest string) string { - match := imageDigestRegexp.FindStringSubmatch(digest) - if len(match) > 1 { - return string(match[1][:6]) - } - return digest -} - -// Writer a slice compact (printDetails == false) in one line, or over multiple line -// with key-value line-by-line (printDetails == true) -func writeSliceDesc(dw printers.PrefixWriter, s []string, label string, labelPrefix string) { - - if len(s) == 0 { - return - } - - if printDetails { - l := labelPrefix + label - for _, value := range s { - dw.WriteColsLn(l, value) - l = labelPrefix - } - return - } - - joined := strings.Join(s, ", ") - if len(joined) > commands.TruncateAt { - joined = joined[:commands.TruncateAt-4] + " ..." - } - dw.WriteAttribute(labelPrefix+label, joined) -} - -// Write request ... limits or only one of them -func writeResources(dw printers.PrefixWriter, label string, request string, limit string) { - value := "" - if request != "" && limit != "" { - value = request + " ... " + limit - } else if request != "" { - value = request - } else if limit != "" { - value = limit - } - - if value == "" { - return - } - - dw.WriteAttribute(label, value) + "(" + commands.Age(desc.revision.CreationTimestamp.Time) + ")" } // Format target percentage that it fits in the revision table @@ -463,148 +328,34 @@ func completeWithUntargetedRevisions(client serving_kn_v1alpha1.KnServingClient, } func newRevisionDesc(revision *v1alpha1.Revision, target *v1alpha1.TrafficTarget, service *v1alpha1.Service) (*revisionDesc, error) { - container := extractContainer(revision) generation, err := strconv.ParseInt(revision.Labels[serving.ConfigurationGenerationLabelKey], 0, 0) if err != nil { return nil, fmt.Errorf("cannot extract configuration generation for revision %s: %v", revision.Name, err) } revisionDesc := revisionDesc{ - name: revision.Name, - logURL: revision.Status.LogURL, - timeoutSeconds: revision.Spec.TimeoutSeconds, - userImage: revision.Annotations[client_serving.UserImageAnnotationKey], - imageDigest: revision.Status.ImageDigest, - creationTimestamp: revision.CreationTimestamp.Time, - + revision: revision, configurationGeneration: int(generation), - configuration: revision.Labels[serving.ConfigurationLabelKey], - - latestCreated: revision.Name == service.Status.LatestCreatedRevisionName, - latestReady: revision.Name == service.Status.LatestReadyRevisionName, + latestCreated: revision.Name == service.Status.LatestCreatedRevisionName, + latestReady: revision.Name == service.Status.LatestReadyRevisionName, } - addStatusInfo(&revisionDesc, revision) addTargetInfo(&revisionDesc, target) - addContainerInfo(&revisionDesc, container) - addResourcesInfo(&revisionDesc, container) - err = addConcurrencyAndScaleInfo(&revisionDesc, revision) if err != nil { return nil, err } return &revisionDesc, nil } -func addStatusInfo(desc *revisionDesc, revision *v1alpha1.Revision) { - for _, condition := range revision.Status.Conditions { - if condition.Type == "Ready" { - desc.reason = condition.Reason - desc.ready = condition.Status - } - } -} - func addTargetInfo(desc *revisionDesc, target *v1alpha1.TrafficTarget) { if target != nil { - desc.percent = *target.Percent + if target.Percent != nil { + desc.percent = *target.Percent + } desc.latestTraffic = target.LatestRevision desc.tag = target.Tag } } -func addContainerInfo(desc *revisionDesc, container *v1.Container) { - addImage(desc, container) - addEnv(desc, container) - addPort(desc, container) -} - -func addResourcesInfo(desc *revisionDesc, container *v1.Container) { - requests := container.Resources.Requests - if !requests.Memory().IsZero() { - desc.requestsMemory = requests.Memory().String() - } - if !requests.Cpu().IsZero() { - desc.requestsCPU = requests.Cpu().String() - } - - limits := container.Resources.Limits - if !limits.Memory().IsZero() { - desc.limitsMemory = limits.Memory().String() - } - if !limits.Cpu().IsZero() { - desc.limitsCPU = limits.Cpu().String() - } -} - -func addConcurrencyAndScaleInfo(desc *revisionDesc, revision *v1alpha1.Revision) error { - min, err := annotationAsInt(revision, autoscaling.MinScaleAnnotationKey) - if err != nil { - return err - } - desc.minScale = min - - max, err := annotationAsInt(revision, autoscaling.MaxScaleAnnotationKey) - if err != nil { - return err - } - desc.maxScale = max - - target, err := annotationAsInt(revision, autoscaling.TargetAnnotationKey) - if err != nil { - return err - } - desc.concurrencyTarget = target - - if revision.Spec.ContainerConcurrency != nil { - desc.concurrencyLimit = revision.Spec.ContainerConcurrency - } - - return nil -} - -func annotationAsInt(revision *v1alpha1.Revision, annotationKey string) (*int, error) { - annos := revision.Annotations - if val, ok := annos[annotationKey]; ok { - valInt, err := strconv.Atoi(val) - if err != nil { - return nil, err - } - return &valInt, nil - } - return nil, nil -} - -func addEnv(desc *revisionDesc, container *v1.Container) { - envVars := make([]string, 0, len(container.Env)) - for _, env := range container.Env { - var value string - if env.ValueFrom != nil { - value = "[ref]" - } else { - value = env.Value - } - envVars = append(envVars, fmt.Sprintf("%s=%s", env.Name, value)) - } - desc.env = envVars -} - -func addPort(desc *revisionDesc, container *v1.Container) { - if len(container.Ports) > 0 { - port := container.Ports[0].ContainerPort - desc.port = &port - } -} - -func addImage(desc *revisionDesc, container *v1.Container) { - desc.image = container.Image -} - -func extractContainer(revision *v1alpha1.Revision) *v1.Container { - if revision.Spec.Containers != nil && len(revision.Spec.Containers) > 0 { - return &revision.Spec.Containers[0] - } - return revision.Spec.DeprecatedContainer -} - func extractRevisionFromTarget(client serving_kn_v1alpha1.KnServingClient, target v1alpha1.TrafficTarget) (*v1alpha1.Revision, error) { var revisionName = target.RevisionName if revisionName == "" { diff --git a/pkg/printers/prefixwriter.go b/pkg/printers/prefixwriter.go index 440ab5212..3eb77019a 100644 --- a/pkg/printers/prefixwriter.go +++ b/pkg/printers/prefixwriter.go @@ -108,7 +108,7 @@ func (pw *prefixWriter) WriteLine(a ...interface{}) { // WriteAttribute writes the attr (as a label) with the given value and returns // a PrefixWriter for writing any subattributes. func (pw *prefixWriter) WriteAttribute(attr, value string) PrefixWriter { - pw.WriteColsLn(l(attr), value) + pw.WriteColsLn(Label(attr), value) return &prefixWriter{pw.out, pw, 0, 1} } @@ -120,6 +120,6 @@ func (pw *prefixWriter) Flush() error { } // Format label (extracted so that color could be added more easily to all labels) -func l(label string) string { +func Label(label string) string { return label + ":" } diff --git a/pkg/serving/revision_template.go b/pkg/serving/revision_template.go index c8cf97759..4eb9fbd1d 100644 --- a/pkg/serving/revision_template.go +++ b/pkg/serving/revision_template.go @@ -16,11 +16,19 @@ package serving import ( "fmt" + "strconv" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/serving/pkg/apis/autoscaling" servingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1" ) +type Scaling struct { + Min *int + Max *int +} + func ContainerOfRevisionTemplate(template *servingv1alpha1.RevisionTemplateSpec) (*corev1.Container, error) { return ContainerOfRevisionSpec(&template.Spec) } @@ -36,8 +44,55 @@ func ContainerOfRevisionSpec(revisionSpec *servingv1alpha1.RevisionSpec) (*corev return container, nil } +func ScalingInfo(m *metav1.ObjectMeta) (*Scaling, error) { + ret := &Scaling{} + var err error + ret.Min, err = annotationAsInt(m, autoscaling.MinScaleAnnotationKey) + if err != nil { + return nil, err + } + ret.Max, err = annotationAsInt(m, autoscaling.MaxScaleAnnotationKey) + if err != nil { + return nil, err + } + return ret, nil +} + +func UserImage(m *metav1.ObjectMeta) string { + return m.Annotations[UserImageAnnotationKey] +} + +func ConcurrencyTarget(m *metav1.ObjectMeta) *int { + ret, _ := annotationAsInt(m, autoscaling.TargetAnnotationKey) + return ret +} + +func Port(revisionSpec *servingv1alpha1.RevisionSpec) *int32 { + c, err := ContainerOfRevisionSpec(revisionSpec) + if err != nil { + return nil + } + if len(c.Ports) > 0 { + p := c.Ports[0].ContainerPort + return &p + } + return nil +} + // ======================================================================================= func usesOldV1alpha1ContainerField(revisionSpec *servingv1alpha1.RevisionSpec) bool { return revisionSpec.DeprecatedContainer != nil } + +func annotationAsInt(m *metav1.ObjectMeta, annotationKey string) (*int, error) { + annos := m.Annotations + if val, ok := annos[annotationKey]; ok { + valInt, err := strconv.Atoi(val) + if err != nil { + return nil, err + } + return &valInt, nil + } + return nil, nil +} diff --git a/pkg/serving/revision_template_test.go b/pkg/serving/revision_template_test.go new file mode 100644 index 000000000..32ce88998 --- /dev/null +++ b/pkg/serving/revision_template_test.go @@ -0,0 +1,73 @@ +// 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 serving + +import ( + "testing" + + "gotest.tools/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "knative.dev/serving/pkg/apis/autoscaling" +) + +type scalingInfoTest struct { + min string + max string + minRes int + maxRes int + e bool +} + +func TestScalingInfo(t *testing.T) { + sentinel := -0xdead + tests := []scalingInfoTest{ + {"3", "4", 3, 4, false}, + {"", "5", sentinel, 5, false}, + {"4", "", 4, sentinel, false}, + {"", "", sentinel, sentinel, false}, + {"", "funtimes", sentinel, sentinel, true}, + } + for _, c := range tests { + m := metav1.ObjectMeta{} + m.Annotations = map[string]string{} + if c.min != "" { + m.Annotations[autoscaling.MinScaleAnnotationKey] = c.min + } + if c.max != "" { + m.Annotations[autoscaling.MaxScaleAnnotationKey] = c.max + } + s, err := ScalingInfo(&m) + if c.e { + assert.Assert(t, err != nil) + continue + } else { + assert.NilError(t, err) + } + if c.minRes != sentinel { + assert.Assert(t, s.Min != nil) + assert.Equal(t, c.minRes, *s.Min) + } else { + assert.Assert(t, s.Min == nil) + } + if c.maxRes != sentinel { + assert.Assert(t, s.Max != nil) + assert.Equal(t, c.maxRes, *s.Max) + } else { + assert.Assert(t, s.Max == nil) + } + + } +} diff --git a/test/e2e/basic_workflow_test.go b/test/e2e/basic_workflow_test.go index 4410382fb..68f6388b9 100644 --- a/test/e2e/basic_workflow_test.go +++ b/test/e2e/basic_workflow_test.go @@ -17,7 +17,6 @@ package e2e import ( - "fmt" "strings" "testing" @@ -134,10 +133,5 @@ func (test *e2eTest) revisionDescribe(t *testing.T, serviceName string) { out, err := test.kn.RunWithOpts([]string{"revision", "describe", revName}, runOpts{}) assert.NilError(t, err) - - expectedGVK := `apiVersion: serving.knative.dev/v1alpha1 -kind: Revision` - expectedNamespace := fmt.Sprintf("namespace: %s", test.kn.namespace) - expectedServiceLabel := fmt.Sprintf("serving.knative.dev/service: %s", serviceName) - assert.Check(t, util.ContainsAll(out, expectedGVK, expectedNamespace, expectedServiceLabel)) + assert.Check(t, util.ContainsAll(out, revName, test.kn.namespace, serviceName, "++ Ready", "TARGET=kn")) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 9dc90c147..15e081dd9 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -387,9 +387,9 @@ k8s.io/api/storage/v1beta1 k8s.io/apimachinery/pkg/api/errors k8s.io/apimachinery/pkg/apis/meta/v1 k8s.io/apimachinery/pkg/util/duration +k8s.io/apimachinery/pkg/api/resource k8s.io/apimachinery/pkg/apis/meta/v1beta1 k8s.io/apimachinery/pkg/runtime -k8s.io/apimachinery/pkg/api/resource k8s.io/apimachinery/pkg/util/sets k8s.io/apimachinery/pkg/api/meta k8s.io/apimachinery/pkg/util/runtime