Pipelines kustomize v3 manifest

[Bobgy]

Which issue is resolved by this Pull Request:
Resolves #1048
Resolves #897
Resolves kubeflow/pipelines#3993

Description of your changes:
Add kustomize v3 manifest for pipelines.
In addition to existing pipelines components, I also integrated the newly introduced components:

• cache server
• metadata writer

/cc @Ark-kun

I verified these with gcp blueprint deployment, caching and metadata writer both works properly.

Checklist:

• Unit tests have been rebuilt:
  1. cd manifests/tests
  2. make generate-changed-only
  3. make test

[kubeflow-bot]

This change is 

[Bobgy]

/cc @rmgogogo
/assign @jlewi

[jlewi]

Reviewed 19 of 196 files at r1.
Reviewable status: 9 of 152 files reviewed, 8 unresolved discussions (waiting on @Ark-kun, @Bobgy, and @rmgogogo)

---

pipeline/installs/generic/kustomization.yaml, line 35 at r2 (raw file):

- name: mysql-secret
  env: params-db-secret.env
vars:

What are these vars for? Can we get rid of them?
https://github.com/kubeflow/manifests/blob/master/docs/KustomizeBestPractices.md#eschew-vars

---

pipeline/installs/generic/params.yaml, line 2 at r2 (raw file):

varReference:
- path: spec/http/route/destination/host

What are you using vars for?

---

pipeline/installs/generic/virtual-service.yaml, line 18 at r2 (raw file):

    route:
    - destination:
        host: $(KFP_UI_SERVICE).$(KFP_UI_NAMESPACE).svc.$(clusterDomain)

How about adding kpt setters for the clusterDomain so that we don't need to rely on the vars for GCP?
Likewise do we need a vars for $(KFP_UI_SERVICE)? Doesn't the service have a fixed name?

---

pipeline/minio/installs/gcp-pd/kustomization.yaml, line 16 at r1 (raw file):

- name: pipeline-minio-install-config
  env: params.env
vars:

Can we get rid of these vars?
https://github.com/kubeflow/manifests/blob/master/docs/KustomizeBestPractices.md#eschew-vars

---

pipeline/minio/installs/gcp-pd/params.yaml, line 2 at r1 (raw file):

varReference:
- path: spec/gcePersistentDisk/pdName

Do we need all these custom vars?
https://github.com/kubeflow/manifests/blob/master/docs/KustomizeBestPractices.md#eschew-vars

---

pipeline/minio/installs/gcp-pd/persistent-volume.yaml, line 4 at r1 (raw file):

kind: PersistentVolume
metadata:
  name: $(kfpMinioPvName)

Why is this a var?
https://github.com/kubeflow/manifests/blob/master/docs/KustomizeBestPractices.md#eschew-vars

---

pipeline/minio/installs/gcp-pd/persistent-volume.yaml, line 11 at r1 (raw file):

  - ReadWriteOnce
  gcePersistentDisk:
    pdName: $(kfpMinioPd)

Why is this a var? On GCP could we use a kpt setter?

---

pipeline/minio/installs/gcp-pd/persistent-volume-claim.yaml, line 6 at r1 (raw file):

  name: minio-pvc
spec:
  volumeName: $(kfpMinioPvName)

Why vars?

[Bobgy]

Reviewable status: 9 of 186 files reviewed, 8 unresolved discussions (waiting on @Ark-kun, @Bobgy, @jlewi, and @rmgogogo)

---

pipeline/installs/generic/kustomization.yaml, line 35 at r2 (raw file):

Previously, jlewi (Jeremy Lewi) wrote…

What are these vars for? Can we get rid of them?
https://github.com/kubeflow/manifests/blob/master/docs/KustomizeBestPractices.md#eschew-vars

Yes, got rid of in kfp rc 2

---

pipeline/installs/generic/params.yaml, line 2 at r2 (raw file):

Previously, jlewi (Jeremy Lewi) wrote…

What are you using vars for?

this is exactly https://github.com/kubeflow/manifests/blob/master/docs/KustomizeBestPractices.md#global-substitution
substituting virtual service

---

pipeline/installs/generic/virtual-service.yaml, line 18 at r2 (raw file):

Previously, jlewi (Jeremy Lewi) wrote…

How about adding kpt setters for the clusterDomain so that we don't need to rely on the vars for GCP?
Likewise do we need a vars for $(KFP_UI_SERVICE)? Doesn't the service have a fixed name?

I will fix first two vars, for cluster domain, we can change it to kpt setter. Would you want me do it in this PR or separate?

[Bobgy]

@jlewi I removed most of the vars after rebasing to kfp rc2 manifest with changes in kubeflow/pipelines#3978.

The remaining usages of var are:

• virtual service: clusterDomain and namespace, https://github.com/kubeflow/manifests/pull/1248/files#diff-adc2be74d4efe3becc4b6ea7bec4d15bR37
• minio and mysql: pv name and pd name
  

  manifests/pipeline/mysql/installs/gcp-pd/kustomization.yaml

  Lines 20 to 36 in f68a7fb

  	vars:
  	- name: kfpMysqlPd
  	objref:
  	kind: ConfigMap
  	name: pipeline-mysql-install-config
  	apiVersion: v1
  	fieldref:
  	fieldpath: data.mysqlPd
  	- name: kfpMysqlPvName
  	objref:
  	kind: ConfigMap
  	name: pipeline-mysql-install-config
  	apiVersion: v1
  	fieldref:
  	fieldpath: data.mysqlPvName
  	configurations:
  	- params.yaml

  and

  manifests/pipeline/minio/installs/gcp-pd/kustomization.yaml

  Lines 16 to 32 in f68a7fb

  	vars:
  	- name: kfpMinioPd
  	objref:
  	kind: ConfigMap
  	name: pipeline-minio-install-config
  	apiVersion: v1
  	fieldref:
  	fieldpath: data.minioPd
  	- name: kfpMinioPvName
  	objref:
  	kind: ConfigMap
  	name: pipeline-minio-install-config
  	apiVersion: v1
  	fieldref:
  	fieldpath: data.minioPvName
  	configurations:
  	- params.yaml

  ( they are using the same pattern )

I'm not sure why we are parameterizing PV name, it was consistent with KF 1.0 manifest. I'll take a look at commit history to find why. I'm guessing that's because persistence volume is immutable after creation, so for proper devops action of changing a PV, it's best to customize the name to use a new PV.

Because this PR is huge. I suggest we separate a PR for how to get rid of the last 4 vars (if we do want to).

[Bobgy]

I did some investigation, the PV, PVC name parameterization by var was added in #104. However, it didn't explain any reasoning about why they were added.

@gabrielwen Do you have context why we needed to parameterize PV and PVC name for pipeline's mysql and minio db?

[jlewi]

Thanks @Bobgy I think most of the vars you mentioned are for internal substitution; e.g.
namespace. Per the guide this is ok as long as we give the vars unique enough names so we don't conflict with similar vars from other packages.

A better solution for clusterDomain is being tracked in #1007 (comment)

You can keep that one as is for now. On GCP we can probably just switch to kpt setters since we are already using kpt setter.

@gabrielwen Do you have context why we needed to parameterize PV and PVC name for pipeline's mysql and minio db?

I think it might have been related to how we were creating the PD. I think we may have been using deployment manager (and now KCC) to create the PD and thus needed to substitute in the correct name to the package.

I think the reasoning was we wanted to use KCC/DM and not the dynamic provisioner to create the PD because we wanted to make sure the PD wasn't destroyed if KF was deleted because we didn't want to destroy any data.

I'm good with submitting PR as is and filing issues to track fixing things later.

[jlewi]

/lgtm
/approve

[Bobgy]

@jlewi the conflicts were caused by env to envs change: https://github.com/kubeflow/manifests/commits/master/pipeline/upstream
#1204

but kfp standalone manifests don't support envs yet, because we wanted to maintain kubectl kustomize compatibility, which uses a kustomize v2.x version: https://github.com/kubernetes-sigs/kustomize#kubectl-integration.

I'll ask the PR owner for a resolution.

[Bobgy]

I updated kfp pull upstream script to use sed for a quick HACK that transforms env: to envs: during pull: a842b7b

What do you think?

[jlewi]

SGTM

Although you could also write a kpt FN transformer.

kustomize 2.x is very old. A lot of things won't work if you rely on it.

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

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 [jlewi]

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

[Bobgy]

Although you could also write a kpt FN transformer.

Thanks for sharing, will try that next time

kustomize 2.x is very old. A lot of things won't work if you rely on it.

convenience from kubectl is the major reason we keep using it. We will consider upgrading when we get the first blocking issue