search

chef / knife-windows

Plugin for Chef's knife tool for working with Windows nodes
Apache License 2.0
152 stars 110 forks source link

adding client.d functionality for bootstrapping windows nodes #443

Closed stubblyhead closed 5 years ago

stubblyhead commented 6 years ago

Starting in Chef 12.8 you can optionally have the contents of the client.d directory (or alternate path if preferred) from workstation to a new node during bootstrap, but for whatever reason this never made it into the knife-windows plugin. chef-client already knows what to do with it once it's there, it's just a matter of getting the bits from point A to point B. This branch adds that functionality.

spuder commented 6 years ago

I suspect if the client.d directory already exists, cmd.exe will throw an error on this line mkdir <%= bootstrap_directory %>\client.d

This may be safer

if not exist "<%= bootstrap_directory -%>\client.d\" mkdir <%= bootstrap_directory -%>\client.d

The trailing slash after client.d is intentional. See comments on this SO for why:

https://stackoverflow.com/a/20688004/1626687

Also shouldn't <%= bootstrap_directory %> be <%= bootstrap_directory -%> to prevent newline characters?

stubblyhead commented 6 years ago

Sorry for the radio silence on my part--I don't think I got a notification of your comment and just saw it the other day. I'll dig into this again and hopefully have a new commit for you sometime next week.

stubblyhead commented 6 years ago

I've finally had a chance to look at this again, thanks for your patience.

Regarding the pre-existence of the client.d directory on the target node--Windows doesn't seem to care in the slightest if the directory already exists, or even if there's already a regular file called client.d. I can't find so much as a warning about it even with debug level logging.

As to <%= bootstrap_directory %> vs <%= bootstrap_directory -%>, the former is used liberally in the master I forked, while the latter isn't used at all. I don't see a situation where there'd be a newline to strip, and it seems like if this was a potential pitfall someone would have fallen into it by now.

spuder commented 6 years ago

You are correct. I no longer think my concerns are worth investigating.

  1. mkdir safety - not needed since other locations in .bat execute mkdir without checks
  2. newline characters - probably not needed
btm commented 5 years ago

Combined this into #461