ungleich-public/cdist

Fork 10

No output for manifest or remotes #140

New issue

Closed

opened 2021-11-20 13:24:36 +00:00 by ungleich-gitea · 8 comments

ungleich-gitea commented

2021-11-20 13:24:36 +00:00

Owner

Created by: antifob

Commit 9703e0f08e "broke" things for us.

We used to, among other things, trace execution by calling set -x from inside our remote scripts. This patch silences the output.

Why was this change introduced? Why is there no documentation? And why is there no way to configure this behavior?

*Created by: antifob* Commit 9703e0f08e979640fc1956fbe7fd2e763dde39f1 "broke" things for us. We used to, among other things, trace execution by calling `set -x` from inside our remote scripts. This patch silences the output. Why was this change introduced? Why is there no documentation? And why is there no way to configure this behavior?

ungleich-gitea closed this issue

2021-11-20 13:24:36 +00:00

ungleich-gitea commented

2021-11-20 15:42:40 +00:00

Author

Owner

Created by: antifob

$ ./cdist/bin/cdist config -S --remote-copy ./remote/copy --remote-exec ./remote/exec localhost
+ ssh localhost sh -c "rm -rf /var/lib/cdist"

👍

*Created by: antifob* ``` $ ./cdist/bin/cdist config -S --remote-copy ./remote/copy --remote-exec ./remote/exec localhost + ssh localhost sh -c "rm -rf /var/lib/cdist" ``` :+1:

ungleich-gitea commented

2021-11-20 15:42:41 +00:00

Author

Owner

Created by: darko-poljak

@uqam-fob @asteven @tom-ee https://github.com/darko-poljak/cdist/tree/output_streams_switch
Can you clone my repo and test this branch?
If you run cdist config as usual then output streams will be saved.
If you run cdist config with -S option then saving output streams should be disabled.

@tom-ee Yes, those files are saved to cache only after run has finished. In case of error those files are not saved to cache. But tmp directory is not removed and all known info is printed, e.g.

$ ./bin/cdist config -v -i ~/.cdist/manifest/init-output-streams $(cat ~/ungleich/data/opennebula-debian9-test )
INFO: 185.203.112.42: Starting configuration run
INFO: 185.203.112.42: Processing __myline/test
ERROR: 185.203.112.42: Command failed: '/bin/sh -e /tmp/tmpow6cwemh/75ee6a79e32da093da23fe4a13dd104b/data/object/__myline/test/.cdist-kisrqlpw/code-local'
return code: 1
---- BEGIN stdout ----
---- END stdout ----

Error processing object '__myline/test'
========================================
name: __myline/test
path: /tmp/tmpow6cwemh/75ee6a79e32da093da23fe4a13dd104b/data/object/__myline/test/.cdist-kisrqlpw
source: /home/darko/.cdist/manifest/init-output-streams
type: /tmp/tmpow6cwemh/75ee6a79e32da093da23fe4a13dd104b/data/conf/type/__myline

---- BEGIN manifest:stderr ----
myline manifest stderr

---- END manifest:stderr ----

---- BEGIN gencode-remote:stderr ----
test gencode-remote error

---- END gencode-remote:stderr ----

---- BEGIN code-local:stderr ----
error

---- END code-local:stderr ----

ERROR: cdist: Failed to configure the following hosts: 185.203.112.42

Since it is not removed, you can discover tmp dir.

With new -S option the output is as the following:

$ ./bin/cdist config -v -S -i ~/.cdist/manifest/init-output-streams $(cat ~/ungleich/data/opennebula-debian9-test )
INFO: 185.203.112.42: Starting configuration run
test stdout output streams
test stderr output streams
myline manifest stdout
myline manifest stderr
test gencode-remote error
INFO: 185.203.112.42: Processing __myline/test
error
ERROR: 185.203.112.42: Command failed: '/bin/sh -e /tmp/tmpzomy0wis/75ee6a79e32da093da23fe4a13dd104b/data/object/__myline/test/.cdist-n566pqut/code-local'
return code: 1
---- BEGIN stdout ----
---- END stdout ----

Error processing object '__myline/test'
========================================
name: __myline/test
path: /tmp/tmpzomy0wis/75ee6a79e32da093da23fe4a13dd104b/data/object/__myline/test/.cdist-n566pqut
source: /home/darko/.cdist/manifest/init-output-streams
type: /tmp/tmpzomy0wis/75ee6a79e32da093da23fe4a13dd104b/data/conf/type/__myline


ERROR: cdist: Failed to configure the following hosts: 185.203.112.42

*Created by: darko-poljak* @uqam-fob @asteven @tom-ee https://github.com/darko-poljak/cdist/tree/output_streams_switch Can you clone my repo and test this branch? If you run cdist config as usual then output streams will be saved. If you run cdist config with `-S` option then saving output streams should be disabled. @tom-ee Yes, those files are saved to cache only after run has finished. In case of error those files are not saved to cache. But tmp directory is not removed and all known info is printed, e.g. ``` $ ./bin/cdist config -v -i ~/.cdist/manifest/init-output-streams $(cat ~/ungleich/data/opennebula-debian9-test ) INFO: 185.203.112.42: Starting configuration run INFO: 185.203.112.42: Processing __myline/test ERROR: 185.203.112.42: Command failed: '/bin/sh -e /tmp/tmpow6cwemh/75ee6a79e32da093da23fe4a13dd104b/data/object/__myline/test/.cdist-kisrqlpw/code-local' return code: 1 ---- BEGIN stdout ---- ---- END stdout ---- Error processing object '__myline/test' ======================================== name: __myline/test path: /tmp/tmpow6cwemh/75ee6a79e32da093da23fe4a13dd104b/data/object/__myline/test/.cdist-kisrqlpw source: /home/darko/.cdist/manifest/init-output-streams type: /tmp/tmpow6cwemh/75ee6a79e32da093da23fe4a13dd104b/data/conf/type/__myline ---- BEGIN manifest:stderr ---- myline manifest stderr ---- END manifest:stderr ---- ---- BEGIN gencode-remote:stderr ---- test gencode-remote error ---- END gencode-remote:stderr ---- ---- BEGIN code-local:stderr ---- error ---- END code-local:stderr ---- ERROR: cdist: Failed to configure the following hosts: 185.203.112.42 ``` Since it is not removed, you can discover tmp dir. With new `-S` option the output is as the following: ``` $ ./bin/cdist config -v -S -i ~/.cdist/manifest/init-output-streams $(cat ~/ungleich/data/opennebula-debian9-test ) INFO: 185.203.112.42: Starting configuration run test stdout output streams test stderr output streams myline manifest stdout myline manifest stderr test gencode-remote error INFO: 185.203.112.42: Processing __myline/test error ERROR: 185.203.112.42: Command failed: '/bin/sh -e /tmp/tmpzomy0wis/75ee6a79e32da093da23fe4a13dd104b/data/object/__myline/test/.cdist-n566pqut/code-local' return code: 1 ---- BEGIN stdout ---- ---- END stdout ---- Error processing object '__myline/test' ======================================== name: __myline/test path: /tmp/tmpzomy0wis/75ee6a79e32da093da23fe4a13dd104b/data/object/__myline/test/.cdist-n566pqut source: /home/darko/.cdist/manifest/init-output-streams type: /tmp/tmpzomy0wis/75ee6a79e32da093da23fe4a13dd104b/data/conf/type/__myline ERROR: cdist: Failed to configure the following hosts: 185.203.112.42 ```

ungleich-gitea commented

2021-11-20 15:42:42 +00:00

Author

Owner

Created by: tom-ee

I'd second adding a switch to disable the "save output streams". In particular the stderr/*- and stdout/*-files created by the new features are only available after the config-run has finished.

The messages-file is only created if the config-run does not error-out. Is this also the case for output-streams?

*Created by: tom-ee* I'd second adding a switch to disable the "save output streams". In particular the `stderr/*`- and `stdout/*`-files created by the new features are only available _after_ the config-run has finished. The `messages`-file is only created if the config-run does not error-out. Is this also the case for output-streams?

ungleich-gitea commented

2021-11-20 15:42:44 +00:00

Author

Owner

Created by: antifob

@asteven Yes, I understand the problem and why it might be desired. Overall, I think it is a good solution. I just don't see why it should be forced on users.

@darko-poljak This looks like a good change for us. Whether the default or not, if the feature can be opted-in/-out, it would be greatly appreciated 👍 Documentation would help to prevent confusion too, so would be a nice addition.

*Created by: antifob* @asteven Yes, I understand the problem and why it might be desired. Overall, I think it is a good solution. I just don't see why it should be forced on users. @darko-poljak This looks like a good change for us. Whether the default or not, if the feature can be opted-in/-out, it would be greatly appreciated 👍 Documentation would help to prevent confusion too, so would be a nice addition.

ungleich-gitea commented

2021-11-20 15:42:45 +00:00

Author

Owner

Created by: darko-poljak

@uqam-fob @asteven Adding option to turn this off shouldn't be hard. I will take this task and this new command line/config option. If you agree, by default, saving output streams will be turned on.
I also plan to add docs chapter where saving output streams will be described/explained in more details.

*Created by: darko-poljak* @uqam-fob @asteven Adding option to turn this off shouldn't be hard. I will take this task and this new command line/config option. If you agree, by default, saving output streams will be turned on. I also plan to add docs chapter where saving output streams will be described/explained in more details.

ungleich-gitea commented

2021-11-20 15:42:46 +00:00

Author

Owner

Created by: asteven

I understand your problem: it used to work, now it doesn't. Without you changing anything yourself. That sucks from a users point of view.

I implemented the output stream capturing because I had some real problems debugging things when running multiple cdist runs in parallel. The unexpected output, e.g. warning and error messages from commands run in generated code are all printed in random order to stderr. There's no prefix, like we have for log messages. You simply don't have a clue which warning/error message belongs to which command.

This is now cleanly handled. All created output is bound to the context where it was produced. Overall I think this is a big win and I want to stick to this feature.

But we can discuss if/how we can make the output capturing optional. Problem is that it could hurt performance. We will investigate.

For debugging remote-copy and remote-exec I also run them with set -x. But I send that output to a file. Would that also work for you? IMHO while running remote-exec with set -x there's so much output that it's almost useless if not redirected to a dedicated file.

*Created by: asteven* I understand your problem: it used to work, now it doesn't. Without you changing anything yourself. That sucks from a users point of view. I implemented the output stream capturing because I had some real problems debugging things when running multiple cdist runs in parallel. The unexpected output, e.g. warning and error messages from commands run in generated code are all printed in random order to stderr. There's no prefix, like we have for log messages. You simply don't have a clue which warning/error message belongs to which command. This is now cleanly handled. All created output is bound to the context where it was produced. Overall I think this is a big win and I want to stick to this feature. But we can discuss if/how we can make the output capturing optional. Problem is that it could hurt performance. We will investigate. For debugging remote-copy and remote-exec I also run them with ```set -x```. But I send that output to a file. Would that also work for you? IMHO while running remote-exec with ```set -x``` there's so much output that it's almost useless if not redirected to a dedicated file.

ungleich-gitea commented

2021-11-20 15:42:48 +00:00

Author

Owner

Created by: antifob

Thanks for the information.

When I wrote "remote scripts", I was mentioning "remote-copy" and "remote-exec" scripts; as this patch affects more than the scripts you mentioned. Knowing exactly what commands are executed helps us analyzing the execution and what is being done on our systems. Also, set -x only prints executed commands to stderr, nothing specific about shell scripts here.

I understand it might be useful to silence output sometimes, but I don't think having code totally silenced is useful or even desired. Of course, we could collect the information as an after-fact, or write a program that constantly polls output files and prints them to the screen, but I don't believe it is an elegant solution as it blocks real-time feedback and the notion of a timeline (can it even be reconstructed?).

A switch to turn this off would provide backward-compatibility, or having access to a facility that would allow messages to be printed to the screen would be useful here.

Makes me wonder if ssh connection prompts (e.g. accept certificates) are hidden now.

*Created by: antifob* Thanks for the information. When I wrote "remote scripts", I was mentioning "remote-copy" and "remote-exec" scripts; as this patch affects more than the scripts you mentioned. Knowing exactly what commands are executed helps us analyzing the execution and what is being done on our systems. Also, `set -x` only prints executed commands to stderr, nothing specific about shell scripts here. I understand it might be useful to silence output sometimes, but I don't think having code totally silenced is useful or even desired. Of course, we could collect the information as an after-fact, or write a program that constantly polls output files and prints them to the screen, but I don't believe it is an elegant solution as it blocks real-time feedback and the notion of a timeline (can it even be reconstructed?). A switch to turn this off would provide backward-compatibility, or having access to a facility that would allow messages to be printed to the screen would be useful here. Makes me wonder if ssh connection prompts (e.g. accept certificates) are hidden now.

ungleich-gitea commented

2021-11-20 15:42:49 +00:00

Author

Owner

Created by: darko-poljak

@uqam-fob The reason for implementing output-streams: important information was lost during a config run, hidden in all the other output.
We now store all that, including error messages.
This all is saved into following files under host entry (sub-directory) under ~/.cdist/cache:

./stdout/init
./stdout/remote
./stderr/init
./stderr/remote

and for each object under

~/.cdist/cache/<host-dir>/object/<object-type-name>/<object-marker>/<stdout or stderr/

there could be files like manifest, gencode-remote, code-remote, gencode-local, code-local (if output was produced) which contain stdout/stderr content.

Setting set -x and seeing the output is sort of what one would expect when running a shell script. Then again we're not just running shell scripts.

Also, now in case of an error, cdist can exit and show all information it has about the error.

Can you use this new saved output streams?

*Created by: darko-poljak* @uqam-fob The reason for implementing output-streams: important information was lost during a config run, hidden in all the other output. We now store all that, including error messages. This all is saved into following files under host entry (sub-directory) under ~/.cdist/cache: ``` ./stdout/init ./stdout/remote ./stderr/init ./stderr/remote ``` and for each object under ``` ~/.cdist/cache/<host-dir>/object/<object-type-name>/<object-marker>/<stdout or stderr/ ``` there could be files like manifest, gencode-remote, code-remote, gencode-local, code-local (if output was produced) which contain stdout/stderr content. Setting set -x and seeing the output is sort of what one would expect when running a shell script. Then again we're not just running shell scripts. Also, now in case of an error, cdist can exit and show all information it has about the error. Can you use this new saved output streams?