Disable istio-sidecar injection #1778
Conversation
Pull Request Test Coverage Report for Build 4434355170
💛 - Coveralls |
| scheduler_name=get_pod_spec_scheduler_name(GANG_SCHEDULER_NAME), | ||
| )), | ||
| template=V1PodTemplateSpec( | ||
| metadata=V1ObjectMeta(annotations={'sidecar.istio.io/inject': "false"}), |
There was a problem hiding this comment.
Should we move sidecar.istio.io/inject to constants.py?
There was a problem hiding this comment.
we can move but with this approach, we might end up having a lot of constants. Also, since sidecar.istio.io/inject is a standard annotation, this looks more readable to me.
And it's consistent with what we already have in training utils.
https://github.com/kubeflow/training-operator/blob/master/sdk/python/kubeflow/training/utils/utils.py#L325
Any specific reason to move it?
There was a problem hiding this comment.
Also, since sidecar.istio.io/inject is a standard annotation, this looks more readable to me.
We should avoid redeclaring the same words in many places since that causes bugs.
There was a problem hiding this comment.
And it's consistent with what we already have in training utils.
https://github.com/kubeflow/training-operator/blob/master/sdk/python/kubeflow/training/utils/utils.py#L325
It would be better to move sidecar.istio.io/inject to sdk/python/kubeflow/training/constants/constants.py.
There was a problem hiding this comment.
okay, let me move it.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johnugeorge, nagar-ajay The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
Currently none of the e2e tests run successfully in
istio-enablednamespaces. All training jobs (created by tests) get stuck inRunningstate and eventually these tests fail after timeout. I think it's because istio sidecar keeps running even after the main containers runs successfully.This is required as we want to run these tests as part of the training operator conformance test. As per the design doc of training operator conformance test, all these tests will run in the namespace created by the Kubeflow profile resource which is istio-enabled by default.
Disable Istio sidecar injection for training job containers.
Testing: Tested updated tests in
defaultnamespace and a istio enabled namespace.Which issue(s) this PR fixes (optional, in
Fixes #<issue number>, #<issue number>, ...format, will close the issue(s) when PR gets merged):Fixes #
Checklist: