In 0.3.14, command assertions that wrap powershell can no longer assert against stdout/stderr successfully

[petemounce]

Edit: Problem In Chair, Not In Computer on my part

see comment thread. Thanks @jsturtevant.

This is now a PR to add an integration test that illustrates how to assert via command using powershell

Original content of PR description:

Assertion that fails in 0.3.14 that passed in 0.3.13:

command:
  Anonymous logins are disabled:
    exit-status: 0
    exec: powershell -noprofile -noninteractive -command "(get-itemproperty -path 'HKLM:/SYSTEM/CurrentControlSet/Control/Lsa/').restrictanonymous"
    stdout:
      - '1'
    timeout: 10000 # in milliseconds
    skip: false

0.3.13 passes

0.3.14 fails

(Note goss.exe being referenced via ./ rather than taking the 0.3.13 as on PATH)

Checklist

• make test-all (UNIX) passes. CI will also test this
• unit and/or integration tests are included (if applicable)
• documentation is changed or added (if applicable)

Description of change

In 0.3.14, my command assertions that used powershell.exe to run powershell started to fail, and I suspect the new cmd /c wrapping, so I'm adding an integration test first. This also happens to be how I'm using goss to assert against the windows registry (note #616).

If this integration test fails I'll try to make changes to allow it to pass - I'm expecting to want to make it possible to use powershell as the shell within command assertions to achieve that.

cc @jsturtevant in case you'd like to take a stab at this?

[petemounce]

There are two tests because I don't know how Travis secures their build machines; one of these should pass, though, based on 0.3.13 behaviour.

[jsturtevant]

makes sense :-)

[jsturtevant]

I gave some thought to making the command either be powershell or cmd but did some basic testing and found I could invoke powershell from the cmd wrapping. Maybe we can revisit this if necessary.

I think in this case, it is the way the quotes are processed. I removed the quotes:

command:
  Anonymous logins are disabled:
    exit-status: 0
    exec: powershell -noprofile -noninteractive -command (get-itemproperty -path 'HKLM:/SYSTEM/CurrentControlSet/Control/Lsa/').restrictanonymous
    stdout:
      - '1'
    timeout: 10000 # in milliseconds
    skip: false

and it passed:

Count: 2, Failed: 0, Skipped: 0

I had to remove some quotes from the commands in #633 as well. Maybe there is something we can do to process the quotes properly?

[petemounce]

@jsturtevant thank you! That allows me to work around this very handily.

[jsturtevant]

looks like this command fails:

wrap a powershell - expect 1: stdout: patterns not found: [1]

Are both of these examples needed?

[petemounce]

No, definitely not; I added both because I have no idea what the expected value should be within Travis. Adding both was reconnaissance.

[jsturtevant]

makes sense :-)

[jsturtevant]

Always love more tests! LGTM

[aelsabbahy]

Appreciate the discussion here and the added test coverage. Many thanks to both of you for the continued support of windows on goss.

Given that this was non-intuitive initially for @petemounce is there an opportunity to improve the documentation here for goss windows users?

[aelsabbahy]

Another question for you both. How stable is windows binary at this point?

Should we consider renaming the "alpha" to "community" or is it (and Mac) not ready for that yet?

[petemounce]

In terms of stability, we haven't noticed anything obviously non-functional or non-performant. We've had 0.3.14 in production since a week after it was released, on Windows and macOS in addition to Linux.

There is a change I may want to do around how Windows process priorities are handled - we discovered that goss doesn't inherit onto sub-processes the goss serve process priority. This leads to resource starvation inside our build farm, since builds are trying to get as much of the computer's resources as possible; we've set up goss serve to have a high priority, but when it runs (for example) command assertions, that process priority is not inherited, and so we get timing out checks sometimes.

That's something we'll probably look at soonish, but not immediately. I don't think that needs to block renaming "alpha" to "community"; the integration-test coverage for both platforms is reasonably encompassing.

[petemounce]

re: docs - I'm not sure. I'm actually still not sure why this was a fix or a quoting issue, really.

[aelsabbahy]

Cool, mostly asking because I'm trying to get v4 out by end of dec or early Jan. Hoping to rename mac and windows as part of that release.

[jsturtevant]

Cool, mostly asking because I'm trying to get v4 out by end of dec or early Jan. Hoping to rename mac and windows as part of that release.

@aelsabbahy just wanted to let you know we are now using this in image-builder as part for cluster api: kubernetes-sigs/image-builder#563

Thanks for all your hard work!

[petemounce]

@jsturtevant that's great! Thanks for circling back and saying so, that's given me a warm fuzzy feeling :)