Fix packagerevisions for repos with subdirs (#4097)

* Fix packagerevisions for repos with subdirs Signed-off-by: John Belamaric <jbelamaric@google.com> * Update key used during deletion and close Signed-off-by: John Belamaric <jbelamaric@google.com> * Fix erroneous and extraneous log message Signed-off-by: John Belamaric <jbelamaric@google.com> --------- Signed-off-by: John Belamaric <jbelamaric@google.com>
kptdev · Nov 30, 2023 · 2d6d49e · 2d6d49e
1 parent 82b7c85
commit 2d6d49e
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 6 deletions.
diff --git a/porch/pkg/cache/cache.go b/porch/pkg/cache/cache.go
@@ -119,7 +119,7 @@ func (c *Cache) OpenRepository(ctx context.Context, repositorySpec *configapi.Re
 		if !isPackageContent(repositorySpec.Spec.Content) {
 			return nil, fmt.Errorf("git repository supports Package content only; got %q", string(repositorySpec.Spec.Content))
 		}
-		key := "git://" + gitSpec.Repo
+		key := "git://" + gitSpec.Repo + gitSpec.Directory
 
 		c.mutex.Lock()
 		defer c.mutex.Unlock()
@@ -175,7 +175,7 @@ func (c *Cache) CloseRepository(repositorySpec *configapi.Repository) error {
 		if git == nil {
 			return fmt.Errorf("git not configured for %s:%s", repositorySpec.ObjectMeta.Namespace, repositorySpec.ObjectMeta.Name)
 		}
-		key = "git://" + git.Repo
+		key = "git://" + git.Repo + git.Directory
 
 	default:
 		return fmt.Errorf("unknown repository type: %q", repositorySpec.Spec.Type)

diff --git a/porch/pkg/cache/repository.go b/porch/pkg/cache/repository.go
@@ -309,7 +309,9 @@ func (r *cachedRepository) Close() error {
 			// There isn't much use in returning an error here, so we just log it
 			// and create a PackageRevisionMeta with just name and namespace. This
 			// makes sure that the Delete event is sent.
-			klog.Warningf("Error looking up PackageRev CR for %s: %v")
+			if !apierrors.IsNotFound(err) {
+				klog.Warningf("Error deleting PackageRev CR %s/%s: %s", nn.Namespace, nn.Name, err)
+			}
 			pkgRevMeta = meta.PackageRevisionMeta{
 				Name:      nn.Name,
 				Namespace: nn.Namespace,

diff --git a/porch/pkg/git/git.go b/porch/pkg/git/git.go
@@ -316,7 +316,9 @@ func (r *gitRepository) listPackageRevisions(ctx context.Context, filter reposit
 				if err != nil {
 					return nil, fmt.Errorf("failed to load package draft %q: %w", name.String(), err)
 				}
-				draftLoaded += 1
+				if draft != nil {
+					draftLoaded += 1
+				}
 			}
 			if draft != nil {
 				drafts = append(drafts, draft)
@@ -342,7 +344,9 @@ func (r *gitRepository) listPackageRevisions(ctx context.Context, filter reposit
 					// this tag is not associated with any package (e.g. could be a release tag)
 					continue
 				}
-				tagLoaded += 1
+				if tagged != nil {
+					tagLoaded += 1
+				}
 			}
 			if tagged != nil && filter.Matches(tagged) {
 				result = append(result, tagged)

diff --git a/porch/pkg/registry/porch/background.go b/porch/pkg/registry/porch/background.go
@@ -174,7 +174,7 @@ func (b *background) isSharedRepository(ctx context.Context, repo *configapi.Rep
 				return true, nil
 			}
 		case configapi.RepositoryTypeGit:
-			if r.Spec.Git.Repo == repo.Spec.Git.Repo {
+			if r.Spec.Git.Repo == repo.Spec.Git.Repo && r.Spec.Git.Directory == repo.Spec.Git.Directory {
 				return true, nil
 			}
 		default: