Update states.chef for version 16.x and 17.x Chef Infra Client output

[ericham]

What does this PR do?

Update the regex search for output lines of Chef Infra Client versions 16.x and 17.x to not throw " AttributeError: 'NoneType' object has no attribute 'group'".

What issues does this PR fix or reference?

Fixes: #61891

Previous Behavior

The regex searched for "Chef Client finished, (\d+)" which appears to match some EOL version of Chef Infra Client. The currently supported versions of Chef Infra Client are 16.x and 17.x which no longer output this line at the end of a chef-client run.

New Behavior

The regex now searches for "Infra \w+ (complete|finished), (\d+)" which will match the output as follows:

Version 16.x Chef Infra Client has the following output line at the end of a successful chef-client run:
Chef Infra Client finished, 47 resources updated

Version 17.x Chef Infra Client has the following output line at the end of a successful chef-client run:
Infra Phase complete, 36 resources updated

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

- [ ] Docs

• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

I have applied these changes successfully on my Ubuntu 18.04 and 20.04 minions, the AttributeError no longer occurs and the stdout is displayed correctly. I'm not sure if a new or existing test needs to be written or updated.

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

[ericham]

As to the needs-changelog-file label, I did include changelog/61891.fixed in this PR, is that not the correct way to do it?

As to the Needs-Testcase, I see there is salt/tests/pytests/unit/states/test_chef.py. However, I am new to unit tests and I'm not sure what I'm looking at here. Does this existing test cover my change? Or do I need to modify this test somehow or create a brand new test?

[garethgreenaway]

@ericham Apologies. I missed that there was in fact a changelog included with the PR.

[garethgreenaway]

@ericham Looking at the changes and the current tests, this would require additional test coverage. Another concern would be whether support needs to be preserved for previous versions of Chef with the current test.

[ericham]

I've changed the regex to maintain the original version output and add my changes to support the 2 current versions of Chef.

"(Chef Client finished|Chef Infra Client finished|Infra Phase complete), (\d+)"

Previous versions
Chef Client finished, 45 resources updated
Version 16.x
Chef Infra Client finished, 56 resources updated
Version 17.x
Infra Phase complete, 67 resources updated

I plan to attend the Twitch Test Clinic tomorrow to get help on writing the test case.

[waynew]

This PR is also blocked by #61919 - when that's merged (and tests a la test-clinic) have been added this should be ready for a re-review 👍

[ericham]

@waynew thanks again for all the help today in the Twitch Test Clinic, I've cherry-picked your 2 commits to add the necessary unit test.

[waynew]

@ericham #61919 was fixed, so if all goes well everything should be 👍 and this should be ready for a re-review/merge later today 🎉

[ericham]

@waynew after your merge there were 2 tests that failed, I decided to merge from master again as there were more commits outstanding, and now there is 1 failing test. So not sure if there is still an issue in the test suite or something with my code?

[waynew]

For some of the tests I had to (re)approve and run, so those are going now 👍

[ericham]

@waynew I merged from master again and got 27 successful tests, I believe the 6 expected checks get triggered on review?

@garethgreenaway I believe this is ready for review now as the Needs-Testcase label has been satisfied with Wayne's help on the unit test.

[ericham]

@dwoz I see you're assigned as the reviewer, I believe this is ready now to be reviewed? If not, please let me know what else I need to complete.