Salt: file.line not inserting new line as expected
Description of Issue/Question
Given a dynamic state file.line does not insert a new line as expected. Given the state provided in the setup section it is expected to get behavior that continually inserts a new line above the line AAAAAAA. Instead, the first attempt adds a new line, however subsequent executions of the state do not modify the file at all.
I tried using the ensure mode to see if that would be a valid workaround for this issue. This worked, but replaced the line with the new content instead of inserting the new line as expected.
Further testing revealed that if the beginning of the string changes, then the line gets inserted. But changing on the end of the line will not update the line.
Setup
Setup a 2018.3.2 master and minion.
Create a file called /var/log/salt/test.log with write permissions for the root user:
AAAAAAA
Add this state:
{% set curtime = None | strftime('%Y-%m-%d %H:%M:%S') %}
update_missing_list:
file.line:
- name: /var/log/salt/test.log
- mode: insert
- content: {{ curtime }}
- before: AAAAAAA
First execution of state
f420deb9bac6:
----------
ID: update_missing_list
Function: file.line
Name: /var/log/salt/test.log
Result: True
Comment: Changes were made
Started: 19:20:15.094866
Duration: 8.982 ms
Changes:
----------
diff:
---
+++
@@ -1 +1,2 @@
+2018-10-01 19:20:15
AAAAAAA
Summary for f420deb9bac6
------------
Succeeded: 1 (changed=1)
Failed: 0
------------
Total states run: 1
Total run time: 8.982 ms
Second execution of state
f420deb9bac6:
----------
ID: update_missing_list
Function: file.line
Name: /var/log/salt/test.log
Result: True
Comment: No changes needed to be made
Started: 19:20:53.272621
Duration: 7.865 ms
Changes:
Summary for f420deb9bac6
------------
Succeeded: 1
Failed: 0
------------
Total states run: 1
Total run time: 7.865 ms
Versions Report
Salt Version:
Salt: 2018.3.2
Dependency Versions:
cffi: Not Installed
cherrypy: Not Installed
dateutil: Not Installed
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
ioflo: Not Installed
Jinja2: 2.7.2
libgit2: Not Installed
libnacl: Not Installed
M2Crypto: Not Installed
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.5.6
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: Not Installed
Python: 2.7.5 (default, Jul 13 2018, 13:06:57)
python-gnupg: Not Installed
PyYAML: 3.11
PyZMQ: 15.3.0
RAET: Not Installed
smmap: Not Installed
timelib: Not Installed
Tornado: 4.2.1
ZMQ: 4.1.4
System Versions:
dist: centos 7.5.1804 Core
locale: ANSI_X3.4-1968
machine: x86_64
release: 4.9.93-linuxkit-aufs
system: Linux
version: CentOS Linux 7.5.1804 Core
doesitblend
All 10 comments
ZD-2860
doesitblend
on 1 Oct 2018
looks like i'm able to replicate this on 2018.3.2 and the head of 2018.3
The reason for this is right here: https://github.com/saltstack/salt/blob/v2018.3.2/salt/modules/file.py#L1711
We are checking b_buff.startswith(prb) where prb is '2018-10-02'. So it is only trying to match the beginning of the date line, which looks like it was coded this way by design. Not sure if we want to change this so its forced to match the entire line, or add an option that allows someone to specify if we should check the entire line.
ping @saltstack/team-core any thoughts here?
Ch3LL
on 2 Oct 2018
This was added by @isbm in d1263b6a85c9ad2744ce4b55424bdb315aab1b73. @isbm, can you explain why partial line matches work here?
terminalmage
on 20 Nov 2018
@terminalmage I cannot remember anymore the exact reason for this, but I would agree that looking at the entire line would be a better way. We had couple of major changes on this since, so I am opened to fix remaining issues in develop.
isbm
on 27 Nov 2018
I expect I am facing the same issue here, which I was just about to enter as a new bug. I am finding that 2017.7 salt ~worked OK~ had different behavior in my example, and 2018.3 regressed. Interesting that the code in question seemed to be introduced in 2015 though.
This fails to make changes in the second state:
{% set first_search_pattern = "auth substack password" %}
{% set insert_line = "auth apple" %}
7 base file:
file.managed:
- name: /tmp/testfile
- contents: |
auth required pam_sepermit.so
auth substack password
auth include postlogin
7 mod 1:
file.line:
- name: /tmp/testfile
- mode: ensure
- after: {{first_search_pattern}}
- content: {{insert_line}}
- show_changes: True
with output as follows:
----------
ID: 7 base file
Function: file.managed
Name: /tmp/testfile
Result: True
Comment: File /tmp/testfile updated
Started: 17:24:06.875782
Duration: 16.411 ms
Changes:
----------
diff:
New file
----------
ID: 7 mod 1
Function: file.line
Name: /tmp/testfile
Result: True
Comment: No changes needed to be made
Started: 17:24:06.892596
Duration: 7.416 ms
Changes:
Summary for local
------------
Succeeded: 2 (changed=1)
Failed: 0
------------
Total states run: 2
Total run time: 23.827 ms
This succeeds at making changes in the second state, the only difference is the x added to {% set insert_line = "xauth apple" %}:
{% set first_search_pattern = "auth substack password" %}
{% set insert_line = "xauth apple" %}
7 base file:
file.managed:
- name: /tmp/testfile
- contents: |
auth required pam_sepermit.so
auth substack password
auth include postlogin
7 mod 1:
file.line:
- name: /tmp/testfile
- mode: ensure
- after: {{first_search_pattern}}
- content: {{insert_line}}
- show_changes: True
with output as follows:
----------
ID: 7 base file
Function: file.managed
Name: /tmp/testfile
Result: True
Comment: File /tmp/testfile updated
Started: 17:27:38.707319
Duration: 17.071 ms
Changes:
----------
diff:
New file
----------
ID: 7 mod 1
Function: file.line
Name: /tmp/testfile
Result: True
Comment: Changes were made
Started: 17:27:38.724768
Duration: 8.86 ms
Changes:
----------
diff:
---
+++
@@ -1,3 +1,4 @@
auth required pam_sepermit.so
auth substack password
+xauth apple
auth include postlogin
Summary for local
------------
Succeeded: 2 (changed=2)
Failed: 0
------------
Total states run: 2
Total run time: 25.931 ms
For the experiment noted above, my versions:
[root@ipa salt]# salt --versions-report
Salt Version:
Salt: 2018.3.3
Dependency Versions:
cffi: 1.6.0
cherrypy: unknown
dateutil: 2.5.3
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
ioflo: Not Installed
Jinja2: 2.8
libgit2: Not Installed
libnacl: Not Installed
M2Crypto: Not Installed
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.5.6
mysql-python: Not Installed
pycparser: 2.14
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: Not Installed
Python: 2.7.5 (default, Oct 30 2018, 23:45:53)
python-gnupg: 0.4.2
PyYAML: 3.11
PyZMQ: 15.3.0
RAET: Not Installed
smmap: Not Installed
timelib: Not Installed
Tornado: 4.2.1
ZMQ: 4.1.4
System Versions:
dist: centos 7.6.1810 Core
locale: UTF-8
machine: x86_64
release: 3.10.0-957.1.3.el7.x86_64
system: Linux
version: CentOS Linux 7.6.1810 Core
icycle77
on 21 Feb 2019
Now, repeating the same exact experiment on 2017.7.7, I see that while both states make changes, the "working" case is actually destructive and surfaces another bug.
See my previous comment for the state file used below.
Here's the case with {% set insert_line = "auth apple" %} -- note how this line replaced the line after my pattern match, rather than being inserted:
----------
ID: 7 base file
Function: file.managed
Name: /tmp/testfile
Result: True
Comment: File /tmp/testfile updated
Started: 17:39:59.753568
Duration: 6.78 ms
Changes:
----------
diff:
New file
----------
ID: 7 mod 1
Function: file.line
Name: /tmp/testfile
Result: True
Comment: Changes were made
Started: 17:39:59.760591
Duration: 1.442 ms
Changes:
----------
diff:
---
+++
@@ -1,3 +1,3 @@
auth required pam_sepermit.so
auth substack password
-auth include postlogin
+auth apple
Summary for local
------------
Succeeded: 2 (changed=2)
Failed: 0
------------
Total states run: 2
Total run time: 8.222 ms
Here's the case with {% set insert_line = "xauth apple" %}:
----------
ID: 7 base file
Function: file.managed
Name: /tmp/testfile
Result: True
Comment: File /tmp/testfile updated
Started: 17:35:45.561827
Duration: 6.836 ms
Changes:
----------
diff:
New file
----------
ID: 7 mod 1
Function: file.line
Name: /tmp/testfile
Result: True
Comment: Changes were made
Started: 17:35:45.568878
Duration: 1.405 ms
Changes:
----------
diff:
---
+++
@@ -1,3 +1,4 @@
auth required pam_sepermit.so
auth substack password
+xauth apple
auth include postlogin
Summary for local
------------
Succeeded: 2 (changed=2)
Failed: 0
------------
Total states run: 2
Total run time: 8.241 ms
Version report for this:
[root@ipa salt]# salt --versions-report
Salt Version:
Salt: 2017.7.7
Dependency Versions:
cffi: 1.6.0
cherrypy: unknown
dateutil: 2.5.3
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
ioflo: Not Installed
Jinja2: 2.8
libgit2: Not Installed
libnacl: Not Installed
M2Crypto: Not Installed
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.5.6
mysql-python: Not Installed
pycparser: 2.14
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: Not Installed
Python: 2.7.5 (default, Oct 30 2018, 23:45:53)
python-gnupg: 0.4.2
PyYAML: 3.11
PyZMQ: 15.3.0
RAET: Not Installed
smmap: Not Installed
timelib: Not Installed
Tornado: 4.2.1
ZMQ: 4.1.4
System Versions:
dist: centos 7.6.1810 Core
locale: UTF-8
machine: x86_64
release: 3.10.0-957.1.3.el7.x86_64
system: Linux
version: CentOS Linux 7.6.1810 Core
I went back to 2018.3.3 (note: I can't find 2018.3.4 in your repo for installation) and added a log to confirm the same issue:
[INFO ] Executing state file.line for [/tmp/testfile]
[INFO ] Executing command [u'lsattr', u'/tmp/testfile'] in directory '/root'
[DEBUG ] stdout: ---------------- /tmp/testfile
[DEBUG ] output: ---------------- /tmp/testfile
[DEBUG ] BRETT a_buff "[u'auth', u'apple']" b_buff "auth include postlogin" prb "auth"
[INFO ] No changes needed to be made
[INFO ] Completed state [/tmp/testfile] at time 19:59:26.392837 (duration_in_ms=7.386)
So yeah, looks like it's because of the odd partial string compare.
icycle77
on 21 Feb 2019
2018.3.4 is not officially released yet.
It is tagged, and probably built, so just waiting for it to be released,
hopefully next week
On Thu, Feb 21, 2019 at 2:04 PM Brett Russ notifications@github.com wrote:
I went back to 2018.3.3 (note: I can't find 2018.3.4 in your repo for
installation) and added a log to confirm the same issue:[INFO ] Executing state file.line for [/tmp/testfile]
[INFO ] Executing command [u'lsattr', u'/tmp/testfile'] in directory '/root'
[DEBUG ] stdout: ---------------- /tmp/testfile
[DEBUG ] output: ---------------- /tmp/testfile
[DEBUG ] BRETT a_buff "[u'auth', u'apple']" b_buff "auth include postlogin" prb "auth"
[INFO ] No changes needed to be made
[INFO ] Completed state [/tmp/testfile] at time 19:59:26.392837 (duration_in_ms=7.386)So yeah, looks like it's because of the odd partial match.
—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
https://github.com/saltstack/salt/issues/49855#issuecomment-466145828,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAssoUtEILH7XZREXHM0rptib_qqWo_Rks5vPvuzgaJpZM4XCvH_
.
gtmanfred
on 22 Feb 2019
can anyone give the fix in https://github.com/saltstack/salt/pull/52333 a try?
Ch3LL
on 30 Apr 2019
closing as merged and will be in Sodium Release
sagetherage
on 8 May 2020
Related issues
qiushics
·
3Comments
seanacais
·
3Comments
zieba88
·
3Comments
golmaal
·
3Comments
saurabhnemade
·
3Comments
Most helpful comment
@terminalmage I cannot remember anymore the exact reason for this, but I would agree that looking at the entire line would be a better way. We had couple of major changes on this since, so I am opened to fix remaining issues in
develop.