Skip to content
This repository has been archived by the owner on Jun 5, 2020. It is now read-only.

(CLOUD-295) Add RDS resource support #165

Merged
merged 1 commit into from
Jun 9, 2015

Conversation

garethr
Copy link
Contributor

@garethr garethr commented May 14, 2015

This takes on the work from @petems in #161 and:

  • Fixes the failing tests
  • Adds lots more tests
  • Adds types checking of parameters and properties
  • Removes a number of intermediary commits
  • Standardises formatting of property descriptions in the type
  • Expand the assertions in the acceptance tests
  • Add documentation for the new types and properties to the README

@petems
Copy link
Contributor

petems commented May 14, 2015

Awesome! 👍

#### Type: route53*
#### Type: rds_db_parameter_group

Note that this type is currently only available to list via puppet
Copy link
Contributor

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.

@jbondpdx
Copy link
Contributor

jbondpdx commented Jun 1, 2015

@garethr and @petems , I've made comments on the readmes; however, an alternative would be to just merge the docs as they are, and then I can take an editorial pass at them after that. I'd be happy to do do that and then PR docs-only changes.

@garethr garethr force-pushed the rds-support branch 3 times, most recently from f555f4b to 0bded84 Compare June 2, 2015 08:36
@garethr
Copy link
Contributor Author

garethr commented Jun 2, 2015

@jbondpdx I've updated the PR with all the suggested changes, and I've rebased with master to include the other README updates.

@jbondpdx
Copy link
Contributor

jbondpdx commented Jun 2, 2015

Awesome, thank you, @garethr . +1

def destroy
Puppet.info("Deleting database #{name} in region #{resource[:region]}")
rds = rds_client(resource[:region])
Puppet.info("Skip Final Snapshot: #{resource[:skip_final_snapshot]}")
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, fixed.

@garethr garethr force-pushed the rds-support branch 4 times, most recently from c67b307 to 36641b9 Compare June 9, 2015 10:40
@garethr
Copy link
Contributor Author

garethr commented Jun 9, 2015

@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

[:present, :creating, :available].include? @property_hash[:ensure]
end

end
Copy link
Contributor

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)

Copy link
Contributor Author

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

@DavidS
Copy link
Contributor

DavidS commented Jun 9, 2015

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:

vagrant@localhost:/vagrant/git/puppetlabs-aws$ AWS_REGION=eu-west-1 rbenv exec bundle exec puppet resource --libdir=./lib --debug rds_db_parameter_group
Debug: Runtime environment: puppet_version=3.7.5, ruby_version=1.9.3, run_mode=user, default_encoding=UTF-8
Info: Checking if DB Parameter Group testing exists
rds_db_parameter_group { 'testing':
  ensure      => 'present',
  description => 'Bar',
  family      => 'mysql5.1',
  region      => 'eu-west-1',
}
vagrant@localhost:/vagrant/git/puppetlabs-aws$ AWS_REGION=eu-west-1 rbenv exec bundle exec puppet resource --libdir=./lib --debug rds_db_parameter_group testing ensure=present description=foo region=eu-west-1
Debug: Runtime environment: puppet_version=3.7.5, ruby_version=1.9.3, run_mode=user, default_encoding=UTF-8
Debug: Loaded state in 0.00 seconds
Info: Checking if DB Parameter Group testing exists
Error: Could not set 'present' on ensure: undefined method `create' for Rds_db_parameter_group[testing]:Puppet::Type::Rds_db_parameter_group
Error: Could not set 'present' on ensure: undefined method `create' for Rds_db_parameter_group[testing]:Puppet::Type::Rds_db_parameter_group
Wrapped exception:
undefined method `create' for Rds_db_parameter_group[testing]:Puppet::Type::Rds_db_parameter_group
Error: /Rds_db_parameter_group[testing]/ensure: change from absent to present failed: Could not set 'present' on ensure: undefined method `create' for Rds_db_parameter_group[testing]:Puppet::Type::Rds_db_parameter_group
Debug: Finishing transaction 15127380
Debug: Storing state
Debug: Stored state in 0.00 seconds
Info: Checking if DB Parameter Group testing exists
rds_db_parameter_group { 'testing':
  ensure => 'absent',
}
vagrant@localhost:/vagrant/git/puppetlabs-aws$

@DavidS
Copy link
Contributor

DavidS commented Jun 9, 2015

  • Listing a rds_db_securitygroup with puppet resource doesn't show the description.
  • Running puppet resource with ensure=absent, doesn't remove it.
vagrant@localhost:/vagrant/git/puppetlabs-aws$ AWS_REGION=eu-west-1 rbenv exec bundle exec puppet resource --libdir=./lib --debug rds_db_securitygroup test
Debug: Runtime environment: puppet_version=3.7.5, ruby_version=1.9.3, run_mode=user, default_encoding=UTF-8
Info: Checking if DB Security Group test exists
rds_db_securitygroup { 'test':
  ensure   => 'present',
  owner_id => '482693910459',
  region   => 'eu-west-1',
}
vagrant@localhost:/vagrant/git/puppetlabs-aws$ AWS_REGION=eu-west-1 rbenv exec bundle exec puppet resource --libdir=./lib --debug rds_db_securitygroup test ensure=absent
Debug: Runtime environment: puppet_version=3.7.5, ruby_version=1.9.3, run_mode=user, default_encoding=UTF-8
Debug: Loaded state in 0.00 seconds
Debug: Prefetching v2 resources for rds_db_securitygroup
Info: Checking if DB Security Group test exists
Debug: /Rds_db_securitygroup[test]: Nothing to manage: no ensure and the resource doesn't exist
Debug: Finishing transaction 16506260
Debug: Storing state
Debug: Stored state in 0.01 seconds
Info: Checking if DB Security Group test exists
rds_db_securitygroup { 'test':
  ensure => 'absent',
}
vagrant@localhost:/vagrant/git/puppetlabs-aws$ AWS_REGION=eu-west-1 rbenv exec bundle exec puppet resource --libdir=./lib --debug rds_db_securitygroup test
Debug: Runtime environment: puppet_version=3.7.5, ruby_version=1.9.3, run_mode=user, default_encoding=UTF-8
Info: Checking if DB Security Group test exists
rds_db_securitygroup { 'test':
  ensure   => 'present',
  owner_id => '482693910459',
  region   => 'eu-west-1',
}
vagrant@localhost:/vagrant/git/puppetlabs-aws$

@garethr
Copy link
Contributor Author

garethr commented Jun 9, 2015

Fixed

  • puppet resource not showing the description for db_securitygroups
  • removed ensure from the db_parameter_group

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.

DavidS added a commit that referenced this pull request Jun 9, 2015
(CLOUD-295) Add RDS resource support
@DavidS DavidS merged commit b5991e4 into puppetlabs-toy-chest:master Jun 9, 2015
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.

4 participants