Skip to content

Msiexec tag array#661

Merged
mx-psi merged 3 commits intoDataDog:masterfrom
alexberry:msiexec_tagarray
Oct 15, 2020
Merged

Msiexec tag array#661
mx-psi merged 3 commits intoDataDog:masterfrom
alexberry:msiexec_tagarray

Conversation

@alexberry
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
cegeka-jenkins pushed a commit to cegeka/puppet-datadog_agent that referenced this pull request Feb 5, 2026
* Testing an inline transform to fix tag arrays using msiexec

* Converting the array to a quoted string separated by commas, else msiexec opens a gui error message and hangs the puppet run when the $tags array has more than one member.

* Puppet lint fixes

Co-authored-by: Alex Berry <alexb@directferries.com>
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