unexpected result using __line type #24

Open
opened 2021-11-20 11:24:54 +00:00 by ungleich-gitea · 7 comments

execute test

cat > /tmp/interfaces <<EOF
auto eno1
iface eno1 inet manual
auto eno2
iface eno2 inet manual
EOF

cp /tmp/interfaces /tmp/interfaces.orig

cdist config localhost -i - <<EOF
__line test_eno1 \
  --file /tmp/interfaces \
  --after 'iface eno1' \
  --line  '    mtu 9000' \
  --state present

__line test_eno2 \
  --file /tmp/interfaces \
  --after 'iface eno2' \
  --line  '    mtu 9000' \
  --state present
EOF

diff -u --color /tmp/interfaces.orig /tmp/interfaces

result:

[02:37:21] $ diff -u --color /tmp/interfaces.orig /tmp/interfaces
--- /tmp/interfaces.orig	2020-10-20 02:37:20.312333544 +0200
+++ /tmp/interfaces	2020-10-20 02:37:21.600376393 +0200
@@ -2,3 +2,4 @@
 iface eno1 inet manual
 auto eno2
 iface eno2 inet manual
+    mtu 9000

execute test ``` cat > /tmp/interfaces <<EOF auto eno1 iface eno1 inet manual auto eno2 iface eno2 inet manual EOF cp /tmp/interfaces /tmp/interfaces.orig cdist config localhost -i - <<EOF __line test_eno1 \ --file /tmp/interfaces \ --after 'iface eno1' \ --line ' mtu 9000' \ --state present __line test_eno2 \ --file /tmp/interfaces \ --after 'iface eno2' \ --line ' mtu 9000' \ --state present EOF diff -u --color /tmp/interfaces.orig /tmp/interfaces ``` result: ```diff [02:37:21] $ diff -u --color /tmp/interfaces.orig /tmp/interfaces --- /tmp/interfaces.orig 2020-10-20 02:37:20.312333544 +0200 +++ /tmp/interfaces 2020-10-20 02:37:21.600376393 +0200 @@ -2,3 +2,4 @@ iface eno1 inet manual auto eno2 iface eno2 inet manual + mtu 9000 ```
Author
Owner

@pedro The __hosts type has many issues of its own (cf. #826). Don't use it as an example.

@pedro The `__hosts` type has many issues of its own (cf. #826). Don't use it as an example.
Author
Owner

@pedro If you're interested in modifying network interfaces you might be interested in !838.
The MR is actually working, but still marked WIP because I can't make up my mind how and if dual stack configurations should be handled.

@pedro If you're interested in modifying network interfaces you might be interested in !838. The MR is actually working, but still marked WIP because I can't make up my mind how and if dual stack configurations should be handled.
Author
Owner

@ander that's nice, object-ids are unique, so if we are always appending automatically the object ID as a comment on each line introduced, then it would work. I saw something in __hosts type, which, oh wow, it uses __line type too in a very similar use case, and they solved it implementing what you said:

https://code.ungleich.ch/ungleich-public/cdist/-/blob/master/cdist/conf/type/__hosts/manifest#L26

so what about moving that marker in __line type so more types and custom manifests can benefit of using __line multiple times

@ander that's nice, object-ids are unique, so if we are always appending automatically the object ID as a comment on each line introduced, then it would work. I saw something in __hosts type, which, oh wow, it uses __line type too in a very similar use case, and they solved it implementing what you said: https://code.ungleich.ch/ungleich-public/cdist/-/blob/master/cdist/conf/type/__hosts/manifest#L26 so what about moving that *marker* in __line type so more types and custom manifests can benefit of using __line multiple times
Author
Owner

can confirm.

reason is that __line will allow only one occurence of --line X.

usecase is totally valid, but __line is not designed this usecase in mind. not a bug, but rather shortcoming.

one possible workaround is to make lines somewhat unique, for example:

__line test_eno1 \
    --file /tmp/interfaces \
    --after 'iface eno1' \
    --line  '    mtu 9000 # eno1 mtu' \
    --state present

__line test_eno2 \
    --file /tmp/interfaces \
    --after 'iface eno2' \
    --line  '    mtu 9000 # eno2 mtu' \
    --state present

but this doesn't fly with every file format.

my awk-fu isn't that good, so I can't help with a "fix" here 😿

can confirm. reason is that `__line` will allow only one occurence of `--line X`. usecase is totally valid, but `__line` is not designed this usecase in mind. not a bug, but rather shortcoming. one possible workaround is to make lines somewhat unique, for example: ``` __line test_eno1 \ --file /tmp/interfaces \ --after 'iface eno1' \ --line ' mtu 9000 # eno1 mtu' \ --state present __line test_eno2 \ --file /tmp/interfaces \ --after 'iface eno2' \ --line ' mtu 9000 # eno2 mtu' \ --state present ``` but this doesn't fly with every file format. my awk-fu isn't that good, so I can't help with a "fix" here :crying_cat_face:
Author
Owner

I have the same output, I hope is easy enough to test

cat > /tmp/interfaces <<EOF
auto eno1
iface eno1 inet manual
auto eno2
iface eno2 inet manual
EOF

cp /tmp/interfaces /tmp/interfaces.orig

cdist config localhost -i - <<EOF
__line test_eno1 \
  --file /tmp/interfaces \
  --after 'iface eno1' \
  --line  '    mtu 9000' \
  --state present

require='__line/test_eno1' \
  __line test_eno2 \
    --file /tmp/interfaces \
    --after 'iface eno2' \
    --line  '    mtu 9000' \
    --state present
EOF
diff -u --color /tmp/interfaces.orig /tmp/interfaces
I have the same output, I hope is easy enough to test ``` cat > /tmp/interfaces <<EOF auto eno1 iface eno1 inet manual auto eno2 iface eno2 inet manual EOF cp /tmp/interfaces /tmp/interfaces.orig cdist config localhost -i - <<EOF __line test_eno1 \ --file /tmp/interfaces \ --after 'iface eno1' \ --line ' mtu 9000' \ --state present require='__line/test_eno1' \ __line test_eno2 \ --file /tmp/interfaces \ --after 'iface eno2' \ --line ' mtu 9000' \ --state present EOF diff -u --color /tmp/interfaces.orig /tmp/interfaces ```
Author
Owner

race condition, because no dependency defined and parallel execution?

add require='__line/test_eno1' \ before __line test_eno2 \

race condition, because no dependency defined and parallel execution? add `require='__line/test_eno1' \` before `__line test_eno2 \`
Author
Owner

changed the description

changed the description
Sign in to join this conversation.
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: ungleich-public/cdist#24
No description provided.