Fixes #8952: Add generic methods using new package promises #427

amousset · 2016-09-06T16:48:55Z

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

amousset · 2016-09-06T16:49:04Z

WIP

jooooooon · 2016-09-07T06:53:00Z

tree/30_generic_methods/package_present.cf

+# ```
+# methods:
+#     "any" usebundle => package_present("postgresql", "9.1", "x86_64", "enforce");
+#     "any" usebundle => package_present("postgresql", "lastest", "default", "enforce");


s/lastest/latest/

amousset · 2016-09-07T09:23:26Z

Commit modified

amousset · 2016-09-07T13:32:25Z

Commit modified

ncharles · 2016-09-07T13:37:57Z

tree/30_generic_methods/package_present.cf

+
+    debian.architecure_specified.version_specified::
+      "${name}"
+          policy => "present",


could you align the => , please ?
Thank you !

amousset · 2016-09-07T16:20:25Z

Commit modified

ncharles · 2016-09-08T07:38:49Z

tree/30_generic_methods/package_present.cf

+
+}
+
+body package_module apt_get


shouldn't this body be in the ncf_lib?
Plus it seem it doesn't do anything on redhat like system (nor suse)

It is still a WIP, I will add Red Hat support after. The body is there for convenience, it will be moved in a "3.7 ncf_lib" after.

amousset · 2016-09-08T09:58:14Z

Commit modified

amousset · 2016-09-08T14:18:34Z

Commit modified

amousset · 2016-09-08T14:20:16Z

Commit modified

ncharles · 2016-09-08T14:59:58Z

tree/30_generic_methods/package_absent.cf

+
+  vars:
+      "canonified_name"  string => canonify("${name}");
+      "old_class_prefix" string => "package_present_${canonified_name}";


shouldn't it be package_absent ?

amousset · 2016-09-09T10:29:13Z

Commit modified

amousset · 2016-09-09T13:11:13Z

Commit modified

amousset · 2016-09-09T13:16:06Z

tree/20_cfe_basics/package_lib.cf

+body package_module yum
+{
+    # Query package updates only every 24 hours.
+    query_updates_ifelapsed => "1440";


@ncharles How do do you think we should make this configurable?

i can make a dumb answer: add parameters in the body
a more clever answer would be to have logical default, and be able to define them globally (it doesn't make sense to have different values for different package on a given node). So it may be a generic method "configure package update frequency" that would define it through a bundle common something.
What do you think of it ?

amousset · 2016-09-09T15:33:50Z

Commit modified

amousset · 2016-09-09T15:54:18Z

Commit modified

amousset · 2016-09-12T09:01:46Z

Commit modified

jooooooon · 2016-09-14T16:37:07Z

It's time to implement drop-down / multiple selection lists in ncf builder :) cc @VinceMacBuche @RaphaelGauthier

jooooooon · 2016-09-14T16:37:50Z

OK, thanks, Alexis. I'll ask @aanriot to test then.

amousset · 2016-09-15T14:31:14Z

Commit modified

amousset · 2016-09-15T14:37:23Z

Fixed a typo that broke specified architectures
Improved the test.

amousset · 2016-09-15T15:07:30Z

We still have two open questions:

Distributing the modules

Normation/rudder-techniques#1029.

Configuring the modules

Each module needs a body like:

body package_module apt_get
{
    # Query list of installed packages from package manager
    query_installed_ifelapsed => "60";
    # Query list of available updates from package manager
    query_updates_ifelapsed => "1440";
}

We should probably have default values and can make them overridable:

With a specific call/generic method
Using ncf.conf

I think I prefer the second, as it is really a global configuration parameter and not a part of any policy (like execution frequency for example).
Ideally, It would be great to have a setting in Rudder to set this parameter by node/group/global.

amousset · 2016-09-16T09:58:34Z

Added the body for the test module to package_lib.cf as it is now in the _package_state bundle.

amousset · 2016-09-21T15:55:01Z

Added a zypper module based on the yum module. Commands and options are adapted, but the logic from parsing zypper output is not there yet.

amousset · 2016-09-21T16:41:59Z

Added parsing for the list of available updates. We still need:

Build package arguments (probably not very different, still not tested)
Handle installation from repositories, a bit more test needed for update/install handling

amousset · 2016-09-23T14:44:56Z

Commit modified

ncharles

This is one impressive piece of work - i do have some questions (as comments in the PR)

ncharles · 2016-09-26T11:02:18Z

tests/acceptance/30_generic_methods/package_present.cf

@@ -0,0 +1,57 @@
+bundle common acc_path


You are missing the header of the file

ncharles · 2016-09-26T11:03:24Z

tree/10_ncf_internals/modules/packages/apt_get

@@ -0,0 +1,361 @@
+#!/usr/bin/python


shouldn't we mention somewhere that it comes from the CFEngine repository ?

ncharles · 2016-09-26T11:07:49Z

tree/10_ncf_internals/modules/packages/yum

@@ -0,0 +1,408 @@
+#!/usr/bin/python


ncharles · 2016-09-26T11:08:05Z

tree/10_ncf_internals/modules/packages/zypper

@@ -0,0 +1,394 @@
+#!/usr/bin/python


shouldn't we add our header ?

ncharles · 2016-09-26T11:10:42Z

tree/30_generic_methods/_package_state.cf

+      "architecture_specified" not => strcmp("default", "${architecture}");
+
+      # Select the right package manager
+      # Always use the specified one, and select a default based on the OS


i don't understand how the default one would be selected

because of the "provider" string => "${package_module_knowledge.platform_default}", if_match_regex => "default"; above.

But this comment should be moved.

amousset · 2016-09-26T12:08:13Z

Commit modified

ncharles · 2016-09-26T12:10:36Z

tree/30_generic_methods/_package_state.cf

+      "architecture" string => "default", if_match_regex => "";
+      "provider"     string => "default", if_match_regex => "";
+      # Select the default packager for this platform if "default"
+      "provider"     string => "${package_module_knowledge.platform_default}", if_match_regex => "default";


can you chain the default (does it work as expected ?)

amousset · 2016-09-26T12:36:58Z

Commit modified

amousset · 2016-09-26T12:51:27Z

Commit modified

ncharles

This is great !

Normation-Quality-Assistant · 2016-09-26T16:50:20Z

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/ncf/pull/427
-- Your faithful QA

amousset · 2016-09-26T16:52:38Z

OK, merging this PR

jooooooon reviewed Sep 7, 2016
View reviewed changes

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from e9b37ca to 3689453 Compare September 7, 2016 09:23

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from 3689453 to 512289a Compare September 7, 2016 13:32

ncharles reviewed Sep 7, 2016
View reviewed changes

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from 512289a to 0005020 Compare September 7, 2016 16:20

ncharles reviewed Sep 8, 2016
View reviewed changes

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from 0005020 to 125acc6 Compare September 8, 2016 09:58

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from 125acc6 to 78c0504 Compare September 8, 2016 14:18

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from 78c0504 to 68d6d68 Compare September 8, 2016 14:20

ncharles reviewed Sep 8, 2016
View reviewed changes

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from 68d6d68 to 4d74f0d Compare September 9, 2016 10:29

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from 4d74f0d to d6dc5a2 Compare September 9, 2016 13:11

amousset reviewed Sep 9, 2016
View reviewed changes

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from d6dc5a2 to fb9f99b Compare September 9, 2016 15:33

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from fb9f99b to 855e3dd Compare September 9, 2016 15:54

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from 855e3dd to 738ec96 Compare September 12, 2016 09:01

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch 2 times, most recently from baf570f to c0a0305 Compare September 15, 2016 14:36

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from c0a0305 to 9092690 Compare September 16, 2016 09:43

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from 9092690 to e16124d Compare September 21, 2016 15:52

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from e16124d to b345837 Compare September 21, 2016 16:39

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from b345837 to fb551ca Compare September 23, 2016 14:44

ncharles reviewed Sep 26, 2016

View reviewed changes

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from fb551ca to dab2c0a Compare September 26, 2016 12:08

ncharles reviewed Sep 26, 2016

View reviewed changes

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from dab2c0a to 1a04fab Compare September 26, 2016 12:37

Fixes #8952: Add generic methods using new package promises

7fb553c

amousset force-pushed the ust_8952/add_generic_methods_using_new_package_promises branch from 1a04fab to 7fb553c Compare September 26, 2016 12:51

ncharles approved these changes Sep 26, 2016

View reviewed changes

ncharles added the Ready for merge label Sep 26, 2016

Normation-Quality-Assistant added the qa: Can't merge label Sep 26, 2016

amousset merged commit 7fb553c into Normation:master Sep 26, 2016

Fixes #8952: Add generic methods using new package promises #427

Fixes #8952: Add generic methods using new package promises #427

Conversation

amousset commented Sep 6, 2016

amousset commented Sep 6, 2016

Choose a reason for hiding this comment

amousset commented Sep 7, 2016

amousset commented Sep 7, 2016

Choose a reason for hiding this comment

amousset commented Sep 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amousset commented Sep 8, 2016

amousset commented Sep 8, 2016

amousset commented Sep 8, 2016

Choose a reason for hiding this comment

amousset commented Sep 9, 2016

amousset commented Sep 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amousset commented Sep 9, 2016

amousset commented Sep 9, 2016

amousset commented Sep 12, 2016

jooooooon commented Sep 14, 2016

jooooooon commented Sep 14, 2016

amousset commented Sep 15, 2016

amousset commented Sep 15, 2016

amousset commented Sep 15, 2016

Distributing the modules

Configuring the modules

amousset commented Sep 16, 2016

amousset commented Sep 21, 2016

amousset commented Sep 21, 2016

amousset commented Sep 23, 2016

ncharles left a comment

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

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

Choose a reason for hiding this comment

amousset commented Sep 26, 2016

Choose a reason for hiding this comment

amousset commented Sep 26, 2016

amousset commented Sep 26, 2016

ncharles left a comment

Choose a reason for hiding this comment

Normation-Quality-Assistant commented Sep 26, 2016

amousset commented Sep 26, 2016