Skip to content

Kitchen::Base and Windows support#92

Merged
jjasghar merged 12 commits intomasterfrom
new_kitchen_openstack
Sep 9, 2015
Merged

Kitchen::Base and Windows support#92
jjasghar merged 12 commits intomasterfrom
new_kitchen_openstack

Conversation

@jjasghar
Copy link
Copy Markdown
Contributor

@jjasghar jjasghar commented Sep 1, 2015

This PR is for Windows and Kitchen::Base support.

  • Updating Fog to newest
  • Updating test-kitchen to newest
  • Removing Cane
  • Added better wording for key injection for README.

@jjasghar jjasghar mentioned this pull request Sep 3, 2015
@tfitch
Copy link
Copy Markdown

tfitch commented Sep 4, 2015

FYI - one of my customers is excited about this change and will be trying it out once it is available.

Comment thread lib/kitchen/driver/openstack.rb Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd create a helper method for the hints path:

def hints_path
  Ohai::Config[:hints_path][0]
end

... and use that here so you're not having to dissect an array twice.

JJ Asghar added 3 commits September 8, 2015 14:14
Windows blows up if setup_ssh and add_ohai_hint is called.
Comment thread README.md Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Need to add this due to 1.4 tk.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If username and password are available in the state file (from the driver), you don't need to add them to transport, unless you want to connect with a different account.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'd like to keep this in the readme, but that's good to know. I know with my image i had to put a different administrator password in.

@jjasghar jjasghar changed the title [WIP]: Kitchen::Base and Windows support Kitchen::Base and Windows support Sep 9, 2015
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Where would this option ever get set? I don't see a configuration option in your driver or the base driver for :disable_ssl_validation

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So it's mentioned in the README, about being turned on ONLY if you know what you're doing. I don't think it's supposed to be used or acknowledged without specific reasons.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

To @smurawski's point, while TK doesn't complain if you pass in configuration options that are not specified, it's good practice to define a default so the code self-documents the fact that options are available.

I'd recommend putting this at the top of your class:

default_config :disable_ssl_validation, false

That way it's 100% clear what the intention is, and you never have to worry about an edge case where this being nil bites you :)

@mwrock
Copy link
Copy Markdown
Member

mwrock commented Sep 9, 2015

LGTM but odd that winrm flaps at the beginning causing a need for the winrm_wait. what win version are you testing with? Was the image sysprepped to run a script on initial startup that might restart winrm?

Comment thread lib/kitchen/driver/openstack.rb Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You could offer windows equivalents of these commands. mkdir could just be "mkdir #{hints_path}" and touch could be " '' > #{hints_path}\\openstack.json" (just off the top of my head.

@usepowershell
Copy link
Copy Markdown

@mwrock The flaps are likely due to the cloud-init process as the instance is created. @jjasghar I'm pretty 👍 with a few minor nitpicks.

@jjasghar
Copy link
Copy Markdown
Contributor Author

jjasghar commented Sep 9, 2015

Added this to the readme for #77

Comment thread README.md Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd suggest making this a little clearer...

Test Kitchen 1.4 supports multiple transports, and transports can be configure globally:

transport:
  username: ubuntu
  password: mysecretpassword

... or per-platform:

platforms:
  name: ubuntu-14.04
  transport:
    password: myrootpassword
  name: windows-2012r2
    password: myadministratorpassword

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice, done.

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 this pull request may close these issues.

5 participants