kubeflow / pipelines

Machine Learning Pipelines for Kubeflow
https://www.kubeflow.org/docs/components/pipelines/
Apache License 2.0
3.62k stars 1.63k forks source link

feat(backend): Add Semaphore and Mutex fields to Workflow CR #11370

Open DharmitD opened 1 week ago

DharmitD commented 1 week ago

Resolves https://github.com/kubeflow/pipelines/issues/6553

Description of your changes: This PR introduces support for Pipeline-level Semaphores and Mutexes in the KFP backend.

Changes Introduced:

This PR should be merged only after https://github.com/kubeflow/pipelines/pull/11340 gets merged. Testing instructions

google-oss-prow[bot] commented 1 week ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign chensun for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[backend/OWNERS](https://github.com/kubeflow/pipelines/blob/master/backend/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
gregsheremeta commented 1 week ago

add fixes #6553 to the PR description

gregsheremeta commented 1 week ago

The semaphore is configured via a fixed ConfigMap named semaphore-config

We should edit the kubeflow manifest to deploy a skeleton of this configmap. You can do that in here or in a follow-up PR.

gregsheremeta commented 1 week ago

The Workflow CR now includes a Synchronization field

I would probably delete this line (and maybe edit the PR title), because that reads like things you enhanced on Workflow itself. We're just setting fields on it...

gregsheremeta commented 1 week ago
platforms:
  kubernetes:
    pipelineConfig:
      mutexName: mutex
      semaphoreKey: semaphore

The expected output should include the semaphore and mutex references:

What does Argo Workflows do when both are set?

A better verification would be to do two separate test pipelines -- one where you use mutex, and one where you use semaphore. And then in addition to verifying the Workflow yaml, also verify that multiple runs are being locked like they should be.

DharmitD commented 4 days ago

/hold until https://github.com/kubeflow/pipelines/pull/11384 and https://github.com/kubeflow/pipelines/pull/11340 get merged