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

Incorrect handling of non-root directory based repos in Porch #451

Closed
johnbelamaric opened this issue Nov 29, 2023 · 14 comments · Fixed by nephio-project/porch#9
Closed
Assignees

Comments

@johnbelamaric
Copy link
Member

Our catalog repository contains multiple sub-directories. Porch assumes a Repository (note the capital, I mean the K8s resource) is a "flat" space for holding packages. We structured the catalog Git repo with the intent to use multiple Porch repos to represent that same repository, like this:

apiVersion: config.porch.kpt.dev/v1alpha1
kind: Repository
metadata:
  name: blueprints-infra-gcp
  namespace: default
  labels:
    kpt.dev/repository-access: read-only
    kpt.dev/repository-content: external-blueprints
spec:
  content: Package
  deployment: false
  git:
    branch: main
    directory: /infra/gcp
    repo: https://github.com/nephio-project/catalog.git
  type: git
---
apiVersion: config.porch.kpt.dev/v1alpha1
kind: Repository
metadata:
  name: blueprints-workloads-free5gc
  namespace: default
  labels:
    kpt.dev/repository-access: read-only
    kpt.dev/repository-content: external-blueprints
spec:
  content: Package
  deployment: false
  git:
    branch: main
    directory: /workloads/free5gc
    repo: https://github.com/nephio-project/catalog.git
  type: git
---
apiVersion: config.porch.kpt.dev/v1alpha1
kind: Repository
metadata:
  name: blueprints-workloads-oai
  namespace: default
  labels:
    kpt.dev/repository-access: read-only
    kpt.dev/repository-content: external-blueprints
spec:
  content: Package
  deployment: false
  git:
    branch: main
    directory: /workloads/oai
    repo: https://github.com/nephio-project/catalog.git
  type: git

That is, we have multiple Porch Repository resources, pointing to different subdirectories. In theory this is fine, but it is not a feature of Porch that is well exercised, and I am seeing some issues.

  1. It is necessary for tags to include the full path. I think this is actually probably the correct behavior, since that way you have true independence across Repository objects. So, for example we must use "infra/gcp/cc-repo-csr/v1" not "cc-repo-csr/v1" as the tag.

  2. Each package revision gets duplicated across all instances with the same base git repo. In other words, it sees this as duplicated repositories rather than separate repos. So with the three Repository resources registered above, we see:

[hi on] jbelamaric@jbelamaric:~/proj/gh/johnbelamaric/nephio$ k get packagerevisions --context gke_jbelamaric-dev_us-central1_nephio
NAME                                                            PACKAGE                              WORKSPACENAME   REVISION   LATEST   LIFECYCLE   REPOSITORY
blueprints-infra-gcp-13e9002337c2cc7a143523b47efedc7d31920f98   cc-cluster-gke-std-csr-cs            main            main       false    Published   blueprints-infra-gcp
blueprints-infra-gcp-336b4f189458637bca8203a06c389e0ca2c2f0af   cc-cluster-gke-std-csr-cs            v1              v1         true     Published   blueprints-infra-gcp
blueprints-infra-gcp-995246eab970328dbe08de5c06d117698d12f861   cc-repo-csr                          main            main       false    Published   blueprints-infra-gcp
blueprints-infra-gcp-b565038bb061184bfd04a3d33b6c5b53a5848bef   cc-repo-csr                          v1              v1         true     Published   blueprints-infra-gcp
blueprints-infra-gcp-fc58b20435dd4d14d3384466fcbedba673099ebf   nephio-blueprint-repo                main            main       false    Published   blueprints-infra-gcp
blueprints-infra-gcp-989f590d68008a9a3f88e39f7a563f844c48f810   nephio-blueprint-repo                v1              v1         true     Published   blueprints-infra-gcp
blueprints-infra-gcp-00b92adc8ca9e87149a60e289db96abd00290f08   nephio-workload-cluster-gke          main            main       false    Published   blueprints-infra-gcp
blueprints-infra-gcp-5a9ea99ed52ccc679df5698cbddd74074e1b31d2   nephio-workload-cluster-gke          v1              v1         true     Published   blueprints-infra-gcp
blueprints-infra-gcp-13e9002337c2cc7a143523b47efedc7d31920f98   cc-cluster-gke-std-csr-cs            main            main       false    Published   blueprints-infra-gcp
blueprints-infra-gcp-336b4f189458637bca8203a06c389e0ca2c2f0af   cc-cluster-gke-std-csr-cs            v1              v1         true     Published   blueprints-infra-gcp
blueprints-infra-gcp-995246eab970328dbe08de5c06d117698d12f861   cc-repo-csr                          main            main       false    Published   blueprints-infra-gcp
blueprints-infra-gcp-b565038bb061184bfd04a3d33b6c5b53a5848bef   cc-repo-csr                          v1              v1         true     Published   blueprints-infra-gcp
blueprints-infra-gcp-fc58b20435dd4d14d3384466fcbedba673099ebf   nephio-blueprint-repo                main            main       false    Published   blueprints-infra-gcp
blueprints-infra-gcp-989f590d68008a9a3f88e39f7a563f844c48f810   nephio-blueprint-repo                v1              v1         true     Published   blueprints-infra-gcp
blueprints-infra-gcp-00b92adc8ca9e87149a60e289db96abd00290f08   nephio-workload-cluster-gke          main            main       false    Published   blueprints-infra-gcp
blueprints-infra-gcp-5a9ea99ed52ccc679df5698cbddd74074e1b31d2   nephio-workload-cluster-gke          v1              v1         true     Published   blueprints-infra-gcp
blueprints-infra-gcp-13e9002337c2cc7a143523b47efedc7d31920f98   cc-cluster-gke-std-csr-cs            main            main       false    Published   blueprints-infra-gcp
blueprints-infra-gcp-336b4f189458637bca8203a06c389e0ca2c2f0af   cc-cluster-gke-std-csr-cs            v1              v1         true     Published   blueprints-infra-gcp
blueprints-infra-gcp-995246eab970328dbe08de5c06d117698d12f861   cc-repo-csr                          main            main       false    Published   blueprints-infra-gcp
blueprints-infra-gcp-b565038bb061184bfd04a3d33b6c5b53a5848bef   cc-repo-csr                          v1              v1         true     Published   blueprints-infra-gcp
blueprints-infra-gcp-fc58b20435dd4d14d3384466fcbedba673099ebf   nephio-blueprint-repo                main            main       false    Published   blueprints-infra-gcp
blueprints-infra-gcp-989f590d68008a9a3f88e39f7a563f844c48f810   nephio-blueprint-repo                v1              v1         true     Published   blueprints-infra-gcp
blueprints-infra-gcp-00b92adc8ca9e87149a60e289db96abd00290f08   nephio-workload-cluster-gke          main            main       false    Published   blueprints-infra-gcp
blueprints-infra-gcp-5a9ea99ed52ccc679df5698cbddd74074e1b31d2   nephio-workload-cluster-gke          v1              v1         true     Published   blueprints-infra-gcp

I think 1) is ok, but I am not sure if Porch properly creates tags like this. I will have to try it. I think I can fix 2) pretty quickly. But I wanted to raise this as a concern; we may want to consider breaking this out into multiple Git repos to be safe.

@johnbelamaric
Copy link
Member Author

Not sure if the UI will handle these correctly, even if I fix 2) and the one I fear may be latent (how Porch creates new tags).

@johnbelamaric
Copy link
Member Author

FYI, what we should see in that list is the set of packages only once, since these are all under the GCP infra Repository, and the other repos should show up as empty.

@johnbelamaric
Copy link
Member Author

I also confirmed this is the behavior in the Porch version used in R1 (v0.0.20), so it is not new with my latest PRs.

@johnbelamaric
Copy link
Member Author

Ok, actually I think I found the issue and it's a pretty easy fix.

@johnbelamaric johnbelamaric self-assigned this Nov 29, 2023
@anurag-rajawat
Copy link

Any updates and is there anything I can help you with?

@johnbelamaric
Copy link
Member Author

Fixed in the latest porch release: kptdev/kpt#4097

@johnbelamaric
Copy link
Member Author

But it seems I may have introduced some issues in kptdev/kpt#4094 so I think we will be doing a new release without some of those changes.

@johnbelamaric
Copy link
Member Author

Looks like @mortent did a release while I was out that excluded the problematic changes. So, we should update our system to use Porch v0.0.35

@tliron
Copy link

tliron commented Dec 14, 2023

@johnbelamaric The SIG#2 community expressed a preference for putting Porch code in its own "porch" repository here. I'm not quite sure if/how to merge the v0.0.35 release in the PR. Should we redo the PR, or have you also incorporated new fixes in it?

@johnbelamaric
Copy link
Member Author

@tliron did you do anything with the PR yet? If not, if you can just get the new repo created, with OWNERS file, the right permissions, I can put together a PR that matches the v35 release, then after that, a separate PR that has my fixes which apparently still need some work. I am hoping to get some specific, reproducible tests from the internal team that found the issues, so that we can use them to figure out what's wrong with my changes (or we can punt on my changes since they become obsolete depending where we take the Porch architecture).

@tliron
Copy link

tliron commented Dec 14, 2023

I did not do anything yet -- once I found out about the new Porch release I didn't know what to do. :) I actually seem to have less than full permissions on the GitHub org. I wouldn't be too worried about OWNERS and everything else yet, and I wouldn't bother with the regular review process for this task. I'm in favor of just pushing the code directly to a new repo.

@johnbelamaric
Copy link
Member Author

Works for me. I will make you an org owner

@johnbelamaric
Copy link
Member Author

I put v35 into nephio-project/catalog#13. However, it looks like v35 does not contain the fix from kptdev/kpt#4097 - I see some of them in the PR to bring back the 4094 changes here: nephio-project/porch#8.

I will submit a Porch PR with just the 4097 changes in our repo, so we will have a fix.

@johnbelamaric
Copy link
Member Author

@anurag-rajawat the images are now available in the Nephio docker hub, tagged "v2.0.0-beta.1" and also (for now) "latest". These are built from the repo after nephio-project/porch#9 merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants