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

Add support for looking up AMIs with the EC2 API #177

Merged
merged 5 commits into from
Oct 27, 2015

Conversation

zl4bv
Copy link
Contributor

@zl4bv zl4bv commented Jul 16, 2015

Addresses #147. When a hash of filters are provided for the image_id it will try to find the AMI with the most recent creation date that matches the supplied filters.

Example usage (.kitchen.yml):

platforms:
  - name: windows2012r2
    driver:
      image_id:
        name: Windows_Server-2012-R2_RTM-English-64Bit-Base-*

Example usage (amis.json):

{
  "regions": {
    "ap-southeast-2": {
      "windows-2012r2": {
        "name": "Windows_Server-2012-R2_RTM-English-64Bit-Base-*"
      }
    }
  }
}

@willejs
Copy link

willejs commented Aug 24, 2015

Is there an update on this? Would love to see it merged!

@puckel
Copy link

puckel commented Sep 8, 2015

Hi,

👍 for this PR, it'll be very useful. Maybe add an environment variable for image_id too.

Thanks

@zl4bv
Copy link
Contributor Author

zl4bv commented Sep 8, 2015

Hey @puckel, if you want to set the image_id with an environment variable then you can add something like the following to your .kitchen.yml file:

platforms:
  - name: blah
    driver:
      image_id: "<%= ENV['IMAGE_ID'] %>"

where the environment variable IMAGE_ID has your desired AMI ID. This will work without the pull request.

@tjnicholas
Copy link

@tyler-ball any estimate when this might get merged and/or released? It looks like it'll address several open issues. #174 #117 #147

@tyler-ball
Copy link
Contributor

Sorry this got dropped. The code and tests look good, but I see a few missing features to add before this can be merged.

First, while it is nice to use an existing key I think that will be confusing in the long run. Instead of overloading image_id to take either a string or a hash I think we should just introduce a new config key. Call it image_search and have it take in a hash. If both image_id and image_search are provided then give image_id precedence. Because image_id is currently defaulted you will need to move the defaulting into the finalize_config method. It will look something like

if config[:image_id].nil? && config[:image_search].nil?
  config[:image_id] = instance.driver.default_ami
end

Second, this PR needs to also update the README to outline how to leverage this new logic.

We should leave the existing amis.json logic alone but I think that is another issue we could address in the 1.0 update. Ideally, I would like to get rid of it and use a default image of the latest ubuntu image (using the search tool here). But that should be another PR. My time has been prioritized off of Test Kitchen so if anyone wants to submit that PR I would be happy to review and merge it.

@willejs
Copy link

willejs commented Oct 26, 2015

@zl4bv do you think your going to get round to fixing this with tylers comments?

@zl4bv
Copy link
Contributor Author

zl4bv commented Oct 26, 2015

@willejs sorry, yes, I will try to get it done some time this week.

Addresses requirement that image_id be set:

```
 Kitchen::Driver::Ec2#finalize_config! when image_id is not provided when image_search is provided searches for an image ID
 Failure/Error: driver.finalize_config!(instance)
 Kitchen::UserError:
  Kitchen::Driver::Ec2str#config[:image_id] cannot be blank
 # /home/travis/.rvm/gems/ruby-1.9.3-p551/gems/test-kitchen-1.4.2/lib/kitchen/configurable.rb:434:in `block in required_config'
 # /home/travis/.rvm/gems/ruby-1.9.3-p551/gems/test-kitchen-1.4.2/lib/kitchen/configurable.rb:288:in `call'
 # /home/travis/.rvm/gems/ruby-1.9.3-p551/gems/test-kitchen-1.4.2/lib/kitchen/configurable.rb:288:in `block in validate_config!'
 # /home/travis/.rvm/gems/ruby-1.9.3-p551/gems/test-kitchen-1.4.2/lib/kitchen/configurable.rb:287:in `each'
 # /home/travis/.rvm/gems/ruby-1.9.3-p551/gems/test-kitchen-1.4.2/lib/kitchen/configurable.rb:287:in `validate_config!'
 # /home/travis/.rvm/gems/ruby-1.9.3-p551/gems/test-kitchen-1.4.2/lib/kitchen/configurable.rb:55:in `finalize_config!'
 # ./lib/kitchen/driver/ec2.rb:163:in `finalize_config!'
 # ./spec/kitchen/driver/ec2_spec.rb:125:in `block (5 levels) in <top (required)>'
```
@zl4bv
Copy link
Contributor Author

zl4bv commented Oct 27, 2015

Hey @tyler-ball (and other collaborators), I've made the suggested changes. Do you think it is ready to be merged now?

tyler-ball added a commit that referenced this pull request Oct 27, 2015
Add support for looking up AMIs with the EC2 API
@tyler-ball tyler-ball merged commit ed68c6f into test-kitchen:master Oct 27, 2015
@willejs
Copy link

willejs commented Oct 27, 2015

YES! Can you cut a release please @tyler-ball

@zl4bv zl4bv deleted the ami_lookup branch October 28, 2015 06:48
@puckel
Copy link

puckel commented Nov 5, 2015

Hi,
When the next release is planned?
Thanks

@willejs
Copy link

willejs commented Nov 5, 2015

new release FTW!

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

Successfully merging this pull request may close these issues.

5 participants