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

Adding NF CRDs in workload cluster package #10

Merged
merged 7 commits into from
Jan 9, 2024

Conversation

arora-sagar
Copy link
Contributor

Currently, we are not installing CRDs via the workload package. It is being done via NF operators. Doing it as part of the workload cluster package is probably a good practice.

  • NFConfig
  • NFDeployment
  • ConfigRef

If this is not a good place to put the CRDs then we can think of making a separate package.

- NFConfig
- NFDeployment
- ConfigRef
@electrocucaracha
Copy link
Member

@johnbelamaric I'm also not sure about the right place to allocate these CRDs, any idea?

@johnbelamaric
Copy link
Member

Questions:

  1. These CRDs live on the management cluster or the workload cluster?
  2. These CRDs are OAI specific or general Nephio?

Assuming the answer to 1) is "management" and to 2) is "general", then these should be included with the Nephio controller manager package. In general, CRDs should be included with the reconciler that owns the corresponding CRs. If the CR may be managed by multiple controllers (say, depending on class), then they probably belong in their own package (or, for example, with the core Nephio controller manager).

I don't think we have built packages for "Nephio core" yet, outside of the R1 "nephio-example-packages" repository. The plan is that those packages would move to here: https://github.com/nephio-project/catalog/tree/main/nephio/core

@henderiw
Copy link

These CRDs are used in specialisation on mgmt cluster but are not actuated on the k8s API.
So this is needed on the workload cluster where they get actuated. So there are the nephio CRDs if you will for the workload cluster and are meta CRDs that are agnostic to the vendor.
So the scenario this solves is if you have multiple operators, they don't need to install them themselves or duplicate them, but they now have a dependency that they gets installed.

@arora-sagar
Copy link
Contributor Author

These CRDs are needed on the workload cluster and in R2 both RAN and Core controllers need them. The core controllers and packages reside in the OAI GitHub repository and the RAN controller and packages reside in the catalog repository. So we need a common location from where they can be applied on the workload cluster.

@johnbelamaric
Copy link
Member

A couple things:

  1. This package doesn't make sense where it is. It should be in the infra/capi subdirectory and shared between the OAI and free5gc code. Is there something OAI specific about this?
  2. This package is applied to the management cluster, so adding the CRDs here doesn't make sense if they are supposed to land on the workload cluster itself. I would create a separate package containing all the stuff we need on the workload cluster, then create a PV in this package to deliver that package to the workload cluster repository.

@arora-sagar
Copy link
Contributor Author

arora-sagar commented Dec 18, 2023

A couple things:

  1. This package doesn't make sense where it is. It should be in the infra/capi subdirectory and shared between the OAI and free5gc code. Is there something OAI specific about this?

There is nothing OAI specific we can move but I am not sure if infra/capi is a good place. We need to install these CRDs on all the workload clusters. At the moment we aren't using infra/capi.

  1. This package is applied to the management cluster, so adding the CRDs here doesn't make sense if they are supposed to land on the workload cluster itself. I would create a separate package containing all the stuff we need on the workload cluster, then create a PV in this package to deliver that package to the workload cluster repository.

Ah okay now I somewhat understand your point. I will create a separate package and its PV. Where should that package and its PV should reside? infra/capi ?

@johnbelamaric
Copy link
Member

The package for provisioning a workload cluster should live in infra/capi. The package that contains the various Nephio CRDs probably should live in nephio/core and be called something like "nephio-workload-crds" to indicate that it contains the CRDs that should be loaded on all workload clusters.

The "nephio-workload-cluster" package would then include a PV that installs that package on all new workload clusters.

@arora-sagar
Copy link
Contributor Author

The package for provisioning a workload cluster should live in infra/capi. The package that contains the various Nephio CRDs probably should live in nephio/core and be called something like "nephio-workload-crds" to indicate that it contains the CRDs that should be loaded on all workload clusters.

The "nephio-workload-cluster" package would then include a PV that installs that package on all new workload clusters.

@johnbelamaric Do you mean something like this? Also, will we change the locations of other packages too? like kindnet, multus which are required by workload cluster? Right now they are in examples.

@johnbelamaric
Copy link
Member

johnbelamaric commented Dec 20, 2023

Yes, that's the right idea. The main thing is to separate out the platform from the use case. So:

  • nephio/core directory contains all those things that should be in any given Nephio installation
  • nephio/optional contains other things that may or may not be needed (for example, resource backend)
  • distros contains individual opinionated distributions for different environments. In there we clone packages from nephio/* and create derivative packages that are customized to their specific environment. That is, they will will have an upstream in nephio/core, for example.
  • infra contains packages for deploying particular kinds of infrastructure, such as a Kind cluster via CAPI, or a GKE cluster via Config Controller.
  • workloads contains the packages for deploying workloads on top of those clusters

The "repo" for the various PVs won't be "catalog". Instead, we need to register the directories as individual repositories, in any particular opinionated installation. See for example the repo-*.yaml files here: https://github.com/nephio-project/catalog/tree/main/distros/gcp/nephio-mgmt for the ones I register for the GCP installation.

We may want to include a package of repositories in the "optional" directory, much like the "nephio-stock-repositories" package in examples. This would register a bunch of

The

@arora-sagar
Copy link
Contributor Author

@johnbelamaric @henderiw @tliron @electrocucaracha so I am moving the packages from nephio-example-packages to catalog as per what @johnbelamaric mentioned in the previous comment.

Is everyone okay with this move?

.
├── distros
│   ├── gcp
│   │   ├── cc-rootsync
│   │   └── nephio-mgmt
│   │       ├── cert-manager
│   │       ├── nephio-controllers 
│   │       │   ├── app
│   │       │   │   └── controller
│   │       │   └── crd
│   │       │       └── bases
│   │       ├── nephio-webui
│   │       ├── network-config
│   │       │   ├── app
│   │       │   │   └── controller
│   │       │   └── crd
│   │       ├── porch
│   │       └── resource-backend
│   │           ├── app
│   │           │   └── controller
│   │           │       └── grpc
│   │           └── crd
│   │               └── bases
│   ├── openshift
│   └── sandbox
├── infra
│   ├── capi
│   │   ├── cert-manager
│   │   ├── cluster-capi
│   │   ├── cluster-capi-infrastructure-docker
│   │   ├── cluster-capi-kind
│   │   ├── cluster-capi-kind-docker-templates
│   │   ├── gitea
│   │   ├── kindnet
│   │   ├── local-path-provisioner
│   │   ├── metallb
│   │   ├── nephio-workload-cluster
│   │   ├── repository
│   │   └── vlanindex
│   └── gcp
│       ├── cc-cluster-gke-std-csr-cs
│       ├── cc-repo-csr
│       ├── nephio-blueprint-repo
│       └── nephio-workload-cluster-gke
├── nephio
│   ├── core
│   │   ├── configsync
│   │   ├── network-config
│   │   │   ├── app
│   │   │   │   └── controller
│   │   │   └── crd
│   │   ├── rootsync
│   │   └── workload-crds
│   └── optional
│       ├── nephio-stock-repos
│       ├── nephio-webui
│       └── resource-backend
│           ├── app
│           │   └── controller
│           │       └── grpc
│           └── crd
│               └── bases
└── workloads
    ├── free5gc
    └── oai
        ├── network
        ├── oai-ran-operator
        │   └── operator
        ├── package-variants
        ├── pkg-example-cucp-bp
        ├── pkg-example-cuup-bp
        ├── pkg-example-du-bp
        └── pkg-example-ue-bp
            └── oai-ue

nephio-controller and porch will be moved in nephio/core after merging #14 and #13

In nephio-stock-repo I will mention all package repositories individually for example cluster-capi, multus, kindnet etc.

I was thinking of mentioning the revisions in packages main because in the main branch all the packages should correspond to main right?

Is this okay? Did I understand it correctly? I just want to double-check before making the commit because once I committed I want to test and have to make changes in test-infra too.

@electrocucaracha
Copy link
Member

Maybe we can do it in several PRs to facilitate the review

@johnbelamaric
Copy link
Member

Generally this looks OK. Gitea and metal-lb are sandbox choices, and are distinct from "capi". Similarly while cert-manager is needed for CAPI it's needed for other things too, so I think it should be in distros. So:

The following should move from infra/capi to distros/sandbox: cert-manager, gitea, metallb, repository

The following should move from nephio/core to nephio/optional: network-config

The following should move from workloads/oai to distros/sandbox: network

I think "multus" also is needed in infra/capi. "vlanindex" probably is good in infra/capi, although maybe that belongs in distros/sandbox.

We will also I think eventually clone things from nephio/core and nephio/optional into distros/sandbox. This is how I did the distros/gcp, for example. Take the base Nephio package, and then customize it for the specific distro.

@electrocucaracha
Copy link
Member

Generally this looks OK. Gitea and metal-lb are sandbox choices, and are distinct from "capi". Similarly while cert-manager is needed for CAPI it's needed for other things too, so I think it should be in distros. So:

The following should move from infra/capi to distros/sandbox: cert-manager, gitea, metallb, repository

The following should move from nephio/core to nephio/optional: network-config

The following should move from workloads/oai to distros/sandbox: network

I think "multus" also is needed in infra/capi. "vlanindex" probably is good in infra/capi, although maybe that belongs in distros/sandbox.

We will also I think eventually clone things from nephio/core and nephio/optional into distros/sandbox. This is how I did the distros/gcp, for example. Take the base Nephio package, and then customize it for the specific distro.

What about creating a workloads/common folder for multus package? It's used by free5gc and oai test scenarios and they don't require a specific version.

@johnbelamaric
Copy link
Member

It's not a workload, it's a part of the cluster infrastructure, and applies specifically to CAPI clusters. It may be used by workloads, but for example it cannot be installed in GKE. And other cluster provisioners (e.g., OpenShift) probably have their own opinionated version and way to install it.

@electrocucaracha
Copy link
Member

I think that we can merge this as it is and create another PRs for moving the rest of the packages

/lgtm

@nephio-prow nephio-prow bot removed the lgtm label Jan 8, 2024
…tions

- oai ran and core repository
- workload crd repo
@electrocucaracha
Copy link
Member

/lgtm

@nephio-prow nephio-prow bot added the lgtm label Jan 9, 2024
@electrocucaracha
Copy link
Member

/approve

@johnbelamaric
Copy link
Member

/approve
/lgtm

Copy link
Contributor

nephio-prow bot commented Jan 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arora-sagar, electrocucaracha, johnbelamaric

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:

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

@nephio-prow nephio-prow bot added the approved label Jan 9, 2024
@nephio-prow nephio-prow bot merged commit f1edddc into nephio-project:main Jan 9, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants