Skip to content

cleanup trainer overlay#3314

Closed
juliusvonkohout wants to merge 2 commits into
masterfrom
cleanup-trainer-overlay
Closed

cleanup trainer overlay#3314
juliusvonkohout wants to merge 2 commits into
masterfrom
cleanup-trainer-overlay

Conversation

@juliusvonkohout
Copy link
Copy Markdown
Member

@juliusvonkohout juliusvonkohout commented Dec 30, 2025

/hold
to be rebased and merged once kubeflow/trainer#3259 is merged and synchronized
CC @andreyvelich

@google-oss-prow
Copy link
Copy Markdown

[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 ask for approval from juliusvonkohout. For more information see the Kubernetes Code Review Process.

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

Details 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

Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR cleans up the trainer overlay by removing local patches that are expected to be moved upstream. The PR is marked with /hold and depends on kubeflow/trainer#3021 being merged first.

Changes:

  • Removes all patches from applications/trainer/overlays/kustomization.yaml, leaving only the upstream resource reference
  • Updates the trainer test workflow to trigger on changes to any file under applications/trainer/** instead of just applications/trainer/upstream/**

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
applications/trainer/overlays/kustomization.yaml Removes patches for seccompProfile and Istio annotations that were previously applied to kubeflow-trainer-controller-manager and jobset-controller-manager deployments
.github/workflows/trainer_test.yaml Broadens the path trigger to include both upstream and overlay changes in the trainer directory

Comment thread applications/trainer/overlays/kustomization.yaml
Comment thread applications/trainer/overlays/kustomization.yaml
Comment thread applications/trainer/overlays/kustomization.yaml
@juliusvonkohout
Copy link
Copy Markdown
Member Author

juliusvonkohout commented Feb 27, 2026

Instead of just opening the port we can also disable it altoghether with https://istio.io/latest/docs/setup/additional-setup/sidecar-injection/#controlling-the-injection-policy so sidecar.istio.io/inject: "false" instead of traffic.sidecar.istio.io/excludeInboundPorts: "9443". That could prevent all istio related problems in the future :-)

@juliusvonkohout
Copy link
Copy Markdown
Member Author

juliusvonkohout commented Feb 28, 2026

Merged in kubeflow/trainer#3259 once there is a release we can synchronize, rebase and merge this PR.

@juliusvonkohout
Copy link
Copy Markdown
Member Author

closed in favor of #3413

@juliusvonkohout juliusvonkohout deleted the cleanup-trainer-overlay branch March 20, 2026 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants