saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Install Salt from the Salt package repositories here:
https://docs.saltproject.io/salt/install-guide/en/latest/
Apache License 2.0
14.19k stars 5.48k forks source link

[BUG] module.wait can no longer be run like module.run in 3005.1 #63400

Closed tj90241 closed 1 year ago

tj90241 commented 1 year ago

Description In 3004.2, this worked:

apt-package-database-update:
  module.wait:
    - pkg.refresh_db:
    - watch:
      - file: apt-manage-sources.list

In 3005.1, it does not, and throws this:

          ID: apt-package-database-update
    Function: module.wait
      Result: False
     Comment: Module function apt-package-database-update is not available
     Started: 16:11:00.667687
    Duration: 2.307 ms
     Changes: 

As the error suggests, it's looking at the state name for the function, unconditionally? Thus, this works:

pkg.refresh_db:
  module.wait:
    - watch:
      - file: apt-manage-sources.list

as does this:

apt-package-database-update:
  module.wait:
    - name: pkg.refresh_db
    - watch:
      - file: apt-manage-sources.list

I've tried adding and removing both module.run and module.wait to superseded: in the minion's configuration, to no avail (even though my understanding is that 3005 should not need this anymore).

Setup This is insensitive to configuration AFAICT, just run this state:

apt-manage-sources.list:
  file.managed:
    - name: /etc/apt/sources.list
    - source: salt://apt/sources.list.jinja
    - template: jinja
    - user: root
    - group: root

apt-package-database-update:
  module.wait:
    - pkg.refresh_db:
    - watch:
      - file: apt-manage-sources.list

... and use whatever file you want for the sources.list.

Steps to Reproduce the behavior See above.

Expected behavior The state should work in the same way in did in 3004.2 and below.

Screenshots N/A

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) ```yaml Salt Version: Salt: 3005.1 Dependency Versions: cffi: Not Installed cherrypy: unknown dateutil: 2.8.1 docker-py: Not Installed gitdb: 4.0.5 gitpython: 3.1.14 Jinja2: 2.11.3 libgit2: Not Installed M2Crypto: Not Installed Mako: Not Installed msgpack: 1.0.0 msgpack-pure: Not Installed mysql-python: Not Installed pycparser: Not Installed pycrypto: Not Installed pycryptodome: 3.9.7 pygit2: Not Installed Python: 3.9.2 (default, Feb 28 2021, 17:03:44) python-gnupg: Not Installed PyYAML: 5.3.1 PyZMQ: 20.0.0 smmap: 4.0.0 timelib: Not Installed Tornado: 4.5.3 ZMQ: 4.3.4 System Versions: dist: debian 11 bullseye locale: utf-8 machine: x86_64 release: 6.1.1slvvirt system: Linux version: Debian GNU/Linux 11 bullseye ```

Additional context The "new" (superseded) format for module.run can take state names fine still, e.g.:

show off module.run with args:
  module.run:
    # Note the lack of `name: `, and trailing `:`
    - test.random_hash:
      - size: 42
      - hash_type: sha256

If module.wait cannot and this is deemed not a bug, it may be worth it to make explicit mention of that.

tj90241 commented 1 year ago

Looks like it's not a bug based on the documentation...

hatifnatt commented 1 year ago

Faced this issue too. I think this is still an issue at least with documentation. Previously (pre 3005) module.run and module.wait have similar semantics. But since Salt version 3005 they behave differently. My sates have workaround to render proper syntax based on use_superseded configuration or Salt version. In all occurrences where module.run is called - everything works fine after upgrading to 3005, and in all occurrences where module.wait is used - states became broken, Salt is trying to use state_id as module name.

Some examples: module.run - ok

  pki_root_ca_cert_publish:
    module.run:
    - mine.send:
      - name: pki_root_ca
      - mine_function: x509.get_pem_entry
      - text: /etc/pki/root_ca/root_ca.crt

module.wait - broken (required to use old (documented?) syntax

  common.restart_salt_minion_reload_systemd:
    module.wait:
    - service.systemctl_reload: {}
    - watch:
      - file: common.restart_salt_minion_systemd_drop-in

Error:

----------
          ID: common.restart_salt_minion_reload_systemd
    Function: module.wait
      Result: False
     Comment: Module function common.restart_salt_minion_reload_systemd is not available
     Started: 17:06:37.117036
    Duration: 1397.993 ms
     Changes:

Replacing module.wait with module.run - ok

  common.restart_salt_minion_reload_systemd:
    module.run:
    - service.systemctl_reload: {}
    - watch:
      - file: common.restart_salt_minion_systemd_drop-in

Success:

----------
          ID: common.restart_salt_minion_reload_systemd
    Function: module.run
      Result: True
     Comment: service.systemctl_reload: True
     Started: 17:08:47.031335
    Duration: 278.853 ms
     Changes:
              ----------
              service.systemctl_reload:
                  True

module.wait - ok, but legacy syntax

  reload_systemd:
    module.wait:
    - name: service.systemctl_reload
    - watch:
      - file: common.restart_salt_minion_systemd_drop-in

IMO module.run and module.wait must behave similarly like in older versions, or documentation must explicitly state that only module.run is using new syntax. In current state there is a lot of confusion.

Can this issue be reopened?