concourse / pool-resource

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

Triggering suppression #33

Closed glyn closed 6 years ago

glyn commented 6 years ago

Sometimes it is not desirable to trigger a job when modifying a pool resource. This commit adds an optional parameter to suppress triggering. The default is that triggering may occur, which is compatible with the behaviour before this change was introduced.

Note: the generate statement for fake_lock_handler.go is updated to use the correct output directory.

glyn commented 6 years ago

I couldn't access the concourse build failure. Of course, I made sure the build passed locally before submitting the PR...

vito commented 6 years ago

Sorry, we should make those jobs public. I just checked and the failure isn't the fault of the tests - our CI is a bit borked at the moment and is unable to build images.

Thanks for submitting this, but I'm not totally clear on the use case. Can't triggering be suppressed on a consumer-by-consumer basis by just not setting trigger: true on the get steps?

I'm also not seeing how this prevents the pool resource from detecting the commit. What's more, by being produced by a put, the resource will collect the version regardless.

Could you provide some context? Are you putting locks in a repo that also contains other bits, and you don't want to cause lock changes to trigger other jobs?

Others have also identified ci skip as a point of contention; users have been bitten by it when team A considers a commit ci skip-worthy for their own pipeline, but then that prevents any other consumers from fetching it in their pipeline, which may actually want it.

glyn commented 6 years ago

@vito thanks for probing. Let me try to explain our requirements, our implementation, and the problem this PR is attempting to solve. Hopefully there will be a better solution.

Requirements

We want CF environment cleanup to be performed out of band relative to test jobs which use the environments. The aim is to locate cleanup logic in a common “laundromat” pipeline rather than having multiple pipelines clean up their own environments.

Each environment is in either a “clean” or a “dirty” set. A clean environment is claimed by a test job, used for testing, and then moved to the dirty set and unclaimed. A dirty environment is claimed by a laundromat job, cleaned up, and then moved to the clean set and unclaimed.

Cleaning up should be automatic and so the laundromat job needs to be triggered when an environment is moved to the dirty set.

Current Implementation

The “clean” and “dirty” sets of environments are represented as “clean” and “dirty” pool resources.

A test job acquires an environment from the clean pool, uses the environment in its tests, and moves the environment to the dirty pool and releases it.

A laundromat job acquires an environment from the dirty pool, cleans up the environment, and moves the environment to the clean pool and releases it.

Triggering is implemented by using a git resource which refers to the dirty pool and which triggers the laundromat job.

Problem

When the laundromat job acquires an environment from the dirty pool, this updates the dirty pool which triggers the laundromat job again.

The PR attempts to avoid this spurious triggering by including “[skip ci]” in the commit log associated with acquiring an environment from the dirty pool.

vito commented 6 years ago

Thanks. This actually sounds like exactly what https://github.com/cfmobile/pool-trigger-resource is for - have you tried that resource out?

glyn commented 6 years ago

Thanks @vito. We hadn't heard of pool-trigger-resource until now but we'll certainly take a look. Closing this PR, at least for the present.