Project

General

Profile

Actions

Bug #10379

closed

When upgrading to 4.1, rudder.community.checkpromises.command=/bin/true option is lost

Added by Nicolas CHARLES about 7 years ago. Updated almost 7 years ago.

Status:
Released
Priority:
N/A
Category:
Web - Config management
Target version:
Severity:
Minor - inconvenience | misleading | easy workaround
UX impact:
User visibility:
Infrequent - complex configurations | third party integrations
Effort required:
Small
Priority:
14
Name check:
Fix check:
Regression:

Description

If we set rudder.community.checkpromises.command=/bin/true , it's to avoid check of generated policy
However, when we upgrade to 4.1, the post generation check is made by

/opt/rudder/etc/hooks.d/policy-generation-node-ready/10-cf-promise-check

so I ended up with much slower policy generation - and piling of validation that I did not expect

This is surprising


Subtasks 1 (0 open1 closed)

Bug #10418: checkpromise+sighup config should be commented in properties file and bad name of corresponding hookReleasedBenoît PECCATTEActions

Related issues 2 (0 open2 closed)

Related to Rudder - User story #10411: Add migration notes for 4.1RejectedAlexis MoussetActions
Related to Rudder - User story #10412: Add a convention for ".disabled" hooks to not be executedReleasedVincent MEMBRÉActions
Actions #1

Updated by François ARMAND about 7 years ago

Hum. You're right.

It does not seem very grave, and it is easy to revert to previous behaviour. But perhaps we should add a migration script from * -> 4.1.

Actions #2

Updated by Jonathan CLARKE about 7 years ago

  • User visibility set to Infrequent - complex configurations | third party integrations
Actions #3

Updated by Jonathan CLARKE about 7 years ago

  • Effort required set to Small
Actions #4

Updated by François ARMAND about 7 years ago

  • Subject changed from Surprising change of behaviour when upgrading from 3.x or 4.0 to 4.1 if rudder.community.checkpromises.command=/bin/true to When upgrading to 4.1, rudder.community.checkpromises.command=/bin/true option is lost
Actions #5

Updated by François ARMAND about 7 years ago

So, we are missing a migration script that is looking in property file, grep for the option set to /bin/true and is so, set a-x /opt/rudder/etc/hooks.d/policy-generation-node-ready/10-cf-promise-check (and perhaps remove the option from the file to not be misleading for users, replacing it with comment).

Actions #6

Updated by François ARMAND about 7 years ago

That being said, shouldn't the properties "rudder.community.checkpromises.command" and "rudder.nova.checkpromises.command" be completelly commented? It is misleading that they are still present, even in fresh install (it is good that there is a text talking about hooks,thought)

Actions #7

Updated by Nicolas CHARLES about 7 years ago

Indeed, they should be commented
However, I think we should replace the values in the script, not disabling it
However, how would it behave at upgrade ? Will it create a new script with .rpmnew, that would be executed also ?

Actions #8

Updated by François ARMAND about 7 years ago

OK, so after some more thought, the value may have been changed to something else than the default value or/bin/true, like something they wrote themself. It does not seem reliable on that case to try to change policy-generation-node-ready/10-cf-promise-check hook (because it is not intented to be), so I'm seeing two possibilities: disable policy-generation-node-ready/10-cf-promise-check, copy it into policy-generation-node-ready/11-user-cf-promise-check, sed in it the commands ; or write a big warning "you did something advanced for cf-promise, please write you hook in place (example in policy-generation-node-ready/10-cf-promise-check).

Actions #9

Updated by François ARMAND about 7 years ago

Nicolas CHARLES wrote:

However, how would it behave at upgrade ? Will it create a new script with .rpmnew, that would be executed also ?

The condition is for it to be executable, the name is not taken into account. So yes, if the .rpmnew file is executable, it will be executed.

Actions #10

Updated by Alexis Mousset about 7 years ago

Actions #11

Updated by François ARMAND about 7 years ago

  • Related to User story #10412: Add a convention for ".disabled" hooks to not be executed added
Actions #12

Updated by François ARMAND about 7 years ago

In fact, we can always migrate with three cases:

- if the value of the property is the default value, do nothing,
- if the value of the property is /bin/true, disable the hook
- if the value is some other command, the implicit contract was that the command would receive the parameters "-f ${pathOfPromises}/promises.cf" so in this case, we just have to disable the existing hook and create a new hook as follow (modulo good shell skills:)

#!/bin/sh

# Hooks parameter are passed by environment variable: 
#
# - RUDDER_GENERATION_DATETIME    : generation datetime: ISO-8601 YYYY-MM-ddTHH:mm:ss.sssZ date/time that identify that policy generation start 
# - RUDDER_NODEID                 : the nodeId
# - RUDDER_NEXT_POLICIES_DIRECTORY: new policies directory (for ex for nodes under root: /var/rudder/share/$RUDDER_NODEID/rules.new)
# - RUDDER_AGENT_TYPE             : agent type ("cfengine-nova" or "cfengine-community")

HERE_NAME_OF_COMMAND_FROM_PROPERTY -f "${RUDDER_NEXT_POLICIES_DIRECTORY}/promises.cf" 
Actions #13

Updated by Benoît PECCATTE about 7 years ago

  • Status changed from New to In progress
  • Assignee set to Benoît PECCATTE
Actions #14

Updated by François ARMAND about 7 years ago

The same logic must be applied for property: rudder.cfengine.reload.server.command ; default value: /opt/rudder/bin/rudder-reload-cf-serverd ; if the value is not that one, then hook: /opt/rudder/hooks.d/policy-generation-finished/50-sigup-cf-serverd must be disabled and a new hook with the value created (no parameter expected).

Actions #15

Updated by Benoît PECCATTE about 7 years ago

  • Status changed from In progress to Pending technical review
  • Assignee changed from Benoît PECCATTE to Alexis Mousset
  • Pull Request set to https://github.com/Normation/rudder-packages/pull/1292
Actions #16

Updated by Benoît PECCATTE about 7 years ago

  • Status changed from Pending technical review to Pending release
Actions #17

Updated by Benoît PECCATTE about 7 years ago

  • Priority set to 14
Actions #18

Updated by Benoît PECCATTE almost 7 years ago

  • Status changed from Pending release to Released

This bug has been fixed in Rudder 4.1.0 which was released today.

Actions

Also available in: Atom PDF