Updated examples/pytorch to disable istio sidecar injection

[jdcfd]

What this PR does / why we need it:
Examples do not work because of istio sidecar injection causing trouble
Which issue(s) this PR fixes (optional, in Fixes #<issue number>, #<issue number>, ... format, will close the issue(s) when PR gets merged):
Fixes #1637

Checklist:

• Docs included if any changes are user facing (PR Updated CRs documentation to include comment about istio annotation website#3686)

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[tenzen-y]

Thank you!
/lgtm
/approve

/assign @kubeflow/wg-training-leads
for approval CI.

[tenzen-y]

/hold
for CI

[andreyvelich]

Thank you for creating this PR @jdcfd!
@tenzen-y @johnugeorge I remember previously we had discussion that we should not add Istio annotation to the Examples to not confuse users: kubeflow/katib#1359 (comment)
At least we did it for Katib.
Should we follow the same model for Training Operator examples, and just update the documentation ?

[tenzen-y]

Thank you for creating this PR @jdcfd! @tenzen-y @johnugeorge I remember previously we had discussion that we should not add Istio annotation to the Examples to not confuse users: kubeflow/katib#1359 (comment) At least we did it for Katib. Should we follow the same model for Training Operator examples, and just update the documentation ?

@andreyvelich I'm ok with either way. I think that the inconsistent examples and docs brought confusions :(
So, if we remove the istio annotation, we should remove the annotation from all examples and add documentation as you say.

[coveralls]

Pull Request Test Coverage Report for Build 8015652309

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 8 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.09%) to 42.805%

---

Files with Coverage Reduction	New Missed Lines	%
pkg/controller.v1/mpi/mpijob_controller.go	8	80.48%

Totals
Change from base Build 7758874698:	-0.09%
Covered Lines:	3748
Relevant Lines:	8756

---

💛 - Coveralls

[jdcfd]

Hi all, thanks for the feedback. I have to admit this is all new to me, even the process of submitting a PR to an open-source project so bear with me.

From what I understood, it is not always necessary to add the Istio annotation, therefore, the user should do it manually if necessary. That was the path you took for katib. In that case, one option is to remove annotation from all examples and add a comment in examples/pytorch/README.md .

Is this the same with all other training operators? or only pytorch?

[tenzen-y]

Hi all, thanks for the feedback. I have to admit this is all new to me, even the process of submitting a PR to an open-source project so bear with me.

From what I understood, it is not always necessary to add the Istio annotation, therefore, the user should do it manually if necessary. That was the path you took for katib. In that case, one option is to remove annotation from all examples and add a comment in examples/pytorch/README.md .

Is this the same with all other training operators? or only pytorch?

Oh, I see. Welcome to the kubeflow community :)
Yes, we should remove the istio annotations from all training operators.

[andreyvelich]

Welcome to the community @jdcfd!
Yes, @tenzen-y is right, we can remove istio annotations from all examples.
That would be great if you could submit PR in the Kubeflow Website repo to explain how users can disable Istio for every job.
Maybe we can follow the TFJob guide and update other CRs accordingly: https://www.kubeflow.org/docs/components/training/tftraining/#what-is-tfjob

[tenzen-y]

/lgtm cancel
/approve cancel

[tenzen-y]

/approve cancel

[jdcfd]

@tenzen-y @andreyvelich I have removed the istio annotations from the examples, added some comments to the README file for PyTorch jobs and submitted a PR to the kubeflow/website repo: kubeflow/website#3686

[andreyvelich]

You can remove this imagePullPolicy, since IfNotPresent is set by default in Kubernetes container.

[jdcfd]

@tenzen-y I removed the istio annotations as discussed. I also submitted a PR to the documentation website repo.

[andreyvelich]

Thank you for your contribution @jdcfd 🎉
/lgtm
/approve

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich, jdcfd

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:

• OWNERS [andreyvelich]

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

[andreyvelich]

/hold cancel