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 Cloud Init installer script #83

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

m03
Copy link
Contributor

@m03 m03 commented Sep 25, 2016

This PR does the following:

  • Adds a script to install Cloudbase Init (cloud initialization service for OpenStack, EC2, etc) into the OS image.

Testing:

  • Tested successfully as part of a Packer build of Windows Server 2012R2 images.

@dragetd
Copy link

dragetd commented Feb 27, 2019

This PR is over two years old. I lack the knowlege to verify the script content.

I wonder: What are the standards by which such tools are included in the images or now? As useful as cloudinit is, is it something most people would benefit?

And the script is not called anywhere, just added to floppy/. Which makes me assume that all floppy/* scripts are run every time - and I would rather like to have LESS installed by default than more.

If the original author is still interested, I'd welcome a different view. If not, we should close this PR.

@m03
Copy link
Contributor Author

m03 commented Feb 27, 2019

I wonder: What are the standards by which such tools are included in the images or now? As useful as cloudinit is, is it something most people would benefit?

At least as beneficial as some of the things that are already included in Boxcutter (OpenSSH, Cygwin, etc), but utility is highly subjective.

And the script is not called anywhere, just added to floppy/. Which makes me assume that all floppy/* scripts are run every time - and I would rather like to have LESS installed by default than more.

The intention is to have it available as something that can be explicitly included if desired. Looking at the example templates, I don't think there is ever an instance where everything in floppy/ is included in the floppy_files array for installation.

If the original author is still interested, I'd welcome a different view. If not, we should close this PR.

See above.

@dragetd
Copy link

dragetd commented Feb 27, 2019

Oh, in this case, this would make a wonderful addition (if this is something that is opt-in). I did not figure out all the workings of this project yet.

The script has a similar pattern to the other scripts. No hardcoded credentials or such. So by my limited Powershell knowledge, this looks good to me!

I'd love to see some life being brought back into this project. :)

@arizvisa arizvisa self-requested a review January 3, 2020 20:37
@arizvisa arizvisa added enhancement This will introduce or improve an already existing capability PR Priority (3) -- Minor This PR refactors a particular component (no major side effects), or introduces a cool feature labels Jan 3, 2020
@arizvisa
Copy link
Contributor

arizvisa commented Jan 3, 2020

@dragetd, @Mo3. Sorry that we're getting to this PR soo much later.. Now that I've been invited into helping maintain this repo, I just added some new labels to help prioritize some of these PRs that we've been lagging on.

Currently, we do provisioning inside scripts/cmtool.bat, are either of you interested in updating this PR to move the contents of install-cloudinit.cmd into that script? Something similar to PR #182 is a good example (maybe at some point in the future we can consider splitting each of the CM_* options into separate scripts)

Although if y'all lost interest in maintaining this, I get it and will try and incorporate this specific PR after I get through some of the other PRs/issues. Lmk.

@arizvisa arizvisa added the stale This issue has had no recent or new discussion and will be forcefully closed in 1 month label Jan 17, 2020
@arizvisa
Copy link
Contributor

Due to lack of discussion, I'm marking this PR as stale. This PR will be closed in a few weeks. If you think this is in error, please let me know and I'll keep it open.

@m03
Copy link
Contributor Author

m03 commented Jan 18, 2020

Cloud Init isn't really config management in the sense that the other tools within cmtool.bat are IMHO, so I don't know that it's the proper place for it vs leaving it standalone. Regardless, if the maintainers require it to be slotted into cmtool.bat, I don't have the time at present to do so.

@arizvisa
Copy link
Contributor

No worries if you don't have the time.

Most of the maintainers have moved onto the boxcutter/windows-ps repository, so this particular is kind of left up to the community to maintain and I've been doing a lot of work to try and get as many abandoned PRs merged in as well as standardize some aspects of boxcutter/windows.

At the moment, I'm hesitant to add things that require floppy/_packer_config.cmd (as its not even documented) or anything that introduces a ton of options that need to be configured before some of other more important problems get fixed and cleaned up (template issues, proxy support for corporate environments, etc).

Currently adding stuff to the floppy/* scripts is really not-easily configurable other than _packer_config.cmd which isn't even documented (unless you dig for it). And if possible adding things as a provisioner step would be much more ideal as the floppy/* scripts are really intended just for rigging things so that Packer can communicate with the guest.

Something like the following could be used to add a provisioner that's easy to configure in your own fork of the templates.

jq '.provisioners[].environment_vars += ["MYENVCONFIG={{user `myenvconfig`}}"] | .provisioners[].scripts += ["script/cloudinit.cmd"]' template.json

But anyways, if you haven't moved on yet and are still interested in getting this merged in, I can try and consider a clean way to do this after I get through some of the other prior things I mentioned. Which would you prefer?

@daxgames
Copy link
Contributor

@arizvisa & @m03 - I don't understand why this is in floppy. It is not required to run this from the floppy before winrm communication is possible.

It should be in script and any env vars should be passed in using the packer template file like everything else in script. The ONLY reason I see to use floppy/_packer_config*.cmd to inject env vars is to minimize packer template modifications to use it.

This is just a really nice example of a script to have in the repo just in case an advanced user wants to use it.

You could leave floppy/_packer_config.cmd alone and just add a floppy/_packer_config_cloud_init.cmd with the following contents:

set CLOUDINIT_64_URL=[alternate x64 url]
set CLOUDINIT_32_URL=[alternate_x86 url]

If the above is not there or or set to nothing then the defaults are used from the script.

I think it should be included as is with some supplemental docs that explain this requires a custom Packer Template supplied by the user.

@daxgames
Copy link
Contributor

@arizvisa I documented _packer_config in #229

@arizvisa
Copy link
Contributor

Hmm.

I really like the solution of using wildcards to allow people customize templates for situations such as CloudInit.

However, if possible I'd really like to have each PR cover features individually to reduce PRs into just their individual capabilities. It makes things hella easier when testing each feature too, as when I was testing each cm tool, I literally needed to verify that every possible combination worked prior to merge... I'll continue the discussion in PR #229 once I get setup for it.

@daxgames
Copy link
Contributor

@arizvisa When I say as-is I would move it to script and add the documentation I spoke of so if a user wanted it he would need to add a custom prefixed packerfile and the new prefix functionality.

Not usre I understand the 'However, if possible I'd really like to have each PR cover features individually to reduce PRs into just their individual capabilities'. This pr is one single feature. Unless you are referring to the floppy/_packer_config*.command in #229. This just enables a capability already in initial install and provisioner scripts already in this repo that most users are probably unaware of unless they dig into the code like we have.

@arizvisa
Copy link
Contributor

Yes, I'm referring to the _packer_config*.cmd that was added to all of the templates. It's not there by default w/o tampering w/ the template directly. This capability which was an addition to all the templates, and the proxy support which updates the download scripts I consider as two different things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This will introduce or improve an already existing capability PR Priority (3) -- Minor This PR refactors a particular component (no major side effects), or introduces a cool feature stale This issue has had no recent or new discussion and will be forcefully closed in 1 month
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants