Skip to content

Commit

Permalink
chore: fix listing services and jobs in an environment (#2345)
Browse files Browse the repository at this point in the history
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
  • Loading branch information
efekarakus authored May 18, 2021
1 parent 110a6e1 commit 3595860
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 104 deletions.
38 changes: 21 additions & 17 deletions internal/pkg/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"fmt"
"sort"

"github.com/aws/copilot-cli/internal/pkg/manifest"

rg "github.com/aws/copilot-cli/internal/pkg/aws/resourcegroups"
"github.com/aws/copilot-cli/internal/pkg/aws/sessions"
"github.com/aws/copilot-cli/internal/pkg/config"
Expand All @@ -25,8 +27,6 @@ const (
)

const (
svcWorkloadType = "service"
jobWorkloadType = "job"
stackResourceType = "cloudformation:stack"
)

Expand All @@ -38,6 +38,7 @@ type resourceGetter interface {
type ConfigStoreClient interface {
GetEnvironment(appName string, environmentName string) (*config.Environment, error)
ListEnvironments(appName string) ([]*config.Environment, error)
ListWorkloads(appName string) ([]*config.Workload, error)
GetService(appName, svcName string) (*config.Workload, error)
GetJob(appName, jobname string) (*config.Workload, error)
}
Expand Down Expand Up @@ -77,24 +78,29 @@ func NewStore(store ConfigStoreClient) (*Store, error) {

// ListDeployedServices returns the names of deployed services in an environment.
func (s *Store) ListDeployedServices(appName string, envName string) ([]string, error) {
return s.listDeployedWorkloads(appName, envName, svcWorkloadType)
return s.listDeployedWorkloads(appName, envName, manifest.ServiceTypes)
}

// ListDeployedJobs returns the names of deployed jobs in an environment.
func (s *Store) ListDeployedJobs(appName string, envName string) ([]string, error) {
return s.listDeployedWorkloads(appName, envName, jobWorkloadType)
return s.listDeployedWorkloads(appName, envName, manifest.JobTypes)
}

func (s *Store) listDeployedWorkloads(appName string, envName string, workloadType string) ([]string, error) {
var getWorkload func(string, string) (*config.Workload, error)
switch workloadType {
case jobWorkloadType:
getWorkload = s.configStore.GetJob
case svcWorkloadType:
getWorkload = s.configStore.GetService
default:
return nil, fmt.Errorf("unrecognized workload type %s", workloadType)
func (s *Store) listDeployedWorkloads(appName string, envName string, workloadType []string) ([]string, error) {
allWorkloads, err := s.configStore.ListWorkloads(appName)
if err != nil {
return nil, fmt.Errorf("list all workloads in application %s: %w", appName, err)
}
filteredWorkloadNames := make(map[string]bool)
for _, wkld := range allWorkloads {
for _, t := range workloadType {
if wkld.Type != t {
continue
}
filteredWorkloadNames[wkld.Name] = true
}
}

rgClient, err := s.newRgClientFromIDs(appName, envName)
if err != nil {
return nil, err
Expand All @@ -113,11 +119,9 @@ func (s *Store) listDeployedWorkloads(appName string, envName string, workloadTy
// To avoid listing duplicate service entry in a case when service has addons stack.
continue
}
wkld, err := getWorkload(appName, name)
if err != nil {
return nil, fmt.Errorf("get %s %s: %w", workloadType, name, err)
if _, ok := filteredWorkloadNames[name]; ok {
wklds = append(wklds, name)
}
wklds = append(wklds, wkld.Name)
}
sort.Strings(wklds)
return wklds, nil
Expand Down
66 changes: 47 additions & 19 deletions internal/pkg/deploy/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"fmt"
"testing"

"github.com/aws/copilot-cli/internal/pkg/manifest"

rg "github.com/aws/copilot-cli/internal/pkg/aws/resourcegroups"
"github.com/aws/copilot-cli/internal/pkg/config"
"github.com/aws/copilot-cli/internal/pkg/deploy/mocks"
Expand Down Expand Up @@ -35,6 +37,7 @@ func TestStore_ListDeployedServices(t *testing.T) {

setupMocks: func(m storeMock) {
gomock.InOrder(
m.configStore.EXPECT().ListWorkloads("mockApp").Return([]*config.Workload{}, nil),
m.rgGetter.EXPECT().GetResourcesByTags(stackResourceType, map[string]string{
AppTagKey: "mockApp",
EnvTagKey: "mockEnv",
Expand All @@ -50,6 +53,7 @@ func TestStore_ListDeployedServices(t *testing.T) {

setupMocks: func(m storeMock) {
gomock.InOrder(
m.configStore.EXPECT().ListWorkloads("mockApp").Return([]*config.Workload{}, nil),
m.rgGetter.EXPECT().GetResourcesByTags(stackResourceType, map[string]string{
AppTagKey: "mockApp",
EnvTagKey: "mockEnv",
Expand All @@ -65,35 +69,44 @@ func TestStore_ListDeployedServices(t *testing.T) {

setupMocks: func(m storeMock) {
gomock.InOrder(
m.configStore.EXPECT().ListWorkloads("mockApp").Return(nil, errors.New("some error")),
m.rgGetter.EXPECT().GetResourcesByTags(stackResourceType, map[string]string{
AppTagKey: "mockApp",
EnvTagKey: "mockEnv",
}).Return([]*rg.Resource{{ARN: "mockARN", Tags: map[string]string{ServiceTagKey: "mockSvc"}}}, nil),
m.configStore.EXPECT().GetService("mockApp", "mockSvc").Return(nil, errors.New("some error")),
}).Times(0),
)
},

wantedError: fmt.Errorf("get service mockSvc: some error"),
wantedError: fmt.Errorf("list all workloads in application mockApp: some error"),
},
"success": {
inputApp: "mockApp",
inputEnv: "mockEnv",

setupMocks: func(m storeMock) {
gomock.InOrder(
m.configStore.EXPECT().ListWorkloads("mockApp").Return([]*config.Workload{
{
App: "mockApp",
Name: "mockSvc1",
Type: manifest.LoadBalancedWebServiceType,
},
{
App: "mockApp",
Name: "mockSvc2",
Type: manifest.RequestDrivenWebServiceType,
},
{
App: "mockApp",
Name: "mockJob",
Type: manifest.ScheduledJobType,
},
}, nil),
m.rgGetter.EXPECT().GetResourcesByTags(stackResourceType, map[string]string{
AppTagKey: "mockApp",
EnvTagKey: "mockEnv",
}).Return([]*rg.Resource{{ARN: "mockARN1", Tags: map[string]string{ServiceTagKey: "mockSvc1"}},
{ARN: "mockARN2", Tags: map[string]string{ServiceTagKey: "mockSvc2"}}}, nil),
m.configStore.EXPECT().GetService("mockApp", "mockSvc1").Return(&config.Workload{
App: "mockApp",
Name: "mockSvc1",
}, nil),
m.configStore.EXPECT().GetService("mockApp", "mockSvc2").Return(&config.Workload{
App: "mockApp",
Name: "mockSvc2",
}, nil),
)
},

Expand Down Expand Up @@ -150,19 +163,33 @@ func TestStore_ListDeployedJobs(t *testing.T) {

setupMocks: func(m storeMock) {
gomock.InOrder(
m.configStore.EXPECT().ListWorkloads("mockApp").Return([]*config.Workload{
{
App: "mockApp",
Name: "mockSvc1",
Type: manifest.LoadBalancedWebServiceType,
},
{
App: "mockApp",
Name: "mockSvc2",
Type: manifest.RequestDrivenWebServiceType,
},
{
App: "mockApp",
Name: "mockJob1",
Type: manifest.ScheduledJobType,
},
{
App: "mockApp",
Name: "mockJob2",
Type: manifest.ScheduledJobType,
},
}, nil),
m.rgGetter.EXPECT().GetResourcesByTags(stackResourceType, map[string]string{
AppTagKey: "mockApp",
EnvTagKey: "mockEnv",
}).Return([]*rg.Resource{{ARN: "mockARN1", Tags: map[string]string{ServiceTagKey: "mockJob1"}},
{ARN: "mockARN2", Tags: map[string]string{ServiceTagKey: "mockJob2"}}}, nil),
m.configStore.EXPECT().GetJob("mockApp", "mockJob1").Return(&config.Workload{
App: "mockApp",
Name: "mockJob1",
}, nil),
m.configStore.EXPECT().GetJob("mockApp", "mockJob2").Return(&config.Workload{
App: "mockApp",
Name: "mockJob2",
}, nil),
)
},

Expand All @@ -174,6 +201,7 @@ func TestStore_ListDeployedJobs(t *testing.T) {

setupMocks: func(m storeMock) {
gomock.InOrder(
m.configStore.EXPECT().ListWorkloads("mockApp").Return([]*config.Workload{}, nil),
m.rgGetter.EXPECT().GetResourcesByTags(stackResourceType, map[string]string{
AppTagKey: "mockApp",
EnvTagKey: "mockEnv",
Expand Down
15 changes: 15 additions & 0 deletions internal/pkg/deploy/mocks/mock_deploy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

68 changes: 0 additions & 68 deletions internal/pkg/describe/mocks/mock_service.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 3595860

Please sign in to comment.