First draft of this ticket up. There's a couple things that aren't perfect:

(1) I think the current description is a bit confusing, I'm not sure if another team has enough context in the ticket's current state.

(2) I haven't tagged the team who would work this ticket. Probably rel-eng?

Scap would be releng ye

I think I have the context to understand this.

It looks like /srv/deployment/wdqs/wdqs-cache/revs/$CURRENT_DEPLOY_COMMIT_HASH/.git/config-files/etc/query_service is symlinked at /etc/query_service/ldf-config.json is that true?

I see this line in log output:

/etc/query_service/ldf-config.json is already linked to current rev(use --force to override)

Which is exactly what you're describing. It comes from the check in scap deploy here: https://gitlab.wikimedia.org/repos/releng/scap/-/blob/master/scap/deploy.py#L212

The assumption is that if a file is already symlinked, there's no need to regenerate the file. But it sounds like it's a bad assumption in this case, is that true?

@thcipriani that's correct. For context, we are manually deleting the directory because puppet runs scap before git-fat is installed, and it results in an incomplete deploy (more details here ) . The deletion results in a broken symlink, maybe scap could check for that?

Also, if we try and run scap before deleting these directories, scap does not detect the missing files (large files provided by git-fat) and reports a successful deployment.

I'm not trying to cross the streams with what might be 2 different bugs (or even expected behavior), just trying to explain why we're manually deleting that directory.

In T342162#9025774, @thcipriani wrote:

I think I have the context to understand this.

It looks like /srv/deployment/wdqs/wdqs-cache/revs/$CURRENT_DEPLOY_COMMIT_HASH/.git/config-files/etc/query_service is symlinked at /etc/query_service/ldf-config.json is that true?

I see this line in log output:

/etc/query_service/ldf-config.json is already linked to current rev(use --force to override)

Which is exactly what you're describing. It comes from the check in scap deploy here: https://gitlab.wikimedia.org/repos/releng/scap/-/blob/master/scap/deploy.py#L212

The assumption is that if a file is already symlinked, there's no need to regenerate the file. But it sounds like it's a bad assumption in this case, is that true?

Right, I think we want to check that the file associated with the symlink actually exists, in addition to checking that the symlink itself is present.

Config-deploy/symlink checking

In T342162#9025804, @bking wrote:

@thcipriani that's correct. For context, we are manually deleting the directory because puppet runs scap before git-fat is installed, and it results in an incomplete deploy (more details here ) . The deletion results in a broken symlink, maybe scap could check for that?

Yeah, I think that makes sense for this task

In T342162#9025921, @RKemper wrote:

Right, I think we want to check that the file associated with the symlink actually exists, in addition to checking that the symlink itself is present.

Agreed.

I'll re-title this task to reflect this symlink-checking bug in scap deploy.

Other parts of what you're seeing could use a different ticket. There are a few issues there. Summary below.

Other issues

In T342162#9025804, @bking wrote:

Also, if we try and run scap before deleting these directories, scap does not detect the missing files (large files provided by git-fat) and reports a successful deployment.

I'm not trying to cross the streams with what might be 2 different bugs (or even expected behavior), just trying to explain why we're manually deleting that directory.

That makes sense. Scap tries to avoid doing work where possible, so exiting early is "expected" But(!) it sounds like some of the checks are bad/confusing—which is a bug. Possibly one of several bugs here 😅

Additional thoughts:

Initial run (as I understand it from T331300#8983005)

1. Puppet runs scap
2. scap's git checkout succeeds
3. git checkout triggers the smudge filter, which calls git-fat
4. git-fat fails (since the executable doesn't exist)

Is that right?

Subsequent scap deploy runs

1. scap deploy scap doesn't re-try the git checkout (unless you use --force) due to this check (which is different than the symlink check above. The logic of this check is: git rev-parse --verify HEAD == <rev you're trying to deploy> ? nothing to do && exit )
2. Smudge is never triggered
3. git-fat is never called to do it's thing
4. git-fat never pulls the actual files

Bugs in this process

I suppose there are a few problems:
0. scap is trying to be too smart/helpful and exiting in confusing ways

1. git-fat should exist before the scap run, so there's some missing dependency/race condition/erroneous graph in the initial puppet run
2. scap should probably check if git-fat was successful, otherwise the bad initial puppet run is unrecoverable without manual intervention
3. maybe we caused 1 because we're phasing out git-fat in favor of git-lfs.

Anyway, this one is more gnarly than adding a symlink check—let's make a separate task. :)