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
93 stars 52 forks source link

Unable to use packages created with kpt cli in porch #664

Open liamfallon opened 2 months ago

liamfallon commented 2 months ago

Original issue URL: https://github.com/kptdev/kpt/issues/3599 Original issue user: https://github.com/ChristopherFry Original issue created at: 2022-10-04T23:25:07Z Original issue last updated at: 2023-01-27T23:47:09Z Original issue body: ### Expected behavior New revisions should be able to be created from packages that were originally created outside of Porch.

Actual behavior

New revision is created, however all resources added to the package are removed, and Kptfile changes are reverted.

Information

Porch and kpt are on build 30858290.

Steps to reproduce the behavior

  1. Follow Namespace Provisioning CLI Guide in creating and publishing the basens package to an already registered Porch repository
  2. Using kpt alpha rpkg copy create a new revision of the package
  3. ERROR - Compare the resources in the new revision (either by comparing the draft branch to main in git, or using kpt alpha rpkg pull) and notice that any resources added to the basens package are now removed in the new revision (the namespace, resource quota, role binding, and apply replacements resources) as well as any Kptfile updates.

Original issue comments: Comment user: https://github.com/ChristopherFry Comment created at: 2022-10-04T23:26:12Z Comment last updated at: 2022-10-04T23:26:12Z Comment body: cc @droot @mortent

Comment user: https://github.com/mortent Comment created at: 2022-10-11T17:31:26Z Comment last updated at: 2022-10-11T17:31:26Z Comment body: The problem here is that our current copy functionality relies on the task list in order to replay the changes to effectively recreate the state of the packagerevision. I can think of two approaches to this issue:

First, we can create a Copy task that references the source PackageRevision that we will use. So to create a new PackageRevision, the user can just use the regular Create endpoint for PackageRevisions just like we do with Init and Clone, but instead provide just the Copy task as the first item in the task list. This will essentially create two different ways to copy a package, similar to how https://github.com/GoogleContainerTools/kpt/issues/3329 suggests two separate ways to update a package (merge or reclone-and-replay/rebase). This does mean that the task list for the resulting PackageRevision will start with a Copy task, so we need to make sure our logic handles this the same way we do with Init and Clone tasks. We do leverage this when we scan for commits for a specific package, so we need to make sure we handle that correctly too. We need to determine how we move the commits to the new branch, whether we can identify the necessary commits and keep the separate, or if we just squash them all into one commit. The latter seems like it might be easiest.

The second option could be that we check in Porch whether we have a task list that permits us to do a replay, or whether we need to create the new packagerevision by copying the source PackageRevision. I'm not sure if this is possible and whether the resulting PackageRevision resource and its task will make sense. The advantage of this approach over the first alternative would be that it puts less responsibility on the user in terms of deciding which approach to Copy it should use.

Overall I think we should try out the first alternative. It seems to stay closest to our current model for PackageRevisions.

Comment user: https://github.com/droot Comment created at: 2022-10-11T18:24:10Z Comment last updated at: 2022-10-11T18:24:10Z Comment body: > replay the changes to effectively recreate the state of the packagerevision.

curious if thereplay approach helpful with the packages so far ?

First, we can create a Copy task that references the source PackageRevision that we will use. So to create a new PackageRevision, the user can just use the regular Create endpoint for PackageRevisions just like we do with Init and Clone, but instead provide just the Copy task as the first item in the task list. This will essentially create two different ways to copy a package, similar to how #3329 suggests two separate ways to update a package (merge or reclone-and-replay/rebase). This does mean that the task list for the resulting PackageRevision will start with a Copy task, so we need to make sure our logic handles this the same way we do with Init and Clone tasks. We do leverage this when we scan for commits for a specific package, so we need to make sure we handle that correctly too. We need to determine how we move the commits to the new branch, whether we can identify the necessary commits and keep the separate, or if we just squash them all into one commit. The latter seems like it might be easiest.

So, this copy mode is essentially clone without tasks replay. Re: how we move the commits to the new branch, it seems squashing all commits in this draft in one commit might be consistent with don't care about replay semantics.

Given that we don't know which model (replay or not) will work well in practice, I think supporting this copy mode seems ok to me.

Comment user: https://github.com/mortent Comment created at: 2023-01-27T23:47:09Z Comment last updated at: 2023-01-27T23:47:09Z Comment body: Coming back to this issue now. I think introducing a copy task is the best approach here and it seems consistent with the way we handle updates.