Rudder

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.

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.

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

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

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.

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).

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.

Nicolas PERRON wrote:

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

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.

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