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

Changed artifact version #427

Merged
merged 1 commit into from Jul 28, 2015
Merged

Conversation

ravbaba
Copy link
Contributor

@ravbaba ravbaba commented Jul 27, 2015

Changed to a new artifact version with hvm virtualization

@@ -8,7 +8,7 @@ resource "atlas_artifact" "mesos-slave" {
/* Mesos slave instances */
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a comment here exaplaining why is this being harcoded and linking to terraform issue?

Copy link
Member

Choose a reason for hiding this comment

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

could we not do some kind of map here in the variables?

amis = {
 eu-west-1 => 'ami-123',
 us-east-1 => 'ami-456',
}

etc..

and then use that var to replace on the end..

 "${atlas_artifact.mesos-master.metadata_full.region-${var.amis.${region}}}"

above code is syntactically incorrect but you get the gist

or even just use the region string without the AMi map? -

varname = "atlas_artifact.mesos-master.metadata_full.region-${var.region}”

ami = ${varname}

Copy link
Contributor

Choose a reason for hiding this comment

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

Dont think that interpolation is supported by terraform. -> "${atlas_artifact.mesos-master.metadata_full.region-${var.amis.${region}}}"

Think we can harcode it as ${atlas_artifact.mesos-master.metadata_full.region-eu-west-1}
or create a static map region -> ami-id
Then just ami = lookup(var.amis, var.region)

Static mapping would need to be updated every time we build images and the atlas_artifact resource shoudl be removed as it would be useless if we go for second option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As enxebre mentioned the interpolation is not supported by terraform. I tested it with terraform version 0.6.1 (the newest one). So the code:

varname = "atlas_artifact.mesos-master.metadata_full.region-${var.region}”

ami = ${varname}

and my version mentioned in the terraform ticket:

ami = "${lookup(atlas_artifact.mesos-master.metadata_full, concat("region-", var.region))}"

don't work.

@enxebre
Copy link
Contributor

enxebre commented Jul 27, 2015

Labeling as needs work as this needs to wait for having vagrant image with same version of ubuntu and linux kernel

@ravbaba ravbaba force-pushed the aws-source-ami-change branch 3 times, most recently from 181bf18 to c2a0c7b Compare July 27, 2015 15:06
@enxebre
Copy link
Contributor

enxebre commented Jul 27, 2015

This change would need to be applied to aws provider too.

@wallies
Copy link
Contributor

wallies commented Jul 27, 2015

this lgtm

enxebre added a commit that referenced this pull request Jul 28, 2015
@enxebre enxebre merged commit e0164f3 into Capgemini:master Jul 28, 2015
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

4 participants