Page MenuHomePhabricator

[scap] Syncing wmf-config/PrivateSettings.php syncs symlink and not file contents
Closed, ResolvedPublic

Description

Chad tripped over this today in prod. He made a change to /a/common/wmf-config/PrivateSettings.php (which is a symlink to /a/common/private/PrivateSettings.php) and then ran sync-file wmf-config/PrivateSettings.php to sync the file with the cluster.

This resulted in an rsync command similar to this running on each cluster host:

sudo -u mwdeploy -n -- /usr/bin/rsync --archive --delete-delay --delay-updates --compress --delete --exclude=/.svn/lock --exclude=/.git/objects --exclude=/.git//objects --exclude=**/cache/l10n/*.cdb --no-perms --include=/wmf-config --include=/wmf-config/PrivateSettings.php --exclude=* tin.eqiad.wmnet::common /usr/local/apache/common-local

Scap is dutifully syncing the file that it was asked to sync, but that file is just a symlink so rsync will only actually sync a symlink and not the contents of the file the symlink points to on tin.

The manual fix that a deployer can use is to run sync-file private/PrivateSettings.php instead to sync the actual file rather than the symlink. This seems error prone to say the least however. Scap should do something (at least emit a warning) when asked to sync a symlink. Ideally it could sync both the symlink and the target file but that may be too much magic.


Version: wmf-deployment
Severity: normal

Details

Reference
bz70054

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 3:28 AM
bzimport added a project: Deployments.
bzimport set Reference to bz70054.
bzimport added a subscriber: Unknown Object (MLST).

I like warning you here - sometimes you want to sync out a symlink? Probably.

I can imagine how one get tricked in that:

vim symlink.txt
sync-file symlink.txt

+1 on emitting a warning that a symlink is being send instead of the actual target.

Change 180200 had a related patch set uploaded (by 20after4):
Add a warning when attempting to sync a symlink. This fixes T72054 Bug: T72054 Reviewers: bd808

https://gerrit.wikimedia.org/r/180200

Patch-For-Review

So far I just emit a simple warning without requiring confirmation. Should this be more heavy handed and require a --really-sync-symlink arg or something like that?

Change 180200 merged by jenkins-bot:
Add a warning when attempting to sync a symlink. This fixes T72054 Bug: T72054 Reviewers: bd808

https://gerrit.wikimedia.org/r/180200

I deployed the patch to beta, but not to prod yet. @mmodell and @Reedy can take care of the prod deploy when they get a chance.

@Reedy: What is involved with deploying this? scap is deployed via puppet, yes?

mmodell raised the priority of this task from Low to Medium.Jan 14 2015, 5:05 AM