Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check operation #66

Merged
merged 6 commits into from
Feb 23, 2023
Merged

Add check operation #66

merged 6 commits into from
Feb 23, 2023

Conversation

adnankobir
Copy link
Contributor

@adnankobir adnankobir commented Feb 5, 2023

This PR adds the functionality to check a given lock in a pool and block/wait until that lock becomes unclaimed.
The rational behind it is as follows:

Given that we have multiple disparate pipelines that may trigger in any order:

  • we would like a build pipeline to be able to claim the lock for the duration of a number of tasks (e.g Docker build/push) and release the lock afterwards
  • the second (or multiple) deploy pipeline should wait until the build pipeline is completed before proceeding to pull in the updated output (ie. the Docker image built in build pipeline)

The intent is to have the deploy pipelines always use the latest available output (Docker image), and if there is an active build taking place to produce said output (Docker image), then we want to wait/block until that output is ready/published.

Now we could simply use acquire across all the pipelines, but this feel clunky since we'd end up dealing with deploy pipelines acquiring the locks unnecessarily, when all we want to do is check for existence of a claimed lock by the build pipeline only. This is especially true in cases of many:1 dependency in pipelines - we want to avoid hundreds of pipelines trying to acquire a lock unnecessarily.

check details in docs:

  • check: If set, we will check for an existing claimed lock in the pool and wait until it becomes unclaimed.

    • If there is an existing lock in claimed state: wait until lock is unclaimed
    • If there is an existing lock in unclaimed state: no-op
    • If no lock exists: fail

    The purpose is to simply block until a given lock in a pool is moved from a
    claimed state to an unclaimed state. This functionality allows us to build
    dependencies between disparate pipelines without the need to acquire locks.

    Note: the lock must be present to perform a check. In other words, a get
    step to fetch metadata about the lock is necessary before a put step can
    check the existence of the lock.

Thanks in advance for the review!

cc @xtremerui

Signed-off-by: Adnan Kobir <adnan.kobir@gmail.com>
Signed-off-by: Adnan Kobir <adnan.kobir@gmail.com>
Signed-off-by: Adnan Kobir <adnan.kobir@gmail.com>
Signed-off-by: Adnan Kobir <adnan.kobir@gmail.com>
@taylorsilva taylorsilva requested review from taylorsilva and removed request for clarafu February 23, 2023 21:41
@taylorsilva
Copy link
Member

Late but I'll take a look at this. Feels like a useful feature for this resource

Copy link
Member

@taylorsilva taylorsilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! Just some nits

out/lock_pool.go Outdated Show resolved Hide resolved
out/out_request.go Outdated Show resolved Hide resolved
Copy link
Member

@taylorsilva taylorsilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decided to just commit my nits and approve this. Thanks for the PR!

@taylorsilva
Copy link
Member

and of course the tests fail. I'm gonna approve this PR and make my own to fix the tests since that's a mess I made 😅

@taylorsilva taylorsilva merged commit bed111e into concourse:master Feb 23, 2023
taylorsilva added a commit that referenced this pull request Feb 23, 2023
Signed-off-by: Taylor Silva <dev@taydev.net>
taylorsilva added a commit that referenced this pull request Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants