Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2017.7.0rc1] use_superseded and module.run changes from release notes do nothing? #42148

Closed
sjorge opened this issue Jul 5, 2017 · 19 comments
Closed
Labels
Documentation Relates to Salt documentation
Milestone

Comments

@sjorge
Copy link
Contributor

sjorge commented Jul 5, 2017

Description of Issue/Question

Following the release notes here https://docs.saltstack.com/en/develop/topics/releases/2017.7.0.html#state-module-changes

I run into the following issue...

  1. add the following to the minion config
use_superseded:
  - module.run
  1. restart the minion
  2. update one of the states to the new format
  3. run state.sls my.state.here
local:
    Data failed to compile:
----------
    State 'run_something' in SLS 'my.state.here' is not formed as a list

I played tried a few different things based on the example

# super simplified example from notes
run_something:
  module.run:
    test.ping:
      - name: some name

# failed attempts to fix my code
#package::upgrade:
#  module.run:
#    pkg.upgrade:
#      - name: package::do_upgrade
#package::upgrade:
#  module.run:
#    pkg.upgrade
#package::upgrade:
#  module.run:
#    pkg.upgrade:

For now i have reverted to the old state file which works fine:

######
## common.package.upgrade
## -----------------------------------
######

## full update
package::refresh:
 module.run:
    - name: pkg.refresh_db

package::upgrade:
 module.run:
    - name: pkg.upgrade

This works with or without the following in the minion config

use_superseded:
  - module.run

Setup

See above

Steps to Reproduce Issue

See above

Versions Report

           Salt: 2017.7.0rc1

Dependency Versions:
           cffi: 1.9.1
       cherrypy: unknown
       dateutil: 2.5.3
      docker-py: Not Installed
          gitdb: 2.0.0
      gitpython: 0.3.2 RC1
          ioflo: 1.6.7
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: 1.5.0
       M2Crypto: 0.22
           Mako: 1.0.6
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: 1.2.5
      pycparser: 2.17
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.12 (default, Jan 26 2017, 18:07:59)
   python-gnupg: 2.2.0
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: 0.6.7
          smmap: 0.9.0
        timelib: 0.2.4
        Tornado: 4.3
            ZMQ: 4.1.4

System Versions:
           dist:
         locale: UTF-8
        machine: i86pc
        release: 5.11
         system: SunOS
        version: Not Installed```
@sjorge sjorge changed the title [2017.7.0rc1] [2017.7.0rc1] use_superseded and module.run changes from release notes do nothing? Jul 5, 2017
@Ch3LL
Copy link
Contributor

Ch3LL commented Jul 6, 2017

Looks like i'm able to replicate this. This feature was added here #39891

@isbm any thoughts here? Are we possibly setting it up incorrectly? Thanks

@Ch3LL Ch3LL added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Jul 6, 2017
@Ch3LL Ch3LL added this to the Blocked milestone Jul 6, 2017
@isbm
Copy link
Contributor

isbm commented Jul 6, 2017

@Ch3LL please assign me to this issue. I'll look at this tomorrow.

@Ch3LL
Copy link
Contributor

Ch3LL commented Jul 6, 2017

looks like i can't assign it to you via github, but we will wait for your analysis and not assign it out to anyone else. Thanks for taking a look :)

@isbm
Copy link
Contributor

isbm commented Jul 7, 2017

@Ch3LL @sjorge
OK.
In order to use new syntax of module.run, you should add the following to your minion config (I usually add /etc/salt/minion.d/deprecations.conf with this:

use_superseded:                                           
  - module.run 

Restart your minion, from now on your module.run goes so with the new syntax. I am still working how to deliver this switch to the minions without config. But this is coming later.

Now for the calling syntax itself. As per YAML syntax, the correct way to call the argument-less functions like test.ping is this:

how_to_call_without_arguments:
  module.run:
    - test.ping:  # Notice the colon at the end!

This colon at the end is required in this case, as the state compiler will cry that the function is not formed as a list (you've got it too, BTW). But those functions with the arguments called simply adding the arguments in obvious way:

how_to_call_with_the_arguments:
  module.run:
    - test.echo:
      - text: "Hi, there!"

Quotes are optional. Or something like this all together:

how_to_call_everything_at_once:
  module.run:
    - cmd.run:
      - cmd: uname -a
    - test.echo:
      - text: Hi, there! No quotes here
    - test.ping:

The above gives me the following result:

saltdevel:
----------
          ID: foobar_to_run_scripted
    Function: module.run
      Result: True
     Comment: test.ping: True, test.echo: hello, there!, cmd.run: Linux zeus 4.4.73-18.17-default SMP PREEMPT Fri Mar 18 14:42:07 UTC 2016 (0a392b2) x86_64 x86_64 x86_64 GNU/Linux
     Started: 15:41:14.203743
    Duration: 19.226 ms
     Changes:   
              ----------
              cmd.run:
                  Linux zeus 4.4.73-18.17-default SMP PREEMPT Fri Mar 18 14:42:07 UTC 2016 (0a392b2) x86_64 x86_64 x86_64 GNU/Linux
              test.echo:
                  hello, there!
              test.ping:
                  True

Summary for saltdevel
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:  19.226 ms

Now, if you leave the state untouched but remove the configuration of the minion deprecations.conf, you will get the following output:

saltdevel:
----------
          ID: foobar_to_run_scripted
    Function: module.run
      Result: False
     Comment: Module function foobar_to_run_scripted is not available
     Started: 15:49:40.847648
    Duration: 0.632 ms
     Changes:   

Summary for saltdevel
------------
Succeeded: 0
Failed:    1
------------
Total states run:     1
Total run time:   0.632 ms

And your log file will have the following entries:

[WARNING ] The function "module.run" is using its deprecated version and will expire in version "Sodium".
[ERROR   ] Module function foobar_to_run_scripted is not available

If you want it still run in old way, modify your SLS file to the following:

run_test_ping:                                             
  module.run:                                              
    - name: test.ping                                      
                                                           
run_test_echo:                                             
  module.run:                                              
    - name: test.echo                                      
    - text: 'Hi, there!'                                   
                                                           
run_cmd_run:                                               
  module.run:                                              
    - name: cmd.run                                        
    - cmd: uname 

You will now be able to run it, but you will be running three operations three times and will get three warnings in your log. 😉 So the difference here is that the new syntax allows you:

  • run many things in one go (that compresses the voluminous repetitive SLS into a few-liner)
  • no need to define arbitrary keywords as an embedded JSON with the ad-hoc keywords param
  • lack of other ad-hoc magic keywords like m_name, m_names, m_fun, m_state and others that you probably do not even know. 😉

Hope it helps! Greetings from sunny Germany and have a nice weekend!

@gtmanfred
Copy link
Contributor

I have confirmed that this works.

[root@salt salt]# salt-call mine.get \* test.echo
local:
    ----------
[root@salt salt]# salt-call state.apply test
local:
----------
          ID: test echo
    Function: module.run
      Result: True
     Comment: mine.send: True
     Started: 14:18:39.939795
    Duration: 511.205 ms
     Changes:
              ----------
              mine.send:
                  True

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time: 511.205 ms
[root@salt salt]# salt-call mine.get \* test.echo
local:
    ----------
    salt.c.gtmanfred-1263.internal:
        hi!
[root@salt salt]# cat /srv/salt/test.sls
test echo:
  module.run:
    - mine.send:
      - func: test.echo
      - text: hi!

@Ch3LL
Copy link
Contributor

Ch3LL commented Jul 7, 2017

@isbm thanks for providing an example for us and thanks @gtmanfred for confirming it works as well.

@sjorge can you also confirm this works for you?

@Ch3LL Ch3LL added info-needed waiting for more info and removed ZRELEASED - 2017.7.0 Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged labels Jul 7, 2017
@sjorge
Copy link
Contributor Author

sjorge commented Jul 7, 2017

I can confirm this works.

Perhaps some extra clarification should be added to the release notes (and docs)

@gtmanfred
Copy link
Contributor

gtmanfred commented Jul 7, 2017 via email

@The-Loeki
Copy link
Contributor

actually the examples in the docs rightfully cause the error https://github.com/saltstack/salt/blob/develop/salt/states/module.py#L27

That's incorrect in both the old & new syntax ;)

@gtmanfred gtmanfred added the Documentation Relates to Salt documentation label Jul 11, 2017
@gtmanfred gtmanfred modified the milestones: Approved, Blocked Jul 11, 2017
@gtmanfred gtmanfred removed the info-needed waiting for more info label Jul 11, 2017
@gtmanfred
Copy link
Contributor

Good Catch, that needs to be fixed.

It should be

fetch_out_of_band:
  module.run:
    - git.fetch:
      - cwd: /path/to/my/repo
      - user: myuser
      - opts: '--all'

@The-Loeki
Copy link
Contributor

Exactly, which is what tripped me (& probably @sjorge as well) up; notice it's almost consistently wrong too ;)

@dmaziuk
Copy link

dmaziuk commented Aug 2, 2017

Apologies for being dense, but how exactly does it need to look in python? I have now

    for u in __pillar__["user_list"].keys() :
            rc[u] = { "module.run" : [
                { "name"     : "shadow.set_password" },
                { "m_name"   : __pillar__["user_list"][u]["name"] },
                { "password" : __pillar__["user_list"][u]["password"] }
            ] }
...
    return rc

and it works but throws the warning

The function "module.run" is using its deprecated version and will expire in version "Sodium".

Do I just need deprecation.conf to shut the warning up? Or what?

@gtmanfred
Copy link
Contributor

gtmanfred commented Aug 2, 2017 via email

@dmaziuk
Copy link

dmaziuk commented Aug 2, 2017

@gtmanfred: Nope.

I just saw the warning in the logs while investigating something else and google turned up this issue. Like I said, it all seems to be working fine.

@marco-m
Copy link

marco-m commented Mar 4, 2018

Hello, any news ? It looks like this can be closed as fixed (unless I am missing something)

@dmaziuk
Copy link

dmaziuk commented Mar 4, 2018

worksforme

@isbm
Copy link
Contributor

isbm commented Mar 4, 2018

@marco-m I think, RC1 is fine, but look at 2018.3 instead. 😉 Maybe we can close the issue, @rallytime?

@dreampuf
Copy link
Contributor

any idea for enable this feature under salt-ssh?

@isbm
Copy link
Contributor

isbm commented Nov 27, 2018

@dreampuf good point. In general, this is not about syntax change, but overall migration mechanism, since everything in Salt is subject to change in ~1 year. So as long as you keep using older versions, this is a very good idea to think of!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Relates to Salt documentation
Projects
None yet
Development

No branches or pull requests

9 participants