From 73597be504407129d56640777453661846a213e3 Mon Sep 17 00:00:00 2001 From: Michail Kargakis Date: Fri, 27 Nov 2015 15:52:31 +0100 Subject: [PATCH] Add suggestions in oc status --- contrib/completions/bash/oc | 2 + contrib/completions/bash/openshift | 2 + docs/generated/oc_by_example_content.adoc | 8 +- pkg/api/graph/interfaces.go | 12 ++ pkg/build/graph/analysis/bc.go | 14 +-- pkg/build/graph/analysis/bc_test.go | 4 +- pkg/cmd/cli/cmd/status.go | 122 +++++++++++++-------- pkg/cmd/cli/describe/projectstatus.go | 65 ++++++++--- pkg/cmd/cli/describe/projectstatus_test.go | 2 +- pkg/deploy/graph/analysis/dc.go | 18 +-- pkg/deploy/graph/analysis/dc_test.go | 6 +- pkg/route/graph/analysis/analysis.go | 11 +- pkg/route/graph/analysis/analysis_test.go | 2 +- 13 files changed, 179 insertions(+), 89 deletions(-) diff --git a/contrib/completions/bash/oc b/contrib/completions/bash/oc index 644991620c05..d3fb365eba6f 100644 --- a/contrib/completions/bash/oc +++ b/contrib/completions/bash/oc @@ -609,6 +609,8 @@ _oc_status() flags+=("--output=") two_word_flags+=("-o") + flags+=("--verbose") + flags+=("-v") flags+=("--alsologtostderr") flags+=("--api-version=") flags+=("--boot-id-file=") diff --git a/contrib/completions/bash/openshift b/contrib/completions/bash/openshift index 6e3be285f682..11e5bf9e16da 100644 --- a/contrib/completions/bash/openshift +++ b/contrib/completions/bash/openshift @@ -5336,6 +5336,8 @@ _openshift_cli_status() flags+=("--output=") two_word_flags+=("-o") + flags+=("--verbose") + flags+=("-v") flags+=("--alsologtostderr") flags+=("--api-version=") flags+=("--boot-id-file=") diff --git a/docs/generated/oc_by_example_content.adoc b/docs/generated/oc_by_example_content.adoc index 50e0ea538067..01b2ad1945da 100644 --- a/docs/generated/oc_by_example_content.adoc +++ b/docs/generated/oc_by_example_content.adoc @@ -959,8 +959,14 @@ Show an overview of the current project [options="nowrap"] ---- - # Show an overview of the current project + # See an overview of the current project. $ oc status + + # Export the overview of the current project in an svg file. + $ oc status -o dot | dot -T svg -o project.svg + + # See an overview of the current project including details for any identified issues. + $ oc status -v ---- ==== diff --git a/pkg/api/graph/interfaces.go b/pkg/api/graph/interfaces.go index 66ee00dfe03d..c0ba77cca9d7 100644 --- a/pkg/api/graph/interfaces.go +++ b/pkg/api/graph/interfaces.go @@ -1,6 +1,8 @@ package graph import ( + "fmt" + "github.com/gonum/graph" ) @@ -15,8 +17,12 @@ type Marker struct { Severity Severity // Key is a short string to identify this message Key string + // Message is a human-readable string that describes what is interesting Message string + // Suggestion is a human-readable string that holds advice for resolving this + // marker. + Suggestion Suggestion } // Severity indicates how important this problem is. @@ -97,3 +103,9 @@ func (m ByKey) Swap(i, j int) { m[i], m[j] = m[j], m[i] } func (m ByKey) Less(i, j int) bool { return m[i].Key < m[j].Key } + +type Suggestion string + +func (s Suggestion) String() string { + return fmt.Sprintf("try: %s", string(s)) +} diff --git a/pkg/build/graph/analysis/bc.go b/pkg/build/graph/analysis/bc.go index 957c665ee5bc..086578684b28 100644 --- a/pkg/build/graph/analysis/bc.go +++ b/pkg/build/graph/analysis/bc.go @@ -15,9 +15,9 @@ import ( ) const ( - MissingRequiredRegistryWarning = "MissingRequiredRegistry" - MissingImageStreamWarning = "MissingImageStream" - CyclicBuildConfigWarning = "CyclicBuildConfig" + MissingRequiredRegistryErr = "MissingRequiredRegistry" + MissingImageStreamErr = "MissingImageStream" + CyclicBuildConfigWarning = "CyclicBuildConfig" ) // FindUnpushableBuildConfigs checks all build configs that will output to an IST backed by an ImageStream and checks to make sure their builds can push. @@ -35,8 +35,8 @@ bc: Node: bcNode, RelatedNodes: []graph.Node{istNode}, - Severity: osgraph.WarningSeverity, - Key: MissingImageStreamWarning, + Severity: osgraph.ErrorSeverity, + Key: MissingImageStreamErr, Message: fmt.Sprintf("%s is pushing to %s that is using %s, but that image stream does not exist.", bcNode.(*buildgraph.BuildConfigNode).ResourceString(), istNode.(*imagegraph.ImageStreamTagNode).ResourceString(), imageStreamNode.ResourceString()), }) @@ -49,8 +49,8 @@ bc: Node: bcNode, RelatedNodes: []graph.Node{istNode}, - Severity: osgraph.WarningSeverity, - Key: MissingRequiredRegistryWarning, + Severity: osgraph.ErrorSeverity, + Key: MissingRequiredRegistryErr, Message: fmt.Sprintf("%s is pushing to %s that is using %s, but the administrator has not configured the integrated Docker registry. (oadm registry)", bcNode.(*buildgraph.BuildConfigNode).ResourceString(), istNode.(*imagegraph.ImageStreamTagNode).ResourceString(), imageStreamNode.ResourceString()), }) diff --git a/pkg/build/graph/analysis/bc_test.go b/pkg/build/graph/analysis/bc_test.go index 0ad73bbfd021..17604930bcf9 100644 --- a/pkg/build/graph/analysis/bc_test.go +++ b/pkg/build/graph/analysis/bc_test.go @@ -24,7 +24,7 @@ func TestUnpushableBuild(t *testing.T) { t.Fatalf("expected %v, got %v", e, a) } - if got, expected := markers[0].Key, MissingRequiredRegistryWarning; got != expected { + if got, expected := markers[0].Key, MissingRequiredRegistryErr; got != expected { t.Fatalf("expected marker key %q, got %q", expected, got) } @@ -53,7 +53,7 @@ func TestUnpushableBuild(t *testing.T) { t.Fatalf("expected %v, got %v", e, a) } - if got, expected := markers[0].Key, MissingImageStreamWarning; got != expected { + if got, expected := markers[0].Key, MissingImageStreamErr; got != expected { t.Fatalf("expected marker key %q, got %q", expected, got) } } diff --git a/pkg/cmd/cli/cmd/status.go b/pkg/cmd/cli/cmd/status.go index ffafdbfe656f..20006c7c24c8 100644 --- a/pkg/cmd/cli/cmd/status.go +++ b/pkg/cmd/cli/cmd/status.go @@ -1,9 +1,9 @@ package cmd import ( + "errors" "fmt" "io" - "strings" "github.com/gonum/graph/encoding/dot" "github.com/spf13/cobra" @@ -14,9 +14,10 @@ import ( "github.com/openshift/origin/pkg/cmd/util/clientcmd" ) -const ( - StatusRecommendedName = "status" +// StatusRecommendedName is the recommended command name. +const StatusRecommendedName = "status" +const ( statusLong = ` Show a high level overview of the current project @@ -28,36 +29,59 @@ oc describe deploymentConfig, oc describe service). You can specify an output format of "-o dot" to have this command output the generated status graph in DOT format that is suitable for use by the "dot" command.` - statusExample = ` # Show an overview of the current project - $ %[1]s` + statusExample = ` # See an overview of the current project. + $ %[1]s + + # Export the overview of the current project in an svg file. + $ %[1]s -o dot | dot -T svg -o project.svg + + # See an overview of the current project including details for any identified issues. + $ %[1]s -v` ) -// NewCmdStatus implements the OpenShift cli status command +// StatusOptions contains all the necessary options for the Openshift cli status command. +type StatusOptions struct { + namespace string + outputFormat string + describer *describe.ProjectStatusDescriber + out io.Writer + verbose bool +} + +// NewCmdStatus implements the OpenShift cli status command. func NewCmdStatus(name, fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Command { - outputFormat := "" + opts := &StatusOptions{} cmd := &cobra.Command{ - Use: "status", + Use: fmt.Sprintf("%s [-o dot | -v ]", StatusRecommendedName), Short: "Show an overview of the current project", Long: statusLong, Example: fmt.Sprintf(statusExample, fullName), Run: func(cmd *cobra.Command, args []string) { - if strings.ToLower(outputFormat) == "dot" { - cmdutil.CheckErr(RunGraph(f, out)) - return + err := opts.Complete(f, cmd, args, out) + cmdutil.CheckErr(err) + + if err := opts.Validate(); err != nil { + cmdutil.CheckErr(cmdutil.UsageError(cmd, err.Error())) } - cmdutil.CheckErr(RunStatus(f, out)) + err = opts.RunStatus() + cmdutil.CheckErr(err) }, } - cmd.Flags().StringVarP(&outputFormat, "output", "o", outputFormat, "Output format. One of: dot.") + cmd.Flags().StringVarP(&opts.outputFormat, "output", "o", opts.outputFormat, "Output format. One of: dot.") + cmd.Flags().BoolVarP(&opts.verbose, "verbose", "v", opts.verbose, "See details for resolving issues.") return cmd } -// RunStatus contains all the necessary functionality for the OpenShift cli status command -func RunStatus(f *clientcmd.Factory, out io.Writer) error { +// Complete completes the options for the Openshift cli status command. +func (o *StatusOptions) Complete(f *clientcmd.Factory, cmd *cobra.Command, args []string, out io.Writer) error { + if len(args) > 0 { + return cmdutil.UsageError(cmd, "no arguments should be provided") + } + client, kclient, err := f.Clients() if err != nil { return err @@ -72,45 +96,53 @@ func RunStatus(f *clientcmd.Factory, out io.Writer) error { if err != nil { return err } + o.namespace = namespace - describer := &describe.ProjectStatusDescriber{K: kclient, C: client, Server: config.Host} - s, err := describer.Describe(namespace, "") - if err != nil { - return err - } + o.describer = &describe.ProjectStatusDescriber{K: kclient, C: client, Server: config.Host, Suggest: o.verbose} + + o.out = out - fmt.Fprintf(out, s) return nil } -// RunGraph contains all the necessary functionality for the OpenShift cli graph command -func RunGraph(f *clientcmd.Factory, out io.Writer) error { - client, kclient, err := f.Clients() - if err != nil { - return err +// Validate validates the options for the Openshift cli status command. +func (o StatusOptions) Validate() error { + if len(o.outputFormat) != 0 && o.outputFormat != "dot" { + return fmt.Errorf("invalid output format provided: %s", o.outputFormat) } - - config, err := f.OpenShiftClientConfig.ClientConfig() - if err != nil { - return err - } - - namespace, _, err := f.DefaultNamespace() - if err != nil { - return err - } - - describer := &describe.ProjectStatusDescriber{K: kclient, C: client, Server: config.Host} - g, _, err := describer.MakeGraph(namespace) - if err != nil { - return err + if len(o.outputFormat) > 0 && o.verbose { + return errors.New("cannot provide suggestions when output format is dot") } + return nil +} - data, err := dot.Marshal(g, namespace, "", " ", false) - if err != nil { - return err +// RunStatus contains all the necessary functionality for the OpenShift cli status command. +func (o StatusOptions) RunStatus() error { + var ( + s string + err error + ) + + switch o.outputFormat { + case "": + s, err = o.describer.Describe(o.namespace, "") + if err != nil { + return err + } + case "dot": + g, _, err := o.describer.MakeGraph(o.namespace) + if err != nil { + return err + } + data, err := dot.Marshal(g, o.namespace, "", " ", false) + if err != nil { + return err + } + s = string(data) + default: + return fmt.Errorf("invalid output format provided: %s", o.outputFormat) } - fmt.Fprintf(out, "%s", string(data)) + fmt.Fprintf(o.out, s) return nil } diff --git a/pkg/cmd/cli/describe/projectstatus.go b/pkg/cmd/cli/describe/projectstatus.go index 782f724f42ab..844898693af4 100644 --- a/pkg/cmd/cli/describe/projectstatus.go +++ b/pkg/cmd/cli/describe/projectstatus.go @@ -48,9 +48,10 @@ const ForbiddenListWarning = "Forbidden" // ProjectStatusDescriber generates extended information about a Project type ProjectStatusDescriber struct { - K kclient.Interface - C client.Interface - Server string + K kclient.Interface + C client.Interface + Server string + Suggest bool } func (d *ProjectStatusDescriber) MakeGraph(namespace string) (osgraph.Graph, sets.String, error) { @@ -210,32 +211,66 @@ func (d *ProjectStatusDescriber) Describe(namespace, name string) (string, error sort.Stable(osgraph.ByKey(allMarkers)) sort.Stable(osgraph.ByNodeID(allMarkers)) - if errorMarkers := allMarkers.BySeverity(osgraph.ErrorSeverity); len(errorMarkers) > 0 { + + errorMarkers := allMarkers.BySeverity(osgraph.ErrorSeverity) + if len(errorMarkers) > 0 { fmt.Fprintln(out, "Errors:") for _, marker := range errorMarkers { fmt.Fprintln(out, indent+"* "+marker.Message) + if len(marker.Suggestion) > 0 && d.Suggest { + fmt.Fprintln(out, indent+" "+marker.Suggestion.String()) + } } } - if warningMarkers := allMarkers.BySeverity(osgraph.WarningSeverity); len(warningMarkers) > 0 { - fmt.Fprintln(out, "Warnings:") + + warningMarkers := allMarkers.BySeverity(osgraph.WarningSeverity) + if len(warningMarkers) > 0 { + if d.Suggest { + fmt.Fprintln(out, "Warnings:") + } for _, marker := range warningMarkers { - fmt.Fprintln(out, indent+"* "+marker.Message) + if d.Suggest { + fmt.Fprintln(out, indent+"* "+marker.Message) + if len(marker.Suggestion) > 0 { + fmt.Fprintln(out, indent+" "+marker.Suggestion.String()) + } + } } } - if infoMarkers := allMarkers.BySeverity(osgraph.InfoSeverity); len(infoMarkers) > 0 { - fmt.Fprintln(out, "Info:") - for _, marker := range infoMarkers { - fmt.Fprintln(out, indent+"* "+marker.Message) - } + + // We print errors by default and warnings if -v is used. If we get none, + // this would be an extra new line. + if len(errorMarkers) != 0 || (d.Suggest && len(warningMarkers) != 0) { + fmt.Fprintln(out) } - fmt.Fprintln(out) + errors, warnings := "", "" + if len(errorMarkers) == 1 { + errors = "1 error" + } else if len(errorMarkers) > 1 { + errors = fmt.Sprintf("%d errors", len(errorMarkers)) + } + if len(warningMarkers) == 1 { + warnings = "1 warning" + } else if len(warningMarkers) > 1 { + warnings = fmt.Sprintf("%d warnings", len(warningMarkers)) + } + + switch { + case !d.Suggest && len(errorMarkers) > 0 && len(warningMarkers) > 0: + fmt.Fprintf(out, "%s and %s identified, use 'oc status -v' to see details.\n", errors, warnings) - if (len(services) == 0) && (len(standaloneDCs) == 0) && (len(standaloneImages) == 0) { + case !d.Suggest && len(errorMarkers) > 0: + fmt.Fprintf(out, "%s identified, use 'oc status -v' to see details.\n", errors) + + case !d.Suggest && len(warningMarkers) > 0: + fmt.Fprintf(out, "%s identified, use 'oc status -v' to see details.\n", warnings) + + case (len(services) == 0) && (len(standaloneDCs) == 0) && (len(standaloneImages) == 0): fmt.Fprintln(out, "You have no services, deployment configs, or build configs.") fmt.Fprintln(out, "Run 'oc new-app' to create an application.") - } else { + default: fmt.Fprintln(out, "To see more, use 'oc describe /'.") fmt.Fprintln(out, "You can use 'oc get all' to see a list of other objects.") } diff --git a/pkg/cmd/cli/describe/projectstatus_test.go b/pkg/cmd/cli/describe/projectstatus_test.go index 8ca4dccb5218..5719d7a0f833 100644 --- a/pkg/cmd/cli/describe/projectstatus_test.go +++ b/pkg/cmd/cli/describe/projectstatus_test.go @@ -278,7 +278,7 @@ func TestProjectStatus(t *testing.T) { o.Add(obj) } oc, kc := testclient.NewFixtureClients(o) - d := ProjectStatusDescriber{C: oc, K: kc, Server: "https://example.com:8443"} + d := ProjectStatusDescriber{C: oc, K: kc, Server: "https://example.com:8443", Suggest: true} out, err := d.Describe("example", "") if !test.ErrFn(err) { t.Errorf("%s: unexpected error: %v", k, err) diff --git a/pkg/deploy/graph/analysis/dc.go b/pkg/deploy/graph/analysis/dc.go index 30af8d2d6999..81ece2872c0a 100644 --- a/pkg/deploy/graph/analysis/dc.go +++ b/pkg/deploy/graph/analysis/dc.go @@ -15,9 +15,9 @@ import ( ) const ( - ImageStreamTagNotAvailableInfo = "ImageStreamTagNotAvailable" - MissingImageStreamWarning = "MissingImageStream" - MissingImageStreamTagWarning = "MissingImageStreamTag" + TagNotAvailableWarning = "ImageStreamTagNotAvailable" + MissingImageStreamErr = "MissingImageStream" + MissingImageStreamTagErr = "MissingImageStreamTag" ) // FindDeploymentConfigTriggerErrors checks for possible failures in deployment config @@ -42,8 +42,8 @@ dc: Node: uncastDcNode, RelatedNodes: []graph.Node{uncastIstNode, isNode}, - Severity: osgraph.WarningSeverity, - Key: MissingImageStreamWarning, + Severity: osgraph.ErrorSeverity, + Key: MissingImageStreamErr, Message: fmt.Sprintf("The image trigger for %s will have no effect because %s does not exist.", dcNode.ResourceString(), isNode.(*imagegraph.ImageStreamNode).ResourceString()), }) @@ -56,8 +56,8 @@ dc: Node: uncastDcNode, RelatedNodes: []graph.Node{uncastIstNode, bcNode}, - Severity: osgraph.InfoSeverity, - Key: ImageStreamTagNotAvailableInfo, + Severity: osgraph.WarningSeverity, + Key: TagNotAvailableWarning, Message: fmt.Sprintf("The image trigger for %s will have no effect because %s does not exist but %s points to %s.", dcNode.ResourceString(), istNode.ResourceString(), bcNode.(*buildgraph.BuildConfigNode).ResourceString(), istNode.ResourceString()), }) @@ -69,8 +69,8 @@ dc: Node: uncastDcNode, RelatedNodes: []graph.Node{uncastIstNode}, - Severity: osgraph.WarningSeverity, - Key: MissingImageStreamTagWarning, + Severity: osgraph.ErrorSeverity, + Key: MissingImageStreamTagErr, Message: fmt.Sprintf("The image trigger for %s will have no effect because %s does not exist.", dcNode.ResourceString(), istNode.ResourceString()), }) diff --git a/pkg/deploy/graph/analysis/dc_test.go b/pkg/deploy/graph/analysis/dc_test.go index cdc8d3132953..24ebb0575453 100644 --- a/pkg/deploy/graph/analysis/dc_test.go +++ b/pkg/deploy/graph/analysis/dc_test.go @@ -23,7 +23,7 @@ func TestMissingImageStreamTag(t *testing.T) { t.Fatalf("expected %v, got %v", e, a) } - if got, expected := markers[0].Key, MissingImageStreamTagWarning; got != expected { + if got, expected := markers[0].Key, MissingImageStreamTagErr; got != expected { t.Fatalf("expected marker key %q, got %q", expected, got) } } @@ -42,7 +42,7 @@ func TestMissingImageStream(t *testing.T) { t.Fatalf("expected %v, got %v", e, a) } - if got, expected := markers[0].Key, MissingImageStreamWarning; got != expected { + if got, expected := markers[0].Key, MissingImageStreamErr; got != expected { t.Fatalf("expected marker key %q, got %q", expected, got) } } @@ -61,7 +61,7 @@ func TestSyntheticImageStreamTag(t *testing.T) { t.Fatalf("expected %v, got %v", e, a) } - if got, expected := markers[0].Key, ImageStreamTagNotAvailableInfo; got != expected { + if got, expected := markers[0].Key, TagNotAvailableWarning; got != expected { t.Fatalf("expected marker key %q, got %q", expected, got) } } diff --git a/pkg/route/graph/analysis/analysis.go b/pkg/route/graph/analysis/analysis.go index 4c42b15e9ce6..cf985291860d 100644 --- a/pkg/route/graph/analysis/analysis.go +++ b/pkg/route/graph/analysis/analysis.go @@ -17,9 +17,9 @@ const ( MissingRoutePortWarning = "MissingRoutePort" // MissingServiceWarning is returned when there is no service for the specific route. MissingServiceWarning = "MissingService" - // MissingTLSTerminationTypeWarning is returned when a route with a tls config doesn't + // MissingTLSTerminationTypeErr is returned when a route with a tls config doesn't // specify a tls termination type. - MissingTLSTerminationTypeWarning = "MissingTLSTermination" + MissingTLSTerminationTypeErr = "MissingTLSTermination" ) // FindMissingPortMapping checks all routes and reports those that don't specify a port while @@ -77,9 +77,10 @@ func FindMissingTLSTerminationType(g osgraph.Graph) []osgraph.Marker { markers = append(markers, osgraph.Marker{ Node: routeNode, - Severity: osgraph.WarningSeverity, - Key: MissingTLSTerminationTypeWarning, - Message: fmt.Sprintf("%s has a tls configuration but no termination type specified.", routeNode.ResourceString())}) + Severity: osgraph.ErrorSeverity, + Key: MissingTLSTerminationTypeErr, + Message: fmt.Sprintf("%s has a TLS configuration but no termination type specified.", routeNode.ResourceString()), + Suggestion: osgraph.Suggestion(fmt.Sprintf("oc patch %s -p '{\"spec\":{\"tls\":{\"termination\":\"\"}}}' (replace with a valid termination type: edge, passthrough, reencrypt)", routeNode.ResourceString()))}) } } diff --git a/pkg/route/graph/analysis/analysis_test.go b/pkg/route/graph/analysis/analysis_test.go index e2dcf0393b69..8f4efc3934a9 100644 --- a/pkg/route/graph/analysis/analysis_test.go +++ b/pkg/route/graph/analysis/analysis_test.go @@ -50,7 +50,7 @@ func TestMissingTLSTerminationType(t *testing.T) { if expected, got := 1, len(markers); expected != got { t.Fatalf("expected %d markers, got %d", expected, got) } - if expected, got := MissingTLSTerminationTypeWarning, markers[0].Key; expected != got { + if expected, got := MissingTLSTerminationTypeErr, markers[0].Key; expected != got { t.Fatalf("expected %s marker key, got %s", expected, got) } }