Add task-def and file flags to local down/ps #789

iamhopaul123 · 2019-06-13T22:18:55Z

Issue #, if available:

Description of changes:
Added two flags "--task-def" and "--file" for "ps" and "down" options for ecs-cli local for filtering.

Enter [N/A] in the box, if an item is not applicable to your change.

Testing

Unit tests passed
Integration tests passed
[N/A] Unit tests added for new functionality
Listed manual checks and their outputs in the comments (example)
[N/A] Add e2e tests for the new flags
[N/A] Link to issue or PR for the integration tests:

Documentation

[N/A] Contacted our doc writer
Updated our README

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…ilter

.gitignore

ecs-cli/modules/cli/local/ps_app.go

efekarakus

Can you run through the checklist in the PR description:

Run make test to make sure the unit tests pass
Run make integ-test to make sure our e2e tests pass
Add manual tests to illustrate your changes
Also add e2e tests for the new flags.

Also please update the title of the PR to be more description such as "Add task-def and file flags to local down/ps"

ecs-cli/modules/cli/local/down_app.go

ecs-cli/modules/cli/local/ps_app.go

ecs-cli/modules/cli/local/down_app.go

ecs-cli/modules/cli/local/ps_app.go

ecs-cli/modules/cli/local/down_app.go

ecs-cli/modules/commands/local/local_command.go

…p.go

efekarakus · 2019-06-14T16:46:56Z

Looks great! @iamhopaul123 can you also run the integration tests to validate that nothing breaks?

ecs-cli/integ/cmd/compose.go

iamhopaul123 · 2019-06-14T23:06:14Z

Manual Testing

Configuration

Parameters

# docker-compose.yml
version: "3"
networks:
  ecs-local-network:
    external: true
services:
  app:
    build:
      context: .
    ports:
    - "6001:80"
    networks: 
    - ecs-local-network
    environment:
      AWS_DEFAULT_REGION: "us-east-1"
      AWS_CONTAINER_CREDENTIALS_RELATIVE_URI: "/creds"
      ECS_CONTAINER_METADATA_URI: "http://169.254.170.2/v3"
    labels:
      ecsLocalTaskDefType: 'remoteFile'
      ecsLocalTaskDefVal: 'arn:aws:ecs:us-east-1:685593908319:task-definition/efe-test:1'

#Dockerfile
FROM ubuntu:latest
RUN apt-get update \
 && apt-get -y install curl \
 && apt-get -y install awscli \
 && apt-get -y install jq \
 && apt-get -y install net-tools
CMD /bin/bash -c 'while true; do sleep 30; done;'

Note that, I put several "docker-compose.yml" in different directories to avoid recreation.

Environment Setup

make build
ecs-cli local up
docker-compose -f docker-compose.local.yml up -d

Tests

Lists all running containers matching the task family or ARN

ecs-cli local ps --task-def arn:aws:ecs:us-east-1:685593908319:task-defini tion/efe-test:3

CONTAINER ID        IMAGE                 STATUS              PORTS                  NAMES                    TASKDEFINITION
ae907267d51a        untitledfolder2_app   Up 4 minutes        0.0.0.0:6004->30/tcp   /untitledfolder2_app_1   arn:aws:ecs:us-east-1:685593908319:task-definition/efe-test:3

Lists all running containers matching the task definition file path

ecs-cli local ps --file /a/b/c

CONTAINER ID        IMAGE                STATUS              PORTS                  NAMES                   TASKDEFINITION
f1ae19996920        untitledfolder_app   Up 5 minutes        0.0.0.0:6002->70/tcp   /untitledfolder_app_1   /a/b/c

List all running containers matching the task definition file path and task family or ARN

ecs-cli local ps --file /a/b/c --task-def arn:aws:ecs:us-east-1:6855939083 19:task-definition/efe-test:1

FATA[0000] task-def and file can not be used together

Stops and removes all running containers matching the task definition file path

ecs-cli local down --file /a/b/c

INFO[0000] Stop and remove 1 container(s)               
INFO[0010] Stopped container with id f1ae19996920       
INFO[0010] Removed container with id f1ae19996920       
INFO[0010] 2 other task container(s) running locally, skipping network removal

Stops and removes all running containers matching the task family or ARN

ecs-cli local down --task-def arn:aws:ecs:us-east-1:685593908319:task-defi nition/efe-test:1

INFO[0000] Stop and remove 1 container(s)               
INFO[0010] Stopped container with id 7b5b1b82ffc3       
INFO[0010] Removed container with id 7b5b1b82ffc3       
INFO[0010] 1 other task container(s) running locally, skipping network removal

efekarakus

LGTM! 🚀

SoManyHs · 2019-06-18T02:53:35Z

ecs-cli/modules/cli/local/create_app.go

+)
+
+const (
+	ecsLocalDockerComposeFileName = "docker-compose.local.yml"


I have a different constant name for this: LocalOutDefaultFileName https://github.com/aws/amazon-ecs-cli/pull/785/files#diff-c72fc3b09ef311d6734cb6f6b9c9b9a6R36. Was this name discussed outside of the context of the Create PR?

SoManyHs · 2019-06-18T03:06:34Z

ecs-cli/modules/cli/local/create_app.go

+const (
+	localTaskDefType  = "localFile"
+	remoteTaskDefType = "remoteFile"
+)


I'm also a little confused why these four constants are defined in create_app.go right now, as I don't see them being used in the create function. Perhaps it would make sense to move these constants to a different file still under the local package namespace?

I think the idea is that eventually Create() will eventually add these as Docker labels to the containers so that we know if the containers were started with a localFile or remoteFile.

SoManyHs · 2019-06-18T03:17:29Z

ecs-cli/modules/cli/local/create_app.go

 func Create(c *cli.Context) {
 	// 1. read in task def (from file or arn)
 	// 2. parse task def into go object
 	// 3. write to docker-compose.local.yml file
 	fmt.Println("foo") // placeholder
 }
+
+func validateOptions(c *cli.Context) error {


Similarly, not sure why this function lives in this file?

The flags that are being validated apply to all the sub-commands (create, up, down, ps). Should this live in some other file? 🤔 I think we decided to move it in create_app.go because it's the first command that users will use for local but I can see how that's not a strong reason.

iamhopaul123 added 3 commits June 12, 2019 23:55

first edit

0466dd2

add Type filter

551e12c

changed the name for optionsPreCheck and added back the ALL to down f…

46b8b94

…ilter

iamhopaul123 requested a review from a team June 13, 2019 22:18

PettitWesley reviewed Jun 13, 2019

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

PettitWesley reviewed Jun 13, 2019

View reviewed changes

ecs-cli/modules/cli/local/ps_app.go Outdated Show resolved Hide resolved

PettitWesley reviewed Jun 13, 2019

View reviewed changes

ecs-cli/modules/cli/local/ps_app.go Outdated Show resolved Hide resolved

remove Dockerfile from .gitignore and renamed functions

88383b0

iamhopaul123 requested a review from a team June 13, 2019 22:46

efekarakus reviewed Jun 13, 2019

View reviewed changes

iamhopaul123 added 2 commits June 14, 2019 09:32

changed format for string; fatal error text; moved const to create_ap…

0329378

…p.go

solve conflict

72a769e

fixed Timeout Error for TestCreateClusterWithFargateService

520d177

hencrice reviewed Jun 14, 2019

View reviewed changes

ecs-cli/integ/cmd/compose.go Show resolved Hide resolved

iamhopaul123 changed the title ~~Ecs local add filter v1~~ Add task-def and file flags to local down/ps Jun 17, 2019

efekarakus approved these changes Jun 17, 2019

View reviewed changes

iamhopaul123 merged commit 6de165f into aws:ecs-local Jun 17, 2019

iamhopaul123 deleted the ecs-local-add-filter-v1 branch June 17, 2019 23:34

SoManyHs reviewed Jun 18, 2019

View reviewed changes

efekarakus mentioned this pull request Jun 19, 2019

Add local create metadata to local Compose file #801

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add task-def and file flags to local down/ps #789

Add task-def and file flags to local down/ps #789

iamhopaul123 commented Jun 13, 2019 •

edited

Loading

efekarakus left a comment •

edited

Loading

efekarakus commented Jun 14, 2019

iamhopaul123 commented Jun 14, 2019 •

edited

Loading

efekarakus left a comment

SoManyHs Jun 18, 2019 •

edited

Loading

SoManyHs Jun 18, 2019

efekarakus Jun 18, 2019

SoManyHs Jun 18, 2019

efekarakus Jun 18, 2019

Add task-def and file flags to local down/ps #789

Add task-def and file flags to local down/ps #789

Conversation

iamhopaul123 commented Jun 13, 2019 • edited Loading

Description of changes: Added two flags "--task-def" and "--file" for "ps" and "down" options for ecs-cli local for filtering.

efekarakus left a comment • edited Loading

Choose a reason for hiding this comment

efekarakus commented Jun 14, 2019

iamhopaul123 commented Jun 14, 2019 • edited Loading

Manual Testing

Configuration

Parameters

Environment Setup

Tests

Lists all running containers matching the task family or ARN

Lists all running containers matching the task definition file path

List all running containers matching the task definition file path and task family or ARN

Stops and removes all running containers matching the task definition file path

Stops and removes all running containers matching the task family or ARN

efekarakus left a comment

Choose a reason for hiding this comment

SoManyHs Jun 18, 2019 • edited Loading

Choose a reason for hiding this comment

SoManyHs Jun 18, 2019

Choose a reason for hiding this comment

efekarakus Jun 18, 2019

Choose a reason for hiding this comment

SoManyHs Jun 18, 2019

Choose a reason for hiding this comment

efekarakus Jun 18, 2019

Choose a reason for hiding this comment

iamhopaul123 commented Jun 13, 2019 •

edited

Loading

Description of changes:
Added two flags "--task-def" and "--file" for "ps" and "down" options for ecs-cli local for filtering.

efekarakus left a comment •

edited

Loading

iamhopaul123 commented Jun 14, 2019 •

edited

Loading

SoManyHs Jun 18, 2019 •

edited

Loading