Adding a lock should reliably fail when the pool contains a lock by that name

[jaresty]

You can get a pool into a bad state where it has a lock by the same name in both the claimed and the unclaimed state. You do this by adding a lock with a name that matches the name of an existing lock, but adding it the opposite state of claimedness from the existing lock.

For example, you if you have a pool with a claimed lock called knox, and you add an unclaimed lock called knox, it will happily create and ruin your pool.

If you try to add a lock to a pool and state that already contains a lock by that name, it will fail to add (and retry). It gives the following (obscure) output:

adding unclaimed lock: luna to pool: cf-deployment/fresh
failed to add the lock: luna! (err: exit status 1) retrying...
failed to add the lock: luna! (err: exit status 1) retrying...
failed to add the lock: luna! (err: exit status 1) retrying...
failed to add the lock: luna! (err: exit status 1) retrying...

We'd prefer it if, instead, it said something like this in both cases:

adding unclaimed lock: luna to pool: cf-deployment/fresh
failed to add lock: luna; A lock by this name already exists in this pool. retrying...

Would you be open to receiving a pull request that implemented this?

Signed-off by: @anEXPer

[concourse-bot]

Hi there!

We use Pivotal Tracker to provide visibility into what our team is working on. A story for this issue has been automatically created.

The current status is as follows:

• [ ] #142188851 Adding a lock should reliably fail when the pool contains a lock by that name

This comment, as well as the labels on the issue, will be automatically updated as the status in Tracker changes.

[jtarchie]

@jaresty, how are you creating a lock with the same name in the claim and unclaim pools? Please clarify what your jobs were doing and how they interacted with the locks.

[jaresty]

Hi @jtarchie,

• We already had an unclaimed lock named luna cf-deployment/fresh/unclaimed/luna
• We accidentally ran a task that uses the pool resource to "add_claimed: luna", which would go in cf-deployment/fresh/claimed/luna. We would expect this to fail, but it did not. After the task succeeded, we could no longer use the pool without manually deleting one of the luna locks.