[BUG] onchanges is firing the requiring state when required is also prereq by a failing third state

[HL-SaltBae]

Description

foo:
    grains.present:
        - value: {{ determined_externally }}

State B:
    something.else:
        - prereq:
            - grains: foo

Notify Team:
    slack.post_message:
        - onchanges:
            - grains: foo

The above is stripped-down example code I have in production. The intention is that whenever the foo grain is about to be changed, then State B should be run, and a Slack message should be generated. If every state succeeds properly, then this actually works fine, and the grain gets changed.

The problem that I am seeing is when State B happens to fail, the grain is not set (which is aligned with the documentation for prereq), but the Slack message gets sent anyway. This results in one Slack message per highstate until State B passes.

In the highstate runs where State B fails, the returns log shows grains: foo state is clearly only run in test mode. The results include "Is set to be added" and has a changes dictionary, but the grain never gets set.

In the highstate runs where State B passes, the returns log shows grains: foo state returns with changes and "Has been set to. . . " language, and the grain IS set.

While it is in-line with the prereq docs for a failure in State B to stop the execution of grains:foo, it is also called out in the onchanges documentation that the required state (grains: foo) needs to return True before the requiring state (slack: Notify Team) executes. Here the grains call is clearly failing, but the slack call happens anyway.

Theory:

• prereq on State B causes grains: foo state to be run in test mode
• State B failing means that grains: foo state is never re-run so that it completes (as designed)
• onchanges on slack: Notify Team state finds the "True with changes" results of the grain: foo state, is unaware that these are test-mode results prompted by a prereq requisite in a different state, and allows slack: Notify Team state to run as though the grain had been changed

Setup
(Please provide relevant configs and/or SLS files (be sure to remove sensitive info. There is no general set-up of Salt.)

Please be as specific as possible and give set-up details.

• on-prem machine (minion)
• VM running on a cloud service, (Master)

Steps to Reproduce the behavior
The following testing SLS illustrates the problem:

foo:
  test.configurable_test_state:
    - result: True
    - changes: True
    - name: fail

State B:
  test.configurable_test_state:
    - changes: True
    - result: False
    - prereq:
      - test: foo

Notify Team:
  test.nop:
    - onchanges:
      - test: foo

The results of this state run are below:

local:
----------
          ID: State B
    Function: test.configurable_test_state
      Result: False
     Comment:
     Started: 21:59:36.285339
    Duration: 0.991 ms
     Changes:
              ----------
              testing:
                  ----------
                  new:
                      Something pretended to change
                  old:
                      Unchanged
----------
          ID: foo
    Function: test.configurable_test_state
        Name: fail
      Result: None
     Comment: This is a test
     Changes:
              ----------
              testing:
                  ----------
                  new:
                      Something pretended to change
                  old:
                      Unchanged
----------
          ID: Notify Team
    Function: test.nop
      Result: True
     Comment: Success!
     Started: 21:59:36.286674
    Duration: 0.805 ms
     Changes:

Summary for local
------------
Succeeded: 2 (unchanged=1, changed=2)
Failed:    1
------------
Total states run:     3
Total run time:   1.796 ms

Expected behavior
If State B fails, and so prereq prevents the actual run of the required state, then the onchanges requisite should prevent the run of the onchanges-requiring state since the onchanges-required state never completed.

Actual Behavior
When State B fails, the required state is never re-evaluated as failing, and the onchanges requisite picks up the test-mode results as evidence that the onchanges requisite is satisfied, firing the final state incorrectly.

Screenshots
If applicable, add screenshots to help explain your problem.

Versions Report

salt --versions-report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

Master

Salt Version:
          Salt: 3004.2
 
Dependency Versions:
          cffi: 1.15.0
      cherrypy: Not Installed
      dateutil: 2.7.3
     docker-py: 4.1.0
         gitdb: 2.0.6
     gitpython: 3.0.7
        Jinja2: 2.10.1
       libgit2: 1.4.1
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.21
      pycrypto: 2.6.1
  pycryptodome: 3.6.1
        pygit2: 1.9.0
        Python: 3.8.10 (default, Jun 22 2022, 20:18:18)
  python-gnupg: 0.4.5
        PyYAML: 5.3.1
         PyZMQ: 18.1.1
         smmap: 2.0.5
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.2
 
System Versions:
          dist: ubuntu 20.04 focal
        locale: utf-8
       machine: x86_64
       release: 5.15.0-1015-aws
        system: Linux
       version: Ubuntu 20.04 focal

minion

Salt Version:
          Salt: 3004.1
 
Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.6.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.10
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 0.5.6
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: 3.4.7
        pygit2: Not Installed
        Python: 3.6.9 (default, Jan 26 2021, 15:33:00)
  python-gnupg: 0.4.1
        PyYAML: 3.12
         PyZMQ: 17.1.2
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.2.5
 
System Versions:
          dist: ubuntu 18.04 Bionic Beaver
        locale: UTF-8
       machine: x86_64
       release: 4.15.0-142-generic
        system: Linux
       version: Ubuntu 18.04 Bionic Beaver

Additional context
Add any other context about the problem here.

[welcome]

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

• Community Wiki
• Salt’s Contributor Guide
• Join our Community Slack
• IRC on LiberaChat
• Salt Project YouTube channel
• Salt Project Twitch channel

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at [email protected]. We’re glad you’ve joined our community and look forward to doing awesome things with you!