[Feature] Support managed by external controller

[mszadkow]

What this PR does / why we need it:
Add support for ManagedBy to be able delegate reconciliation process to external controller.

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

Checklist:

• Docs included if any changes are user facing

[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.

[coveralls]

Pull Request Test Coverage Report for Build 10849366606

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 100.0%

---

Totals
Change from base Build 10793288019:	0.0%
Covered Lines:	66
Relevant Lines:	66

---

💛 - Coveralls

[andreyvelich]

Thank you for this effort @mszadkow!
We will review it soon!
/ok-to-test
/assign @kubeflow/wg-training-leads

[andreyvelich]

/rerun-all

[mszadkow]

/rerun-all

[andreyvelich]

@mszadkow @mimowo I think, I have only one question here: #2203 (comment).

@kubeflow/wg-training-leads Please take a look at this change.

It would be nice if you could also open PR to update the Kubeflow docs with that API change: https://www.kubeflow.org/docs/components/training/user-guides/.

Maybe we can introduce another user-guide on how to modify managedBy API and why it is needed.

[tenzen-y]

Additionally, could you implement E2E testing here?

https://github.com/kubeflow/training-operator/tree/e9766d171507ad3772cc04c740acb4d1ee4c544c/sdk/python/test/e2e

[mszadkow]

Hey @tenzen-y @andreyvelich, I have addressed most of the comments.
What left is:

1. e2e tests - I would be very grateful for any help or advice on how to run these tests locally.
   From what I see, they are being run in docker sdk/python/Dockerfile.conformance - but seems that they need running cluster or something because that's what I see in result:

test/e2e/test_e2e_xgboostjob.py:41: in <module>
    TRAINING_CLIENT = TrainingClient(job_kind=constants.XGBOOSTJOB_KIND)
kubeflow/training/api/training_client.py:79: in __init__
    config.load_kube_config(config_file=config_file, context=context)
/usr/local/lib/python3.10/site-packages/kubernetes/config/kube_config.py:819: in load_kube_config
    loader = _get_kube_config_loader(
/usr/local/lib/python3.10/site-packages/kubernetes/config/kube_config.py:776: in _get_kube_config_loader
    raise ConfigException(
E   kubernetes.config.config_exception.ConfigException: Invalid kube-config file. No configuration found.
=========================== short test summary info ============================
ERROR test/e2e/test_e2e_mpijob.py - kubernetes.config.config_exception.Config...
ERROR test/e2e/test_e2e_paddlejob.py - kubernetes.config.config_exception.Con...
ERROR test/e2e/test_e2e_pytorchjob.py - kubernetes.config.config_exception.Co...
ERROR test/e2e/test_e2e_tfjob.py - kubernetes.config.config_exception.ConfigE...
ERROR test/e2e/test_e2e_xgboostjob.py - kubernetes.config.config_exception.Co...
!!!!!!!!!!!!!!!!!!! Interrupted: 5 errors during collection !!!!!!!!!!!!!!!!!!!!
============================== 5 errors in 0.33s ===============================

2. Where should I locate the skip in the end: reconcile, predicate or both?
   There are some situations that @mimowo mentioned that it would be beneficial to have it in both places.
   [Feature] Support managed by external controller #2203 (comment)

Would you agree to make those changes in the follow up possibly?

[tenzen-y]

1. e2e tests - I would be very grateful for any help or advice on how to run these tests locally.
   From what I see, they are being run in docker sdk/python/Dockerfile.conformance - but seems that they need running cluster or something because that's what I see in result:

Actually, the conformance container image does not related to our E2E testing.
So, you can reproduce the CI step locally in the following:

1. Create K8s Cluster:

   training-operator/.github/workflows/integration-tests.yaml

   Lines 74 to 79 in 00eef58

   	- name: Create k8s Kind Cluster
   	uses: helm/kind-action@v1.10.0
   	with:
   	node_image: kindest/node:${{ matrix.kubernetes-version }}
   	cluster_name: training-operator-cluster
   	kubectl_version: ${{ matrix.kubernetes-version }}

2. Build and load new container image to K8s cluster:

   training-operator/.github/workflows/integration-tests.yaml

   Lines 81 to 85 in 00eef58

   	- name: Build training-operator
   	run: |
   	./scripts/gha/build-image.sh
   	env:
   	TRAINING_CI_IMAGE: kubeflowtraining/training-operator:test

3. Deploy the training-operator with new image:

   training-operator/.github/workflows/integration-tests.yaml

   Lines 87 to 94 in 00eef58

   	- name: Deploy training operator
   	run: |
   	./scripts/gha/setup-training-operator.sh
   	env:
   	KIND_CLUSTER: training-operator-cluster
   	TRAINING_CI_IMAGE: kubeflowtraining/training-operator:test
   	GANG_SCHEDULER_NAME: ${{ matrix.gang-scheduler-name }}
   	KUBERNETES_VERSION: ${{ matrix.kubernetes-version }}

4. install Python library, and perform tests:

   training-operator/.github/workflows/integration-tests.yaml

   Lines 96 to 99 in 00eef58

   	- name: Run tests
   	run: |
   	pip install pytest
   	python3 -m pip install -e sdk/python; pytest -s sdk/python/test --log-cli-level=debug --namespace=default

[mimowo]

/lgtm
Reviewed implementation and tests.
PTAL @tenzen-y and @andreyvelich

[google-oss-prow]

@mimowo: changing LGTM is restricted to collaborators

In response to this:

/lgtm
Reviewed implementation and tests.
PTAL @tenzen-y and @andreyvelich

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.

[tenzen-y]

@mszadkow Thank you for driving this feature!
@mimowo Additionally, I would like to express my appreciation to help review this PR!

/lgtm
/approve

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tenzen-y

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 [tenzen-y]

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