Improve cf apps with --no-stats and better error handling (#2795)

* Introduce `--no-stats` for `cf apps`

Fetching process stats requires a `/stats` request for each process.
If a space contains a large number apps or processes this slows down `cf apps`.
In case a user is not interested in stats the `--no-stats` flag can be helpful.

* Handle `ProcessNotFoundError` in `cf apps`

As a user calls `cf apps` in a large space it might happen that a app
was deleted while the CLI is putting all the different information (processes, stats & routes) together.
This leads to a failing `cf apps` command.
This change prevents this by catching `ProcessNotFoundError` errors and returning `nil` instead.
Related to #2734

Author: johha

actor/v7action/application_summary.go

@@ -2,6 +2,7 @@ package v7action
 
 import (
 	"code.cloudfoundry.org/cli/actor/actionerror"
+	"code.cloudfoundry.org/cli/api/cloudcontroller/ccerror"
 	"code.cloudfoundry.org/cli/api/cloudcontroller/ccv3"
 	"code.cloudfoundry.org/cli/resources"
 	"code.cloudfoundry.org/cli/util/batcher"
@@ -32,7 +33,7 @@ func (a ApplicationSummary) hasIsolationSegment() bool {
 		len(a.ProcessSummaries[0].InstanceDetails[0].IsolationSegment) > 0
 }
 
-func (actor Actor) GetAppSummariesForSpace(spaceGUID string, labelSelector string) ([]ApplicationSummary, Warnings, error) {
+func (actor Actor) GetAppSummariesForSpace(spaceGUID string, labelSelector string, omitStats bool) ([]ApplicationSummary, Warnings, error) {
 	var allWarnings Warnings
 	var allSummaries []ApplicationSummary
 
@@ -43,56 +44,33 @@ func (actor Actor) GetAppSummariesForSpace(spaceGUID string, labelSelector strin
 	if len(labelSelector) > 0 {
 		keys = append(keys, ccv3.Query{Key: ccv3.LabelSelectorFilter, Values: []string{labelSelector}})
 	}
-	apps, warnings, err := actor.CloudControllerClient.GetApplications(keys...)
-	allWarnings = append(allWarnings, warnings...)
+	apps, ccv3Warnings, err := actor.CloudControllerClient.GetApplications(keys...)
+	allWarnings = append(allWarnings, ccv3Warnings...)
 	if err != nil {
 		return nil, allWarnings, err
 	}
-	var processes []resources.Process
 
-	warnings, err = batcher.RequestByGUID(toAppGUIDs(apps), func(guids []string) (ccv3.Warnings, error) {
-		batch, warnings, err := actor.CloudControllerClient.GetProcesses(ccv3.Query{
-			Key: ccv3.AppGUIDFilter, Values: guids,
-		})
-		processes = append(processes, batch...)
-		return warnings, err
-	})
-	allWarnings = append(allWarnings, warnings...)
-	if err != nil {
-		return nil, allWarnings, err
-	}
+	var processSummariesByAppGUID map[string]ProcessSummaries
+	var warnings Warnings
 
-	processSummariesByAppGUID := make(map[string]ProcessSummaries, len(apps))
-	for _, process := range processes {
-		instances, warnings, err := actor.CloudControllerClient.GetProcessInstances(process.GUID)
-		allWarnings = append(allWarnings, Warnings(warnings)...)
+	if !omitStats {
+		processSummariesByAppGUID, warnings, err = actor.getProcessSummariesForApps(apps)
+		allWarnings = append(allWarnings, warnings...)
 		if err != nil {
 			return nil, allWarnings, err
 		}
-
-		var instanceDetails []ProcessInstance
-		for _, instance := range instances {
-			instanceDetails = append(instanceDetails, ProcessInstance(instance))
-		}
-
-		processSummary := ProcessSummary{
-			Process:         resources.Process(process),
-			InstanceDetails: instanceDetails,
-		}
-
-		processSummariesByAppGUID[process.AppGUID] = append(processSummariesByAppGUID[process.AppGUID], processSummary)
 	}
 
 	var routes []resources.Route
 
-	warnings, err = batcher.RequestByGUID(toAppGUIDs(apps), func(guids []string) (ccv3.Warnings, error) {
+	ccv3Warnings, err = batcher.RequestByGUID(toAppGUIDs(apps), func(guids []string) (ccv3.Warnings, error) {
 		batch, warnings, err := actor.CloudControllerClient.GetRoutes(ccv3.Query{
 			Key: ccv3.AppGUIDFilter, Values: guids,
 		})
 		routes = append(routes, batch...)
 		return warnings, err
 	})
-	allWarnings = append(allWarnings, warnings...)
+	allWarnings = append(allWarnings, ccv3Warnings...)
 	if err != nil {
 		return nil, allWarnings, err
 	}
@@ -144,6 +122,51 @@ func (actor Actor) GetDetailedAppSummary(appName, spaceGUID string, withObfuscat
 	return detailedSummary, allWarnings, err
 }
 
+func (actor Actor) getProcessSummariesForApps(apps []resources.Application) (map[string]ProcessSummaries, Warnings, error) {
+	processSummariesByAppGUID := make(map[string]ProcessSummaries)
+	var allWarnings Warnings
+	var processes []resources.Process
+
+	warnings, err := batcher.RequestByGUID(toAppGUIDs(apps), func(guids []string) (ccv3.Warnings, error) {
+		batch, warnings, err := actor.CloudControllerClient.GetProcesses(ccv3.Query{
+			Key: ccv3.AppGUIDFilter, Values: guids,
+		})
+		processes = append(processes, batch...)
+		return warnings, err
+	})
+	allWarnings = append(allWarnings, warnings...)
+	if err != nil {
+		return nil, allWarnings, err
+	}
+
+	for _, process := range processes {
+		instances, warnings, err := actor.CloudControllerClient.GetProcessInstances(process.GUID)
+		allWarnings = append(allWarnings, Warnings(warnings)...)
+
+		if err != nil {
+			switch err.(type) {
+			case ccerror.ProcessNotFoundError, ccerror.InstanceNotFoundError:
+				continue
+			default:
+				return nil, allWarnings, err
+			}
+		}
+
+		var instanceDetails []ProcessInstance
+		for _, instance := range instances {
+			instanceDetails = append(instanceDetails, ProcessInstance(instance))
+		}
+
+		processSummary := ProcessSummary{
+			Process:         resources.Process(process),
+			InstanceDetails: instanceDetails,
+		}
+
+		processSummariesByAppGUID[process.AppGUID] = append(processSummariesByAppGUID[process.AppGUID], processSummary)
+	}
+	return processSummariesByAppGUID, allWarnings, nil
+}
+
 func (actor Actor) createSummary(app resources.Application, withObfuscatedValues bool) (ApplicationSummary, Warnings, error) {
 	var allWarnings Warnings
 

actor/v7action/application_summary_test.go

@@ -72,6 +72,7 @@ var _ = Describe("Application Summary Actions", func() {
 		var (
 			spaceGUID     string
 			labelSelector string
+			omitStats     bool
 
 			summaries  []ApplicationSummary
 			warnings   Warnings
@@ -81,10 +82,11 @@ var _ = Describe("Application Summary Actions", func() {
 		BeforeEach(func() {
 			spaceGUID = "some-space-guid"
 			labelSelector = "some-key=some-value"
+			omitStats = false
 		})
 
 		JustBeforeEach(func() {
-			summaries, warnings, executeErr = actor.GetAppSummariesForSpace(spaceGUID, labelSelector)
+			summaries, warnings, executeErr = actor.GetAppSummariesForSpace(spaceGUID, labelSelector, omitStats)
 		})
 
 		When("getting the application is successful", func() {
@@ -361,6 +363,93 @@ var _ = Describe("Application Summary Actions", func() {
 				Expect(warnings).To(ConsistOf("get-apps-warning"))
 			})
 		})
+
+		When("omitStats flag is provided", func() {
+			BeforeEach(func() {
+				omitStats = true
+
+				fakeCloudControllerClient.GetApplicationsReturns(
+					[]resources.Application{
+						{
+							Name:  "some-app-name",
+							GUID:  "some-app-guid",
+							State: constant.ApplicationStarted,
+						},
+					},
+					ccv3.Warnings{"get-apps-warning"},
+					nil,
+				)
+
+				listedProcesses := []resources.Process{
+					{
+						GUID:       "some-process-web-guid",
+						Type:       "web",
+						Command:    *types.NewFilteredString("[Redacted Value]"),
+						MemoryInMB: types.NullUint64{Value: 64, IsSet: true},
+						AppGUID:    "some-app-guid",
+					},
+				}
+
+				fakeCloudControllerClient.GetProcessesReturns(
+					listedProcesses,
+					ccv3.Warnings{"get-app-processes-warning"},
+					nil,
+				)
+			})
+			It("doesn't call the stats endpoint", func() {
+				Expect(fakeCloudControllerClient.GetProcessInstancesCallCount()).To(Equal(0))
+			})
+		})
+
+		When("an application is deleted in between", func() {
+
+			BeforeEach(func() {
+				fakeCloudControllerClient.GetApplicationsReturns(
+					[]resources.Application{
+						{
+							Name:  "some-app-name",
+							GUID:  "some-app-guid",
+							State: constant.ApplicationStarted,
+						},
+					},
+					nil,
+					nil,
+				)
+
+				fakeCloudControllerClient.GetProcessesReturns(
+					[]resources.Process{
+						{
+							GUID:       "some-process-web-guid",
+							Type:       "web",
+							Command:    *types.NewFilteredString("[Redacted Value]"),
+							MemoryInMB: types.NullUint64{Value: 64, IsSet: true},
+							AppGUID:    "some-app-guid",
+						},
+					},
+					nil,
+					nil,
+				)
+
+				fakeCloudControllerClient.GetProcessInstancesReturns(nil, nil, ccerror.ProcessNotFoundError{})
+			})
+
+			It("does not fail and has empty ProcessSummaries & Routes", func() {
+				Expect(executeErr).ToNot(HaveOccurred())
+
+				Expect(summaries).To(Equal([]ApplicationSummary{
+					{
+						Application: resources.Application{
+							Name:  "some-app-name",
+							GUID:  "some-app-guid",
+							State: constant.ApplicationStarted,
+						},
+						ProcessSummaries: nil,
+						Routes:           nil,
+					},
+				}))
+
+			})
+		})
 	})
 
 	Describe("GetDetailedAppSummary", func() {

command/v7/actor.go

@@ -87,7 +87,7 @@ type Actor interface {
 	EnableServiceAccess(offeringName, brokerName, orgName, planName string) (v7action.SkippedPlans, v7action.Warnings, error)
 	EntitleIsolationSegmentToOrganizationByName(isolationSegmentName string, orgName string) (v7action.Warnings, error)
 	GetAppFeature(appGUID string, featureName string) (resources.ApplicationFeature, v7action.Warnings, error)
-	GetAppSummariesForSpace(spaceGUID string, labels string) ([]v7action.ApplicationSummary, v7action.Warnings, error)
+	GetAppSummariesForSpace(spaceGUID string, labels string, omitStats bool) ([]v7action.ApplicationSummary, v7action.Warnings, error)
 	GetApplicationByNameAndSpace(appName string, spaceGUID string) (resources.Application, v7action.Warnings, error)
 	GetApplicationMapForRoute(route resources.Route) (map[string]resources.Application, v7action.Warnings, error)
 	GetApplicationDroplets(appName string, spaceGUID string) ([]resources.Droplet, v7action.Warnings, error)

command/v7/apps_command.go

@@ -13,7 +13,8 @@ type AppsCommand struct {
 	usage           interface{} `usage:"CF_NAME apps [--labels SELECTOR]\n\nEXAMPLES:\n   CF_NAME apps\n   CF_NAME apps --labels 'environment in (production,staging),tier in (backend)'\n   CF_NAME apps --labels 'env=dev,!chargeback-code,tier in (backend,worker)'"`
 	relatedCommands interface{} `related_commands:"events, logs, map-route, push, scale, start, stop, restart"`
 
-	Labels string `long:"labels" description:"Selector to filter apps by labels"`
+	Labels    string `long:"labels" description:"Selector to filter apps by labels"`
+	OmitStats bool   `long:"no-stats" description:"Do not retrieve process stats"`
 }
 
 func (cmd AppsCommand) Execute(args []string) error {
@@ -34,7 +35,7 @@ func (cmd AppsCommand) Execute(args []string) error {
 	})
 	cmd.UI.DisplayNewline()
 
-	summaries, warnings, err := cmd.Actor.GetAppSummariesForSpace(cmd.Config.TargetedSpace().GUID, cmd.Labels)
+	summaries, warnings, err := cmd.Actor.GetAppSummariesForSpace(cmd.Config.TargetedSpace().GUID, cmd.Labels, cmd.OmitStats)
 	cmd.UI.DisplayWarnings(warnings)
 	if err != nil {
 		return err
@@ -45,22 +46,29 @@ func (cmd AppsCommand) Execute(args []string) error {
 		return nil
 	}
 
-	table := [][]string{
-		{
-			cmd.UI.TranslateText("name"),
-			cmd.UI.TranslateText("requested state"),
-			cmd.UI.TranslateText("processes"),
-			cmd.UI.TranslateText("routes"),
-		},
+	fields := []string{
+		cmd.UI.TranslateText("name"),
+		cmd.UI.TranslateText("requested state"),
 	}
 
+	if !cmd.OmitStats {
+		fields = append(fields, cmd.UI.TranslateText("processes"))
+	}
+
+	fields = append(fields, cmd.UI.TranslateText("routes"))
+
+	table := [][]string{fields}
+
 	for _, summary := range summaries {
-		table = append(table, []string{
+		tableRow := []string{
 			summary.Name,
 			cmd.UI.TranslateText(strings.ToLower(string(summary.State))),
-			summary.ProcessSummaries.String(),
-			getURLs(summary.Routes),
-		})
+		}
+		if !cmd.OmitStats {
+			tableRow = append(tableRow, summary.ProcessSummaries.String())
+		}
+		tableRow = append(tableRow, getURLs(summary.Routes))
+		table = append(table, tableRow)
 	}
 
 	cmd.UI.DisplayTableWithHeader("", table, ui.DefaultTableSpacePadding)

command/v7/apps_command_test.go

@@ -243,9 +243,10 @@ var _ = Describe("apps Command", func() {
 				Expect(testUI.Err).To(Say("warning-2"))
 
 				Expect(fakeActor.GetAppSummariesForSpaceCallCount()).To(Equal(1))
-				spaceGUID, labels := fakeActor.GetAppSummariesForSpaceArgsForCall(0)
+				spaceGUID, labels, omitStats := fakeActor.GetAppSummariesForSpaceArgsForCall(0)
 				Expect(spaceGUID).To(Equal("some-space-guid"))
 				Expect(labels).To(Equal(""))
+				Expect(omitStats).To(Equal(false))
 			})
 		})
 
@@ -274,9 +275,10 @@ var _ = Describe("apps Command", func() {
 				Expect(testUI.Err).To(Say("warning"))
 
 				Expect(fakeActor.GetAppSummariesForSpaceCallCount()).To(Equal(1))
-				spaceGUID, labelSelector := fakeActor.GetAppSummariesForSpaceArgsForCall(0)
+				spaceGUID, labelSelector, omitStats := fakeActor.GetAppSummariesForSpaceArgsForCall(0)
 				Expect(spaceGUID).To(Equal("some-space-guid"))
 				Expect(labelSelector).To(Equal(""))
+				Expect(omitStats).To(Equal(false))
 			})
 		})
 
@@ -301,9 +303,46 @@ var _ = Describe("apps Command", func() {
 
 		It("passes the flag to the API", func() {
 			Expect(fakeActor.GetAppSummariesForSpaceCallCount()).To(Equal(1))
-			_, labelSelector := fakeActor.GetAppSummariesForSpaceArgsForCall(0)
+			_, labelSelector, _ := fakeActor.GetAppSummariesForSpaceArgsForCall(0)
 			Expect(labelSelector).To(Equal("fish=moose"))
 		})
 	})
 
+	Context("when '--skip-stats' flag is set", func() {
+		BeforeEach(func() {
+			cmd.OmitStats = true
+
+			appSummaries := []v7action.ApplicationSummary{
+				{
+					Application: resources.Application{
+						GUID:  "app-guid-1",
+						Name:  "some-app-1",
+						State: constant.ApplicationStarted,
+					},
+					ProcessSummaries: []v7action.ProcessSummary{},
+					Routes: []resources.Route{
+						{
+							Host: "some-app-1",
+							URL:  "some-app-1.some-domain",
+						},
+					},
+				},
+			}
+			fakeActor.GetAppSummariesForSpaceReturns(appSummaries, v7action.Warnings{}, nil)
+
+		})
+
+		It("prints the application summary without process information", func() {
+			Expect(executeErr).ToNot(HaveOccurred())
+
+			Expect(testUI.Out).To(Say(`Getting apps in org some-org / space some-space as steve\.\.\.`))
+
+			Expect(testUI.Out).To(Say(`name\s+requested state\s+routes`))
+			Expect(testUI.Out).To(Say(`some-app-1\s+started\s+some-app-1.some-domain`))
+			Expect(fakeActor.GetAppSummariesForSpaceCallCount()).To(Equal(1))
+			_, _, omitStats := fakeActor.GetAppSummariesForSpaceArgsForCall(0)
+			Expect(omitStats).To(Equal(true))
+		})
+	})
+
 })