chore: upstream istio support#3189
Conversation
Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Add security context and annotations for Istio traffic management. Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Added a patch to modify the jobset-controller-manager deployment annotations and security context. Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Removed security context seccomp profile from deployment. Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Removed security context seccompProfile configuration. Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Removed security context seccompProfile from spec. Signed-off-by: Julius von Kohout <45896133+juliusvonkohout@users.noreply.github.com>
Signed-off-by: Surya Sameer Datta Vaddadi <f20220373@goa.bits-pilani.ac.in>
|
🎉 Welcome to the Kubeflow Trainer! 🎉 Thanks for opening your first PR! We're happy to have you as part of our community 🚀 Here's what happens next:
Join the community:
Feel free to ask questions in the comments if you need any help or clarification! |
Pull Request Test Coverage Report for Build 22163625294Details
💛 - Coveralls |
Signed-off-by: Surya Sameer Datta Vaddadi <f20220373@goa.bits-pilani.ac.in>
|
Please add the actual istio labels and what was in the previous PR in the new schema. |
Signed-off-by: Surya Sameer Datta Vaddadi <f20220373@goa.bits-pilani.ac.in>
Signed-off-by: Surya Sameer Datta Vaddadi <f20220373@goa.bits-pilani.ac.in>
|
@juliusvonkohout , |
charts/kubeflow-trainer/values.yaml
Outdated
There was a problem hiding this comment.
Please can you remove this label by default and update the README on how to set it:
| podAnnotations: | |
| traffic.sidecar.istio.io/excludeInboundPorts: "9443" | |
| podAnnotations: {} |
|
|
||
| patches: | ||
| - path: patches/remove-namespace.yaml | ||
| - target: |
There was a problem hiding this comment.
Please can you also move the JobSet patch for Istio to this file?
https://github.com/sameerdattav/trainer/blob/c0369539a34d98e61f2c584a54f39f33cba7c07a/manifests/third-party/jobset/kustomization.yaml#L21-L30
Signed-off-by: Surya Sameer Datta Vaddadi <f20220373@goa.bits-pilani.ac.in>
Signed-off-by: Surya Sameer Datta Vaddadi <f20220373@goa.bits-pilani.ac.in>
Signed-off-by: Surya Sameer Datta Vaddadi <f20220373@goa.bits-pilani.ac.in>
andreyvelich
left a comment
There was a problem hiding this comment.
Thank you for this work @sameerdattav!
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andreyvelich The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold |
|
@sameerdattav You should change branch to |
|
/reopen |
|
@andreyvelich: Reopened this PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@andreyvelich , I think @juliusvonkohout wanted me to raise a pr against his branch, i.e pss-restricted-fixes? |
|
Yes, let's commit this feature directly to master branch please. |
|
/retest |
|
@sameerdattav: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@andreyvelich could you please leave an ok-to-test or a retest comment? |
|
/ok-to-test @sameerdattav Our E2Es are failing due to this: kubeflow/sdk#307 |
| annotations: | ||
| traffic.sidecar.istio.io/excludeInboundPorts: "9443" |
There was a problem hiding this comment.
You don't need this since you already have a patch, right ?
| annotations: | |
| traffic.sidecar.istio.io/excludeInboundPorts: "9443" |
| matchLabels: | ||
| app: kubeflow-trainer | ||
| template: | ||
| metadata: | ||
| labels: | ||
| app: kubeflow-trainer |
There was a problem hiding this comment.
Why do you need these labels? Can we preserve the original one:
app.kubernetes.io/name: trainer
app.kubernetes.io/component: manager
app.kubernetes.io/part-of: kubeflow
| matchLabels: | |
| app: kubeflow-trainer | |
| template: | |
| metadata: | |
| labels: | |
| app: kubeflow-trainer | |
| template: | |
| metadata: |
This PR builds on #3021 and implements the suggested follow-up:
@juliusvonkohout