Fix cached pillar errors on state.apply

[agraul]

What does this PR do?

On state.apply, only look for errors in the fresh pillar data. Errors in the in-memory pillar can be ignored because the in-memory pillar data is not used anyway.

What issues does this PR fix or reference?

Fixes: #52354
Fixes: #59339
Fixes: #57180

Previous Behavior

When the in-memory pillar (__pillar__) contains _errors, state.apply aborts when it looks for errors in the pillar.

New Behavior

Errors in the in-memory pillar are only looked for if the fresh pillar is empty.

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

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

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

[agraul]

Failing test:

salt/tests/pytests/unit/modules/state/test_state.py

Lines 1262 to 1267 in 584415d

	def test_get_pillar_errors_CE():
	"""
	Test _get_pillar_errors function.
	CC: External clean, Internal erroneous
	:return:
	"""

The test, and _get_pillar_errors's docstring focus on an internal vs external pillar. Is that really the case? When running state.apply the two pillars seems to be the in-memory pillar and the freshly generated pillar (through salt.state.State), not internal plus external.

I think the docstring and tests are outdated, but I'm not sure.

[agraul]

I've now changed the doc string and updated the unit tests. Please let me know if this was the correct thing to do, or if external/internal is the correct differentiation. In the state.apply, in-memory vs. fresh pillar data is clearer to me, similar to the docs

[agraul]

Hey @saltstack/core-pr-reviewers, all checks are green now. It would be great if one of you could review this pull request 😃 when you have time.

[MKLeb]

re-run all

[Ch3LL]

The failure on photon looks to be related to these changes.

[agraul]

The failure on photon looks to be related to these changes.

That's correct, the failing test is the new one I'm adding in this PR. In the test I have a file.managed state which copies over a test file. On photon, this test file is written with a different file mode than on any other OS. I'll specify it in the file.managed state, that will make the test deterministic (at least in this regard)

[github-actions]

Hi! I'm your friendly PR bot!

You might be wondering what I'm doing commenting here on your PR.

Yes, as a matter of fact, I am...

I'm just here to help us improve the documentation. I can't respond to
questions or anything, but what I can do, I do well!

Okay... so what do you do?

I detect modules that are missing docstrings or "CLI Example" on existing docstrings!
When I was created we had a lot of these. The documentation for these
modules need some love and attention to make Salt better for our users.

So what does that have to do with my PR?

I noticed that in this PR there are some files changed that have some of these
issues. So I'm leaving this comment to let you know your options.

Okay, what are they?

Well, my favorite, is that since you were making changes here I'm hoping that
you would be the most familiar with this module and be able to add some other
examples or fix any of the reported issues.

If I can, then what?

Well, you can either add them to this PR or add them to another PR. Either way is fine!

Well... what if I can't, or don't want to?

That's also fine! We appreciate all contributions to the Salt Project. If you
can't add those other examples, either because you're too busy, or unfamiliar,
or you just aren't interested, we still appreciate the contributions that
you've made already.

Whatever approach you decide to take, just drop a comment here letting us know!

Detected Issues (click me)

Check Known Missing Docstrings...........................................Failed
- hook id: invoke
- exit code: 1
/home/runner/.cache/pre-commit/repolfxap482/py_env-python3/lib/python3.9/site-packages/distutils_hack/init.py:33: UserWarning: Setuptools is replacing distutils.

warnings.warn("Setuptools is replacing distutils.")

The function 'get_pauses' on 'salt/modules/state.py' does not have a 'CLI Example:' in its docstring

The function 'apply' on 'salt/modules/state.py' does not have a 'CLI Example:' in its docstring

The function 'test' on 'salt/modules/state.py' does not have a 'CLI Example:' in its docstring

Found 3 errors

---

Thanks again!

[github-actions]

Hi! I'm your friendly PR bot!

You might be wondering what I'm doing commenting here on your PR.

Yes, as a matter of fact, I am...

I'm just here to help us improve the documentation. I can't respond to
questions or anything, but what I can do, I do well!

Okay... so what do you do?

I detect modules that are missing docstrings or "CLI Example" on existing docstrings!
When I was created we had a lot of these. The documentation for these
modules need some love and attention to make Salt better for our users.

So what does that have to do with my PR?

I noticed that in this PR there are some files changed that have some of these
issues. So I'm leaving this comment to let you know your options.

Okay, what are they?

Well, my favorite, is that since you were making changes here I'm hoping that
you would be the most familiar with this module and be able to add some other
examples or fix any of the reported issues.

If I can, then what?

Well, you can either add them to this PR or add them to another PR. Either way is fine!

Well... what if I can't, or don't want to?

That's also fine! We appreciate all contributions to the Salt Project. If you
can't add those other examples, either because you're too busy, or unfamiliar,
or you just aren't interested, we still appreciate the contributions that
you've made already.

Whatever approach you decide to take, just drop a comment here letting us know!

Detected Issues (click me)

Check Known Missing Docstrings...........................................Failed
- hook id: invoke
- exit code: 1
/home/runner/.cache/pre-commit/repolfxap482/py_env-python3/lib/python3.9/site-packages/distutils_hack/init.py:33: UserWarning: Setuptools is replacing distutils.

warnings.warn("Setuptools is replacing distutils.")

The function 'get_pauses' on 'salt/modules/state.py' does not have a 'CLI Example:' in its docstring

The function 'apply' on 'salt/modules/state.py' does not have a 'CLI Example:' in its docstring

The function 'test' on 'salt/modules/state.py' does not have a 'CLI Example:' in its docstring

Found 3 errors

---

Thanks again!

[agraul]

Is the debian failure a known issue? I'd be surprised if my change caused any runtime performance regression, it reduces the number of dict.get calls.

For completeness, this is the failing test

        ...
        duration = 4                     <============================= duration defined here
        if salt.utils.platform.spawning_platform():
            duration = 30
            # mac needs some more time to do its makeup
            if salt.utils.platform.is_darwin():
                duration += 15
    
        with pytest.helpers.temp_file("retry.sls", sls_contents, state_tree):
            ret = state.sls(
                "retry", __pub_jid="1"
            )  # Because these run in parallel we need a fake JID
    
            for state_return in ret:
                assert state_return.result is True
>               assert state_return.full_return["duration"] < duration 
E               assert 10.765 < 4

[MKLeb]

@agraul Yes, I've seen that fail sporadically before. Going to re-run the failed tests here, as they seem to be unrelated.