Project

General

Profile

Bug #10379

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

Added by Nicolas CHARLES 7 months ago. Updated 6 months ago.

Status:
Released
Priority:
N/A
Category:
Web - Config management
Target version:
Target version (plugin):
Severity:
Minor - inconvenience | misleading | easy workaround
User visibility:
Infrequent - complex configurations | third party integrations
Effort required:
Small
Priority:
14

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

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


Related issues

Related to Rudder - User story #10411: Add migration notes for 4.1 New
Related to Rudder - User story #10412: Add a convention for ".disabled" hooks to not be executed Released

Associated revisions

Revision 79e95874
Added by Benoît PECCATTE 6 months ago

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

History

#1 Updated by François ARMAND 6 months 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.

#2 Updated by Jonathan CLARKE 6 months ago

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

#3 Updated by Jonathan CLARKE 6 months ago

  • Effort required set to Small

#4 Updated by François ARMAND 6 months 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

#5 Updated by François ARMAND 6 months 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).

#6 Updated by François ARMAND 6 months 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)

#7 Updated by Nicolas CHARLES 6 months 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 ?

#8 Updated by François ARMAND 6 months 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).

#9 Updated by François ARMAND 6 months 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.

#10 Updated by Alexis MOUSSET 6 months ago

#11 Updated by François ARMAND 6 months ago

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

#12 Updated by François ARMAND 6 months 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" 

#13 Updated by Benoît PECCATTE 6 months ago

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

#14 Updated by François ARMAND 6 months 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).

#15 Updated by Benoît PECCATTE 6 months 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

#16 Updated by Benoît PECCATTE 6 months ago

  • Status changed from Pending technical review to Pending release

#17 Updated by Benoît PECCATTE 6 months ago

  • Priority set to 14

#18 Updated by Benoît PECCATTE 6 months ago

  • Status changed from Pending release to Released

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

Also available in: Atom PDF