Katib: Move manifests development upstream #1432

yanniszark · 2021-02-15T14:24:35Z

Issue Resolved

Resolves: #1431
Umbrella issue: kubeflow/manifests#1740

Description

As part of the work of wg-manifests for 1.3
(kubeflow/manifests#1735), we are moving manifests
development in upstream repos. This gives the application developers full
ownership of their manifests, tracked in a single place.

This PR copies the manifests for application Katib
from path apps/katib/upstream of kubeflow/manifests to path
manifests/v1beta1 of the upstream repo (https://github.com/kubeflow/katib).

cc @kubeflow/wg-automl-leads

k8s-ci-robot · 2021-02-15T14:24:48Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yanniszark
To complete the pull request process, please assign andreyvelich after the PR has been reviewed.
You can assign the PR to them by writing /assign @andreyvelich in a comment when ready.

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

Needs approval from an approver in each of these files:

OWNERS

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

yanniszark · 2021-02-15T16:21:19Z

/retest

andreyvelich

Thank you for doing this @yanniszark!
Do we have a unique structure for the kustomize manifests for each kubeflow component ?
Or every WG should decide how to store manifest by themselves?

andreyvelich · 2021-02-16T15:53:58Z

manifests/v1beta1/katib-controller/base/katib-controller-deployment.yaml

@@ -0,0 +1,53 @@
+apiVersion: apps/v1


Should we move all of the YAMLs from the manifests/v1beta1/katib-controller/base to the /manifests/v1beta1/components/<appropriate-component-name/ ?
Since we left the old structure for the manifests to not brake kfdef configs before we transfer to the v3 manifests ?

I agree! I was suspecting something like this happened.

andreyvelich · 2021-02-16T15:54:17Z

manifests/v1beta1/katib-controller/overlays/application/application.yaml

@@ -0,0 +1,65 @@
+apiVersion: app.k8s.io/v1beta1


Do we need to maintain these overlays ?

For the application overlays, we are discussing about moving away from them soon:
kubeflow/manifests#1715

What we could do is put them in an installs/kubeflow kustomization, since they are used when Katib is installed alongside other Kubeflow components.

@yanniszark So should we add the application overlay for Katib now or it's better to remove all labels and add them later with an Application, once we need it.
Since currently, many Katib labels are incorrect for the Application: kubeflow/manifests#1669.

@andreyvelich in general, we are moving away from using the Application CR: kubeflow/manifests#1715 (comment)

For this PR, I would suggest leaving them as is. The initial plan is to just get the manifests under the control of the upstream repo (katib) and then we can make changes incrementally.

yanniszark · 2021-02-21T20:49:54Z

@andreyvelich those are some good questions. I tried to answer in the comments above. As for the final question:

Do we have a unique structure for the kustomize manifests for each kubeflow component ?
Or every WG should decide how to store manifest by themselves?

We plan to propose a common way to organize manifests soon (step 4 in this plan kubeflow/manifests#1735). The first goal is to get the content of kubeflow/manifests back to the respective upstream repos.

In general, applications are expected to provide kustomizations that can be used to install the application.

yanniszark · 2021-03-01T20:58:32Z

@andreyvelich I have made the changed suggested here:

Should we move all of the YAMLs from the manifests/v1beta1/katib-controller/base to the /manifests/v1beta1/components/<appropriate-component-name/ ?

Could you take another look?

andreyvelich · 2021-03-01T21:49:49Z

@yanniszark Thank you for the update!

Do you need istio-authz-policy-odl.yaml and istio-authz-policy-old.yaml in this PR ?
Please rebase to fix CI.

andreyvelich · 2021-03-01T21:51:01Z

manifests/v1beta1/components/katib-controller/katib-ui-virtual-service.yaml

+        uri: /katib/
+      route:
+        - destination:
+            host: katib-ui.$(katib-ui-namespace).svc.$(clusterDomain)


Will we get these values from the appropriate stack.
For example: https://github.com/kubeflow/manifests/blob/master/distributions/stacks/gcp/kustomization.yaml ?

Yes exactly! Let's keep those as they are and we can introduce changes afterwards if we find it's not a good pattern.

yanniszark · 2021-03-04T20:57:07Z

Do you need istio-authz-policy-odl.yaml and istio-authz-policy-old.yaml in this PR ?
No, this was a mistaken git add.

Please rebase to fix CI.
I rebased but the Github Actions CI seems broken. Here is the Github worker message:

/home/runner/work/_temp/47c2af1d-bea5-4f4b-85f5-af8d0b594511.sh: line 1: juju: command not found

andreyvelich · 2021-03-04T20:59:02Z

Thank you @yanniszark.
Yes there are some problems with CI as I mentioned here: #1450 (comment).

yanniszark · 2021-03-06T11:55:26Z

@andreyvelich thanks, so what are the next steps for the CI?
Wait for #1450 to be merged?

andreyvelich · 2021-03-06T17:29:55Z

@yanniszark Yeah, please can you rebase and we can move forward?

As part of the work of wg-manifests for 1.3 (kubeflow/manifests#1735), we are moving manifests development in upstream repos. This gives the application developers full ownership of their manifests, tracked in a single place. This commit copies the manifests for application `Katib` from path `apps/katib/upstream` of kubeflow/manifests to path `manifests/v1beta1` of the upstream repo (https://github.com/kubeflow/katib). Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

yanniszark · 2021-03-06T19:04:35Z

@andreyvelich great, I just rebased!

andreyvelich · 2021-03-06T19:33:35Z

Thanks @yanniszark.
/lgtm
/approve
/retest

google-oss-robot · 2021-03-06T19:33:39Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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

k8s-ci-robot added the size/XXL label Feb 15, 2021

k8s-ci-robot requested review from andreyvelich and sperlingxx February 15, 2021 14:24

yanniszark force-pushed the feature-move-katib-manifests-upstream branch from 0658cc9 to 7c97e53 Compare February 15, 2021 14:44

yanniszark marked this pull request as draft February 15, 2021 14:45

k8s-ci-robot added the do-not-merge/work-in-progress label Feb 15, 2021

yanniszark force-pushed the feature-move-katib-manifests-upstream branch from 7c97e53 to 5ce880a Compare February 15, 2021 14:48

yanniszark marked this pull request as ready for review February 15, 2021 14:50

k8s-ci-robot removed the do-not-merge/work-in-progress label Feb 15, 2021

yanniszark mentioned this pull request Feb 15, 2021

Step 2: Move manifests development upstream kubeflow/manifests#1740

Closed

andreyvelich reviewed Feb 16, 2021

View reviewed changes

yanniszark force-pushed the feature-move-katib-manifests-upstream branch from 5ce880a to bfda6be Compare March 1, 2021 20:58

andreyvelich reviewed Mar 1, 2021

View reviewed changes

yanniszark force-pushed the feature-move-katib-manifests-upstream branch 3 times, most recently from 4b12a58 to 6a1f705 Compare March 4, 2021 15:46

yanniszark force-pushed the feature-move-katib-manifests-upstream branch from 6a1f705 to aea241f Compare March 6, 2021 19:01

yanniszark added 2 commits March 6, 2021 21:02

manifests: Fold base, overlays into components

f61c903

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

yanniszark force-pushed the feature-move-katib-manifests-upstream branch from aea241f to f61c903 Compare March 6, 2021 19:02

google-oss-robot assigned andreyvelich Mar 6, 2021

google-oss-robot added lgtm approved labels Mar 6, 2021

google-oss-robot merged commit ac4f525 into kubeflow:master Mar 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Katib: Move manifests development upstream #1432

Katib: Move manifests development upstream #1432

yanniszark commented Feb 15, 2021

k8s-ci-robot commented Feb 15, 2021

yanniszark commented Feb 15, 2021

andreyvelich left a comment

andreyvelich Feb 16, 2021

yanniszark Feb 21, 2021

andreyvelich Feb 16, 2021

yanniszark Feb 21, 2021

andreyvelich Feb 22, 2021

yanniszark Mar 1, 2021

yanniszark commented Feb 21, 2021 •

edited

Loading

yanniszark commented Mar 1, 2021

andreyvelich commented Mar 1, 2021

andreyvelich Mar 1, 2021

yanniszark Mar 4, 2021

yanniszark commented Mar 4, 2021

andreyvelich commented Mar 4, 2021

yanniszark commented Mar 6, 2021

andreyvelich commented Mar 6, 2021

yanniszark commented Mar 6, 2021

andreyvelich commented Mar 6, 2021

google-oss-robot commented Mar 6, 2021

Katib: Move manifests development upstream #1432

Katib: Move manifests development upstream #1432

Conversation

yanniszark commented Feb 15, 2021

Issue Resolved

Description

k8s-ci-robot commented Feb 15, 2021

yanniszark commented Feb 15, 2021

andreyvelich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yanniszark commented Feb 21, 2021 • edited Loading

yanniszark commented Mar 1, 2021

andreyvelich commented Mar 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yanniszark commented Mar 4, 2021

andreyvelich commented Mar 4, 2021

yanniszark commented Mar 6, 2021

andreyvelich commented Mar 6, 2021

yanniszark commented Mar 6, 2021

andreyvelich commented Mar 6, 2021

google-oss-robot commented Mar 6, 2021

yanniszark commented Feb 21, 2021 •

edited

Loading