Skip to content

Commit 0a8ccbf

Browse files
PettitWesleyPettitWesley
committed
Force-recreate for service discovery resources
 - If CLI detects that a CFN stack already exists, it tries to delete it - For namespaces, it only does force recreation if the namespace could not be found using route53 servicediscovery APIs - This features allows the CLI to clean up old CFN stacks from previous failed attempts - Route53 will prevent the removal of any resource that is actually in use
1 parent e0ab990 commit 0a8ccbf

File tree

2 files changed

+156
-7
lines changed

2 files changed

+156
-7
lines changed

‎ecs-cli/modules/cli/servicediscovery/servicediscovery_app.go

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,31 +119,36 @@ func update(c *cli.Context, networkMode, serviceName, clusterName string, cfnCli
119119

120120
func delete(c *cli.Context, cfnClient cloudformation.CloudformationClient, serviceName, projectName, clusterName string) error {
121121
sdsStackName := cfnStackName(serviceDiscoveryServiceStackNameFormat, clusterName, serviceName)
122-
err := deleteStack(sdsStackName, projectName, "Service Discovery Service", cfnClient)
122+
err := deleteStack(sdsStackName, projectName, "Service Discovery Service", cfnClient, true, false)
123123
if err != nil {
124124
return err
125125
}
126126

127127
if c.Bool(flags.DeletePrivateNamespaceFlag) {
128128
namspaceStackName := cfnStackName(privateDNSNamespaceStackNameFormat, clusterName, serviceName)
129-
err = deleteStack(namspaceStackName, projectName, "Private DNS Namespace", cfnClient)
129+
err = deleteStack(namspaceStackName, projectName, "Private DNS Namespace", cfnClient, false, false)
130130
if err != nil {
131131
return err
132132
}
133133
}
134134
return nil
135135
}
136136

137-
func deleteStack(stackName, projectName, resource string, cfnClient cloudformation.CloudformationClient) error {
137+
func deleteStack(stackName, projectName, resource string, cfnClient cloudformation.CloudformationClient, ignoreValidation, cleanUp bool) error {
138138
if err := cfnClient.ValidateStackExists(stackName); err != nil {
139+
if ignoreValidation {
140+
return nil
141+
}
139142
return errors.Wrapf(err, "no %s CloudFormation stack found for project '%s'", resource, projectName)
140143
}
141-
144+
if cleanUp {
145+
logrus.Info("Cleaning up existing CloudFormation stack...")
146+
} else {
147+
logrus.Infof("Waiting for your %s resource to be deleted...", resource)
148+
}
142149
if err := cfnClient.DeleteStack(stackName); err != nil {
143150
return err
144151
}
145-
146-
logrus.Infof("Waiting for your %s resource to be deleted...", resource)
147152
return cfnClient.WaitUntilDeleteComplete(stackName)
148153
}
149154

@@ -174,6 +179,12 @@ func create(c *cli.Context, networkMode, serviceName string, cfnClient cloudform
174179
}
175180

176181
sdsStackName := cfnStackName(serviceDiscoveryServiceStackNameFormat, config.Cluster, serviceName)
182+
183+
// first try to delete the SDS Stack to clean up previous attempts that failed
184+
if err := deleteStack(sdsStackName, serviceName, "Service Discovery Service", cfnClient, true, true); err != nil {
185+
return nil, errors.Wrapf(err, "A Service Discovery Service CloudFormation stack for %s already exists, failed to delete existing stack", serviceName)
186+
}
187+
177188
if _, err := cfnClient.CreateStack(cloudformation.GetSDSTemplate(), sdsStackName, false, sdsParams); err != nil {
178189
return nil, err
179190
}
@@ -194,13 +205,22 @@ func create(c *cli.Context, networkMode, serviceName string, cfnClient cloudform
194205
return serviceRegistry, err
195206
}
196207

208+
// createNamespace creates a private DNS namespace
209+
// This function is used if getExistingNamespace() fails to find an existing namespace with the required settings
210+
// If a CFN stack with the same name exists already, we therefore know that it doesn't contain a namespace (stack create failed),
211+
// So we can safely delete the existing stack
197212
func createNamespace(c *cli.Context, networkMode, serviceName, clusterName string, cfnClient cloudformation.CloudformationClient, input *utils.ServiceDiscovery) (*string, error) {
198213
namespaceParams := getNamespaceCFNParams(input)
199214
if err := namespaceParams.Validate(); err != nil {
200215
return nil, err
201216
}
202217

203218
namespaceStackName := cfnStackName(privateDNSNamespaceStackNameFormat, clusterName, serviceName)
219+
220+
if err := deleteStack(namespaceStackName, serviceName, "Private DNS Namespace", cfnClient, true, true); err != nil {
221+
return nil, errors.Wrapf(err, "A Private DNS Namespace CloudFormation stack for %s already exists, failed to delete existing stack: %s", serviceName, err)
222+
}
223+
204224
if _, err := cfnClient.CreateStack(cloudformation.GetPrivateNamespaceTemplate(), namespaceStackName, false, namespaceParams); err != nil {
205225
return nil, err
206226
}

‎ecs-cli/modules/cli/servicediscovery/servicediscovery_app_test.go

Lines changed: 130 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,88 @@ func TestCreateServiceDiscoveryHost(t *testing.T) {
137137
assert.Equal(t, int64(80), aws.Int64Value(registry.ContainerPort), "Expected container port to match")
138138
}
139139

140+
func TestCreateServiceDiscoveryForceRecreate(t *testing.T) {
141+
oldFindPrivateNamespace := findPrivateNamespace
142+
defer func() { findPrivateNamespace = oldFindPrivateNamespace }()
143+
findPrivateNamespace = func(name, vpc string, config *config.CommandConfig) (*string, error) {
144+
// In this test the namespace does pre-exist
145+
return nil, nil
146+
}
147+
148+
ctrl := gomock.NewController(t)
149+
defer ctrl.Finish()
150+
151+
describeNamespaceStackResponse := &sdk.DescribeStacksOutput{
152+
Stacks: []*sdk.Stack{
153+
&sdk.Stack{
154+
Outputs: []*sdk.Output{
155+
&sdk.Output{
156+
OutputKey: aws.String(cfnTemplateOutputPrivateNamespaceID),
157+
OutputValue: aws.String(testNamespaceID),
158+
},
159+
},
160+
},
161+
},
162+
}
163+
164+
describeSDSStackResponse := &sdk.DescribeStacksOutput{
165+
Stacks: []*sdk.Stack{
166+
&sdk.Stack{
167+
Outputs: []*sdk.Output{
168+
&sdk.Output{
169+
OutputKey: aws.String(cfnTemplateOutputSDSARN),
170+
OutputValue: aws.String(testSDSARN),
171+
},
172+
},
173+
},
174+
},
175+
}
176+
177+
mockCloudformation := mock_cloudformation.NewMockCloudformationClient(ctrl)
178+
179+
gomock.InOrder(
180+
mockCloudformation.EXPECT().ValidateStackExists(testNamespaceStackName).Return(nil),
181+
// validate that existing SDS stack is deleted
182+
mockCloudformation.EXPECT().DeleteStack(testNamespaceStackName).Return(nil),
183+
mockCloudformation.EXPECT().WaitUntilDeleteComplete(testNamespaceStackName).Return(nil),
184+
mockCloudformation.EXPECT().CreateStack(gomock.Any(), testNamespaceStackName, false, gomock.Any()).Do(func(x, y, w, z interface{}) {
185+
stackName := y.(string)
186+
capabilityIAM := w.(bool)
187+
cfnParams := z.(*cloudformation.CfnStackParams)
188+
validateCFNParam(testNamespaceName, parameterKeyNamespaceName, cfnParams, t)
189+
validateCFNParam(testVPCID, parameterKeyVPCID, cfnParams, t)
190+
assert.False(t, capabilityIAM, "Expected capability capabilityIAM to be false")
191+
assert.Equal(t, testNamespaceStackName, stackName, "Expected stack name to match")
192+
}).Return("", nil),
193+
mockCloudformation.EXPECT().WaitUntilCreateComplete(testNamespaceStackName).Return(nil),
194+
mockCloudformation.EXPECT().DescribeStacks(testNamespaceStackName).Return(describeNamespaceStackResponse, nil),
195+
mockCloudformation.EXPECT().ValidateStackExists(testSDSStackName).Return(nil),
196+
// Validate that existing Namespace stack is deleted
197+
mockCloudformation.EXPECT().DeleteStack(testSDSStackName).Return(nil),
198+
mockCloudformation.EXPECT().WaitUntilDeleteComplete(testSDSStackName).Return(nil),
199+
mockCloudformation.EXPECT().CreateStack(gomock.Any(), testSDSStackName, false, gomock.Any()).Do(func(x, y, w, z interface{}) {
200+
stackName := y.(string)
201+
capabilityIAM := w.(bool)
202+
cfnParams := z.(*cloudformation.CfnStackParams)
203+
validateCFNParam(testNamespaceID, parameterKeyNamespaceID, cfnParams, t)
204+
validateCFNParam(testServiceName, parameterKeySDSName, cfnParams, t)
205+
validateCFNParam(servicediscovery.RecordTypeA, parameterKeyDNSType, cfnParams, t)
206+
assert.False(t, capabilityIAM, "Expected capability capabilityIAM to be false")
207+
assert.Equal(t, testSDSStackName, stackName, "Expected stack name to match")
208+
}).Return("", nil),
209+
mockCloudformation.EXPECT().WaitUntilCreateComplete(testSDSStackName).Return(nil),
210+
mockCloudformation.EXPECT().DescribeStacks(testSDSStackName).Return(describeSDSStackResponse, nil),
211+
)
212+
213+
config := &config.CommandConfig{
214+
Cluster: testClusterName,
215+
}
216+
217+
registry, err := create(simpleWorkflowContext(), "awsvpc", testServiceName, mockCloudformation, &utils.ServiceDiscovery{}, config)
218+
assert.NoError(t, err, "Unexpected Error calling create")
219+
assert.Equal(t, testSDSARN, aws.StringValue(registry.RegistryArn), "Expected SDS ARN to match")
220+
}
221+
140222
func TestCreateServiceDiscoveryWithECSParams(t *testing.T) {
141223
input := &utils.ServiceDiscovery{
142224
ContainerName: testContainerName,
@@ -604,15 +686,37 @@ func TestDeleteServiceDiscoveryDeleteNamespace(t *testing.T) {
604686
assert.NoError(t, err, "Unexpected error calling delete")
605687
}
606688

607-
func TestDeleteServiceDiscoveryStackNotFoundError(t *testing.T) {
689+
func TestDeleteServiceDiscoveryStackNotFoundErrorForSDS(t *testing.T) {
608690
ctrl := gomock.NewController(t)
609691
defer ctrl.Finish()
610692
mockCloudformation := mock_cloudformation.NewMockCloudformationClient(ctrl)
611693
gomock.InOrder(
612694
mockCloudformation.EXPECT().ValidateStackExists(testSDSStackName).Return(fmt.Errorf("Stack not found")),
613695
)
614696

697+
// If no stack is found, then there is nothing to delete, so no error is returned
615698
err := delete(emptyContext(), mockCloudformation, testServiceName, testServiceName, testClusterName)
699+
assert.NoError(t, err, "Expected error calling delete")
700+
}
701+
702+
func TestDeleteServiceDiscoveryStackNotFoundErrorForNamespaceWithDeleteNamespaceFlag(t *testing.T) {
703+
ctrl := gomock.NewController(t)
704+
defer ctrl.Finish()
705+
mockCloudformation := mock_cloudformation.NewMockCloudformationClient(ctrl)
706+
gomock.InOrder(
707+
mockCloudformation.EXPECT().ValidateStackExists(testSDSStackName).Return(nil),
708+
mockCloudformation.EXPECT().DeleteStack(testSDSStackName).Return(nil),
709+
mockCloudformation.EXPECT().WaitUntilDeleteComplete(testSDSStackName).Return(nil),
710+
mockCloudformation.EXPECT().ValidateStackExists(testNamespaceStackName).Return(fmt.Errorf("Stack not found")),
711+
)
712+
713+
flagSet := flag.NewFlagSet("create-sd", 0)
714+
flagSet.Bool(flags.DeletePrivateNamespaceFlag, true, "")
715+
716+
context := cli.NewContext(nil, flagSet, nil)
717+
718+
err := delete(context, mockCloudformation, testServiceName, testServiceName, testClusterName)
719+
// Since the user requested us to delete their namespace, if we failed to delete it, then that's an error case
616720
assert.Error(t, err, "Expected error calling delete")
617721
}
618722

@@ -632,6 +736,29 @@ func TestCFNStackName(t *testing.T) {
632736
assert.True(t, len(namespaceStackName) <= 128, "CFN Stack names must be no longer than 128 characters")
633737
}
634738

739+
// Tests the following weird/rare case which is technically allowed:
740+
// SDS wasn't create by CLI, so no SDS stack exists, But
741+
// Namespace was created by CLI, and user wants us to remove it.
742+
func TestDeleteServiceDiscoveryStackNotFoundErrorForSDSWithDeleteNamespaceFlag(t *testing.T) {
743+
ctrl := gomock.NewController(t)
744+
defer ctrl.Finish()
745+
mockCloudformation := mock_cloudformation.NewMockCloudformationClient(ctrl)
746+
gomock.InOrder(
747+
mockCloudformation.EXPECT().ValidateStackExists(testSDSStackName).Return(fmt.Errorf("Stack not found")),
748+
mockCloudformation.EXPECT().ValidateStackExists(testNamespaceStackName).Return(nil),
749+
mockCloudformation.EXPECT().DeleteStack(testNamespaceStackName).Return(nil),
750+
mockCloudformation.EXPECT().WaitUntilDeleteComplete(testNamespaceStackName).Return(nil),
751+
)
752+
753+
flagSet := flag.NewFlagSet("create-sd", 0)
754+
flagSet.Bool(flags.DeletePrivateNamespaceFlag, true, "")
755+
756+
context := cli.NewContext(nil, flagSet, nil)
757+
758+
err := delete(context, mockCloudformation, testServiceName, testServiceName, testClusterName)
759+
assert.NoError(t, err, "Unexpected error calling delete")
760+
}
761+
635762
func testCreateServiceDiscovery(t *testing.T, networkMode string, ecsParamsSD *utils.ServiceDiscovery, c *cli.Context, validateNamespace validateNamespaceParamsFunc, validateSDS validateSDSParamsFunc, createNamespace bool) (*ecs.ServiceRegistry, error) {
636763
ctrl := gomock.NewController(t)
637764
defer ctrl.Finish()
@@ -669,6 +796,7 @@ func testCreateServiceDiscovery(t *testing.T, networkMode string, ecsParamsSD *u
669796
var expectedCFNCalls []*gomock.Call
670797
if createNamespace {
671798
expectedCFNCalls = append(expectedCFNCalls, []*gomock.Call{
799+
mockCloudformation.EXPECT().ValidateStackExists(testNamespaceStackName).Return(fmt.Errorf("Stack Not Found")),
672800
mockCloudformation.EXPECT().CreateStack(gomock.Any(), testNamespaceStackName, false, gomock.Any()).Do(func(x, y, w, z interface{}) {
673801
stackName := y.(string)
674802
capabilityIAM := w.(bool)
@@ -682,6 +810,7 @@ func testCreateServiceDiscovery(t *testing.T, networkMode string, ecsParamsSD *u
682810
}...)
683811
}
684812
expectedCFNCalls = append(expectedCFNCalls, []*gomock.Call{
813+
mockCloudformation.EXPECT().ValidateStackExists(testSDSStackName).Return(fmt.Errorf("Stack Not Found")),
685814
mockCloudformation.EXPECT().CreateStack(gomock.Any(), testSDSStackName, false, gomock.Any()).Do(func(x, y, w, z interface{}) {
686815
stackName := y.(string)
687816
capabilityIAM := w.(bool)

0 commit comments

Comments
 (0)