Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #3925 - Prevent UUID to be removed #119

Merged
merged 7 commits into from
Sep 21, 2013
Merged

Fixes #3925 - Prevent UUID to be removed #119

merged 7 commits into from
Sep 21, 2013

Conversation

nperron
Copy link
Contributor

@nperron nperron commented Sep 12, 2013

Fixes #3925 - Prevent UUID to be removed

See http://www.rudder-project.org/redmine/issues/3925

Nicolas Perron added 2 commits September 12, 2013 10:54
…, and set 'root' to UUID only during rudder-server-root install but use the script during upgrade
@nperron
Copy link
Contributor Author

nperron commented Sep 16, 2013

PR Updated. I've added a new script to install for rudder-agent packages and will be used by cron. This script will check CFEngine and UUID for the moment.
Postinst of rudder-server-root package will launch it to recover UUID if removed

# If the UUID is not valid, regenerate it
if [ ${CHECK_UUID} -ne 1 ]; then
echo -n "INFO: Creating a new UUID for Rudder as the existing one is invalid..."
uuidgen > ${UUID_FILE}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should make a backup copy of the old UUID before savagely overwriting it.

@nperron
Copy link
Contributor Author

nperron commented Sep 17, 2013

Pull Request Updated.

# in order to remove it later without complains of the package manager.
CHECK_RUDDER_AGENT_CRON=`grep "check-rudder-agent" techniques/system/common/1.0/rudder_agent_community_cron.st | wc -l`
# Add it only if the default cron file does not call check-rudder-agent script
if [ ${CHECK_RUDDER_AGENT_CRON} -ne 1 ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like rather sloppy logic in the test. What we want to test is whether the cron file contains a call to "/opt/rudder/bin/check-rudder-agent". Why not use that in your grep rather than a substring? Also, why check" ==1" rather than "!=0", which is what we care about?

Code should reflect intent, otherwise it is hard to link the implementation to the logic that brought the author to it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this check is not sufficient - one should only run the echo and chmod commands if the temporary file does not exist.

@nperron
Copy link
Contributor Author

nperron commented Sep 19, 2013

Pull Request updated ! sorry for bizarre logic and wrong files :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants