concourse / pool-resource

atomically manages the state of the world (e.g. external environments)
Apache License 2.0
56 stars 36 forks source link

File modifications in-place don't break locks #11

Closed rainmaker closed 8 years ago

rainmaker commented 8 years ago

Hi concourse,

To allow batch changes of environment files without affecting in-flight pipelines, we made a small change to assets/in to tolerate modified pool resources which haven't otherwise been moved. We tested this on releng's concourse deployment, and saw that a pipeline continued to hold the lock after we had modified an environment file. We also verified that moving an environment file would still result in a broken lock. We did notice that, in the latter case, that it took 2 pipeline jobs for the lock to break because of resource caching.

It should also be noted that, since the pool resource uses the commit sha from the original lock acquisition, the changes to the environment will not be reflected until the environment has been released and re-acquired.

Let us know if you have any questions about our testing or changes.

Raina Masand and Dave Walter CF Release Engineering

concourse-bot commented 8 years ago

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:

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

vito commented 8 years ago

Cool that it's just a one-line change. Bummer about the resource caching; that makes sense as to why it would let that sneak through. Hrm. Something for us to chew on.

Could you take a look at adding an integration test here? It would probably look similar to this Context, and just modify the file in-place rather than moving it, and see that everything works (instead of failing, as it did before).

rainmaker commented 8 years ago

@vito Added an integration test for modified file, also went back to make sure it failed without our fix. Let me know if there's anything else we need to look at.

vito commented 8 years ago

Looks good, thanks!