[BUG] Integration test for CLI summary output

[plastikos]

It turns out that the summary output is incorrect. This integration test demonstrates the failure.

[plastikos]

It appears that the module __context__ is being reset by a call to yield. This has the result of clearing the retcode that is set into __context__ so that it is never returned to the salt master.

More details available for discussion.

[plastikos]

Likely related to 6838a95

[cachedout]

Hi Thayne. Do you have a fix for this as well or is this just intended to illustrate the issue which still needs a resolution?

[plastikos]

I used to have a fix for it until 6838a95 was committed. I wasn't super happy with my fix, though. So for now it illustrates a problem - although not very well.

The most obvious part of this is that retcode isn't being set in the minion return and so the master fails to identify which minions fails. It's worth noting, however, that the outputer that color codes the output does deep examination (it doesn't look at retcode but walks each state) and so it gets the color output correct.

The deeper problem is that __context__ in the minion execution environment is being completely cleared when a yield is encountered. It's notable that the python object continues to have the same id(), just the state has been completely reset to empty - which could be a problem for anything that stores state in __context__.

At this point, unless I can communicate with someone that has a broader vision for how it all is architected and has development vision, I just don't have a fix. While the classification of output could be fixed by using the same classifier that the outputter uses, it would not address that the __context__ gets reset (maybe that's not a problem?).

[cachedout]

I'd like to bring @jacksontj into this discussion.

[plastikos]

Bump.

[plastikos]

As simple as this is, I feel that it demonstrates what I would classify a serious bug: The salt server classifying failed states as successful means that no results can be trusted. That has significant ramifications when you have 35,000 results to review.

[plastikos]

See #30914

[cachedout]

Go Go Jenkins!

[plastikos]

Sorry I've take sooooo long to get back to this. I pulled it up to develop and it still fails. What I'm saying is that #31164, which is similar to a patch I worked up many moons ago, does not work with the new code.

I think someone that understands the new tornado design, and even possibly python internals with yield, may need to look at this.

[thatch45]

Go Go Jenkins!

[plastikos]

Looks like this still fails - that's an indication that a failed state is being marked as successful (a bad thing).

[cachedout]

Right now I think the best thing to do is to track this in #30914. This is linked to that issue so the test is available to whomever takes on the task of fixing this.