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

[WIP] Import resources into Terraform during refresh #3345

Closed
wants to merge 9 commits into from

Conversation

ross
Copy link
Contributor

@ross ross commented Sep 28, 2015

Adds an optional parameter to terraform refresh, -import=path that enables existing resources to be pulled under terraform management. It's a different take on #2022 that I believe allows a bit more functionality/flexibility.

It ignores resource mappings when the resource is already under terraform management. It also allows ignoring mappings for things that aren't yet configured. Those two things allow you to build a large list of mappings (mechanism not included) and then iterate piece by piece on config for them until you're happy with plan's changes for them, ideally nothing. You'll only see the plan for things that are configured so if you start working on a single resource that'll be the only thing included. As soon as you add others to the .tf's they'll start showing up. As #2022 mentions there are gaps in the schema of some resources that cause small plan changes and in some situations even suggest/require resource replacement. Doesn't seem like those should be tackled as part of this PR, but it would be nice to knock them out.

The mapping format is pretty simplistic: module, resource, and id.

root aws_vpc.primary vpc-a12345bc
...

New to the innards of terraform so there's likely things that could be done more cleanly were I more deeply familiar with it. The only thing I'm not particularly happy with was Context.UpdateState. Since Context.state isn't accessible it was either that, making it public, or completely reloading the context prior to refreshing. None of the options seemed great, but this route avoids double verifying and re-reading inputs.

  • Unit Tests
  • Verify that things work as expected with (sub-)modules
  • Documentation

Thoughts & suggestions welcome.

fixes #581

@josephholsten
Copy link
Contributor

@ross is this building correctly on your end?

@ross
Copy link
Contributor Author

ross commented Sep 30, 2015

@ross is this building correctly on your end?

Missed adding a change on terraform.Context. Should build now. Thanks for letting me know.

@ross
Copy link
Contributor Author

ross commented Oct 4, 2015

Added unit tests for the -import option. I'll probably mess around and make sure that it works as desired for submodules, but pause before doing anything beyond that to make sure this is a direction that people want to take. Fwiw, it's working really well for me.

@devth
Copy link

devth commented Oct 14, 2015

This functionality will determine whether my team adopts Terraform. Would love to see this make it into a release soon! ⚡

@ross
Copy link
Contributor Author

ross commented Oct 14, 2015

This functionality will determine whether my team adopts Terraform. Would love to see this make it into a release soon!

Are you able to try it out? I can see import being an adoption blocker for almost anything with existing infrastructure.

@devth
Copy link

devth commented Oct 14, 2015

@ross sure. Do I need to build off your branch? And will it work for OpenStack?

@ross
Copy link
Contributor Author

ross commented Oct 14, 2015

@ross sure. Do I need to build off your branch?

Yeah, you should be able to follow the developing terraform instructions in the main README. When it says to clone this repository you'd instead clone mine and check out this branch.

git clone https://github.com/ross/terraform.git
git checkout refresh-import

If you haven't built terraform before the vagrant route may be the easiest.

And will it work for OpenStack?

It should, though I've only tested it with AWS so YMMV. There are some ~bugs in the providers where details of the metadata are't pulled down fully during refresh. Those will show up as places where plan is showing changes no matter how much you specify in the .tf files.

Let me know if you run in to any problems or have questions. I'd really like to see this move forward and the best way to do that is to get some people kicking the tires.

@jgmchan
Copy link

jgmchan commented Oct 16, 2015

👍 This or the associated PR would be awesome for our team moving to Terraform

@devth
Copy link

devth commented Oct 17, 2015

@ross I was able to build your branch, but I'm blocked on what appears to be a bug introduced in 0.6.4: #3536

Will try again once that's resolved, but I'll probably have to rebase your branch on latest.

Adds a -import option to terraform refresh that allows specifying a mapping
between resources names and ids so that existing infrastructure can be
imported and managed by terraform.
- reworks import file format, `module resource id`
- cleans up importResources a bit and avoids the helper method
- walks modules pulling out their resources so that we only import things
  that have been configured

Better, but still a bit more cleaning up and polishing to go.
@ross
Copy link
Contributor Author

ross commented Oct 18, 2015

@ross I was able to build your branch, but I'm blocked on what appears to be a bug introduced in 0.6.4: #3536

@devth Just pushed a new test branch, refresh-import-v0.6.3, off of the v0.6.3 release tag. I assume since you said it was broken in 0.6.4 this should work for you. It obviously won't have any of the other changes, just my refresh-import work.

https://github.com/ross/terraform/tree/refresh-import-v0.6.3

@ross
Copy link
Contributor Author

ross commented Oct 18, 2015

Just ran in to some cases with the Heroku provider where this implementation isn't sufficent. The same would be the case with the other branch. Basically its provider requires data beyond the ID to successfully make Read calls. Heroku's url structure has nested nouns: /apps/<app>/addons/<ID>/. So more information is required in order to do the initial refresh. I'm sure there are other providers with similar needs.

This will either require extending the import file format to allow specifying the other data or perhaps some sort of copying of the configured (tf) data over in to the initial setup. The former would definitely work, but the latter (if possible) would be nice.

@sparkprime
Copy link
Contributor

Does this work for named resources like security groups and the google resources, where you specify a name that has to be unique across your project / account?

@ross
Copy link
Contributor Author

ross commented Oct 19, 2015

Does this work for named resources like security groups and the google resources, where you specify a name that has to be unique across your project / account?

I've used it with security groups and that works fine if Terraform can manage them this can import them. I haven't tried Google resources, but they should work.

Only known problem at this point is Heroku addons where more than the ID is required to refresh the resources. I looked in to clean solutions to that yesterday that so far haven't been fruitful. There's something I know will work, but I'm trying to find a cleaner way.

@devth
Copy link

devth commented Oct 19, 2015

@ross not sure why but I'm still hitting the same issue I did on 0.6.4. Maybe I'm doing something wrong. I can't make on your branch either - looks like 2 tests fail. I did make dev, then:

± ~/.golang/src/github.com/hashicorp/terraform/bin/terraform -v
~/.golang/src/github.com/hashicorp/terraform/bin/terraform refresh
Terraform v0.6.3

Your version of Terraform is out of date! The latest version
is 0.6.4. You can update by downloading from www.terraform.io

± ~/.golang/src/github.com/hashicorp/terraform/bin/terraform refresh
Error refreshing state: 1 error(s) occurred:

* Post https://identity.vip.foo.com:5443/v2.0/tokens: dial tcp: lookup identity.vip.foo.com on 10.0.1.1:53: no such host

So it looks like the right version but maybe something strange is going on.

Some providers need more than the ID to look up their resources. Heroku
addons being a good example, they need their app.
@ross
Copy link
Contributor Author

ross commented Oct 21, 2015

OK. Pushed a small commit that provides a way to get the "extra" attributes to the state file during import so that refresh would work. That gets heroku addons working. I feel like there should be a better place to be doing this where that data could be fetched from the config, but didn't find it and I'm not familiar enough with the codebase to tackle the refactoring that would probably be required to get import down in to the lower levels cleanly.

@ross
Copy link
Contributor Author

ross commented Oct 21, 2015

So it looks like the right version but maybe something strange is going on.

No clue what's up there. I don't have an openstack setup to run/test against.

@mtb-xt
Copy link

mtb-xt commented Nov 9, 2015

Is there a way to import several similar AWS resources (i.e. subnets or instances) to a single object in Terraform with count attribute?
Other than that the import works excellent for me, would love to see this merged. 👍

@ross
Copy link
Contributor Author

ross commented Nov 9, 2015

Is there a way to import several similar AWS resources (i.e. subnets or instances) to a single object in Terraform with count attribute?

Haven't specifically tried that, but if I had to guess it'd work if you provide <whatever>.0, <whatever>.1, ... for the ids. That's the way count gets expanded.

@radeksimko radeksimko added the wip label Dec 27, 2015
@josephholsten
Copy link
Contributor

@phinze like #2022, this is a resource import implementation that needs 👀 to see if it's the way to proceed.

@phinze phinze removed the wip label Feb 9, 2016
@blakesmith
Copy link
Contributor

Looks like this hasn't been touched in awhile. @ross Are you still interested in doing this work, or do you want to hand it off? I'd be willing to pick up where you left off if you don't have time. This is a key feature we'd need to adopt Terraform.

@ross
Copy link
Contributor Author

ross commented Apr 1, 2016

@ross Are you still interested in doing this work, or do you want to hand it off?

I was originally hoping to get some feedback/direction before working on it more, but either way I won't likely have the time to do so in the near-term so feel free to take it and run.

@PaulCapestany
Copy link

For those watching this thread (particularly @ross + @blakesmith), not sure if you're aware but it appears that @mitchellh is working on a feature branch for importing pre-existing resources: f-import

@ross
Copy link
Contributor Author

ross commented May 3, 2016

For those watching this thread (particularly @ross + @blakesmith), not sure if you're aware but it appears that @mitchellh is working on a feature branch for importing pre-existing resources: f-import

Thanks for the ping @PaulCapestany, that's a massive set of changes. Will keep an 👀 out.

@mitchellh
Copy link
Contributor

Ah sorry @ross I didn't see this PR! I didn't intend to circumvent your existing work. I am indeed working on such things in my branch. The core is actually very complete. However, let me swing back and review this and see if there is anything I should bring in to my branch.

We'll be releasing more details on import shortly...

@ross
Copy link
Contributor Author

ross commented May 3, 2016

Ah sorry @ross I didn't see this PR! I didn't intend to circumvent your existing work. I am indeed working on such things in my branch. The core is actually very complete. However, let me swing back and review this and see if there is anything I should bring in to my branch.

We'll be releasing more details on import shortly...

No worries. It's unlikely there's anything of use here. I was mostly hacking it in without touching the details/internals which was looking to be needed (and I didn't know enough to really dig into.)

@stack72
Copy link
Contributor

stack72 commented May 9, 2016

Hi @ross

After Mitchell's comment (#3345 (comment)) - I am going to close out this PR. Thanks so much for the work that was done on this :) I am looking forward to seeing these features make Terraform in the future

Paul

@stack72 stack72 closed this May 9, 2016
@ross ross deleted the refresh-import branch May 9, 2016 20:56
@ghost
Copy link

ghost commented Apr 26, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 26, 2020
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Import resources into Terraform