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

(BOLT-742) Add support for collections to install task #312

Merged
merged 1 commit into from Aug 9, 2018

Conversation

MikaelSmith
Copy link
Contributor

@MikaelSmith MikaelSmith commented Aug 8, 2018

Adds a collections parameter that defaults to 'puppet'. Available values

  • 'puppet5'
  • 'puppet6', for future support when it's released
  • 'puppet', aliases to the latest available collection

Install will select either the latest version from that collection, or
only work if you provide a valid version within that collection. To
install versions outside the default collection you'll need to
explicitly specify the collection and version.

@MikaelSmith

This comment has been minimized.

Copy link
Contributor

@adreyer adreyer left a comment

Choose a reason for hiding this comment

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

Is there a reason to have "puppet" instead of null as the latest option?
Will changing the latest value be a breaking module change?

@MikaelSmith
Copy link
Contributor Author

'puppet' implicitly maps to latest (according to our release team). If you want a specific collection, I'd choose one, so I don't want to consider updating 'latest' to a new collection to be breaking. Also, what would 'null' mean in terms of 'collection'?

@MikaelSmith
Copy link
Contributor Author

I'm not very convinced we need to support PC1. It's no longer developed and it introduces lots of special cases.

@MikaelSmith
Copy link
Contributor Author

Removed PC1 support.

@adreyer
Copy link
Contributor

adreyer commented Aug 8, 2018

I didn't realize puppet was a thing on our release servers. +1

results = run_task_on('target', 'puppet_agent::version')
results.each do |res|
expect(res['status']).to eq('success')
expect(res['result']['version']).to eq('5.5.2')
Copy link
Member

@donoghuc donoghuc Aug 8, 2018

Choose a reason for hiding this comment

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

For some reason there is no 5.5.2 for windows in either puppet or puppet5 collections. This will fail the windows acceptance tests. Same for osx 10.11. Unfortunately there does not seem to be a common version for all the different targets in our nodeset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like 5.5.2 was burned. I’ll update to 5.5.3.

@@ -419,8 +417,7 @@ install_file() {
info "installing puppetlabs dmg with hdiutil and installer"
mountpoint="$(mktemp -d -t $(random_hexdump))"
/usr/bin/hdiutil attach "${download_filename?}" -nobrowse -readonly -mountpoint "${mountpoint?}"
latest_macos_version $mountpoint
/usr/sbin/installer -pkg "${mountpoint?}/puppet-agent-${dmg_version?}-installer.pkg" -target /
/usr/sbin/installer -pkg ${mountpoint?}/puppet-agent-*-installer.pkg -target /
Copy link
Member

Choose a reason for hiding this comment

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

Using the wildcard to match version in .pkg path is brilliant. Wish I would have thought of that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is only needed for PC1 packages. But it seems reasonable enough.

Copy link
Member

@donoghuc donoghuc left a comment

Choose a reason for hiding this comment

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

The change to the task code looks great. The spec test to install a specific version will need to be updated to work with every target in the nodeset.

"parameters": {
"version": {
"description": "The version of puppet-agent to install",
"type": "Optional[String]"
},
"collection": {
"description": "The Puppet collection to install from (defaults to puppet, which maps to the latest collection released)",
Copy link
Member

Choose a reason for hiding this comment

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

Should the description be ... maps to the latest release in the puppet collection? I interpret latest collection released to mean the latest version in any collection. Maybe this is intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

puppet maps to puppet5 now, will switch to puppet6 once released. So open to anything to clarify that.

Copy link
Member

Choose a reason for hiding this comment

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

I just assumed that latest would mean latest in the collection. Should have tested before commenting. I think your description is clear as is.

Adds a collections parameter that defaults to 'puppet'. Available values
- 'puppet5'
- 'puppet6', for future support when it's released
- 'puppet', aliases to the latest available collection

Install will select either the latest version from that collection, or
only work if you provide a valid version within that collection. To
install versions outside the default collection you'll need to
explicitly specify the collection and version.
@MikaelSmith
Copy link
Contributor Author

Updated to 5.5.3.

@MikaelSmith MikaelSmith merged commit 6a63f86 into puppetlabs:master Aug 9, 2018
@MikaelSmith MikaelSmith deleted the BOLT-742 branch August 9, 2018 00:02
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.

None yet

3 participants