Project

General

Profile

Actions

Bug #2747

closed

After a migration from 2.3.8 to 2.4.0~beta3, some distributes policies are not commited

Added by Nicolas PERRON over 12 years ago. Updated about 12 years ago.

Status:
Released
Priority:
1 (highest)
Assignee:
Nicolas PERRON
Category:
System techniques
Target version:
Severity:
UX impact:
User visibility:
Effort required:
Priority:
Name check:
Fix check:
Regression:

Description

It seems that rudde-rupgrade doesn't take into account some distribute policies. After a migration, some of them are not commited and errors are displayed in the /var/log/rudder/webapp/XXXX_XX_XX.stderrout.log:

root@squeezedev:/var/rudder/configuration-repository# git status
# On branch master
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#    techniques/system/distributePolicy/1.0/aliveCheck.st
#    techniques/system/distributePolicy/1.0/initCheck.st
#    techniques/system/distributePolicy/1.0/integrityCheck.st
#    techniques/system/distributePolicy/1.0/logrotate.st
#    techniques/system/distributePolicy/1.0/logrotateCheck.st
#    techniques/system/distributePolicy/1.0/networkCheck.st
#    techniques/system/distributePolicy/1.0/passwordCheck.st
#    techniques/system/distributePolicy/1.0/postgresCheck.st
nothing added to commit but untracked files present (use "git add" to track)

[2012-08-01 03:37:20] ERROR com.normation.cfclerk.services.impl.GitTechniqueReader - Template with id distributePolicy/1.0/initCheck was not found
[2012-08-01 03:37:21] ERROR com.normation.rudder.services.policies.RudderCf3PromisesFileWriterServiceImpl - Writing promises error : 
java.lang.RuntimeException: Error when trying to open template 'distributePolicy/1.0/initCheck.st'. Check that the file exists and is correctly commited in Git, or that the metadata for the technique are corrects.
    at com.normation.cfclerk.services.impl.Cf3PromisesFileWriterServiceImpl$$anonfun$writePromisesFiles$1$$anonfun$apply$7.apply(Cf3PromisesFileWriterServiceImpl.scala:151) ~[cfclerk-2.4.0-SNAPSHOT.jar:na]
[2012-08-01 03:37:21] ERROR com.normation.rudder.batch.AsyncDeploymentAgent - Deployment error for process '1' at 2012/08/01 03:37:21: Exception caught during deployment process: Error when trying to open template 'distributePolicy/1.0/initCheck.st'. Check that the file exists and is correctly commited in Git, or that the metadata for the technique are corrects.
java.lang.RuntimeException: Error when trying to open template 'distributePolicy/1.0/initCheck.st'. Check that the file exists and is correctly commited in Git, or that the metadata for the technique are corrects.
    at com.normation.cfclerk.services.impl.Cf3PromisesFileWriterServiceImpl$$anonfun$writePromisesFiles$1$$anonfun$apply$7.apply(Cf3PromisesFileWriterServiceImpl.scala:151) ~[cfclerk-2.4.0-SNAPSHOT.jar:na]
Actions #1

Updated by Nicolas PERRON over 12 years ago

The problem is that in rudder-upgrade, we want to update the folder /var/rudder/configuration-repository/techniques/system/ without committing any of the user modification or new technique.
What we do is that we make a copy of system techniques and all promises modified are commited. The eight others techniques which are not modified but added are not included in this commit.

We could do a git commit add to include all of the techniques but the user's ones will be committed too.

Actions #2

Updated by Nicolas PERRON over 12 years ago

Nicolas PERRON wrote:

The problem is that in rudder-upgrade, we want to update the folder /var/rudder/configuration-repository/techniques/system/ without committing any of the user modification or new technique.
What we do is that we make a copy of system techniques and all promises modified are commited. The eight others techniques which are not modified but added are not included in this commit.

We could do a git commit add to include all of the techniques but the user's ones will be committed too.

Or we could use this:

git add -u techniques/system/ #Instead of git commit -m "my message of commit" techniques/system
git add techniques/system/distributePolicy/1.0/aliveCheck.st techniques/system/distributePolicy/1.0/initCheck.st [...] # All of the eight techniques missing

But I fear that a hard list of files to add is too dangerous.

Actions #3

Updated by Jonathan CLARKE over 12 years ago

  • Status changed from New to Discussion

Nicolas, I agree with you - neither solution is acceptable. We mustn't commit user modifications on any non-system Techniques, and we mustn't use a fixed list of files to add.

You need to find a git command that will add all files under the techniques/system directory. I thought we had done this in the 2.3 > 2.4 migration (for policy-templates -> techniques rename), check that maybe.

Actions #4

Updated by Nicolas PERRON over 12 years ago

Jonathan CLARKE wrote:

Nicolas, I agree with you - neither solution is acceptable. We mustn't commit user modifications on any non-system Techniques, and we mustn't use a fixed list of files to add.

You need to find a git command that will add all files under the techniques/system directory. I thought we had done this in the 2.3 > 2.4 migration (for policy-templates -> techniques rename), check that maybe.

We can't guarantee that the user will not add Techniques in the /var/rudder/configuration-repository/techniques/system directory, so this is why git add is not used in techniques/system/

I think that I had the solution:

find /opt/rudder/share/techniques/system -type f | cut -d '/' -f 5- | xargs git add

It will search for all the files which was updated from /opt/rudder/share/techniques/system and will add them to git. With this command will should be sure that no user's Techniques will be added from /var/rudder/configuration-repository/techniques/system

Actions #5

Updated by Nicolas PERRON over 12 years ago

  • Status changed from Discussion to Pending technical review
  • % Done changed from 0 to 100

Applied in changeset commit:4a75c87d186ee2765cf9e6aeadb03d50d00a608a.

Actions #6

Updated by Matthieu CERDA over 12 years ago

Works fine. Quite a funny way to resolve the problem :D

Actions #8

Updated by Michael Gliwinski about 12 years ago

Nicolas PERRON wrote:

I think that I had the solution:
[...]

It will search for all the files which was updated from /opt/rudder/share/techniques/system and will add them to git. With this command will should be sure that no user's Techniques will be added from /var/rudder/configuration-repository/techniques/system

There is a minor issue with this solution: if techniques were removed from /opt/rudder/share/techniques/system, they will not be removed from /var/rudder/configuration-repository/techniques/system, which causes the diff -Naur ... check in rudder-upgrade script from rudder-webapp to fail on each upgrade, which in turn causes it to try to update and commit system techniques on each update, and if there are no changes, git commit fails breaking the upgrade.

This (removal of techniques) has happened recently with apacheCheck.st and ldapCheck.st, which is how I noticed this.

In terms of solution, maybe also handle specific files that should be removed? e.g. something like

for fn in distributePolicy/1.0/apacheCheck.st distributePolicy/1.0/ldapCheck.st; do
    rm -f /var/rudder/configuration-repository/techniques/system/$fn
done

It seems if you try to handle deletions generically, you run into the same issue of forbidding users from changing/adding techniques in system.

Actions #9

Updated by Michael Gliwinski about 12 years ago

Michael Gliwinski wrote:

This (removal of techniques) has happened recently with apacheCheck.st and ldapCheck.st, which is how I noticed this.

Just hit this again with distributePolicy/1.0/logrotate.st (nightly build rudder-webapp 2.4.0~beta5~git201209120436-precise0).

Actions #10

Updated by Jonathan CLARKE about 12 years ago

  • Status changed from Pending technical review to Discussion
  • Target version changed from 2.4.0~beta3 to 2.4.0~beta5

Thanks for pointing this out Michael. Let's see how we can work around this.

I'd rather avoid having a manual list of files, because I can just tell that we're going to be deleting more in the future, and not remember to update the list... so I'd rather have an automated solution, if possible.

Assuming that the user does not have control over the system policies is almost already the case - they are central to the way Rudder works, and some parts of the webapp's code rely on them. The real problem here is the way that the Techniques are updated at the moment I think (we're hoping to change that in 2.5 to be able to just "git pull", therefore git could manage any merges nicely).

Actions #11

Updated by Nicolas PERRON about 12 years ago

Jonathan CLARKE wrote:

Thanks for pointing this out Michael. Let's see how we can work around this.

I'd rather avoid having a manual list of files, because I can just tell that we're going to be deleting more in the future, and not remember to update the list... so I'd rather have an automated solution, if possible.

I agree with you, we can't have a manual list of files to upgrade

Assuming that the user does not have control over the system policies is almost already the case - they are central to the way Rudder works, and some parts of the webapp's code rely on them. The real problem here is the way that the Techniques are updated at the moment I think (we're hoping to change that in 2.5 to be able to just "git pull", therefore git could manage any merges nicely).

I agree with you and so I use rsync --delete in my last commit (http://www.rudder-project.org/redmine/projects/packages/repository/revisions/69b397f20a4baa1c8d510fd8b90262110ded69dd) in order to be sure that the files from /opt/rudder/share/techniques/system are exactly the same than in /var/rudder/configuration-repository/techniques/system.

Actions #12

Updated by Nicolas PERRON about 12 years ago

  • Status changed from Discussion to Pending technical review
Actions #13

Updated by Jonathan CLARKE about 12 years ago

Nicolas PERRON wrote:

Jon, could you review my last commit (http://www.rudder-project.org/redmine/projects/packages/repository/revisions/69b397f20a4baa1c8d510fd8b90262110ded69dd) , please ?

Just had a look, and I changed a few things:
  1. The rsync target was wrong! This command would have wiped out all Techniques... You need to use /techniques/*system/*.
  2. Added the -q (quiet) option to rsync, this is a postinst script after all
  3. The whole block of code above the rsync line seems to do the same thing, but handling a specific case (deleting some files and adding another one). I think it can be removed in favour of the simpler rsync approach.

Commiting a revision with these changes, please review.

Actions #14

Updated by Nicolas PERRON about 12 years ago

Jonathan CLARKE wrote:

Nicolas PERRON wrote:

Jon, could you review my last commit (http://www.rudder-project.org/redmine/projects/packages/repository/revisions/69b397f20a4baa1c8d510fd8b90262110ded69dd) , please ?

Just had a look, and I changed a few things:
  1. The rsync target was wrong! This command would have wiped out all Techniques... You need to use /techniques/*system/*.
  2. Added the -q (quiet) option to rsync, this is a postinst script after all
  3. The whole block of code above the rsync line seems to do the same thing, but handling a specific case (deleting some files and adding another one). I think it can be removed in favour of the simpler rsync approach.

Commiting a revision with these changes, please review.

You're right and your commit seems good to me

Actions #15

Updated by Jonathan CLARKE about 12 years ago

  • Status changed from Pending technical review to Released
Actions

Also available in: Atom PDF