Skip to content
This repository has been archived by the owner on Sep 9, 2022. It is now read-only.

Render notification_topic_arn when exporting ElastiCache clusters #436

Merged
merged 1 commit into from
Apr 21, 2019

Conversation

mozamimy
Copy link
Contributor

@mozamimy mozamimy commented Dec 7, 2018

I think it's good to render notification_topic_arn on .tf file and .tfstate file. Because I'm using terraforming with Terraform CLI tool with import subcommand.

In that situation, I work according to the below instructions.

  • Execute terraforming ecc
  • Cut and paste (or process the output by a script) a target cluster TF configuration to a new .tf file.
  • Run terraform import module.elasticache_redis.aws_elasticache_cluster.foo foo to import the actual resource into tfstate file.
  • I confirm that there are no diffs by run terraform plan command.

If the actual resource has a notification configuration, then the terrafrom plan generates the following output.

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  ~ module.elasticache_redis.aws_elasticache_cluster.foo
      notification_topic_arn: "arn:aws:sns:ap-northeast-1:123456789012:bar" => ""


Plan: 0 to add, 1 to change, 0 to destroy.

I’d like the output of terraforming to eliminate diffs after run terraform import. What do you think about this?

I'm using terraforming with the following environment.

  • macOS
  • ruby 2.5.3p105
  • Terraform v0.11.10 with provider.aws v1.39.0

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 92f101d on mozamimy:elasticache-cluster-notify into 1ba96b9 on dtan4:master.

@dtan4
Copy link
Owner

dtan4 commented Apr 21, 2019

Sorry for the late review 🙇

I’d like the output of terraforming to eliminate diffs after run terraform import. What do you think about this?

I agree with this 👍

Copy link
Owner

@dtan4 dtan4 left a comment

Choose a reason for hiding this comment

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

LGTM 👍 It works with the current master (44b4bdd)

@dtan4 dtan4 merged commit 252210e into dtan4:master Apr 21, 2019
@mozamimy mozamimy deleted the elasticache-cluster-notify branch April 21, 2019 21:46
@mozamimy
Copy link
Contributor Author

Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants