-
Notifications
You must be signed in to change notification settings - Fork 217
(CLOUD-295) Add RDS resource support #165
Conversation
|
Awesome! 👍 |
README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not totally sure what this means. I think it is something like:
Note that currently, this type can only be listed via puppet resource, but cannot be created by Puppet.
f555f4b to
0bded84
Compare
|
@jbondpdx I've updated the PR with all the suggested changes, and I've rebased with master to include the other README updates. |
|
Awesome, thank you, @garethr . +1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have a proper message saying what will or will not be done here, instead of just dumping the value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, fixed.
c67b307 to
36641b9
Compare
|
@DavidS I've updated this PR with suggestions and rebased against master. It would be useful to review sooner rather than later in order to make sure @zreichert has time to review in this sprint once it's been through CI. Cheers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When trying to create at rds_db_parameter_group, puppet errors badly:
Error: Could not set 'present' on ensure: undefined method `create' for Rds_db_parameter_group[name=test2]:Puppet::Type::Rds_db_parameter_group
It'd be great if we could add create and destroy methods that fail with useful error messages.
(also, missing newline at EOF)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, why is that even ensurable? I've removed that, much more sensible:
puppet resource rds_db_parameter_group garethr ensure=present region=sa-east-1
Error: Could not run: Invalid parameter ensure
|
I see that rds_db_parameter_group is currently quite limited. It'd be great to have at least one acceptance test for it though. Not the least because I do not understand this behaviour: |
|
|
Fixed
The issue you're seeing with ensure=absent is that you always have to specify the region in the params, not just the ENV variable. This is on purpose, but I think we should change it so what you expected to work just works. See #117, although that work needs prioritising. |
(CLOUD-295) Add RDS resource support
This takes on the work from @petems in #161 and: