Automate the setup of DS Integ and E2E tests #186
Conversation
| os.Exit(errorCode) | ||
| } | ||
|
|
||
| taskDefinitionSleep300 := "esh_test_sleep300" |
There was a problem hiding this comment.
I see you're registering an esh_test_sleep300 task definition, but where are you using it?
There was a problem hiding this comment.
If you're actually using it, can you rename this to ds_test_sleep300?
| @@ -38,11 +46,18 @@ const ( | |||
| invalidCluster = "cluster/cluster" | |||
| badRequestHTTPResponse = "400 Bad Request" | |||
| listEnvironmentsBadRequest = "ListEnvironmentsBadRequest" | |||
|
|
|||
| errorCode = 1 | |||
| configName = "DSASGLaunchConfiguration" | |||
There was a problem hiding this comment.
This is a little ambiguous. Can you rename this variable to launchConfigurationName?
| } | ||
|
|
||
| err = asgWrapper.CreateLaunchConfiguration(configName, clusterName, instanceProfileName, keyPair, amiID) | ||
| if err != nil { |
There was a problem hiding this comment.
What if it's not an awserr.Error?
| } | ||
|
|
||
| err = asgWrapper.CreateAutoScalingGroup(asg, configName, azs) | ||
| if err != nil { |
| os.Exit(errorCode) | ||
| } | ||
|
|
||
| taskDefinitionSleep300 := "esh_test_sleep300" |
There was a problem hiding this comment.
If you're actually using it, can you rename this to ds_test_sleep300?
| } | ||
|
|
||
| asg := wrappers.GetASGName() | ||
| if asg == defaultASGClusterName { |
There was a problem hiding this comment.
What is this for? What if the asg name is different?
There was a problem hiding this comment.
If users wanna pass in their custom ASG, the test will pick up that ASG and not create a new ASG itself.
There was a problem hiding this comment.
Should we validate the user-given ASG?
|
|
||
| func (wrapper AutoScalingWrapper) CreateLaunchConfiguration(configName string, clusterName string, instanceProfileName string, keyName string, amiID string) error { | ||
| // The user data is run when the instance is launched. It registers the instance to the cluster | ||
| // and makes the instance to self-terminate after one hour in case the test fails to clean up. |
There was a problem hiding this comment.
Does self-terminate actually work? If not, might make sense to point out that the instance state is set to stopped (and that it does not actually terminate).
There was a problem hiding this comment.
Sure. I will add a comment here and in the Readme as well.
| - Create ECS cluster (e.g. name=q-daemon-scheduler-test) | ||
| - Create AutoScaling group (e.g. name=ecs-daemon-scheduler-test-asg) which can launch instances into the above ECS cluster. Follow the steps [here](http://docs.aws.amazon.com/AmazonECS/latest/developerguide/launch_container_instance.html) | ||
| - Start blox cluster state service: | ||
| AWS_PROFILE=default AWS_REGION=us-west-2 CSS_LOG_FILE=var/output/logs/css.log ./out/cluster-state-service --queue event_stream --etcd-endpoint localhost:2379 --bind localhost:3000 |
There was a problem hiding this comment.
Can you wrap these two commands into ``` blocks similar to the gucumber ones down below?
| AWS_REGION=... AWS_PROFILE=... gucumber -tags=@integ | ||
| ``` | ||
|
|
||
| - Customization |
There was a problem hiding this comment.
Can you prefix this line with a # instead of a - to make it a header?
|
|
||
| - Customization | ||
| Users can pass in their own cluster, autoscaling group, or EC2 key pair to the tests | ||
| Custom names have to be different from default names: |
There was a problem hiding this comment.
Can you format the options below as a list by prefixing them with *?
| } | ||
|
|
||
| var status string | ||
| if resp.AutoScalingGroups[0].Status != nil { |
There was a problem hiding this comment.
What will happen if resp.AutoScalingGroups has a length of 0? My guess is it will nil pointer panic here.
| os.Exit(errorCode) | ||
| } | ||
|
|
||
| asg := wrappers.GetASGName() |
There was a problem hiding this comment.
Can you change this to asg = instead of asg :=? This is creating a new local variable instead of using the variable defined above.
| forceDelete := true | ||
| // With ForceDelete set to true, this will delete all instances attached to the Autoscaling group | ||
| asgWrapper.DeleteAutoScalingGroup(asg, forceDelete) | ||
| if err != nil { |
There was a problem hiding this comment.
What err are you checking here?
| } | ||
|
|
||
| asgWrapper.DeleteLaunchConfiguration(launchConfigurationName) | ||
| if err != nil { |
There was a problem hiding this comment.
What err are you checking here?
Summary
Automate the setup of DS Integ and E2E tests
#184
Implementation details
Automate the setup of the environment for the CSS E2E test
Testing
cd cluster-state-service; make; cd ../)cd cluster-state-service; make release; cd ../)cd daemon-scheduler; make; cd ../)cd daemon-scheduler; make release; cd ../)New tests cover the changes: no
Description for the changelog
Automate the setup of the environment for the CSS E2E test
Licensing
This contribution is under the terms of the Apache 2.0 License: Yes
Before merging