search

saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.08k stars 5.47k forks source link

test=False not working with state.sls_id #18912

Closed jakubek closed 6 years ago

jakubek commented 9 years ago

All my minions are running in test: true mode by default.

root@saltmaster:/srv/salt/mw# salt 'test1' state.sls_id mycnf mw.mysql test=False
test1:
----------
          ID: mycnf
    Function: file.managed
        Name: /etc/my.cnf
      Result: None
     Comment: The following values are set to be changed:
              diff: ---  
              +++  
              @@ -12,7 +12,6 @@
               character_set_server = utf8
               collation_server = utf8_unicode_ci
               init_connect = 'SET NAMES utf8'
              -

               # Replication settings
               log-bin=mysql-bin
              @@ -33,20 +32,20 @@
               read_buffer_size     = 256K
               sort_buffer_size     = 8M

              -# tune me!
               query_cache_type     = 1
               query_cache_size     = 2G

              -# tune me!
               thread_cache_size    = 32

               max_connections      = 1024

               bind-address         = 0.0.0.0

              -# tune me!
              -innodb_buffer_pool_size         = 12G
              -innodb_buffer_pool_instances    = 6
              +innodb_read_io_threads   = 64
              +innodb_write_io_threads  = 64
              +
              +innodb_buffer_pool_size         = 20G
              +innodb_buffer_pool_instances    = 20
               innodb_data_home_dir            = /var/lib/mysql
               innodb_data_file_path           = ibdata1:1024M:autoextend
               innodb_log_file_size            = 128M
     Started: 13:46:55.576986
    Duration: 892.093 ms
     Changes:   

Summary
------------
Succeeded: 1 (unchanged=1)
Failed:    0
------------
Total states run:     1

VERSIONS:

root@saltmaster:/srv/salt/mw# salt --versions-report
           Salt: 2014.7.0
         Python: 2.7.3 (default, Mar 13 2014, 11:03:55)
         Jinja2: 2.6
       M2Crypto: 0.21.1
 msgpack-python: 0.1.10
   msgpack-pure: Not Installed
       pycrypto: 2.6
        libnacl: Not Installed
         PyYAML: 3.10
          ioflo: Not Installed
          PyZMQ: 13.1.0
           RAET: Not Installed
            ZMQ: 3.2.3
           Mako: 0.7.0

[root@test1 ~]# salt-minion --versions-report
           Salt: 2014.7.0
         Python: 2.6.6 (r266:84292, Jan 22 2014, 09:42:36)
         Jinja2: unknown
       M2Crypto: 0.20.2
 msgpack-python: 0.1.13
   msgpack-pure: Not Installed
       pycrypto: 2.0.1
        libnacl: Not Installed
         PyYAML: 3.10
          ioflo: Not Installed
          PyZMQ: 2.2.0.1
           RAET: Not Installed
            ZMQ: 3.2.4                                                                                                                                                                                                                                                         
           Mako: Not Installed
rallytime commented 9 years ago

Thanks for the report @jakubek. We technically don't actually have a test=False option, and the output that I am seeing from your state looks like it should if you were running test=True. I bet the state is only checking for the existence of the test kwarg, regardless of it's =True or =False status or something along those lines. If you run your state without test=False, the state should run as it normally would. Can you give that a try and see if you get the result you expect?

jakubek commented 9 years ago

@rallytime as I mentioned I'm always running minion with option test: true in /etc/salt/minion. In this case state is always fired as dry-run.

root@saltmaster:/srv/pillar/p-vipnet# salt 'test1' state.sls_id mycnf mw.mysql
test1:
----------
          ID: mycnf
    Function: file.managed
        Name: /etc/my.cnf
      Result: None
     Comment: The following values are set to be changed:
              diff: ---  
              +++  
              @@ -12,7 +12,6 @@
               character_set_server = utf8
               collation_server = utf8_unicode_ci
               init_connect = 'SET NAMES utf8'
              -

               # Replication settings
               log-bin=mysql-bin
              @@ -33,20 +32,20 @@
               read_buffer_size     = 256K
               sort_buffer_size     = 8M

              -# tune me!
               query_cache_type     = 1
               query_cache_size     = 2G

              -# tune me!
               thread_cache_size    = 32

               max_connections      = 1024

               bind-address         = 0.0.0.0

              -# tune me!
              -innodb_buffer_pool_size         = 12G
              -innodb_buffer_pool_instances    = 6
              +innodb_read_io_threads   = 64
              +innodb_write_io_threads  = 64
              +
              +innodb_buffer_pool_size         = 20G
              +innodb_buffer_pool_instances    = 20
               innodb_data_home_dir            = /var/lib/mysql
               innodb_data_file_path           = ibdata1:1024M:autoextend
               innodb_log_file_size            = 128M
     Started: 16:19:42.663503
    Duration: 940.818 ms
     Changes:   

Summary
------------
Succeeded: 1 (unchanged=1)
Failed:    0
------------
Total states run:     1
rallytime commented 9 years ago

Ah, it seems that I was misunderstanding your question, and I was also incorrect about the test=False option. I'd never seen that before.

We do want to support defaulting to test: True in the minion or master config for dry "test" runs on states and setting test=False is completely legitimate to override the config option and actually run the state.

We had a bug with this a while ago where setting test: True in the master config couldn't be overridden that has been fixed. Are you in a position to switch the config option from minion config to master to config? I am wondering if that would be a good work-around for now and is only broken in the minion config option, or if this is something that has regressed for both master and minion config values.

jakubek commented 9 years ago

@rallytime are you sure that there is "test:" key in master config? Maybe we are writing about different things?

jakubek@x240:~/gits/other/salt/conf$ git branch
* develop

jakubek@x240:~/gits/other/salt/conf$ grep '#test' master
jakubek@x240:~/gits/other/salt/conf$ grep '#test' minion 
#test: True
#test.foo: foo
#test.bar: [baz,quo]
#test.baz: {spam: sausage, cheese: bread}

Maybe it's related to this Bug https://github.com/saltstack/salt/issues/12624?

rallytime commented 9 years ago

Bah. You're right. It's not supported in the master, only the minion. Sorry for the confusion. We'll get this fixed up.

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.