Feature/aws ami docker support #11

MatthiasScholz · 2017-10-07T18:34:28Z

EXTENSION: AWS AMI Creation with Docker support

The current solution only allows to create AMIs with nomad and consul preinstalled, docker support is missing. This PR adds this for the already supported Amazon Linux and Ubuntu16 AMIs.

NEW: Putting Docker installation routine into the packer script.
REFACTOR: Image setup routine to make the packer script more readable
-> Extracted setup commands into separate scripts.
TEST: Amazon Linux and Ubuntu16 with Docker support were verified.

To keep the packer template clear the installation of nomad and the consul was moved into a separate shell script.

Added support for Docker for the Amazon Linux and Ubuntu16 AMI creation in conjunction with the usage of the AWS ECR ( private repository ). WARNING: Using AWS credentials inside the AMI.

brikis98

Thanks for the PR. I assume your goal here is to show how to create Nomad client nodes that have Docker installed so Nomad can run Docker containers on them? If so, I think that's a great idea, but please update the appropriate READMEs with this information!

Amazon Linux with Docker support was verified, Ubuntu16 not.

Please test both. Installing Docker on Ubuntu can be a bit quirky :)

brikis98 · 2017-10-07T22:28:23Z

examples/nomad-consul-ami/aws_credentials

@@ -0,0 +1,3 @@
+[default]
+aws_access_key_id = TODO_ADD_YOUR_KEY_HERE
+aws_secret_access_key = TODO_ADD_YOUR_SECRET_HERE


I know this is just example code, but I don't want to encourage users storing their secrets in plain text and copying them to a server. It's not a good pattern to follow. A better pattern to follow is to create an IAM role with ECR permissions and to attach that IAM role to the server (I believe the ECR cred helper supports this just fine).

Indeed this is not the recommended production safe way. This approach was chosen for simplicity.

To adapt this to more secure way this will take some more time to update the approach and an increased configuration effort for the user. For testing nomad it might be acceptable. But nevertheless this security hole should be mentioned in the documentation.

To adapt this to more secure way this will take some more time to update the approach and an increased configuration effort for the user

You can make it part of the example code directly! You just attach ECR permissions to the IAM role of the Nomad client servers. It's actually much easier for the user this way, and there is no security hole.

What do you think about adding this module to do the trick?
https://github.com/cloudposse/terraform-aws-ecr

Interesting find. It seems to do a bit more than we need though. All we really want is an IAM role to read from ECR, right?

It depends on the scope and the predefined environment. Since this terraform module does not provide an AWS ECR preconfiguration the user is in charge to provide or create one. When you start from scratch it could be convenient to include the batteries, but it will also enlarge the example.

Maybe it is the best to remove the AWS ECR support from this PR and create a second PR. Most important at the moment is the docker support.

Sure, handling ECR in a separate PR makes sense.

brikis98 · 2017-10-07T22:32:28Z

examples/nomad-consul-ami/nomad-consul-docker-ecr.json

+    },
+    {
+      "ami_name": "nomad-consul-amazon-linux-{{isotime | clean_ami_name}}",
+      "ami_description": "An Amazon Linux AMI that has Nomad and Consul installed.",


The names and descriptions of both of these should be updated to indicate Docker is installed too

Done: Name and description for AWS Linux and Ubuntu

brikis98 · 2017-10-07T22:33:03Z

examples/nomad-consul-ami/nomad-consul-docker-ecr.json

+    {
+      "type": "file",
+      "source": "setup_ubuntu16.sh",
+      "destination": "/tmp/",


Why use the file provisioner to copy this script rather using the script provisioner to execute it directly? Same goes for the same pattern below.

This way was chosen since the content of the script section for packer became hardly readable. Since the script has to be execute on the machine it needs to be copied before.

Is there a more easy way to do this directly in the script section directly?

Yes, use the script parameter: https://www.packer.io/docs/provisioners/shell.html#script

It both uploads a script file and executes it.

This issue was solved with commit:
.REFACTOR script usage, call the provided script directly (#632a73e)

brikis98 · 2017-10-07T22:34:24Z

examples/nomad-consul-ami/nomad-consul-docker-ecr.json

+      "only": [
+        "amazon-linux-ami"
+      ]
+    },


Per my comment above, this should be done using an IAM role at run time rather than hard-coded, plaintext credentials at build time.

brikis98 · 2017-10-07T22:36:20Z

examples/nomad-consul-ami/nomad-consul.json

+     "type": "shell",
+     "environment_vars": [ "NOMAD_VERSION={{user `nomad_version`}}",
+                           "CONSUL_VERSION={{user `consul_version`}}",
+                           "CONSUL_MODULE_VERSION={{user `consul_module_version`}}"],


JSON formatting puts the square brackets on their own lines.

Done: Improved formatting for the whole file

brikis98 · 2017-10-07T22:37:15Z

examples/nomad-consul-ami/setup_ubuntu16.sh

+
+# NOTE: git is already preinstalled
+#echo "[INFO] [${SCRIPT}] Setup git"
+#sudo apt install -y git


Remove this then

This was kept for convenience since it is a real dependency it should be made visible.

Fair enough. Could you update the comment to say "Note: git is required, but it should already be preinstalled on Ubuntu 16.0"

MatthiasScholz · 2017-10-10T20:34:06Z

Updated initial PR request TEST comment after verification. Ubuntu16 is working as well.

MatthiasScholz · 2017-10-20T09:24:41Z

Sorry for the long delay in providing updates to this PR. Sadly at the moment I am to busy to work on this. I will have a look at it again in the middle of November.

AWS ECR support was removed because it is a securtiy flaw to add the AWS credentials into the AMI directly. This feature will be reintergrated later using AWS IAM roles instead.

MatthiasScholz · 2017-11-19T00:29:55Z

UPDATED:

AWS ECR support removed
Explanation regarding preinstalled git on Ubuntu16 added.

brikis98

This looks great, thanks!

Just a final tweak around how scripts are copied/executed.

brikis98 · 2017-11-19T14:01:41Z

examples/nomad-consul-ami/nomad-consul.json

+    },
+    {
+      "type": "file",
+      "source": "setup_nomad_consul.sh",


Why in this Packer template are you copying the script to the server and running it from /tmp, whereas in the above you are just executing it directly using the script keyword? We should stick with one approach for consistency and it seems like the latter is simpler.

brikis98 · 2017-11-19T14:03:28Z

examples/nomad-consul-ami/nomad-consul-docker.json

+  "provisioners": [
+    {
+      "type": "shell",
+      "script": "setup_ubuntu16.sh",


Please use {{template_dir}} so the paths work, even if you're running Packer from a different folder.

brikis98 · 2017-11-19T14:03:32Z

examples/nomad-consul-ami/nomad-consul-docker.json

+    },
+    {
+      "type": "shell",
+      "script": "setup_amazon-linux.sh",


Please use {{template_dir}} so the paths work, even if you're running Packer from a different folder.

brikis98 · 2017-11-19T14:03:40Z

examples/nomad-consul-ami/nomad-consul-docker.json

+        "CONSUL_VERSION={{user `consul_version`}}",
+        "CONSUL_MODULE_VERSION={{user `consul_module_version`}}"
+      ],
+      "script": "setup_nomad_consul.sh"


Please use {{template_dir}} so the paths work, even if you're running Packer from a different folder.

brikis98

Wonderful, thank you!

brikis98 · 2017-11-22T23:41:32Z

https://github.com/hashicorp/terraform-aws-nomad/releases/tag/v0.0.4

Fix hashicorp#11

MatthiasScholz added 3 commits October 7, 2017 20:19

.REFACTOR packer extracted nomad and consul creation

90e8a8c

To keep the packer template clear the installation of nomad and the consul was moved into a separate shell script.

.NEW packer AWS AMI creation with Docker + AWS ECR

a1398d2

Added support for Docker for the Amazon Linux and Ubuntu16 AMI creation in conjunction with the usage of the AWS ECR ( private repository ). WARNING: Using AWS credentials inside the AMI.

.BUGFIX packer Ubuntu16: docker installation + missing make

8e66039

brikis98 suggested changes Oct 7, 2017

View reviewed changes

MatthiasScholz added 5 commits October 8, 2017 10:32

.UPDATE pretty printing json file

1bbc5b0

.UPDATE adding Docker support in AMI name and description

e4cf5a7

.REFACTOR script usage, call the provided script directly

632a73e

.BUGFIX folder creation for .aws and .docker before file copy

d12a4d1

.CLEANUP removed duplication using $HOME variable

5a8e8ee

MatthiasScholz added 3 commits November 18, 2017 22:11

.UPDATE ubuntu16 git preinstalled comment

d35bbd9

.REFACTOR removed AWS ECR support

7a47e6b

AWS ECR support was removed because it is a securtiy flaw to add the AWS credentials into the AMI directly. This feature will be reintergrated later using AWS IAM roles instead.

.BUGFIX removed aws profile support, after bugfix in Packer 1.1.2

5f2852d

brikis98 suggested changes Nov 19, 2017

View reviewed changes

MatthiasScholz added 2 commits November 22, 2017 19:26

.REFACTOR increase generic approach - make use of template_dir

84e6eaa

.BUGFIX use direct script execution instead of file copy

9cccf7c

brikis98 approved these changes Nov 22, 2017

View reviewed changes

brikis98 merged commit 78e9478 into hashicorp:master Nov 22, 2017

MatthiasScholz deleted the feature/aws-ami-docker-support branch November 24, 2017 20:32

Tethik pushed a commit to Tethik/terraform-aws-nomad that referenced this pull request Feb 27, 2018

Make this module more consistent with the terraform-aws-consul module.

200030d

Fix hashicorp#11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/aws ami docker support #11

Feature/aws ami docker support #11

MatthiasScholz commented Oct 7, 2017 •

edited

Loading

brikis98 left a comment

brikis98 Oct 7, 2017

MatthiasScholz Oct 8, 2017

brikis98 Oct 8, 2017

MatthiasScholz Oct 10, 2017

brikis98 Oct 11, 2017

MatthiasScholz Oct 14, 2017

brikis98 Oct 14, 2017

brikis98 Oct 7, 2017

MatthiasScholz Oct 8, 2017

brikis98 Oct 7, 2017

MatthiasScholz Oct 8, 2017

brikis98 Oct 8, 2017

brikis98 Oct 8, 2017

MatthiasScholz Oct 10, 2017

brikis98 Oct 7, 2017

brikis98 Oct 7, 2017

MatthiasScholz Oct 8, 2017

brikis98 Oct 7, 2017

MatthiasScholz Oct 8, 2017

brikis98 Oct 14, 2017

MatthiasScholz commented Oct 10, 2017 •

edited

Loading

MatthiasScholz commented Oct 20, 2017

MatthiasScholz commented Nov 19, 2017 •

edited

Loading

brikis98 left a comment

brikis98 Nov 19, 2017

brikis98 Nov 19, 2017

brikis98 Nov 19, 2017

brikis98 Nov 19, 2017

brikis98 left a comment

brikis98 commented Nov 22, 2017

Feature/aws ami docker support #11

Feature/aws ami docker support #11

Conversation

MatthiasScholz commented Oct 7, 2017 • edited Loading

EXTENSION: AWS AMI Creation with Docker support

brikis98 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MatthiasScholz commented Oct 10, 2017 • edited Loading

MatthiasScholz commented Oct 20, 2017

MatthiasScholz commented Nov 19, 2017 • edited Loading

brikis98 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brikis98 left a comment

Choose a reason for hiding this comment

brikis98 commented Nov 22, 2017

MatthiasScholz commented Oct 7, 2017 •

edited

Loading

MatthiasScholz commented Oct 10, 2017 •

edited

Loading

MatthiasScholz commented Nov 19, 2017 •

edited

Loading