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

Allow specifying initial attachment name when creating addon. #227

Closed
ylt opened this issue Jul 31, 2019 · 9 comments · May be fixed by #231
Closed

Allow specifying initial attachment name when creating addon. #227

ylt opened this issue Jul 31, 2019 · 9 comments · May be fixed by #231
Assignees

Comments

@ylt
Copy link

ylt commented Jul 31, 2019

There appears to be no way of specifying the initial attachment name when creating an addon (--as parameter on the Heroku command line, also appears to be referred to in the docs as an alias).

This is exposed fairly nicely in the Heroku Go client, https://github.com/heroku/heroku-go/blob/daded33ff8cdcd8127dcfc3c744686ae2e8d4efc/v5/heroku.go#L388-L390

It doesn't seem that far off in it's current state, as it is constructing this very same object, but providing only Name, Config and Plan (everything but Attachment!): https://github.com/terraform-providers/terraform-provider-heroku/blob/4f59dde/heroku/resource_heroku_addon.go#L99

@mars
Copy link
Member

mars commented Aug 3, 2019

Hi @ylt,

Unless I misunderstand, we implemented this in the most recent release, v2.1.0.

@mars mars closed this as completed Aug 3, 2019
@mars
Copy link
Member

mars commented Aug 3, 2019

Sorry for the abrupt closure. I see what you’re asking. Yes, please go ahead and open a Pull Request if you like. Otherwise, we’ll leave this in the open issues for someone to pickup.

@mars mars reopened this Aug 3, 2019
@ylt
Copy link
Author

ylt commented Aug 3, 2019

No problem @mars, the confusion is probably on me for the brief explanation (was pretty tight on time, wanted to get something out before I forgot).

Also can see how it was confusing, addons effectively have two names - the name of the addon itself (which that particular release fixes), and then the attachment name which is the naming from the apps perspective.

Our use case is that we have multiple Redis servers, one to act as a cache and the other to act as a longer lived key/value store (different maxmemory eviction policies on each), as recommended by Rails: https://guides.rubyonrails.org/caching_with_rails.html#activesupport-cache-rediscachestore.

"Deployment note: Redis doesn't expire keys by default, so take care to use a dedicated Redis cache server. Don't fill up your persistent-Redis server with volatile cache data! Read the Redis cache server setup guide in detail."

The workaround for now has been to make an additional attachment as follows:

resource "heroku_addon" "project-cache" {
  app  = "${heroku_app.project-app.name}"
  plan = "heroku-redis:hobby-dev"
  config = {
    maxmemory_policy = "allkeys-lru"
  }
}

# hopefully we don't need this attachment in the future:
# https://github.com/terraform-providers/terraform-provider-heroku/issues/227
resource "heroku_addon_attachment" "project-cache-attachment" {
  app_id  = "${heroku_app.project-app.name}"
  addon_id = "${heroku_addon.project-cache.id}"
  name = "cache"
}

Main problem is that it's not obvious the feature doesn't exist (spent a few hours trying to find a solution), but the other point is that it can also produce a bit of a mess around the naming.

image
Image 2019-08-03 06-17-10

By adding in initial names, it should make it possible for users to address each of their instances, and clean up the default "REDIS" attachment, also avoiding awful attachment names for further instances such as "HEROKU_REDIS_RED" (making neither name usable, as the naming of each then depends on sequence which can possibly change).


I'll spend a few hours to see if I can get something working.

@davidji99
Copy link
Collaborator

davidji99 commented Aug 5, 2019

Hey @ylt,

Happy to add this in since I added one of the two 'name' attributes in an earlier PR. Let me know. Thanks.

@ylt
Copy link
Author

ylt commented Aug 5, 2019

Hi @davidji99, that really would be appreciated thanks!

@davidji99
Copy link
Collaborator

davidji99 commented Aug 6, 2019

Hi @ylt,

Just my initial observation from working on this enhancement, I think you will likely need to recreate your heroku_addon resource(s) in order to set the attachment_name attribute. This is because the Platform API does not allow users to update an existing addon's attachment name.

@mars
Copy link
Member

mars commented Aug 8, 2019

@davidji99 ,

I think you will likely need to recreate your heroku_addon resource(s) in order to set the attachment_name attribute

We could probably offer guidance to manually update Terraform state so that recreation in not necessary.

@davidji99
Copy link
Collaborator

For sure, some state surgery might be needed to not destroy and recreate the addon.

@mars
Copy link
Member

mars commented May 26, 2022

Closing, with a workaround documented on #241

@mars mars closed this as completed May 26, 2022
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 a pull request may close this issue.

3 participants