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

Adding ignore_changes lifecycle flag #2525

Merged
merged 1 commit into from
Oct 14, 2015

Conversation

robzienert
Copy link
Contributor

First stab at adding an "ignore_updates" lifecycle flag; addresses #2437.

@joekhoobyar
Copy link
Contributor

+1

@maxenglander
Copy link
Contributor

+1!

@JeanMertz
Copy link
Contributor

Yes! Just came looking for this after:

~ aws_eip.core
    instance: "i-92eeed5c" => ""

as I can't let Terraform manage this EIP.

@dalehamel
Copy link

👍

@dalehamel
Copy link

Unfortunately, doesn't seem to work:

* aws_route_table.subnet_routing_table.0: diffs didn't match during apply. This is a bug with Terraform and should be reported.

Maybe this is just an issue with resources with count?

@pmoust
Copy link
Contributor

pmoust commented Jul 24, 2015

Hi @robzienert , please have a look at #2018 (comment) where a meta-paremeter is proposed to ignore changes in targeted attributes of a resource.

@robzienert
Copy link
Contributor Author

@pmoust I like this a lot more. Not sure why I didn't see that issue when I was originally working on this PR. I'll work on getting this updated this coming weekend to use a list of properties, rather than being resource-level.

@berset
Copy link

berset commented Jul 30, 2015

Would love to see this feature! 👍

@robzienert robzienert force-pushed the ignore-updates-lifecycle branch 2 times, most recently from 280cde2 to cba29f2 Compare August 9, 2015 08:06
@robzienert
Copy link
Contributor Author

Finally got around to updating the PR. Right now it ignores attributes by their name, not state ID, but I would like to support both however I'm running into an issue with my implementation. For example:

I have a dynamic route on an aws_route_table resource. When defining a state ID, like route.1157028409 in aws_route_table, the route.# value stays the same, saying that a route is still being removed. Makes me think I'm evaling the diff in the wrong place.

@robzienert robzienert changed the title Adding ignore_updates lifecycle flag Adding ignore_changes lifecycle flag Aug 9, 2015
@robzienert robzienert force-pushed the ignore-updates-lifecycle branch from cba29f2 to 9c0efe4 Compare August 9, 2015 08:10
@phinze
Copy link
Contributor

phinze commented Oct 7, 2015

@robzienert This is looking good! If you could add a covering unit test or two in our higher level context tests, I think this should be merge-able. Let me know if the tests are any trouble and one of us can help pick them up.

@robzienert
Copy link
Contributor Author

Welcome back @phinze. Will look into this - until then, do you have any feedback re my previous comment?

@phinze
Copy link
Contributor

phinze commented Oct 12, 2015

Oh right! Sorry about missing that, @robzienert

Right now it ignores attributes by their name, not state ID, but I would like to support both however I'm running into an issue with my implementation.

I'm not sure we should expose the internals of state storage as an interface here. Why would you need to do this instead of just the attribute names?

Based on your example, it looks like you're trying to partially manage the routes on a route table? For that use case I think we should go for a separate aws_route resource.

@robzienert
Copy link
Contributor Author

Not exposing the state ID is fine, I was just thinking if people wanted to ignore an individual list item - but that would certainly be better suited with a join resource like aws_route. Good call.

@pmoust
Copy link
Contributor

pmoust commented Oct 13, 2015

Would love to see that landing in 0.6.4, thanks for your efforts

@robzienert robzienert force-pushed the ignore-updates-lifecycle branch from 9c0efe4 to a1939e7 Compare October 14, 2015 21:34
@robzienert
Copy link
Contributor Author

PR updated with a context test.

@phinze
Copy link
Contributor

phinze commented Oct 14, 2015

Looks good! Thanks for all your work on this, @robzienert 🙇

phinze added a commit that referenced this pull request Oct 14, 2015
@phinze phinze merged commit b430b98 into hashicorp:master Oct 14, 2015
@berset
Copy link

berset commented Oct 15, 2015

Finally :)

@ghost
Copy link

ghost commented Apr 30, 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 30, 2020
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.

9 participants