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

expose with_public_ip var of metadata service #42

Merged
merged 3 commits into from
Nov 29, 2022

Conversation

andreinechaev
Copy link
Contributor

No description provided.

@@ -141,3 +141,9 @@ variable "extra_ui_static_env_vars" {
default = {}
description = "Additional environment variables for UI static app"
}

variable "with_public_ip" {
Copy link
Contributor

Choose a reason for hiding this comment

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

@andreinechaev if you expose, you'll need to add to the README as well

Copy link
Member

Choose a reason for hiding this comment

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

nit: now that I think of it we should probably make the description more descriptive too (I understand this PR copied it from the submodule, but still). Maybe something like "enable public IP assignment for the metadata service".

@oavdeev
Copy link
Member

oavdeev commented Oct 7, 2022

@Erin-Boehmer how did this work for you, since you added the option not to assign the IP in the first place in #37 ?

I believe @andreinechaev ran into this issue with ECS not being able to pull images when it doesn't have IP address but runs in a public subnet. If it was a private subnet, I think it would be fine granted NAT gateway and Internet gateway are set up as usual.

We should maybe add a note that you should either

  1. use a public subnet and remember to set with_public_ip to true
  2. use a private subnet, then with_public_ip doesn't matter (?)
  3. use a public subnet, leave with_public_ip to false, but configure the module to pull container images from a private repo accessible from within the VPC

I'm now trying to think what's the best way to communicate this to module users 🤔 Maybe add something in the description for the subnet setting too.

@andreinechaev andreinechaev requested review from oavdeev and Erin-Boehmer and removed request for oavdeev and Erin-Boehmer October 17, 2022 08:50
Copy link
Member

@oavdeev oavdeev left a comment

Choose a reason for hiding this comment

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

sorry for the delay, im going to include this into the next release

@leeyh20
Copy link

leeyh20 commented Nov 23, 2022

@oavdeev Agree, i ran into the same issue today.
I was running this:
https://github.com/outerbounds/terraform-aws-metaflow/blob/v0.7.1/examples/minimal/minimal_example.tf
And because ECS is set to automatically use a private ip, the ECS cannot pull the image for metadata service.

image

Therefore now the minimal example would not work, since the minimal example uses public subnets for the ECS.

According to AWS, private subnets should have with_public_ip set to false and public subnets should have with_public_ip set to true.
Link: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task_cannot_pull_image.html

Maybe it would be good to add this into a quickstart guide or something.

@oavdeev
Copy link
Member

oavdeev commented Nov 29, 2022

I'm thinking I'll add a note and maybe even remove the default in the next release, so the users have to read the note and make a conscious decision depending on their subnet setup

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.

4 participants