Fixes #7192: Rewrite the service_* methods #544

amousset · 2017-02-28T11:57:47Z

https://www.rudder-project.org/redmine/issues/7192

amousset · 2017-02-28T11:58:26Z

Still being tested, but the implementation should be complete now.

amousset · 2017-02-28T12:00:29Z

tree/30_generic_methods/service_ensure_running_path.cf

-  classes:
-    # This method can be called as a substitution if service_ensure_running is called directly, with service_path=service_name
-    # In this case, we don't need to call service_check_running_ps but can use better OS-specific methods if available
-    "real_path"                       not => strcmp("${service_name}", "${service_path}");


This is an incompatible change, but I see previous behavior like a bug more than a feature...

amousset · 2017-02-28T12:02:02Z

Commit modified

amousset · 2017-02-28T13:29:40Z

Commit modified

ncharles · 2017-02-28T13:37:29Z

tree/20_cfe_basics/ncf_lib.cf

+    debian_8::
+      # This is to workaround a bug in Debian Jessie, that makes systemctl is-enabled
+      # fail if run against a service that still uses SysV style scripts
+      # See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=760616


This bug was marked as fixed on 29/05/2015, is it still relevant to keep this exception ?

It seems it was only fixed in unstable, and not in stable (by a systemd upgrade).

ncharles · 2017-02-28T13:51:43Z

tree/30_generic_methods/service_ensure_stopped.cf

@@ -62,14 +63,17 @@ bundle agent service_ensure_stopped(service_name)
        usebundle  => _classes_copy("${class_prefix}_stop_if_running", "${class_prefix}"),
        ifvarclass => "has_promiser_stack.${class_prefix}_check_running_kept";

+    windows::
+      "action"             usebundle => service_stop("${service_name}");


why not keeping the windows service code there ?

amousset · 2017-02-28T13:53:47Z

Commit modified

amousset · 2017-02-28T13:54:46Z

Replaced command_execution and commands promise by returnszero. Allows:

Avoiding useless or misleading reports from log_default
Avoiding the systemd workaround

ncharles · 2017-02-28T13:54:52Z

tree/20_cfe_basics/ncf_lib.cf

+# Set variables with the error
+# Test behavior with unkown action
+# is-active not implemented for service/init.d
+bundle agent ncf_services(service, action)


If i understand correctly, it will mostly return "repair" outcome, am I right ?

ncharles · 2017-02-28T13:58:51Z

tree/20_cfe_basics/ncf_lib.cf

+      "${configuration.error} The ${action} action is not supported on Windows (use 'start', 'stop' or 'restart')";
+
+    pass3.info.method_found.(!windows|is_valid_action)::
+      "${configuration.info} Executing ${action} on ${service} using the ${method} method";


ncharles · 2017-02-28T14:06:06Z

tree/30_generic_methods/service_ensure_running_path.cf

@@ -75,14 +67,17 @@ bundle agent service_ensure_running_path(service_name, service_path)
        usebundle  => _classes_success("${old_class_prefix}"),
        ifvarclass => "(real_path.service_check_running_${canonified_service_path}_kept)|(!real_path.service_check_running_${canonified_service_name}_kept)";

+    windows::


Why not keeping this as a servies: promise type ?

ncharles · 2017-02-28T14:06:20Z

tree/30_generic_methods/service_ensure_started_at_boot.cf

        ifvarclass => "service_check_started_at_boot_${canonified_service_name}_not_ok";

      "copy classes"
        usebundle  => _classes_copy("${class_prefix}_define_at_boot", "${class_prefix}"),
        ifvarclass => "${class_prefix}_check_at_boot_not_ok";

+    windows::


why not keeping this one also ?

amousset · 2017-02-28T14:23:15Z

Commit modified

amousset · 2017-02-28T14:33:33Z

Commit modified

amousset · 2017-02-28T14:45:19Z

Commit modified

amousset · 2017-02-28T14:52:19Z

Commit modified

amousset · 2017-02-28T14:55:09Z

Commit modified

amousset · 2017-02-28T16:07:59Z

Commit modified

amousset · 2017-02-28T17:02:08Z

Commit modified

ncharles · 2017-03-01T09:29:09Z

tree/20_cfe_basics/ncf_lib.cf

+    #####
+    pass2.is_check_action.!windows.!is_process_action.method_found.action_ok::
+      "force_success_class"    usebundle => _classes_success("${old_class_prefix}");
+      "force_success_class"    usebundle => _classes_success("${class_prefix}");


we should have _classes_success_two (or equivalent) (this is not the place of this PR thought)

amousset · 2017-03-01T11:43:51Z

Commit modified

amousset · 2017-03-01T11:44:29Z

Changed the way to handle service ${service status as modifying the action breaks reporting.

amousset · 2017-03-01T11:45:44Z

Commit modified

amousset · 2017-03-01T11:53:28Z

Failing tests:

./30_generic_methods/service_check_started_at_boot.cf
./30_generic_methods/unsafe/service_check_disabled_at_boot.cf

amousset · 2017-03-01T12:09:54Z

Commit modified

amousset · 2017-03-01T12:11:54Z

TODO:

Real tests that enforce state in init and check actual service status in test
Document the way services are handled, possible options, etc.
Document the (small) behavior changes

amousset · 2017-03-01T14:22:12Z

Also missing proper upstart support.

amousset · 2017-03-01T15:08:18Z

Commit modified

amousset · 2017-03-01T15:12:08Z

Commit modified

amousset · 2017-03-01T15:25:27Z

Currently failing on Ubuntu 14.04:

./30_generic_methods/service_check_started_at_boot.cf FAIL
./30_generic_methods/unsafe/service_ensure_running_start.cf FAIL

ncharles · 2017-03-01T15:57:02Z

I'd like to point out that on master branch, the tests

service_start,
service_check_disabled_at_boot.cf,
service_check_running_ps.cf
service_check_started_at_boot.cf
are failing

amousset · 2017-03-01T16:41:05Z

Commit modified

amousset · 2017-03-01T16:42:46Z

Added upstart support!

amousset · 2017-03-01T16:58:38Z

Commit modified

ncharles · 2017-03-01T17:03:07Z

tree/20_cfe_basics/ncf_lib.cf

+      "upstart_action_cmd[reload]"     string => "/sbin/initctl reload ${service}";
+      "upstart_action_cmd[is-active]"  string => "/sbin/initctl status ${service} | grep -q ' start/'";
+      # boot
+      "upstart_action_cmd[enable]"     string => "${paths.path[sed]} -i '/manual/d' /etc/init/${service}.override";


this sed is not anchored, would it be an issue ?

amousset · 2017-03-01T17:03:44Z

Commit modified

amousset · 2017-03-01T17:09:12Z

Commit modified

Normation-Quality-Assistant · 2017-03-01T17:09:50Z

OK, merging this PR

amousset commented Feb 28, 2017

View reviewed changes

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from ecceed9 to f8082a1 Compare February 28, 2017 12:02

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from f8082a1 to ee82423 Compare February 28, 2017 13:29

ncharles reviewed Feb 28, 2017

View reviewed changes

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from ee82423 to 40b05e8 Compare February 28, 2017 13:53

ncharles reviewed Feb 28, 2017

View reviewed changes

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch 2 times, most recently from cf762a1 to 8633500 Compare February 28, 2017 14:33

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from 8633500 to c9cda8f Compare February 28, 2017 14:45

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from c9cda8f to 7aafd2d Compare February 28, 2017 14:52

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from 7aafd2d to 6f01242 Compare February 28, 2017 14:55

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch 2 times, most recently from 5a65e0a to bc9de88 Compare February 28, 2017 17:01

ncharles reviewed Mar 1, 2017

View reviewed changes

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from bd7e014 to d8cf46e Compare March 1, 2017 11:43

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from d8cf46e to f18cb97 Compare March 1, 2017 11:45

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from f18cb97 to a148e98 Compare March 1, 2017 12:09

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from a148e98 to 166364e Compare March 1, 2017 15:08

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from 166364e to 76c2673 Compare March 1, 2017 15:12

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from 76c2673 to bd57e32 Compare March 1, 2017 16:41

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from bd57e32 to 3fbda7e Compare March 1, 2017 16:58

ncharles reviewed Mar 1, 2017

View reviewed changes

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from 3fbda7e to 56ee829 Compare March 1, 2017 17:03

Fixes #7192: Rewrite the service_* methods

86c2e96

amousset force-pushed the arch_7192/rewrite_the_service_methods_to_benefit_from_the_power_of_the_new_aggregation_methods_from_7171 branch from 56ee829 to 86c2e96 Compare March 1, 2017 17:09

ncharles added the Ready for merge label Mar 1, 2017

Normation-Quality-Assistant merged commit 86c2e96 into Normation:master Mar 1, 2017

Fixes #7192: Rewrite the service_* methods #544

Fixes #7192: Rewrite the service_* methods #544

Conversation

amousset commented Feb 28, 2017

amousset commented Feb 28, 2017 • edited Loading

amousset Feb 28, 2017 • edited Loading

Choose a reason for hiding this comment

amousset commented Feb 28, 2017

amousset commented Feb 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amousset commented Feb 28, 2017

amousset commented Feb 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amousset commented Feb 28, 2017

amousset commented Feb 28, 2017

amousset commented Feb 28, 2017

amousset commented Feb 28, 2017

amousset commented Feb 28, 2017

amousset commented Feb 28, 2017

amousset commented Feb 28, 2017

Choose a reason for hiding this comment

amousset commented Mar 1, 2017

amousset commented Mar 1, 2017

amousset commented Mar 1, 2017

amousset commented Mar 1, 2017

amousset commented Mar 1, 2017

amousset commented Mar 1, 2017

amousset commented Mar 1, 2017

amousset commented Mar 1, 2017

amousset commented Mar 1, 2017

amousset commented Mar 1, 2017 • edited Loading

ncharles commented Mar 1, 2017

amousset commented Mar 1, 2017

amousset commented Mar 1, 2017

amousset commented Mar 1, 2017

Choose a reason for hiding this comment

amousset commented Mar 1, 2017

amousset commented Mar 1, 2017

Normation-Quality-Assistant commented Mar 1, 2017

amousset commented Feb 28, 2017 •

edited

Loading

amousset Feb 28, 2017 •

edited

Loading

amousset commented Mar 1, 2017 •

edited

Loading