Skip to content

Conversation

briangallagher
Copy link
Contributor

@briangallagher briangallagher commented Sep 29, 2025

What this PR does / why we need it:
This PR adds a docker backend, extending the current Kubernetes and Local Process backends.
Its designed for Local Execution

Which issue(s) this PR fixes (optional, in Fixes #<issue number>, #<issue number>, ... format, will close the issue(s) when PR gets merged):

Fixes #108

Verification
This WIP PR to add an example notebook for local docker execution can be used to verify this PR
Update the pip install to:
!pip install "git+https://github.com/briangallagher/sdk.git@docker-backend#egg=kubeflow[docker]"

Copy link

[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 astefanutti 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:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coveralls
Copy link

coveralls commented Sep 29, 2025

Pull Request Test Coverage Report for Build 18128142359

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 71.739%

Totals Coverage Status
Change from base Build 17979146135: 0.0%
Covered Lines: 297
Relevant Lines: 414

💛 - Coveralls

@briangallagher briangallagher changed the title Add docker backend feat: Add docker backend Sep 29, 2025
@briangallagher briangallagher force-pushed the docker-backend branch 2 times, most recently from 7546cfc to 92fa6c8 Compare September 29, 2025 20:20
Signed-off-by: Brian Gallagher <[email protected]>
logger = logging.getLogger(__name__)


DOCKER_LABEL_PREFIX = "trainer.kubeflow.ai"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
DOCKER_LABEL_PREFIX = "trainer.kubeflow.ai"
DOCKER_LABEL_PREFIX = "trainer.kubeflow.org"

import uuid

try:
import docker # type: ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

How difficult would it be to support Podman as well: https://pypi.org/project/podman/

Should it be a different backend or could this be a "container" backend that could work for both Docker and Podman depending on what's installed / configured?

Copy link
Contributor Author

@briangallagher briangallagher Oct 2, 2025

Choose a reason for hiding this comment

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

I think it's a little bit cleaner to have a separate backend as it uses the podman client and the code is a bit different. The networking is slightly different.
But, a nice addition might be some auto discovery after all backends are added.
Meaning, look for Podman, then docker finally fall back to subprocess.

Copy link
Member

Choose a reason for hiding this comment

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

I would agree with @astefanutti to unify all container-based functionality under single backend.
From the end-user point of view it doesn't matter whether they use Docker, Podman, or Container backend to run local containers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Docker Backend to support Local Execution
4 participants