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

Dynamically find default images for many platforms #221

Merged
merged 11 commits into from
Feb 12, 2016
Merged

Conversation

jkeiser
Copy link
Contributor

@jkeiser jkeiser commented Feb 10, 2016

  • Use search to find latest official AMI of the given OS and version on your region:
    • ubuntu[-14[.04]]
    • windows[-2008|-2012|-2012r2[sp1|sp2|rtm]]
    • rhel[-major[.minor]]
    • debian[-major]
    • centos[-major[.minor]]
    • freebsd[-major[.minor]]
    • fedora[-major]
  • Prefer images by (in order of priority):
    1. Latest version
    2. SSD support over non-SSD
    3. HVM (modern standard) over paravirtual
    4. 64-bit over 32-bit
    5. More recent image creation date
  • Default username: Auto-detect ssh username from image name
  • Default instance_type: t2.micro or t1.micro depending on image type (t2.micro for hvm, t1.micro for paravirtual)
  • Do not default availability zone (default of "a" doesn't work in all regions, and it's not necessary to pass AZ to create an instance)

- Use search to find latest official AMI of the given OS and version on your region:
  - ubuntu[-14[.04]]
  - windows[-2008|-2012|-2012r2[sp1|sp2|rtm]]
  - el[-<major>[.<minor>]]
  - debian[-<major>]
  - centos[-<major>[.<minor>]]
  - freebsd[-<major>[.<minor>]]
  - fedora[-<major>]
- Prefer images by (in order of priority):
  1. Latest version
  2. SSD support over non-SSD
  3. HVM (modern standard) over paravirtual
  4. 64-bit over 32-bit
  5. More recent image creation date
- Restrict to paravirtual if a paravirtual-only instance type is chosen
- Default username: Auto-detect ssh username from image name
- Default instance_type: t2.micro or t1.micro depending on image type (t2.micro for hvm, t1.micro for paravirtual)
- Do not pass availability zone unless specified (default of "a" doesn't work in all regions)

To get started, you need to install the software and set up your credentials and SSH key. Some of these steps you have probably already done, but we include them here for completeness.

1. Install the latest ChefDK and put it in your path.

Choose a reason for hiding this comment

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

The TK maintainers are trying to ensure the project is agnostic to the use case. So, we should probably change this line to add "or ensure the test-kitchen gem is installed on your workstation"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

{ name: key.to_s, values: Array(value).map { |v| v.to_s } }
end

# We prefer most recent first

Choose a reason for hiding this comment

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

This block from 83-110 is so cool. Neat implementation.

@@ -57,20 +66,20 @@ class Ec2 < Kitchen::Driver::Base # rubocop:disable Metrics/ClassLength
default_config :price, nil
default_config :retryable_tries, 60
default_config :retryable_sleep, 5
default_config :aws_access_key_id, nil
default_config :aws_access_key_id, ENV["AWS_SSH_KEY_ID"]

Choose a reason for hiding this comment

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

I think I'm misunderstanding this change (and the one below in 72). The SSH key ID is not the same as the access_key. Can you help me understand this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's wrong :)

@adamleff
Copy link

LGTM. @jkeiser this is some sooper-sweet stuff. Thanks for doing it.

Paging @test-kitchen/kitchen-ec2-maintainers

- name: ubuntu-i386
# 32-bit version of Debian 6
- name: debian-6-i386
# Latest 64-bit stable minor release of freebsd 10
Copy link

Choose a reason for hiding this comment

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

do you mean 64bit? cos the name says i386

@thommay
Copy link

thommay commented Feb 12, 2016

One teensy nit otherwise 👍

@jkeiser jkeiser merged commit 481bc4e into master Feb 12, 2016
@jkeiser jkeiser mentioned this pull request Feb 12, 2016
@lamont-granquist
Copy link
Contributor

Would changing the priority of "64-bit vs 32-bit" to second change anything? HVM is also most likely more important than SSDs, but I suspect most of the SSD images are HVM 64-bit anyway.

And have you tried to converge chef-client on a t1.micro recently with a non-trivial runlist? The last time I tried running chef-client on a t1.micro it got a few minutes into a first converge and then it triggered the CPU governor which took away 99% of the CPU and it took the first converge a few more hours to run when it only had 1% of a CPU left. On a null run-list they probably finish fast enough that the governor does not kick on. That was several years ago so things might have changed, but they were not fit for purpose last time I checked.

@jkeiser
Copy link
Contributor Author

jkeiser commented Feb 13, 2016

@lamont-granquist hmm, I think doing amd64 > i386 earlier makes a lot of sense from a consistency point of view.

For the priority of HVM over SSD, do you think the SSD generally matters more, or the instance_type? That's the main thing you are selecting--the ability to actually run on most instance types.

As far as t1.micro, what do you suggest is a good default? I was super reluctant to bring it out of the free tier, but paravirtual images literally do not work on t1.micro.

@lamont-granquist
Copy link
Contributor

What happens if you remove paravirt/t1.micro images completely? How much coverage do you lose?

@jkeiser
Copy link
Contributor Author

jkeiser commented Feb 13, 2016

You lose some OS's and versions (can't recall which). Certainly nothing recent. But I can't see a reason to just not support them at all, if we have a way ...

@lamont-granquist
Copy link
Contributor

Yeah, but last time I tried the t1.micros they were pretty much unfit for purpose. That's kinda the point I'm not sure you're 'supporting' them just by adding the config...

@tas50 tas50 deleted the jk/ec2-defaults branch January 30, 2022 01:03
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.

None yet

6 participants