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

Fixes #5594: use windows compatible command for executor daemon #517

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion initial-promises/node-server/promises.cf
Original file line number Diff line number Diff line change
Expand Up @@ -511,9 +511,16 @@ body executor control
{
splaytime => "1";

exec_command => "${sys.cf_agent} -f failsafe.cf && ${sys.cf_agent}";
schedule => { "Min00", "Min05", "Min10", "Min15", "Min20", "Min25", "Min30", "Min35", "Min40", "Min45", "Min50", "Min55" };
executorfacility => "LOG_DAEMON";

windows::
# CFEngine best practice is to use full paths on Windows
exec_command => "${sys.cf_agent} -f \"${sys.workdir}\inputs\failsafe.cf\" & ${sys.cf_agent}";

!windows::
exec_command => "${sys.cf_agent} -f failsafe.cf && ${sys.cf_agent}";

}

########################################################
Expand Down
10 changes: 8 additions & 2 deletions techniques/system/common/1.0/promises.st
Original file line number Diff line number Diff line change
Expand Up @@ -734,10 +734,16 @@ body agent control
body executor control
{
splaytime => "&AGENT_RUN_SPLAYTIME&";

exec_command => "${sys.cf_agent} -f failsafe.cf \&\& ${sys.cf_agent}";
schedule => { &AGENT_RUN_SCHEDULE& };
executorfacility => "LOG_DAEMON";

windows::
# CFEngine best practice is to use full paths on Windows
exec_command => "${sys.cf_agent} -f \"${sys.workdir}\inputs\failsafe.cf\" \& ${sys.cf_agent}";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you specify the full path to failsafe.cf here? If it is necessary in windows for some weird reason, that weird reason must be in a comment on this file so future readers understand and don't break it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is what is done in CFEngine masterfiles; there is no reason to do differently

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand, sorry. We use just "failsafe.cf" on !windows, which is (according to CFEngine docs) a relative path taken from ${sys.workdir}/inputs. The change you are introducing in Rudder is adding a full path where we used to use a relative path.

My question is simple: does this relative path not work on Windows?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i do not know, as stated earlier, CFEngine best practice is to use for windows the full path, as showed in https://github.com/cfengine/masterfiles/blob/master/controls/cf_execd.cf#L34

the change i'm introducing is replacing && (that doesn't work on Windows) by & and using the syntax that is maintained by CFEngine, and assured to work in current and future release

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is a CFEngine best practice! That is what I'm trying to get at!

Yes, your change is to replace && by &. But you're introducing an unrelated change - the full path.

Please add a comment above this line explaining why you're doing this. The comment is very simple to write: "CFEngine best practice is to use full paths on Windows". Then, we can understand why this is different between windows:: and !windows:: in the future. Thank you.


!windows::
exec_command => "${sys.cf_agent} -f failsafe.cf \&\& ${sys.cf_agent}";

}

########################################################
Expand Down