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.

Sign up for GitHub

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

Jump to bottom

Updated installing salt with its bootstrap script, and enabled specifying the CM_OPTIONS for all the configuration management tools #201

Merged
merged 12 commits into from
Jan 9, 2020
Merged

Updated installing salt with its bootstrap script, and enabled specifying the CM_OPTIONS for all the configuration management tools #201

merged 12 commits into from
Jan 9, 2020

Conversation

arizvisa
Copy link
Contributor

@arizvisa arizvisa commented Jan 6, 2020 •
edited
Loading

This PR is based on PR #182, and supersedes it. The original intent of the PR is to fix the installation of the SaltStack configuration management tool. Originally we were hardcoding the path to the SaltStack minion url, dowloading, and then installing it. Since then SaltStack has introduced a bootstrap script (https://github.com/saltstack/salt-bootstrap) to install a minion which is used by this PR to automatically determine the url of the correct package to install.

The original PR introduced a new option, CM_OPTIONS, that allows the user to customize the options for SaltStack. It was discovered that in script/cmtool.bat, all of the configuration tools use their own variable for passing options to their installer (i.e. PUPPET_OPTIONS, CHEF_OPTIONS, etc).

Despite their available, they have each been left undefined. As it seems that passing options to each of these was intended, this PR extends the CM_OPTIONS capability to each of the configuration management tools. This allows the user to explicitly control the options if they so desire.

It should also be mentioned that all of our templates have been modified to support the CM_OPTIONS environment variable. (This is why this PR touches so many files).

@arizvisa arizvisa added bug A bug in the way that a template is built or provisioned enhancement This will introduce or improve an already existing capability labels Jan 6, 2020
@arizvisa arizvisa self-assigned this Jan 6, 2020
@arizvisa arizvisa mentioned this pull request Jan 6, 2020
Closed
@arizvisa
Copy link
Contributor Author

arizvisa commented Jan 6, 2020 •
edited
Loading

Presently these two cases against the eval-win7x64-enterprise.json template.

  • saltlatest
  • salt2019.0

@arizvisa
Copy link
Contributor Author

arizvisa commented Jan 6, 2020

Lol. This PR is blocked by saltstack/salt-bootstrap#1406. Go figure.

Depending on how they respond (I'm not crossing my fingers or anything), this might result in bundling salt-bootstrap.ps1 in this project.

@arizvisa
Copy link
Contributor Author

arizvisa commented Jan 6, 2020

This PR is also blocked by saltstack/salt-bootstrap#1394. SaltStack's such a joke sometimes...

@arizvisa
Copy link
Contributor Author

arizvisa commented Jan 7, 2020

As saltstack has a tendency to drop the ball on their issues and PRs, I'm right now testing with letting the user specify the branch that salt-bootstrap.ps1 is fetched from, and defaulting to a branch without the aforementioned issues.

If this works, i'll update this PR with the changes and the documentation with the revision that's hardcoded prior to merge.

@arizvisa
Copy link
Contributor Author

arizvisa commented Jan 8, 2020

Going to split the CM_OPTIONS parameter from this PR so that the other config management tools can benefit from this. It seems that with the recent merge of PR #200, exposing an external option can be used for things like licensing, branches, etc.

@arizvisa
Copy link
Contributor Author

arizvisa commented Jan 8, 2020

The split of the CM_OPTIONS parameter is done by PR #203, this PR will be rebased onto that one.

rominf and others added 11 commits January 9, 2020 02:08
@rominf @arizvisa
Install Salt using Salt bootstrap script
b6d46ba 
Official bootstrap script written in Powershell is used.
`CM_OPTIONS` are passed as arguments (see `README.md`).

PS: old way of installing Salt was limited and no longer works.
@arizvisa
Added the requested change in README.md for PR boxcutter#182.
3bbf19e
@arizvisa
Added the requested change to script/cmtool.bat for PR boxcutter#182 …
56059c5 
…which retains usability of the CM_VERSION variable when installing salt.
@arizvisa
Added support for applying the CM_OPTIONS parameter to each of the co…
d11167e 
…nfiguration management tools, and updated README.md to reference it.
@arizvisa
Updated script/cmtool.bat to patch the downloaded installer for salt-…
c2979cf 
…bootstrap in order to fix issue saltstack/salt-bootstrap#1406.
@arizvisa
Updated script/cmtool.bat to detect the platform major and minor vers…
8815f01 
…ion.
@arizvisa
Fixed the way the platform version was being extracted in script/cmto…
06fda67 
…ol.bat.
@arizvisa
Added a fallback to the older method of installing salt to script/cmt…
f45f9cc 
…ool.bat when using a platform not supported by salt-bootstrap.
@arizvisa
Added a missing error check to the saltbootstrap branch of script/cmt…
8239cbd 
…ool.bat
@arizvisa
Fixed the major/minor version extraction in script/cmtool.bat so the …
07a92be 
…versions are properly trimmed.
@arizvisa
Older versions of Windows Powershell do not support TLS too well, so …
04b0ad4 
…script/cmtool.bat now fetches using non-ssl when falling back to the saltrepository method.
@arizvisa
Copy link
Contributor Author

arizvisa commented Jan 9, 2020

(force pushed due to rebase on top of master)

@arizvisa arizvisa changed the title [WIP] Updated installing salt with its bootstrap script, and enabled specifying the CM_OPTIONS for all the configuration management tools Updated installing salt with its bootstrap script, and enabled specifying the CM_OPTIONS for all the configuration management tools Jan 9, 2020
@arizvisa
Copy link
Contributor Author

arizvisa commented Jan 9, 2020

Okay. So here's what was done.

  • Original PR introduced salt-bootstrap for fetching the minion, this uses that same code. In script/cmtool.bat this is the :saltbootstrap label.
  • Turns out that though that latest stable release of salt-bootstrap is busted and has an undocumented dependency on the .NET Framework 4.5 (Cannot convert value "Tls,Tls1,Tls2" to type "System.Net.SecurityProtocol.Type" saltstack/salt-bootstrap#1406). This was fixed by patching in the numerical constants since it's not that the webclient doesn't support Tls11 and Tls12, it just doesn't define the enumerations. This was done with Powershell and it seems to work on the 4 youngest templates in our repository.
  • However, Tls11 isn't supported on Win7. To deal with this I added platform detection to script/cmtool.bat and we fall back to the original method over plain-old http/1.1. Platform detection is provided by exposing variables that are available to all the tools. These are PlatformMajorVersion and PlatformMinorVersion. We're fortunate that for supporting CM_OPTIONS the parameters are the same between both methods. In script/cmtool.bat this is done in the :saltrepository label.
  • Because SaltStack is fantastic and they only write smoke-tests instead of comprehensive tests, using latest for salt-bootstrap doesn't work. This is documented in issue powershell sat-minion on windows is breaking! saltstack/salt-bootstrap#1394. We deal with this in the :saltbootstrap label by hardcoding the latest version of salt.

@arizvisa
Copy link
Contributor Author

arizvisa commented Jan 9, 2020 •
edited
Loading

I'm re-testing the following platforms after the force-push.

  • eval-win7x86-enterprise.json 2019.2.2
  • eval-win7x86-enterprise.json (latest)
  • eval-win7x64-enterprise.json 2019.2.2
  • eval-win7x64-enterprise.json (latest)
  • eval-win81x86-enterprise.json 2019.2.2
  • eval-win81x86-enterprise.json (latest)
  • eval-win81x64-enterprise.json 2019.2.2
  • eval-win81x64-enterprise.json (latest

@arizvisa
Updated script/cmtool.bat to emit the detected platform version, and …
012598e 
…added checks for more versions when determining whether to use saltbootstrap or saltrepository.
@arizvisa
Copy link
Contributor Author

arizvisa commented Jan 9, 2020 •
edited
Loading

Added some futureproofing by including some more checks for platform versions as per https://docs.microsoft.com/en-us/windows/win32/sysinfo/operating-system-version

@arizvisa
Copy link
Contributor Author

arizvisa commented Jan 9, 2020

Okay, other than wget.exe issues and other template-specific things (that eventually need fixing). All relevant code checks out. Merging...

@arizvisa arizvisa merged commit 12ed28a into boxcutter:master Jan 9, 2020
@arizvisa
Copy link
Contributor Author

arizvisa commented Jan 9, 2020

Goddammit, that was supposed to be a merge-commit.

@arizvisa arizvisa mentioned this pull request Jan 10, 2020
Merged
@arizvisa arizvisa mentioned this pull request Feb 17, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@arizvisa arizvisa

Labels
bug A bug in the way that a template is built or provisioned enhancement This will introduce or improve an already existing capability
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@arizvisa @rominf