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

Pull Endpoints image if it doesn't exist #786

Merged
merged 3 commits into from
Jun 13, 2019

Conversation

efekarakus
Copy link
Contributor

@efekarakus efekarakus commented Jun 11, 2019

Description of changes:
As a user that's running ecs-cli local up and doesn't have the amazon/amazon-ecs-local-container-endpoints downloaded locally.
I want ecs-cli local up to pull the image for me.
So that I don't have to pull the image myself.


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

Testing

  • Unit tests passed
  • Integration tests passed
  • Unit tests added for new functionality
  • Listed manual checks and their outputs in the comments (example)
  • [N/A] Link to issue or PR for the integration tests: The integ tests need to pull the image to pass.

Documentation

  • 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.

@efekarakus
Copy link
Contributor Author

efekarakus commented Jun 11, 2019

Manual Tests

1. Image is not available locally

$ docker images -f reference=amazon/amazon-ecs-local-container-endpoints                  
REPOSITORY          TAG                 IMAGE ID            CREATED             SIZE
$ ecs-cli local up
INFO[0000] Creating network: ecs-local-network...       
INFO[0015] Created network ecs-local-network with ID b42fe58c8a43cdd61c5c25baf0f83574803f0746df155dd84edfb3c6693e3024 
INFO[0015] Pulling image amazon/amazon-ecs-local-container-endpoints 
INFO[0018] Pulled image amazon/amazon-ecs-local-container-endpoints 
INFO[0018] Created the amazon-ecs-local-container-endpoints container with ID fecd0d8bb61397c5882dbc6905053b6181bcb6e3654997a0e046f8198c46ffe2 
INFO[0019] Started container with ID fecd0d8bb61397c5882dbc6905053b6181bcb6e3654997a0e046f8198c46ffe2

2. Image exists and we do "local up"

$ docker images -f reference=amazon/amazon-ecs-local-container-endpoints               
REPOSITORY                                    TAG                 IMAGE ID            CREATED             SIZE
amazon/amazon-ecs-local-container-endpoints   latest              ccb85faf9315        7 weeks ago         17.4MB
$ ecs-cli local up
INFO[0000] The network ecs-local-network already exists 
INFO[0000] The amazon-ecs-local-container-endpoints container already exists with ID fecd0d8bb61397c5882dbc6905053b6181bcb6e3654997a0e046f8198c46ffe2 
INFO[0000] Started container with ID fecd0d8bb61397c5882dbc6905053b6181bcb6e3654997a0e046f8198c46ffe2 

3. Image download fails due to a network failure

I turned off my wifi while downloading the image here is the experience:

$ ecs-cli local up                        
INFO[0000] The network ecs-local-network already exists 
INFO[0000] Pulling image amazon/amazon-ecs-local-container-endpoints 
FATA[0120] Failed to download the image amazon/amazon-ecs-local-container-endpoints due to context deadline exceeded 

}
defer rc.Close()

ioutil.ReadAll(rc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work?
https://golang.org/pkg/io/ioutil/#ReadAll seems to return two values, both of which are ignored here...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After our discussion offline, I tested with turning off the wifi during img download (see 3. under Manual Tests). Added an error check here, thanks for pointing it out :)

Copy link
Contributor

@PettitWesley PettitWesley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

       !
       !
       ^
      / \
     /___\
    |=   =|
    |     |
    |     |
    |     |
    |     |
    |     |
    |     |
    |     |
    |     |
    |     |
   /|##!##|\
  / |##!##| \
 /  |##!##|  \
|  / ^ | ^ \  |
| /  ( | )  \ |
|/   ( | )   \|
    ((   ))
   ((  :  ))
   ((  :  ))
    ((   ))
     (( ))
      ( )
       .
       .
       .

@efekarakus efekarakus merged commit a6b978a into aws:ecs-local Jun 13, 2019
@efekarakus efekarakus deleted the local/pull-image branch June 19, 2019 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants