[adamjohnson01]

Description

Added AWS EC2 provisioner related to #15

How Has This Been Tested?

Create

Using provider: ec2
Requesting host: boring-wright5 in eu-west-1, from ec2
Host: i-01d86e950d95dacc3, status:
[1/500] Host: i-01d86e950d95dacc3, status:
[2/500] Host: i-01d86e950d95dacc3, status:
[3/500] Host: i-01d86e950d95dacc3, status:
[4/500] Host: i-01d86e950d95dacc3, status:
[5/500] Host: i-01d86e950d95dacc3, status:
[6/500] Host: i-01d86e950d95dacc3, status:
[7/500] Host: i-01d86e950d95dacc3, status:
[8/500] Host: i-01d86e950d95dacc3, status:
[9/500] Host: i-01d86e950d95dacc3, status:
[10/500] Host: i-01d86e950d95dacc3, status:
[11/500] Host: i-01d86e950d95dacc3, status:
[12/500] Host: i-01d86e950d95dacc3, status:
[13/500] Host: i-01d86e950d95dacc3, status:
[14/500] Host: i-01d86e950d95dacc3, status:
[15/500] Host: i-01d86e950d95dacc3, status:
[16/500] Host: i-01d86e950d95dacc3, status:
[17/500] Host: i-01d86e950d95dacc3, status:
[18/500] Host: i-01d86e950d95dacc3, status:
[19/500] Host: i-01d86e950d95dacc3, status:
[20/500] Host: i-01d86e950d95dacc3, status:
[21/500] Host: i-01d86e950d95dacc3, status:
[22/500] Host: i-01d86e950d95dacc3, status:
[23/500] Host: i-01d86e950d95dacc3, status:
[24/500] Host: i-01d86e950d95dacc3, status:
[25/500] Host: i-01d86e950d95dacc3, status:
[26/500] Host: i-01d86e950d95dacc3, status:
[27/500] Host: i-01d86e950d95dacc3, status:
[28/500] Host: i-01d86e950d95dacc3, status:
[29/500] Host: i-01d86e950d95dacc3, status:
[30/500] Host: i-01d86e950d95dacc3, status:
[31/500] Host: i-01d86e950d95dacc3, status:
[32/500] Host: i-01d86e950d95dacc3, status:
[33/500] Host: i-01d86e950d95dacc3, status:
[34/500] Host: i-01d86e950d95dacc3, status:
[35/500] Host: i-01d86e950d95dacc3, status:
[36/500] Host: i-01d86e950d95dacc3, status:
[37/500] Host: i-01d86e950d95dacc3, status:
[38/500] Host: i-01d86e950d95dacc3, status:
[39/500] Host: i-01d86e950d95dacc3, status:
[40/500] Host: i-01d86e950d95dacc3, status:
[41/500] Host: i-01d86e950d95dacc3, status:
[42/500] Host: i-01d86e950d95dacc3, status:
[43/500] Host: i-01d86e950d95dacc3, status:
[44/500] Host: i-01d86e950d95dacc3, status:
[45/500] Host: i-01d86e950d95dacc3, status: active
Inlets OSS exit-node summary:
  IP: 34.245.153.241
  Auth-token: VZby2s9kgDN4hIToHnp0iEAWXCLrcglKeAHyyf3ATc1VpnvwTdQq5qQ1i2CS5lrO

Command:
  export UPSTREAM=http://127.0.0.1:8000
  inlets client --remote "ws://34.245.153.241:8080" \
	--token "VZby2s9kgDN4hIToHnp0iEAWXCLrcglKeAHyyf3ATc1VpnvwTdQq5qQ1i2CS5lrO" \
	--upstream $UPSTREAM

Delete

Using provider: ec2
Deleting host: : i-01d86e950d95dacc3 from ec2

Checklist:

I have:

• updated the documentation and/or roadmap (if required)
• read the CONTRIBUTION guide
• signed-off my commits with git commit -s
• added unit tests

[alexellis]

Hi Adam,

Thank you for this PR.

I would like to suggest a couple of edits:

• please add a commit message body, and wrap it to 72 chars for each line - I like this guide a lot - https://chris.beams.io/posts/git-commit/
• elaborate on the testing done including all the steps you took and output from the console

I'll checkout the code after that. 👍

Alex

[alexellis]

Just a few comments / suggestions.

[mhausenblas]

LGTM, great work @adamjohnson01! Out of curiosity @alexellis: I noticed that the vendored packages are checked in, is that intentional?

[adamjohnson01]

@mhausenblas, I ran a dep ensure after adding the provisioner. Do you not want vendor checked in?

[alexellis]

AFAIK when using dep, the packages are generally checked-in.

[alexellis]

Thank you for this PR and to Michael for the review. I have requested a few minor changes around prints, also have a question on the security group management and on how this was tested (outputs etc).

[mhausenblas]

@adamjohnson01 as I pointed out in this article it's a project policy decision if deps are checked in or not. Seems @alexellis has opted for having them in Git, so we're good. That being said, is there any reason why not to use go mod @alexellis (NB this is obviously an orthogonal discussion and should not block merging this PR)?

[alexellis]

I've seen very few big projects use dep and then not commit the vendor folder, but of course it is possible to ask developers and CI to run "dep ensure" after every sync of git.

is there any reason why not to use go mod

inlets, k3sup, derek etc all fall under the same contributing guide and community of contributors / host company (OpenFaaS Ltd):

https://github.com/openfaas/faas/blob/master/CONTRIBUTING.md#i-need-to-add-a-dependency

[alexellis]

Is DescribeInstanceStatus and DescribeInstances required here, if we know the instance will not be up immediately after the provisioning call?

This feels like it should belong in the Status function

[adamjohnson01]

I am not sure what you mean, this is in the Status function.

[alexellis]

My mistake, the diff must have been showing a merged view.

[alexellis]

Hi Adam, it seems like the code is still blocking, which is not desired for the event-loop which will also be used in the Kubernetes operator.

Provision (fast, non-blocking)
Loop check status (fast, non-blocking)
Print IP (fast, non-blocking)

The same should be the case for Delete.

Could you amend, re-test and then squash the commits into one? Let me know if you have any questions.

[alexellis]

This will be a great addition btw.

[adamjohnson01]

@alexellis , I have removed as much of the blocking as I can. The blocking in the delete function is required as the security group cannot be deleted while the instance exists.

[alexellis]

#16 (comment)

Thank you, and that's fair, do we need to document it with a comment? Additive commits are fine, when they are not reworking behaviour.

[alexellis]

LGTM

[alexellis]

I see you have the comment, awesome! I think the securityGroup function needs a more descriptive name, I've suggested one but will merge now. Thank you for your patience and excellent work.

[alexellis]

Released in https://github.com/inlets/inletsctl/releases/tag/0.3.6

[adamjohnson01]

I see you have the comment, awesome! I think the securityGroup function needs a more descriptive name, I've suggested one but will merge now.

Cool, I will create another PR for the function name

Thank you for your patience and excellent work.

My pleasure, happy to contribute!

[weyert]

Thank you @adamjohnson01, I was wondering if you would consider the instance type to micro as that's part of the Amazon Free Tier part so it would be free for the first year. Now you have to build a custom version of inletsctl to allow this (which I did happily) only this tool is also being used by inlets-operator which will provision it's own instance and doesn't seem to allow reusing an existing one.

Maybe we can offer a parameter for it --instance-type which defaults to micro but allows you to change it to nano?