saltstack-formulas / salt-formula

Yes, Salt can Salt itself!
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
198 stars 421 forks source link

[BUG] Conflicting states run every highstate #527

Closed ajcollett closed 2 years ago

ajcollett commented 2 years ago

Your setup

Formula commit hash / release tag

I am using the master branch for this formula, salt-formula.

Versions reports (master & minion)

Salt is on version 3004 on both master and minion.

Pillar / config used

  salt:                                                             
        ----------                                                    
        key_url:                                                      
            https://repo.saltproject.io/py3/ubuntu/20.04/amd64/latest/salt-archive-keyring.gpg                                               
        lookup:                                                       
            ----------                                                
            pyinotify:                                                
                python3-pyinotify                                     
        minion:                                                       
            ----------                                                
            grains:                                                   
                ----------                                            
                <snip>                            
            master:                                                   
                <snip>                                     
            saltenv:                                                  
                base                                                  
        minion_remove_config:                                         
            True                                                      
        pkgrepo:                                                      
            deb https://repo.saltstack.com/py3/ubuntu/20.04/amd64/latest/ focal main

Bug details

Describe the bug

When using minion_remove_config: true 2 states are enforced every run:

  1. remove-default-minion-conf-file
  2. permissions-minion-config

That is, the minion file is removed, then recreated and managed. In code, only the first is surrounded by an if, but the 2nd always runs.

Steps to reproduce the bug

  1. Set minion_remove_config: true in pillar.
  2. Run state.apply
  3. Observe both above states run each time sate.apply or state.highstate is run.

Expected behaviour

The file should be removed when the minion_remove_config setting is set to true.

Attempts to fix the bug

The permissions state should probably only be run when minion_remove_config is set to false. I have not tested this.

lmf-mx commented 2 years ago

I'm seeing this too and it's creating the file if it doesn't exist. There is an arg create to file.managed that can be set to False to disable creation of the file if it doesn't exist.

ajcollett commented 2 years ago

So I assume that would need to be updated in the formula?

myii commented 2 years ago

@ajcollett @lmf-mx Thanks for the report, this has been fixed in #528. Feel free to comment on how this works out for you.

ajcollett commented 2 years ago

@myii you beat me to it! I was about to create a pull request with exactly the same. Cheers!

I tested in locally before doing a fork. It seemed to work well for me.

saltstack-formulas-travis commented 2 years ago

:tada: This issue has been resolved in version 1.10.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

lmf-mx commented 2 years ago

@myii Checking for remove is even better, avoids the state ever hitting the output.

Thanks!

myii commented 2 years ago

@myii you beat me to it! I was about to create a pull request with exactly the same. Cheers!

I tested in locally before doing a fork. It seemed to work well for me.

You're welcome, @ajcollett -- good to hear that it's working as expected.

@myii Checking for remove is even better, avoids the state ever hitting the output.

@lmf-mx Yes, while there are other options, the minion.sls file is pretty messy, so this was the easiest way to get the job done!