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

[BUG] Conflicting states run every highstate #527

Closed
ajcollett opened this issue Jan 11, 2022 · 7 comments · Fixed by #528
Closed

[BUG] Conflicting states run every highstate #527

ajcollett opened this issue Jan 11, 2022 · 7 comments · Fixed by #528

Comments

@ajcollett
Copy link

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.

@ajcollett ajcollett added the bug label Jan 11, 2022
@lmf-mx
Copy link

lmf-mx commented Feb 2, 2022

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
Copy link
Author

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

myii added a commit to myii/salt-formula that referenced this issue Feb 3, 2022
@myii myii closed this as completed in #528 Feb 3, 2022
@myii
Copy link
Member

myii commented Feb 3, 2022

@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
Copy link
Author

@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 pushed a commit that referenced this issue Feb 3, 2022
## [1.10.1](v1.10.0...v1.10.1) (2022-02-03)

### Bug Fixes

* **minion:** respect `minion_remove_config: true` ([02c31df](02c31df)), closes [#527](#527)

### Continuous Integration

* **gemfile:** allow rubygems proxy to be provided as an env var [skip ci] ([46fc639](46fc639))
* **kitchen+gitlab:** update for new pre-salted images [skip ci] ([d95dac2](d95dac2))
* **windows:** update Salt version installed to `3004-3` [skip ci] ([fdccb9b](fdccb9b))
* **windows:** use Salt version `3004` [skip ci] ([a1e9823](a1e9823))

### Tests

* update for new pre-salted images [skip ci] ([8015fe0](8015fe0))
@saltstack-formulas-travis

🎉 This issue has been resolved in version 1.10.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@lmf-mx
Copy link

lmf-mx commented Feb 3, 2022

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

Thanks!

@myii
Copy link
Member

myii commented Feb 3, 2022

@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!

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

Successfully merging a pull request may close this issue.

4 participants