Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

adding IBM Bluemix (formerly called soft layer) support for terraform instance plugin #321

Merged
merged 2 commits into from
Dec 6, 2016

Conversation

linsun
Copy link
Contributor

@linsun linsun commented Dec 5, 2016

Adding IBM Bluemix support for terraform instance plugin, the main difference is that the tags are in a list format vs a map. Also, the user data is called user_metadata in the most recent terraform plugin for soft layer: https://github.com/softlayer/terraform-provider-softlayer

Signed-off-by: Lin Sun <linsun@us.ibm.com>
@chungers
Copy link
Contributor

chungers commented Dec 5, 2016

Thanks for the PR. My understanding of this PR is that it adds some bluemix specific logic to

  • merge the tags and user data (Init) in Spec with the tf.json which is formatted differently based on terraform providers (e.g. Type == aws_instance or bluemix_vm_guest).
  • extract the tags from the provider-specific tf.json to fit the shape of map[string]string used by instance.Description when describing instances.

Given this context, a few comments

  1. We should have done a better job of externalizing this type of logic/ transformation via a simple templating mechanism. The vagrant plugin is a good example with the user-customizable vagrant file externalized as a template which the plugin evaluates at runtime. This is a good pattern because it eliminates the need for this type of PR where we'd have to add switches that are tied to providers just to do small data transformations.
  2. The Terraform plugin is an example. Anything more complicated than described in 1, should be forked and become a specialized plugin hosted in separate repo.

That said, it's my fault, not yours, for the short-sighted design. So I would like to propose the following:

  • Merge in this PR first, then
  • I will make the terraform plugin more generic for handling the tag/ user data transformation, via template files.

@wfarner @FrenchBen what do you guys think?

@wfarner
Copy link
Contributor

wfarner commented Dec 5, 2016

I would punt on (1). The more work we put into making an example general-purpose, the more we act against the guidance that it's ostensibly for demonstration only.

@linsun
Copy link
Contributor Author

linsun commented Dec 6, 2016

@chungers your understanding is correct, thank you for review it! If we could eliminate the need for this type of logic and not assume the tag data is always a map, I'm all for the simple approach.

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "master" git@github.com:linsun/infrakit.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842353236544
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@chungers chungers merged commit 51f5aa5 into docker-archive:master Dec 6, 2016
chungers pushed a commit to chungers/infrakit that referenced this pull request Sep 30, 2017
Add cloudwatch logging for user containers
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants