Stop Error noise from 'test -e'

[noelmcloughlin]

This PR stops SALT ERROR output when cloning formulas and unless check fails.

[ERROR   ] Command 'test -e /srv/formulas/epel-formula >/dev/null 2>&1' failed with return code: 1
[ERROR   ] retcode: 1
[ERROR   ] Command 'test -e /srv/formulas/openssh-formula >/dev/null 2>&1' failed with return code: 1
[ERROR   ] retcode: 1
[ERROR   ] Command 'test -e /srv/formulas/packages-formula >/dev/null 2>&1' failed with return code: 1
[ERROR   ] retcode: 1

[javierbertoli]

@noelmcloughlin , this onlyif always evaluate to true. Am I missing something?

• gitdir exists and is removed -> true
• gitdir exists but can't be removed for any reason -> ors to true

$ sudo mkdir /tmp/deleteable
$ ls -ld /tmp/deleteable
drwxr-xr-x 2 root root 4096 Dec 13 08:28 /tmp/deleteable
$ rm -rf /tmp/deleteable; echo $?
rm: cannot remove '/tmp/deleteable': Operation not permitted
1

• gitdir does not exist, rm -rf won't fail -> true

[noelmcloughlin]

It is quick solution without adding new file.absent state - the goal is to clone the repo and noise from conditional is a Salt bug imho. Open to other ideas - it's an irritation.

[noelmcloughlin]

Sorry yes - I am following 80/20 rule in assuming rm command just works. sudo rm -fr {{gitdir}}

[javierbertoli]

Yes, I understand. Comparing with other formulas, I see that we use test -<whatever> in many places, both with onlyif and with unless, and they don't add noise when used.

I'm suspicious of the stderr -> stdout redirection here as the culprit of the noise, and I wonder if just using

unless: test -e {{ gitdir }}

(as I see it being used in other formulas) won't fix the issue? It's worth a try, dyt?

[noelmcloughlin]

When using onlyif/unless Salt should generate FALSE or TRUE.

However today Salt generates either TRUE or FALSE+STDERR

Here is example from lvm-formula - FALSE is enough - the STDERR output is unwanted.

[ERROR   ] Command '[u'vgdisplay', u'-c', u'cinder_volumes']' failed with return code: 5
[ERROR   ] stderr:   Volume group "cinder_volumes" not found
  Cannot process volume group cinder_volumes

[noelmcloughlin]

I added the stderr->stdout->/dev/null in #384 but it didn't work ( I probably tried `test -d {{ gitdir}}` too). Any ideas why it happens?

…

On Thu, Dec 13, 2018 at 1:31 PM Javier Bértoli ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In salt/formulas.sls <#389 (comment)> : > @@ -51,7 +51,7 @@ - require: - file: {{ basedir }} {%- if not update %} - - unless: test -e {{ gitdir }} >/dev/null 2>&1 + - onlyif: rm -fr {{ gitdir }} >/dev/null 2>&1 | true Yes, I understand. Comparing with other formulas, I see that we use test -<whatever> in many places, both with onlyif and with unless, and they don't add noise when used. I'm suspicious of the stderr -> stdout redirection here as the culprit of the noise, and I wonder if just using unless: test -e {{ gitdir }} (as I see it being used in other formulas) won't fix the issue? It's worth a try, dyt? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#389 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMtKQgJpsMCj1QjrQyUvK7VzUGrh8fX7ks5u4lawgaJpZM4ZQo0e> .

[noelmcloughlin]

@javierbertoli I found an unexpected and undocumented solution. Studying the API shows that kwargs are supported. I passed output_loglevel: quiet' to git.latest and that worked.

salt.states.git.latest(name, rev=u'HEAD', target=None, branch=None, user=None, password=None,
 update_head=True, force_checkout=False, force_clone=False, force_fetch=False, force_reset=False,
 submodules=False, bare=False, mirror=False, remote=u'origin', fetch_tags=True, sync_tags=True,
 depth=None, identity=None, https_user=None, https_pass=None, onlyif=None, unless=None, 
refspec_branch=u'*', refspec_tag=u'*', output_encoding=None, **kwargs **)

The salt cmdmod.py module sets "output_loglevel" to "debug" by default. Turns out unless/onlyif for various states are supported by same code (I think).

Anyway it works - I'm closing the PR as not needed.

[noelmcloughlin]

I opened replacement PR #390