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] file.comment complains if not found #58269

Closed
prensing opened this issue Aug 21, 2020 · 5 comments
Closed

[BUG] file.comment complains if not found #58269

prensing opened this issue Aug 21, 2020 · 5 comments
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Core relates to code central or existential to Salt severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around State-Module
Milestone

Comments

@prensing
Copy link

Description
The file.comment service complains, with an error, if the regex is not found. This does not match similar behavior in, for instance, file.replace, which does not complain if it is not found.

Steps to Reproduce the behavior
Create file.comment with an expression which does not match.

Expected behavior
Silently ignore that nothing matches

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
Salt Version:
           Salt: 3001.1
 
Dependency Versions:
           cffi: 1.14.0
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.20
       pycrypto: 2.6.1
   pycryptodome: 3.4.7
         pygit2: Not Installed
         Python: 3.6.9 (default, Jul 17 2020, 12:50:27)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 17.1.2
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5
 
System Versions:
           dist: ubuntu 18.04 Bionic Beaver
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-70-generic
         system: Linux
        version: Ubuntu 18.04 Bionic Beaver

@prensing prensing added the Bug broken, incorrect, or confusing behavior label Aug 21, 2020
@DmitryKuzmenko
Copy link
Contributor

It'd say: it's worth to change the default behavior of file.replace to return an error in case the target regex is not found in the file. But it's good idea to have an option to change this behavior to not complain if it's needed for particular cases.

@DmitryKuzmenko DmitryKuzmenko added Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Core relates to code central or existential to Salt severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around State-Module and removed needs-triage labels Aug 27, 2020
@DmitryKuzmenko DmitryKuzmenko removed their assignment Aug 27, 2020
@DmitryKuzmenko DmitryKuzmenko added this to the Approved milestone Aug 27, 2020
@prensing
Copy link
Author

If the team thinks that the default behavior of the methods should be to complain if the pattern does not match, I am fine with that as long as there is some ability to turn it off. Particularly with file.comment, many of the uses would be to "get rid" of a line if it is present, but it does not matter if it is not present.

@ferricoxide
Copy link

If the team thinks that the default behavior of the methods should be to complain if the pattern does not match, I am fine with that as long as there is some ability to turn it off. Particularly with file.comment, many of the uses would be to "get rid" of a line if it is present, but it does not matter if it is not present.

Yeah, I don't so much care that it complains, but there should at least be an "ignore-on-no-match" option. Right now, I'm left with a state that makes me ask "what even is the point of this state?" I mean, if I do two, consecutive runs of a file.comment state on the same system (as part of a periodic "return-to-baseline" task), notionally, after the file.comment state does The Right Thing™ on its initial run, it sh/would fail on subsequent runs (since my ^<REGEX> should return a "not found" on every run subsequent the initial, content-changing run).

@radioipcloud
Copy link

radioipcloud commented Jan 12, 2022

A workaround can simply use onlyif with grep,awk etc... in order to make sure to run only if the pattern is present. It will prevent the state from running if the pattern is not present.

<file_of_some_kind>:
file.comment:
- regex: ^somepattern
- onlyif: grep "^pool" <file_of_some_kind>

@nicholasmhughes
Copy link
Collaborator

addressed in #62045

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Core relates to code central or existential to Salt severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around State-Module
Projects
None yet
Development

No branches or pull requests

6 participants