chore(manifests): Move metadata to third-party and add overlay for separate db

[yanniszark]

This PR is based on #5344, which should be merged first.

Description of your changes:

As mentioned in kubeflow/manifests#1765 (comment), there is a discrepancy between manifests in the kubeflow/manifests repo and manifests in the kubeflow/pipelines repo, regarding metadata-db:

• In KFP repo, MLMD uses the MySQL of Pipelines.
• In kubeflow/manifests, MLMD uses its own DB.

This means that if a distribution upgrades from the MLMD in the manifests repo, they will lose their data.

This PR adds a db overlay to metadata, which ensures backwards-compatibility with previous versions of kubeflow/manifests.
In addition, I moved the metadata kustomization to third-party, same as all other kustomizations that are not developed inside kubeflow/pipelines.

@Bobgy let me know what you think.

[Bobgy]

Hi @yanniszark, thank you for the PR!
I really appreciate the efforts for flagging the problem of potential backward-incompatibility in Kubeflow and you taking efforts to move Kubeflow metadata DB overlays here. These LGTM.

I have some different opinions here regarding where to put these manifests:

• although MLMD is not developed in KFP repo, the official repo https://github.com/google/ml-metadata does not provide kubernetes manifests. Also, some of the manifests like metadata-envoy proxy are KFP-specific deployment. Therefore, this repo is source of truth for metadata manifests.
• metadata is a required component, especially going forward bit.ly/kfp-v2
• It's clearly stated in wgs.yaml that https://github.com/kubeflow/community/blob/011f1e18cda163d00b5030e3c740d2d2a7171141/wgs.yaml#L378-L380 metadata is a subproject of KFP. We'll move metadata in Kubeflow related development to KFP.

Therefore, I think metadata show stay in base/ folder, instead of third-party.

[yanniszark]

Therefore, I think metadata show stay in base/ folder, instead of third-party.

Thanks for the detailed explanation! That is totally fine with me. I have move the metadata folder under base again.
In general, I think that it will make more sense in the future to unfold installs into each separate component and then combine them in envs. But for now, I think we are good. Are you ok with the updated code?

PS. You can check the diff of this code vs 77945669 (previous commit) to check that the diff is only the renames + the metadata kustomization path references.

[Bobgy]

In general, I think that it will make more sense in the future to unfold installs into each separate component and then combine them in envs. But for now, I think we are good. Are you ok with the updated code?

that's an interesting idea, I feel like we are missing some foundational features in kustomize, have you tried https://github.com/kubernetes-sigs/kustomize/blob/master/examples/components.md which allows composing kustomization.yaml files? I feel that helps when decomposing the monolith into separate components, because it makes it easier to compose some config

[Bobgy]

@yanniszark can you resolve the merge conflict?

[Bobgy]

May I confirm your plan with this overlay?
I'd prefer only keeping it for backward compatibility reasons, because of the unnecessary burden of maintaining two DBs. Most distributions should configure their own overlay of the db using managed DB services for both KFP and Metadata.

Therefore, my suggestion would be: split into two platform-agnostic-multi-user envs

• One has name platform-agnostic-multi-user-legacy which uses this db overlay.
• While the other has name platform-agnostic-multi-user which keeps using the base metadata.

What do you think?

[yanniszark]

This is a great suggestion! I implemented it.

[yanniszark]

I feel like we are missing some foundational features in kustomize, have you tried https://github.com/kubernetes-sigs/kustomize/blob/master/examples/components.md which allows composing kustomization.yaml files?

Some of the people who proposed and implemented the components proposal are actually my co-workers from Arrikto 😄 (@apyrgio, @ioandr), so I agree that it's a great feature and that we should use it. Its main goal was to enable kustomize to share patches. The only problem is that it uses a later kustomize version and it's a bit late to change it for the release. However, let's definitely try components in the near future to enable easier configuration!

PS. I rebased my branch to resolve the conflict and implemented the suggested feature.

[Bobgy]

Some of the people who proposed and implemented the components proposal are actually my co-workers from Arrikto 😄 (@apyrgio, @ioandr), so I agree that it's a great feature and that we should use it. Its main goal was to enable kustomize to share patches. The only problem is that it uses a later kustomize version and it's a bit late to change it for the release. However, let's definitely try components in the near future to enable easier configuration!

Ahaa, great contributions! I love the spirit of this feature, it really solves a painpoint.
Regarding its status, I saw that it was still in v1alpha1. Were there any plans to make breaking changes? or were there plans to bump its stability level? (just being cautious as we've got many painful memories for using v1alpha1 features)

[Bobgy]

There's one remaining comment, but I think we can do that later.
Thank you @yanniszark!

/lgtm
/approve

[Bobgy]

shall we add a comment here or a readme in this folder to clarify its purpose?
anyway, I think this is not a blocker you can submit a separate PR too.

[Bobgy]

I was going to edit your branch directly to add this comment, but you didn't set the config to allow maintainers edit your branch. Would you be interested to do that next time? So I can help you edit some minor stuff

[yanniszark]

I was going to edit your branch directly to add this comment, but you didn't set the config to allow maintainers edit your branch. Would you be interested to do that next time? So I can help you edit some minor stuff

Thanks for the heads-up on this. I tried to find this button when creating a PR and noticed the following:

• If I make a PR from my own fork (yanniszark/pipelines), I can allow edits from maintainers.
• If I make a PR from my company's fork (arrikto/kubeflow-pipelines), then no such option is displayed.

Thus, I need to bring it up internally and see what we can do about it, seems it looks like a permissions issue. I agree that it would be helpful to enable.

[yanniszark]

shall we add a comment here or a readme in this folder to clarify its purpose?

I will create a separate PR for that as well.

[google-oss-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bobgy

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:

• manifests/kustomize/OWNERS [Bobgy]

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