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

Msiexec tag array #661

Merged
merged 3 commits into from
Oct 15, 2020
Merged

Msiexec tag array #661

merged 3 commits into from
Oct 15, 2020

Conversation

alexberry
Copy link
Contributor

@alexberry alexberry commented Oct 14, 2020

What does this PR do?

I spotted a bug with the windows manifest - When the package gets updated with an array of tags, the tags are passed literally on the command line and msiexec opens an interactive help window, hanging puppet.

Motivation

It broke our release (we have had an array of tags before, the issue only affected us when the agent got updated).

Additional Notes

Describe your test plan

  1. I branched your module from github to our own repositories, then added it in to our r10k configuration and removed the "mod" declaration from our Puppetfile so that we no longer used the forge version.

  2. I tested as-was, and saw the same error. The error was a result of the following msiexec being run:
    Debug: Executing: 'msiexec.exe /qn /norestart /i C:\Windows\temp\datadog-agent-7-7.22.1.amd64.msi /norestart APIKEY=redacted HOSTNAME=websvr3-stag TAGS=["version:Monolith_uk.1.23.0", "env:stag"]'

  3. I updated my branch with the code in this PR and ran puppet again, puppet ran successfully and the resultant msiexec was:
    Debug: Executing: 'msiexec.exe /qn /norestart /i C:\Windows\temp\datadog-agent-7-7.22.1.amd64.msi /norestart APIKEY=redacted HOSTNAME=websvr3-stag TAGS="version:Monolith_uk.1.23.0,env:stag"'

  4. Finally I updated my branch to remove the second tag, to ensure the functionality works properly with a single member, and the resultant msiexec was:
    Debug: Executing: 'msiexec.exe /qn /norestart /i C:\Windows\temp\datadog-agent-7-7.22.1.amd64.msi /norestart APIKEY=redacted HOSTNAME=websvr3-stag TAGS="version:Monolith_uk.1.23.0"'

Output from both step 3 (array of tags containing more than 1 member) and step 4 (array of tags containing a single member) conforms to datadog's own command-line installation documentation for windows.

Alex Berry added 2 commits October 14, 2020 17:30
…exec opens a gui error message and hangs the puppet run when the $tags array has more than one member.
@alexberry alexberry requested a review from a team as a code owner October 14, 2020 17:19
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Thanks for your PR, we definitely missed that! The logic looks good to me, but you will need the linters to pass in order for us to merge this. You can run them using bundle exec rake test as stated by this guide. In particular, it seems the syntax linter has two suggestions that you should address:

❯ bundle exec rake test
---> syntax:manifests
---> syntax:templates
---> syntax:hiera:yaml
manifests/windows.pp - WARNING: double quoted string containing no variables on line 14
manifests/windows.pp - WARNING: variable not enclosed in {} on line 15

@alexberry
Copy link
Contributor Author

Noted, apologies I should have linted before submitting, PR updated with required lint changes.

@alexberry alexberry requested a review from mx-psi October 15, 2020 08:56
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

LGTM now 🚀, thanks for fixing the small issues :)

@mx-psi mx-psi merged commit a8e4d56 into DataDog:master Oct 15, 2020
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.

2 participants