Merge pull request #140 from Nordix/refactor-porch-core

Refactor Porch core packages for readability and maintainability

pkg/cache/cache.go

@@ -1,3 +1,17 @@
+// Copyright 2024 The Nephio Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 package cache
 
 import (
@@ -8,24 +22,6 @@ import (
 )
 
 type Cache interface {
-	OpenRepository(ctx context.Context, repositorySpec *configapi.Repository) (CachedRepository, error)
+	OpenRepository(ctx context.Context, repositorySpec *configapi.Repository) (repository.Repository, error)
 	CloseRepository(repositorySpec *configapi.Repository, allRepos []configapi.Repository) error
 }
-
-type CachedRepository interface {
-	repository.Repository
-	RefreshCache(ctx context.Context) error
-}
-
-type CachedPackageRevision interface {
-	repository.PackageRevision
-}
-
-type CachedPackageDraft interface {
-	repository.PackageDraft
-}
-
-// Remove?
-type CachedPackage interface {
-	repository.Package
-}

pkg/cache/fake/objectnotifier.go

@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2022, 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -15,13 +15,12 @@
 package fake
 
 import (
-	"github.com/nephio-project/porch/pkg/meta"
 	"github.com/nephio-project/porch/pkg/repository"
 	"k8s.io/apimachinery/pkg/watch"
 )
 
 type ObjectNotifier struct{}
 
-func (o *ObjectNotifier) NotifyPackageRevisionChange(watch.EventType, repository.PackageRevision, meta.PackageRevisionMeta) int {
+func (o *ObjectNotifier) NotifyPackageRevisionChange(watch.EventType, repository.PackageRevision) int {
 	return 0
 }

pkg/cache/memory/cache.go

@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2022, 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -58,7 +58,7 @@ type Cache struct {
 var _ cache.Cache = &Cache{}
 
 type objectNotifier interface {
-	NotifyPackageRevisionChange(eventType watch.EventType, obj repository.PackageRevision, objMeta meta.PackageRevisionMeta) int
+	NotifyPackageRevisionChange(eventType watch.EventType, obj repository.PackageRevision) int
 }
 
 type CacheOptions struct {
@@ -105,7 +105,7 @@ func getCacheKey(repositorySpec *configapi.Repository) (string, error) {
 	}
 }
 
-func (c *Cache) OpenRepository(ctx context.Context, repositorySpec *configapi.Repository) (cache.CachedRepository, error) {
+func (c *Cache) OpenRepository(ctx context.Context, repositorySpec *configapi.Repository) (repository.Repository, error) {
 	ctx, span := tracer.Start(ctx, "Cache::OpenRepository", trace.WithAttributes())
 	defer span.End()
 

pkg/cache/memory/cache_test.go

@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2022, 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -25,7 +25,6 @@ import (
 	"github.com/google/go-cmp/cmp"
 	api "github.com/nephio-project/porch/api/porch/v1alpha1"
 	"github.com/nephio-project/porch/api/porchconfig/v1alpha1"
-	"github.com/nephio-project/porch/pkg/cache"
 
 	fakecache "github.com/nephio-project/porch/pkg/cache/fake"
 	"github.com/nephio-project/porch/pkg/git"
@@ -216,7 +215,7 @@ func TestDeletePublishedMain(t *testing.T) {
 
 }
 
-func openRepositoryFromArchive(t *testing.T, ctx context.Context, testPath, name string) cache.CachedRepository {
+func openRepositoryFromArchive(t *testing.T, ctx context.Context, testPath, name string) repository.Repository {
 	t.Helper()
 
 	tempdir := t.TempDir()

pkg/cache/memory/draft.go

@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2022, 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -18,7 +18,6 @@ import (
 	"context"
 
 	"github.com/nephio-project/porch/api/porch/v1alpha1"
-	"github.com/nephio-project/porch/pkg/cache"
 	"github.com/nephio-project/porch/pkg/repository"
 	"go.opentelemetry.io/otel/trace"
 )
@@ -29,7 +28,6 @@ type cachedDraft struct {
 }
 
 var _ repository.PackageDraft = &cachedDraft{}
-var _ cache.CachedPackageDraft = &cachedDraft{}
 
 func (cd *cachedDraft) Close(ctx context.Context, version string) (repository.PackageRevision, error) {
 	ctx, span := tracer.Start(ctx, "cachedDraft::Close", trace.WithAttributes())

pkg/cache/memory/package.go

@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2022, 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -15,7 +15,6 @@
 package memory
 
 import (
-	"github.com/nephio-project/porch/pkg/cache"
 	"github.com/nephio-project/porch/pkg/repository"
 )
 
@@ -26,7 +25,6 @@ import (
 // between Git and OCI.
 
 var _ repository.Package = &cachedPackage{}
-var _ cache.CachedPackage = &cachedPackage{}
 
 type cachedPackage struct {
 	repository.Package

pkg/cache/memory/packagerevision.go

@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2022, 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,8 +17,7 @@ package memory
 import (
 	"context"
 
-	"github.com/nephio-project/porch/api/porch/v1alpha1"
-	"github.com/nephio-project/porch/pkg/cache"
+	api "github.com/nephio-project/porch/api/porch/v1alpha1"
 	"github.com/nephio-project/porch/pkg/repository"
 )
 
@@ -29,26 +28,33 @@ import (
 // between Git and OCI.
 
 var _ repository.PackageRevision = &cachedPackageRevision{}
-var _ cache.CachedPackageRevision = &cachedPackageRevision{}
 
 type cachedPackageRevision struct {
 	repository.PackageRevision
 	isLatestRevision bool
 }
 
-func (c *cachedPackageRevision) GetPackageRevision(ctx context.Context) (*v1alpha1.PackageRevision, error) {
-	rev, err := c.PackageRevision.GetPackageRevision(ctx)
+func (c *cachedPackageRevision) GetPackageRevision(ctx context.Context) (*api.PackageRevision, error) {
+	apiPR, err := c.PackageRevision.GetPackageRevision(ctx)
 	if err != nil {
 		return nil, err
 	}
+
+	apiPR.Annotations = c.GetMeta().Annotations
+	apiPR.Finalizers = c.GetMeta().Finalizers
+	apiPR.OwnerReferences = c.GetMeta().OwnerReferences
+	apiPR.DeletionTimestamp = c.GetMeta().DeletionTimestamp
+	apiPR.Labels = c.GetMeta().Labels
+
 	if c.isLatestRevision {
 		// copy the labels in case the cached object is being read by another go routine
-		labels := make(map[string]string, len(rev.Labels))
-		for k, v := range rev.Labels {
+		labels := make(map[string]string, len(apiPR.Labels))
+		for k, v := range apiPR.Labels {
 			labels[k] = v
 		}
-		labels[v1alpha1.LatestPackageRevisionKey] = v1alpha1.LatestPackageRevisionValue
-		rev.Labels = labels
+		labels[api.LatestPackageRevisionKey] = api.LatestPackageRevisionValue
+		apiPR.Labels = labels
 	}
-	return rev, nil
+
+	return apiPR, nil
 }

pkg/cache/memory/repository.go

@@ -22,13 +22,12 @@ import (
 
 	"github.com/nephio-project/porch/api/porch/v1alpha1"
 	configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1"
-	"github.com/nephio-project/porch/pkg/cache"
 	"github.com/nephio-project/porch/pkg/git"
 	"github.com/nephio-project/porch/pkg/meta"
 	"github.com/nephio-project/porch/pkg/repository"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/trace"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/klog/v2"
@@ -43,7 +42,6 @@ var tracer = otel.Tracer("cache")
 // between Git and OCI.
 
 var _ repository.Repository = &cachedRepository{}
-var _ cache.CachedRepository = &cachedRepository{}
 
 type cachedRepository struct {
 	id string
@@ -85,7 +83,7 @@ func newRepository(id string, repoSpec *configapi.Repository, repo repository.Re
 	return r
 }
 
-func (r *cachedRepository) RefreshCache(ctx context.Context) error {
+func (r *cachedRepository) Refresh(ctx context.Context) error {
 
 	_, _, err := r.refreshAllCachedPackages(ctx)
 
@@ -261,7 +259,7 @@ func (r *cachedRepository) createMainPackageRevision(ctx context.Context, update
 
 	// Create the package if it doesn't exist
 	_, err := r.metadataStore.Get(ctx, pkgRevMetaNN)
-	if apierrors.IsNotFound(err) {
+	if errors.IsNotFound(err) {
 		pkgRevMeta := meta.PackageRevisionMeta{
 			Name:      updatedMain.KubeObjectName(),
 			Namespace: updatedMain.KubeObjectNamespace(),
@@ -346,19 +344,15 @@ func (r *cachedRepository) Close() error {
 		// the repository, so we have to just delete the PackageRevision regardless of any
 		// finalizers.
 		klog.Infof("repo %s: deleting packagerev %s/%s because repository is closed", r.id, nn.Namespace, nn.Name)
-		pkgRevMeta, err := r.metadataStore.Delete(context.TODO(), nn, true)
+		_, err := r.metadataStore.Delete(context.TODO(), nn, true)
 		if err != nil {
 			// 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("repo %s: error deleting packagerev for %s: %v", r.id, nn.Name, err)
-			pkgRevMeta = meta.PackageRevisionMeta{
-				Name:      nn.Name,
-				Namespace: nn.Namespace,
-			}
 		}
 		klog.Infof("repo %s: successfully deleted packagerev %s/%s", r.id, nn.Namespace, nn.Name)
-		sent += r.objectNotifier.NotifyPackageRevisionChange(watch.Deleted, pr, pkgRevMeta)
+		sent += r.objectNotifier.NotifyPackageRevisionChange(watch.Deleted, pr)
 	}
 	klog.Infof("repo %s: sent %d notifications for %d package revisions during close", r.id, sent, len(r.cachedPackageRevisions))
 	return r.repo.Close()
@@ -474,7 +468,7 @@ func (r *cachedRepository) refreshAllCachedPackages(ctx context.Context) (map[re
 				Name:      prm.Name,
 				Namespace: prm.Namespace,
 			}, true); err != nil {
-				if !apierrors.IsNotFound(err) {
+				if !errors.IsNotFound(err) {
 					// This will be retried the next time the sync runs.
 					klog.Warningf("repo %s: unable to delete PackageRev CR for %s/%s: %v",
 						r.id, prm.Name, prm.Namespace, err)
@@ -488,15 +482,11 @@ func (r *cachedRepository) refreshAllCachedPackages(ctx context.Context) (map[re
 	modSent := 0
 	for kname, newPackage := range newPackageRevisionNames {
 		oldPackage := oldPackageRevisionNames[kname]
-		metaPackage, found := existingPkgRevCRsMap[newPackage.KubeObjectName()]
-		if !found {
-			klog.Warningf("no PackageRev CR found for PackageRevision %s", newPackage.KubeObjectName())
-		}
 		if oldPackage == nil {
-			addSent += r.objectNotifier.NotifyPackageRevisionChange(watch.Added, newPackage, metaPackage)
+			addSent += r.objectNotifier.NotifyPackageRevisionChange(watch.Added, newPackage)
 		} else {
 			if oldPackage.ResourceVersion() != newPackage.ResourceVersion() {
-				modSent += r.objectNotifier.NotifyPackageRevisionChange(watch.Modified, newPackage, metaPackage)
+				modSent += r.objectNotifier.NotifyPackageRevisionChange(watch.Modified, newPackage)
 			}
 		}
 	}
@@ -529,17 +519,7 @@ func (r *cachedRepository) refreshAllCachedPackages(ctx context.Context) (map[re
 			}
 			klog.Infof("repo %s: deleting PackageRev %s/%s because PackageRevision was removed from SoT",
 				r.id, nn.Namespace, nn.Name)
-			metaPackage, err := r.metadataStore.Delete(ctx, nn, true)
-			if err != nil {
-				if !apierrors.IsNotFound(err) {
-					klog.Warningf("repo %s: error deleting PkgRevMeta %s: %v", r.id, nn, err)
-				}
-				metaPackage = meta.PackageRevisionMeta{
-					Name:      nn.Name,
-					Namespace: nn.Namespace,
-				}
-			}
-			delSent += r.objectNotifier.NotifyPackageRevisionChange(watch.Deleted, oldPackage, metaPackage)
+			delSent += r.objectNotifier.NotifyPackageRevisionChange(watch.Deleted, oldPackage)
 		}
 	}
 	klog.Infof("repo %s: addSent %d, modSent %d, delSent for %d old and %d new repo packages", r.id, addSent, modSent, len(oldPackageRevisionNames), len(newPackageRevisionNames))

pkg/cache/memory/util.go

@@ -1,4 +1,4 @@
-// Copyright 2022 The kpt and Nephio Authors
+// Copyright 2022, 2024 The kpt and Nephio Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -57,6 +57,7 @@ func identifyLatestRevisions(result map[repository.PackageRevisionKey]*cachedPac
 			latest[currentKey.Package] = current
 		}
 	}
+
 	// Mark the winners as latest
 	for _, v := range latest {
 		v.isLatestRevision = true