Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve cf apps with --no-stats and better error handling [V8] #2796

Merged
merged 2 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 56 additions & 33 deletions actor/v7action/application_summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand All @@ -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
}
Expand Down Expand Up @@ -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

Expand Down
91 changes: 90 additions & 1 deletion actor/v7action/application_summary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ var _ = Describe("Application Summary Actions", func() {
var (
spaceGUID string
labelSelector string
omitStats bool

summaries []ApplicationSummary
warnings Warnings
Expand All @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion command/v7/actor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
34 changes: 21 additions & 13 deletions command/v7/apps_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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)
Expand Down
45 changes: 42 additions & 3 deletions command/v7/apps_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
})

Expand Down Expand Up @@ -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))
})
})

Expand All @@ -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))
})
})

})
Loading
Loading