Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

relationship between continous-deployment and it's implementation specific incarnations #19

Closed
durandom opened this issue Sep 4, 2020 · 26 comments
Assignees
Labels
priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now.

Comments

@durandom
Copy link
Member

durandom commented Sep 4, 2020

We want to maintain one upstream version of https://github.com/operate-first/continuous-deployment and make it easy for downstream users to build on top of the knowledge collected upstream. While we have control over our own downstream, e.g. CRC or QuickLab, we will not have control over a 3rd party downstream.

The amount of change introduces by downstream may also vary: from just changing a key in KSOPS to replacing KSOPS with Vault.

For a downstream user, it should be easy to follow the documentation, without any context switching to different repositories.
It should also be really easy to incorporate all changes from upstream, once upstream introduces new best practices.

The original idea was to have this continous-deployment repo to be the upstream with no implementation specifics and let other targets be the fork of it.

E.g. continous-deployment <--upstream_of-- continous-deployment-crc

Unfortunately you can't fork into the same account/org (https://github.community/t/alternatives-to-forking-into-the-same-account/10200)

I suggest to create a new or duplicate repo continous-deployment-crc and handle the rebasing without GH, just like explained in https://stackoverflow.com/questions/45748400/git-fork-repo-to-same-organization

The downside is, we don't get a nice UI how many commits each repo is ahead/behind the other one.

Thoughts?

@HumairAK
Copy link
Member

HumairAK commented Sep 4, 2020

hrmm -- what about not doing forks at all, and using a kustomize remote base like we discussed on call?

@durandom
Copy link
Member Author

durandom commented Sep 4, 2020

hrmm -- what about not doing forks at all, and using a kustomize remote base like we discussed on call?

Because we also want the documentation and tooling in the continous-deployment repo to be re-used. That's the main benefit of that repo. Plus some tooling we'll add later.

@HumairAK
Copy link
Member

HumairAK commented Sep 4, 2020

That's fair, I'm good with just using cmd line rebasing + pr, this could probably be automated in some form later

@anishasthana
Copy link
Member

I thought we had said we don't want to rebase where possible.

@durandom
Copy link
Member Author

durandom commented Sep 8, 2020

I thought we had said we don't want to rebase where possible.

Correct, if we reference external manifests. In this case we're adding implementation specifics (like the CRC or QuickLab cluster). Let's see how this works out

@durandom durandom self-assigned this Sep 8, 2020
@durandom
Copy link
Member Author

durandom commented Sep 8, 2020

@tumido mentioned developer docs, e.g. https://firebase.google.com/docs/firestore/quickstart - they tend to have language specific examples embedded via inline tabs.

But then, if you're a user of the upstream repo and maintain your own implementation specifics, would they also show up in such inline tabs?

@durandom durandom changed the title relationship between continous-deployment and it's "forks" relationship between continous-deployment and it's implementation specific incarnations Sep 8, 2020
@durandom
Copy link
Member Author

durandom commented Sep 8, 2020

@anishasthana maybe maintaining the manifests only in this repo works, with referencing them from the downstream versions. Would all changes like in https://github.com/operate-first/continuous-deployment/pull/20/files go to an overlay then?

@anishasthana
Copy link
Member

That's what I'm thinking too, any quick-lab/crc/whatever specific bits can go into overlays in other repos, and they can all pull from this repository. @durandom

@HumairAK
Copy link
Member

HumairAK commented Sep 8, 2020

I like the idea of using remote base in kustomizations

But yeah, then users will need to copy paste the rest of the files (like if they want to re-use docs) from upstream onto their own repo.

@anishasthana
Copy link
Member

Does Markdown not support overlays? :-)

@durandom durandom added this to New in Master Board Sep 16, 2020
@HumairAK HumairAK moved this from Backlog to To do/Next in Master Board Sep 30, 2020
@HumairAK HumairAK added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Sep 30, 2020
@HumairAK
Copy link
Member

There's a bit of confusion regarding forking odh-manifests or using kustomize remotes, we should prioritize this discussion and come to a resolution

@anishasthana

@goern
Copy link
Member

goern commented Sep 30, 2020

@harshad16

@anishasthana
Copy link
Member

So in the case of the ODH operator, remote references to manifests are not possible. This is due to how the operator is structured in terms of requiring tar balls. Having the operator reach out to tan upstream github repository every time it attempts to reconcile some changes will result in a lot of slowness. So from an operator-1st standpoint, I'd propose the following:

  1. Create a fork of odh-manifests and have our own overlays with any specific changes we need (extra user profiles for jhub, different admin password for superset, etc.) If we want to use fully vanilla manifests, we can just directly refer to the upstream manifest from our kfdef file.
  2. FOr all other applications, I think using remote overlays is the way to go

@tumido
Copy link
Member

tumido commented Oct 20, 2020

To me forking odh-manifests sounds like an overkill. I would personally prefer the approach no. 2 described in this Vasek's blogpost. Let's have our repo (not a fork), which can serve as an override for the already present ODH components. This repo can also host additinal demo components we'd like to provide on top of ODH.

Mind that we'd like to stick as close to upstream ODH manifests as possible and feed any our changes upstream anyways.

So, this is how it can be used:

See this demo repo: https://github.com/vpavlin/odh-manifests-overrides

It contains a kfdef file which contais:

kind: KfDef
...
spec:
  applications:
...
  - kustomizeConfig:
      repoRef:
        name: manifests
        path: jupyterhub/jupyterhub
    name: jupyterhub
  - kustomizeConfig:
      repoRef:
        name: manifests-overrides
        path: jupyterhub/jupyterhub
    name: jupyterhub
  repos:
...
  - name: manifests
    uri: https://github.com/opendatahub-io/odh-manifests/tarball/master
  - name: manifests-overrides
    uri: https://github.com/vpavlin/odh-manifests-overrides/tarball/master

This makes the kfdef to merge those manifests together and replace whatever is to be found in jupyter/jupyter in the manifests repo reference by the manifests found in jupyter/jupyter of the manifests-overrides repo ref. (What happens behind the scene, can be simply described as copy and paste of files and then it runs kustomize on the resulting folder structure.).

This way we can provide changeset to the ODH components in case we need to. That's pretty useful for cases when we'd like to modify the already present resource files - like config maps, etc. It's a file based paste/replace method.

On the other hand we can't use this approach to provide additional resource files or overlays to the existing components though, because we can't reference the other repository and kustomize it.

This "override repo" can be also used to define additional manifests as new kfdef components - this can become a home for the demo usecases and such.

@durandom
Copy link
Member Author

@tumido your approach sounds good and it's recommended from the ODH folks. I'm 👎 for a fork.

And of course we'll run into problems like the last paragraph ("On the other hand we can't use this approach to ...") but that's good and if we involve ODH in solving it, that's the goal of the op1st stuff

@HumairAK
Copy link
Member

I looked over Vasek's blog post. It seems he himself recommends the third approach, but I agree in that the 2nd approach seems to work better for us. The only downside he points out is that upstream could change the structure of the manifests, but you would have to watch out for this even if you rebase from an upstream and update overlays (option 3). I also think you could guard against such things via a ci pipeline, or even a script you can run to ensure no new files were added unintentionally.

On the other hand we can't use this approach to provide additional resource files or overlays to the existing components though, because we can't reference the other repository and kustomize it.

Not sure if I understand this bit @tumido can you elaborate?

This "override repo" can be also used to define additional manifests as new kfdef components - this can become a home for the demo usecases and such.

This sounds like we would be using odh as a general cd solution like argocd, might as well use argocd at that point.

@anishasthana
Copy link
Member

@durandom Approach #3 is what is recommended by ODH upstream --
Approach 3: Fork the odh-manifests repository and extend it with component overlays.

@HumairAK I think your interpretation of the second approach is bang-on. At that point we're not really benefiting from using anything ODH, and we should just stick with ArgoCD.

@tumido can you elaborate more on what you're saying?

kind: KfDef
...
spec:
  applications:
...
  - kustomizeConfig:
      repoRef:
        name: manifests
        path: jupyterhub/jupyterhub
    name: jupyterhub
  - kustomizeConfig:
      repoRef:
        name: manifests-overrides
        path: jupyterhub/jupyterhub
    name: jupyterhub
  repos:
...
  - name: manifests
    uri: https://github.com/opendatahub-io/odh-manifests/tarball/master
  - name: manifests-overrides
    uri: https://github.com/vpavlin/odh-manifests-overrides/tarball/master

If this results in the first jupyterhub being completely overwritten by the second why are we specifying the first at all? It seems like unnecessary clutter.
Or am I misunderstanding and what is actually happening is that any extra files in the odh-manifests-overrides repo are used in addition to the files in the odh-manifests repo.

@durandom
Copy link
Member Author

If this results in the first jupyterhub being completely overwritten by the second why are we specifying the first at all? It seems like unnecessary clutter.

If I read the blog post correct, it'll only override/replace the files that are in the override repo, in the example just the config-map.

@durandom
Copy link
Member Author

Maybe I should've read the documentation and the blog post more carefully. I always just saw "fork" and imagined approach No1. - and tbh I'm not sure which approach we used in https://github.com/AICoE/idh-manifests

So No.2 seems like it the sane approach, but introduces the classic "how do I track what I'm changing".
But in No.3 we're also just adding the overlays as new files. Is the benefit that kustomize will complain in No.3. if I overlay an unknown resource and in No.2 it won't?

@tumido
Copy link
Member

tumido commented Oct 21, 2020

@anishasthana

Let me explain elaborate on what kfctl does. The jupyterhub component is not overwriten completely. If you look into the override repo you can see that only a single file is present within the jupyterhub/jupyterhub path and here's why:

The kfctl execution works like this:

  1. Fetch all tarballs in repos of the KfDef and unpack them to a cache location
  2. Go through the applications list, in sequence from top to bottom and copy from /kustomizeConfig/repoRef/name cache folder from step 1 on subpath /kustomizeConfigrepoRef/path within the cache manifests folder to a name folder within kustomize folder.

That's it.

So in this example:

Use this example.yaml
apiVersion: kfdef.apps.kubeflow.org/v1beta1
kind: KfDef
metadata:
  name: opendatahub
  namespace: opendatahub
spec:
  applications:
  - kustomizeConfig:
      repoRef:
        name: manifests
        path: jupyterhub/jupyterhub
    name: jupyterhub
  - kustomizeConfig:
      repoRef:
        name: manifests-overrides
        path: jupyterhub/jupyterhub
    name: jupyterhub
  repos:
  - name: manifests
    uri: https://github.com/opendatahub-io/odh-manifests/tarball/master
  - name: manifests-overrides
    uri: https://github.com/vpavlin/odh-manifests-overrides/tarball/master

Then run:

$ kfctl build -V -f example.yaml
INFO[0000] Fetching https://github.com/opendatahub-io/odh-manifests/tarball/master to .cache/manifests  filename="kfconfig/types.go:498"
INFO[0000] Updating localPath to .cache/manifests/opendatahub-io-odh-manifests-2911878  filename="kfconfig/types.go:565"
INFO[0000] Fetch succeeded; LocalPath .cache/manifests/opendatahub-io-odh-manifests-2911878  filename="kfconfig/types.go:586"
INFO[0000] Fetching https://github.com/vpavlin/odh-manifests-overrides/tarball/master to .cache/manifests-overrides  filename="kfconfig/types.go:498"
INFO[0000] Updating localPath to .cache/manifests-overrides/vpavlin-odh-manifests-overrides-ec9916d  filename="kfconfig/types.go:565"
INFO[0000] Fetch succeeded; LocalPath .cache/manifests-overrides/vpavlin-odh-manifests-overrides-ec9916d  filename="kfconfig/types.go:586"
INFO[0000] Processing application: jupyterhub            filename="kustomize/kustomize.go:515"
INFO[0000] Processing application: jupyterhub            filename="kustomize/kustomize.go:515"
Now I have a `kustomize` folder which contains following files. All of them comes from `manifests` repo, except `jupyterhub-configmap.yaml`, which content was overwritten from `manifests-overrides`.
$ tree kustomize
kustomize
└── jupyterhub
    ├── base
    │   ├── jupyterhub-configmap.yaml
    │   ├── jupyterhub-db-dc.yaml
    │   ├── jupyterhub-db-pvc.yaml
    │   ├── jupyterhub-db-service.yaml
    │   ├── jupyterhub-dc.yaml
    │   ├── jupyterhub-img-imagestream.yaml
    │   ├── jupyterhub-rolebinding.yaml
    │   ├── jupyterhub-role.yaml
    │   ├── jupyterhub-route.yaml
    │   ├── jupyterhub-secret.yaml
    │   ├── jupyterhub-serviceaccount.yaml
    │   ├── jupyterhub-service.yaml
    │   ├── jupyterhub-singleuser-profiles-configmap.yaml
    │   ├── jupyterhub-spark-operator-configmap.yaml
    │   ├── kustomization.yaml
    │   ├── params.env
    │   └── params.yaml
    ├── kustomization.yaml
    └── overlays
        ├── build
        │   ├── jupyterhub-buildconfig.yaml
        │   ├── jupyterhub-dh-buildconfig.yaml
        │   ├── jupyterhub-imagestream.yaml
        │   └── kustomization.yaml
        └── storage-class
            ├── jupyterhub-configmap.yaml
            ├── jupyterhub-db-pvc.yaml
            ├── jupyterhub-dc.yaml
            └── kustomization.yaml

$ cat kustomize/jupyterhub/base/jupyterhub-configmap.yaml
apiVersion: v1
kind: ConfigMap
metadata:
  labels:
    app: jupyterhub
  name: jupyterhub-cfg
data:
  jupyterhub_config.py: "print('The configMap has been overrided')"
  jupyterhub_admins: ""
  gpu_mode: ""
  singleuser_pvc_size: 10Gi

In that sense it differs very much from forking the odh-manifests, since the overrides repo, contains only the manifests which should be overriden, nothing else. And kfctl takes case of the completion.

In my eyes the no. 2 approach seems like the most viable approach, because:

  1. We don't want to modify ODH - Operate First should be running vanilla ODH. And if really have to modify it, we should be able to achieve it with the overrides as a temporary solution with expectation to get those modifications into upstream. The arguments is that there's no more vanilla setup than Operate First. On clean clusters, all vendors.. If we can't run the vanilla manifests then who can? I think nobody will ever get closer to vanilla deployment of this than we do.
  2. We want to extend the ODH with demo components and for that we don't need to maintain the upstream codebase at all, these demo components are separate to ODH manifests - and these can live in the very same repo as our overrides, because of the way how kfctl treats manifests (copy-pasting from one location to another)

And now I'm getting to what @HumairAK asked. Well, yes, we can use ArgoCD for that. The question is what's the end goal... have a demo usecase baked into ODH? Than it should be a component that you can deploy along with ODH, so it's usable even without operate first... Is it a custom Object storage for our clusters we're running ODH on? Well it may be better to deploy this via ArgoCD, so we can better tap into it and monitor it closely. I don't think we'll get a definitive answer to that for every case. But I think approach no.2 still gives us the possibility to go either way.

@tumido
Copy link
Member

tumido commented Oct 21, 2020

Conclusion is:

Approach no 2. allows us to have a single repo which gives us the option to both modify as well as extend ODH. And all of that even without forking upstream. And I see that as a win strategy.

@HumairAK
Copy link
Member

HumairAK commented Oct 21, 2020

Just to clarify, I was more referring to using the "override" feature to add "additional" components, which seems like it might be an abuse of its intention (maybe I'm wrong). When I hear the term override, I assume it's replacing something that existed before.

Maybe that's me just nitpicking.

Regardless, I'm voting for option no.2 as well, this way we can retain complete tarballs whether they are from the original odh-manifest repo or overrides in our own repo. Afaik, that was the primary concern with using remote references and overlays.

@durandom
Copy link
Member Author

Cool, thanks for weighing in everybody. Especially @tumido for his elaborate explanations and @anishasthana for challenging.
Let's go with No.2 and try to explain the reasoning why (i.e. the gist of the conversation in here) in the README of our override repo.

@HumairAK
Copy link
Member

Any objections to storing the actual odh manifests to deploy the operator (i.e. subscription/operator group etc.) alongside the kfdef and overrides all in the same repository (i.e. operate-first /odh ) ?

I would like to avoid introducing more repos if we don't need to. I can create issues there for laying out readme about our override procedure etc.

@tumido
Copy link
Member

tumido commented Oct 21, 2020

@HumairAK no objections on my end. I like that idea. Storing them besides the kfdef and the operator, in a 3rd folder in that repo, make sense to me. 👍

@HumairAK
Copy link
Member

All right going to close this ticket, let's continue any/all discussions regarding this to the odh repository. Thanks everyone for chiming in.

Master Board automation moved this from To do/Next to Done Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now.
Projects
No open projects
Master Board
  
Done
Development

No branches or pull requests

5 participants