-
Notifications
You must be signed in to change notification settings - Fork 227
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
Moving PackageRevision to CRD #3630
Comments
I agree that switching to a CRD makes the most sense in the long run. A couple short comments:
|
We should try to split this up into several issues. The list in this issue is a start, but I suspect there will be more. |
So, what would a "v1alpha2" PackageRevision resource look like if it is completely CRD-based? I think we're going to need a thorough design on this. I see it as an opportunity to revisit the whole Package/PR/PRR model. A few thoughts, these are just questions that come up immediately for me:
|
Background
Porch is primarily built as an aggregated APIServer with the PackageRevision and PackageRevisionResources resources, as well as their subresources, implemented that way rather than as CRDs. Up until recently the only datastore used for these resources were git/oci, although with #3579 we also store data pertaining to each PackageRevision in CRs as well.
It has become clear that in order to make the resources provided by the aggregated APIServer behave like regular resources, it is not possible to only rely on storing data in git commits. That we recently started using CRs in addition to git is a consequence of this (We also discussed alternatives here), but having to storage backends for a resource leads to other issues. It is also non-trivial to implement all aspects of the Kubernetes API through an aggregated APIServer when using a different storage backend that etcd. Examples are proper resourceVersions, generations, and correct semantics for watches.
As a result, we have discussed migrating the PackageRevision type to a CRD.
Details
Implementing the PackageRevision type as a CRD has many benefits, as CRDs already provide all the operations that are expected from KRM resources. But it also comes with limitations as compared to an aggregated apiserver:
As a result, switching to using a CRD involves addressing at least the following items (more will probably come up):
The PackageRevisionResources type will almost certainly still need to be supported through the aggregated APIServer. Storing package content in CRDs does not seem like a feasible solution. But we need to determine how the PackageRevisionResources type will align with a PackageRevision type that is a CRD. Currently, we consider them as two separate views of the same underlying resource. We have discussed an alternative where we only allow push and pull of resources through the PackageRevisionResource, but it needs more thought. If we could have a PackageRevision CRD with subresources backed with an aggregated APIServer, it seems like we would like to have something like
packagerevisions/push
andpackagerevisions/pull
, but combining CRDs and aggregated APIServer in this way is not possible.How do we get there?
We essentially have two ways to approach this.
Full rewrite now
This is taking the “quick(?) and painful” approach and do the full rewrite in one go. This seems very risky as this is a major change to Porch with several challenges, and we will almost certainly discover additional challenges on the way.
Gradual rewrite
So instead of trying to do all these changes in one massive effort, we can do this gradually.
What makes this approach compelling, is that we can make all the necessary changes without actually migrating to using a CRD. We can migrate away from using subresources, move the polling of repositories, and migrate logic to controllers using the current type from the Aggregated APIServer, but it allows us to combine this effort with adding new features (that hopefully doesn’t increase our dependency on the aggregated APIServer). This also allows to reconsider this change if discover that it is misguided.
The path forward is essentially to start addressing the issues listed above and start documenting additional issues we come across.
The text was updated successfully, but these errors were encountered: