nephio-project / nephio

Nephio is a Kubernetes-based automation platform for deploying and managing highly distributed, interconnected workloads such as 5G Network Functions, and the underlying infrastructure on which those workloads depend.
Apache License 2.0
102 stars 53 forks source link

Package-context content is erased when package is cloned #408

Open debeaueric opened 11 months ago

debeaueric commented 11 months ago

The package-context.yaml from a blueprint package is erased when the package is cloned. As a result, all the context elements filled in this file are not usable by Kpt functions contained in the Kptfile. For example, https://github.com/nephio-project/free5gc-packages/blob/main/free5gc-cp/package-context.yaml contains namespace field, but it is not present in the deployed package.

johnbelamaric commented 11 months ago

Yeah, this also makes the package context manipulation in PV basically useless. That's why I think we remove that functionality in the eventual beta version (if not in v1alpha2) of the API. See https://github.com/kptdev/kpt/issues/3973 for additional context.

I think maybe we should just never store the package context. Rather than it being a real, stored resource, it may be useful to introduce the concept of a sort of "virtual" resource that provides access to meta-data and other information (possibly even external?) to functions/specializers.

I am not sure if it's a good idea, but we could also possibly extend this to a sort of "data source" concept seen in other tools. Effectively, these become APIs to other systems. I haven't thought through the implications of that, though. For example, if those external systems do not provide "watch" like functionality, how do we handle it? How do we track changes in return values from those APIs? What would our system look like with that functionality to access IPAM, instead of, say, our existing IPClaim mechanism? Does it introduce something imperative, or are we OK since we still use the declarative resource model to access it?

davidblaisonneau-orange commented 11 months ago

I only have seen the visible face of Nephio, and probably have not understood some element on the underlayers, and I hope this message will be pertinent.

In my opinion, today, the main impact of that behaviour is the difference between the UI/packRev and a packageVariant.

For a package with just a setter function:

To have a package working on both UI and packageVariant, I must have 2 setter functions, the one on package-variables.yaml, the second on package-context.yaml, that make the package less readable.

Sorry for those questions, but:

Whatever the option is choosen, it must be homogeneous for each way of manipulating packages.

johnbelamaric commented 11 months ago

These are really good points, David. The UI is very limited right now. Here's one way to think about it. We have a collection of management planes, each accessible via KRM:

Sitting in front of those, we have Git repositories. For the infra and workload clusters, we generally limit our interaction with them to publishing to their Git repositories. For the Nephio Management cluster, we use both the git repository and direct interaction with the API server.

Today, the UI only allows the user to manipulate packages - ie, the contents of the git repositories. This means you can effectively do anything with the UI, but it is not always the most convenient way to do it. Everything has to be deployed as a package. When we use kubectl directly to modify PackageVariant in the Nephio API Server, we are bypassing the gitops for that operation. I think this can be OK, in many cases.

So, my thinking is this - and I think this addresses exactly your points:

  1. We should look at building GUI features that can interact with the live API server of the Nephio management server. We did this in the pre-R1 workshop last year, where you could at least view "Package Deployments". We just didn't build anything like this out in R1. A step towards this would be just adding the standard Backstage plugin for K8s for the mgmt server, and then for the workload clusters (if they are accessible). Better than that would be special screens for PV, PVS, etc.
  2. We need to improve PV. We can make that a lot better. We may want consider if some first class support for setters in PV is a good idea. I am not sure. The changes to allow direct injection from the PV to the package may be good enough.
  3. I think we should eliminate package context altogether (in kpt/Porch) as a stored resource, as described in my comment above.
liamfallon commented 3 months ago

Triaged Triage Comment: Package context is broken today, it probably should be removed from Package Variant. Don't use this, we need to add something to the documentation to warn people not to use it and PV shouldn't update it