From 6c7ba830a6a279f07b639eaa9ff495bd1081ae24 Mon Sep 17 00:00:00 2001 From: liamfallon Date: Mon, 9 Dec 2024 16:11:41 +0000 Subject: [PATCH 1/2] Use Kubernetes metadata struct rather than Porch derived metadata struct --- pkg/cache/memory/cache_test.go | 4 +-- pkg/cache/memory/repository.go | 7 ++-- pkg/engine/engine.go | 5 +-- pkg/git/package.go | 7 ++-- pkg/meta/fake/memorystore.go | 23 ++++++------ pkg/meta/store.go | 48 +++++++++++--------------- pkg/oci/oci.go | 7 ++-- pkg/repository/fake/packagerevision.go | 8 ++--- pkg/repository/repository.go | 6 ++-- 9 files changed, 55 insertions(+), 60 deletions(-) diff --git a/pkg/cache/memory/cache_test.go b/pkg/cache/memory/cache_test.go index fb130d2b..ed8dd323 100644 --- a/pkg/cache/memory/cache_test.go +++ b/pkg/cache/memory/cache_test.go @@ -272,11 +272,11 @@ func createMetadataStoreFromArchive(t *testing.T, testPath, name string) meta.Me } if os.IsNotExist(err) { return &fakemeta.MemoryMetadataStore{ - Metas: []meta.PackageRevisionMeta{}, + Metas: []metav1.ObjectMeta{}, } } - var metas []meta.PackageRevisionMeta + var metas []metav1.ObjectMeta if err := yaml.Unmarshal(c, &metas); err != nil { t.Fatalf("Error unmarshalling metadata file for repository %s", name) } diff --git a/pkg/cache/memory/repository.go b/pkg/cache/memory/repository.go index d584a2d0..b50e511e 100644 --- a/pkg/cache/memory/repository.go +++ b/pkg/cache/memory/repository.go @@ -28,6 +28,7 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/trace" "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" "k8s.io/klog/v2" @@ -260,7 +261,7 @@ func (r *cachedRepository) createMainPackageRevision(ctx context.Context, update // Create the package if it doesn't exist _, err := r.metadataStore.Get(ctx, pkgRevMetaNN) if errors.IsNotFound(err) { - pkgRevMeta := meta.PackageRevisionMeta{ + pkgRevMeta := metav1.ObjectMeta{ Name: updatedMain.KubeObjectName(), Namespace: updatedMain.KubeObjectNamespace(), } @@ -426,7 +427,7 @@ func (r *cachedRepository) refreshAllCachedPackages(ctx context.Context) (map[re return nil, nil, err } // Create a map so we can quickly check if a specific PackageRevisionMeta exists. - existingPkgRevCRsMap := make(map[string]meta.PackageRevisionMeta) + existingPkgRevCRsMap := make(map[string]metav1.ObjectMeta) for i := range existingPkgRevCRs { pr := existingPkgRevCRs[i] existingPkgRevCRsMap[pr.Name] = pr @@ -497,7 +498,7 @@ func (r *cachedRepository) refreshAllCachedPackages(ctx context.Context) (map[re // a corresponding PackageRev CR. for pkgRevName, pkgRev := range newPackageRevisionNames { if _, found := existingPkgRevCRsMap[pkgRevName]; !found { - pkgRevMeta := meta.PackageRevisionMeta{ + pkgRevMeta := metav1.ObjectMeta{ Name: pkgRevName, Namespace: r.repoSpec.Namespace, } diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 2ccc3fc4..e67179f1 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -30,6 +30,7 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/trace" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" "k8s.io/klog/v2" @@ -187,7 +188,7 @@ func (cad *cadEngine) CreatePackageRevision(ctx context.Context, repositoryObj * if err != nil { return nil, err } - pkgRevMeta := meta.PackageRevisionMeta{ + pkgRevMeta := metav1.ObjectMeta{ Name: repoPkgRev.KubeObjectName(), Namespace: repoPkgRev.KubeObjectNamespace(), Labels: obj.Labels, @@ -325,7 +326,7 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, version string, } func (cad *cadEngine) updatePkgRevMeta(ctx context.Context, repoPkgRev repository.PackageRevision, apiPkgRev *api.PackageRevision) error { - pkgRevMeta := meta.PackageRevisionMeta{ + pkgRevMeta := metav1.ObjectMeta{ Name: repoPkgRev.KubeObjectName(), Namespace: repoPkgRev.KubeObjectNamespace(), Labels: apiPkgRev.Labels, diff --git a/pkg/git/package.go b/pkg/git/package.go index be203b4d..64ff3151 100644 --- a/pkg/git/package.go +++ b/pkg/git/package.go @@ -29,7 +29,6 @@ import ( "github.com/nephio-project/porch/api/porch/v1alpha1" "github.com/nephio-project/porch/internal/kpt/pkg" kptfile "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1" - "github.com/nephio-project/porch/pkg/meta" "github.com/nephio-project/porch/pkg/repository" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -51,7 +50,7 @@ type gitPackageRevision struct { tree plumbing.Hash // Cached tree of the package itself, some descendent of commit.Tree() commit plumbing.Hash // Current version of the package (commit sha) tasks []v1alpha1.Task - metadata meta.PackageRevisionMeta + metadata metav1.ObjectMeta } var _ repository.PackageRevision = &gitPackageRevision{} @@ -318,11 +317,11 @@ func (p *gitPackageRevision) UpdateLifecycle(ctx context.Context, new v1alpha1.P return p.repo.UpdateLifecycle(ctx, p, new) } -func (p *gitPackageRevision) GetMeta() meta.PackageRevisionMeta { +func (p *gitPackageRevision) GetMeta() metav1.ObjectMeta { return p.metadata } -func (p *gitPackageRevision) SetMeta(metadata meta.PackageRevisionMeta) { +func (p *gitPackageRevision) SetMeta(metadata metav1.ObjectMeta) { p.metadata = metadata } diff --git a/pkg/meta/fake/memorystore.go b/pkg/meta/fake/memorystore.go index a3dd72a3..dd48e66d 100644 --- a/pkg/meta/fake/memorystore.go +++ b/pkg/meta/fake/memorystore.go @@ -20,6 +20,7 @@ import ( configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" "github.com/nephio-project/porch/pkg/meta" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" ) @@ -27,28 +28,28 @@ import ( // MemoryMetadataStore is an in-memory implementation of the MetadataStore interface. It // means metadata about packagerevisions will be stored in memory, which is useful for testing. type MemoryMetadataStore struct { - Metas []meta.PackageRevisionMeta + Metas []metav1.ObjectMeta } var _ meta.MetadataStore = &MemoryMetadataStore{} -func (m *MemoryMetadataStore) Get(ctx context.Context, namespacedName types.NamespacedName) (meta.PackageRevisionMeta, error) { +func (m *MemoryMetadataStore) Get(ctx context.Context, namespacedName types.NamespacedName) (metav1.ObjectMeta, error) { for _, meta := range m.Metas { if meta.Name == namespacedName.Name && meta.Namespace == namespacedName.Namespace { return meta, nil } } - return meta.PackageRevisionMeta{}, apierrors.NewNotFound( + return metav1.ObjectMeta{}, apierrors.NewNotFound( schema.GroupResource{Group: "config.kpt.dev", Resource: "packagerevisions"}, namespacedName.Name, ) } -func (m *MemoryMetadataStore) List(ctx context.Context, repo *configapi.Repository) ([]meta.PackageRevisionMeta, error) { +func (m *MemoryMetadataStore) List(ctx context.Context, repo *configapi.Repository) ([]metav1.ObjectMeta, error) { return m.Metas, nil } -func (m *MemoryMetadataStore) Create(ctx context.Context, pkgRevMeta meta.PackageRevisionMeta, repoName string, pkgRevUID types.UID) (meta.PackageRevisionMeta, error) { +func (m *MemoryMetadataStore) Create(ctx context.Context, pkgRevMeta metav1.ObjectMeta, repoName string, pkgRevUID types.UID) (metav1.ObjectMeta, error) { for _, m := range m.Metas { if m.Name == pkgRevMeta.Name && m.Namespace == pkgRevMeta.Namespace { return m, apierrors.NewAlreadyExists( @@ -61,7 +62,7 @@ func (m *MemoryMetadataStore) Create(ctx context.Context, pkgRevMeta meta.Packag return pkgRevMeta, nil } -func (m *MemoryMetadataStore) Update(ctx context.Context, pkgRevMeta meta.PackageRevisionMeta) (meta.PackageRevisionMeta, error) { +func (m *MemoryMetadataStore) Update(ctx context.Context, pkgRevMeta metav1.ObjectMeta) (metav1.ObjectMeta, error) { i := -1 for j, m := range m.Metas { if m.Name == pkgRevMeta.Name && m.Namespace == pkgRevMeta.Namespace { @@ -69,7 +70,7 @@ func (m *MemoryMetadataStore) Update(ctx context.Context, pkgRevMeta meta.Packag } } if i < 0 { - return meta.PackageRevisionMeta{}, apierrors.NewNotFound( + return metav1.ObjectMeta{}, apierrors.NewNotFound( schema.GroupResource{Group: "config.porch.kpt.dev", Resource: "packagerevisions"}, pkgRevMeta.Name, ) @@ -78,10 +79,10 @@ func (m *MemoryMetadataStore) Update(ctx context.Context, pkgRevMeta meta.Packag return pkgRevMeta, nil } -func (m *MemoryMetadataStore) Delete(ctx context.Context, namespacedName types.NamespacedName, _ bool) (meta.PackageRevisionMeta, error) { - var metas []meta.PackageRevisionMeta +func (m *MemoryMetadataStore) Delete(ctx context.Context, namespacedName types.NamespacedName, _ bool) (metav1.ObjectMeta, error) { + var metas []metav1.ObjectMeta found := false - var deletedMeta meta.PackageRevisionMeta + var deletedMeta metav1.ObjectMeta for _, m := range m.Metas { if m.Name == namespacedName.Name && m.Namespace == namespacedName.Namespace { found = true @@ -91,7 +92,7 @@ func (m *MemoryMetadataStore) Delete(ctx context.Context, namespacedName types.N } } if !found { - return meta.PackageRevisionMeta{}, apierrors.NewNotFound( + return metav1.ObjectMeta{}, apierrors.NewNotFound( schema.GroupResource{Group: "config.kpt.dev", Resource: "packagerevisions"}, namespacedName.Name, ) diff --git a/pkg/meta/store.go b/pkg/meta/store.go index bbb6e402..d41805ce 100644 --- a/pkg/meta/store.go +++ b/pkg/meta/store.go @@ -45,11 +45,11 @@ var ( // examples of metadata we want to keep is labels, annotations, owner references, and // finalizers. type MetadataStore interface { - Get(ctx context.Context, namespacedName types.NamespacedName) (PackageRevisionMeta, error) - List(ctx context.Context, repo *configapi.Repository) ([]PackageRevisionMeta, error) - Create(ctx context.Context, pkgRevMeta PackageRevisionMeta, repoName string, pkgRevUID types.UID) (PackageRevisionMeta, error) - Update(ctx context.Context, pkgRevMeta PackageRevisionMeta) (PackageRevisionMeta, error) - Delete(ctx context.Context, namespacedName types.NamespacedName, clearFinalizer bool) (PackageRevisionMeta, error) + Get(ctx context.Context, namespacedName types.NamespacedName) (metav1.ObjectMeta, error) + List(ctx context.Context, repo *configapi.Repository) ([]metav1.ObjectMeta, error) + Create(ctx context.Context, pkgRevMeta metav1.ObjectMeta, repoName string, pkgRevUID types.UID) (metav1.ObjectMeta, error) + Update(ctx context.Context, pkgRevMeta metav1.ObjectMeta) (metav1.ObjectMeta, error) + Delete(ctx context.Context, namespacedName types.NamespacedName, clearFinalizer bool) (metav1.ObjectMeta, error) } // PackageRevisionMeta contains metadata about a specific PackageRevision. The @@ -78,20 +78,20 @@ type crdMetadataStore struct { coreClient client.Client } -func (c *crdMetadataStore) Get(ctx context.Context, namespacedName types.NamespacedName) (PackageRevisionMeta, error) { +func (c *crdMetadataStore) Get(ctx context.Context, namespacedName types.NamespacedName) (metav1.ObjectMeta, error) { ctx, span := tracer.Start(ctx, "crdMetadataStore::Get", trace.WithAttributes()) defer span.End() var internalPkgRev internalapi.PackageRev err := c.coreClient.Get(ctx, namespacedName, &internalPkgRev) if err != nil { - return PackageRevisionMeta{}, err + return metav1.ObjectMeta{}, err } return toPackageRevisionMeta(&internalPkgRev), nil } -func (c *crdMetadataStore) List(ctx context.Context, repo *configapi.Repository) ([]PackageRevisionMeta, error) { +func (c *crdMetadataStore) List(ctx context.Context, repo *configapi.Repository) ([]metav1.ObjectMeta, error) { ctx, span := tracer.Start(ctx, "crdMetadataStore::List", trace.WithAttributes()) defer span.End() @@ -100,14 +100,14 @@ func (c *crdMetadataStore) List(ctx context.Context, repo *configapi.Repository) if err != nil { return nil, err } - var pkgRevMetas []PackageRevisionMeta + var pkgRevMetas []metav1.ObjectMeta for _, ipr := range internalPkgRevList.Items { pkgRevMetas = append(pkgRevMetas, toPackageRevisionMeta(&ipr)) } return pkgRevMetas, nil } -func (c *crdMetadataStore) Create(ctx context.Context, pkgRevMeta PackageRevisionMeta, repoName string, pkgRevUID types.UID) (PackageRevisionMeta, error) { +func (c *crdMetadataStore) Create(ctx context.Context, pkgRevMeta metav1.ObjectMeta, repoName string, pkgRevUID types.UID) (metav1.ObjectMeta, error) { ctx, span := tracer.Start(ctx, "crdMetadataStore::Create", trace.WithAttributes()) defer span.End() @@ -141,12 +141,12 @@ func (c *crdMetadataStore) Create(ctx context.Context, pkgRevMeta PackageRevisio if apierrors.IsAlreadyExists(err) { return c.Update(ctx, pkgRevMeta) } - return PackageRevisionMeta{}, err + return metav1.ObjectMeta{}, err } return toPackageRevisionMeta(&internalPkgRev), nil } -func (c *crdMetadataStore) Update(ctx context.Context, pkgRevMeta PackageRevisionMeta) (PackageRevisionMeta, error) { +func (c *crdMetadataStore) Update(ctx context.Context, pkgRevMeta metav1.ObjectMeta) (metav1.ObjectMeta, error) { ctx, span := tracer.Start(ctx, "crdMetadataStore::Update", trace.WithAttributes()) defer span.End() @@ -157,7 +157,7 @@ func (c *crdMetadataStore) Update(ctx context.Context, pkgRevMeta PackageRevisio } err := c.coreClient.Get(ctx, namespacedName, &internalPkgRev) if err != nil { - return PackageRevisionMeta{}, err + return metav1.ObjectMeta{}, err } // Copy updated labels to the CR and add the repository label @@ -185,12 +185,12 @@ func (c *crdMetadataStore) Update(ctx context.Context, pkgRevMeta PackageRevisio klog.Infof("Updating packagerev %s/%s", internalPkgRev.Namespace, internalPkgRev.Name) if err := c.coreClient.Update(ctx, &internalPkgRev); err != nil { - return PackageRevisionMeta{}, err + return metav1.ObjectMeta{}, err } return toPackageRevisionMeta(&internalPkgRev), nil } -func (c *crdMetadataStore) Delete(ctx context.Context, namespacedName types.NamespacedName, clearFinalizers bool) (PackageRevisionMeta, error) { +func (c *crdMetadataStore) Delete(ctx context.Context, namespacedName types.NamespacedName, clearFinalizers bool) (metav1.ObjectMeta, error) { ctx, span := tracer.Start(ctx, "crdMetadataStore::Delete", trace.WithAttributes()) defer span.End() @@ -210,17 +210,17 @@ func (c *crdMetadataStore) Delete(ctx context.Context, namespacedName types.Name return nil }) if retriedErr != nil { - return PackageRevisionMeta{}, retriedErr + return metav1.ObjectMeta{}, retriedErr } klog.Infof("Deleting packagerev %s/%s", internalPkgRev.Namespace, internalPkgRev.Name) if err := c.coreClient.Delete(ctx, &internalPkgRev); err != nil { - return PackageRevisionMeta{}, err + return metav1.ObjectMeta{}, err } return toPackageRevisionMeta(&internalPkgRev), nil } -func toPackageRevisionMeta(internalPkgRev *internalapi.PackageRev) PackageRevisionMeta { +func toPackageRevisionMeta(internalPkgRev *internalapi.PackageRev) metav1.ObjectMeta { labels := internalPkgRev.Labels delete(labels, PkgRevisionRepoLabel) @@ -232,6 +232,7 @@ func toPackageRevisionMeta(internalPkgRev *internalapi.PackageRev) PackageRevisi ownerReferences = append(ownerReferences, or) } } + internalPkgRev.OwnerReferences = ownerReferences var finalizers []string for _, f := range internalPkgRev.Finalizers { @@ -240,15 +241,8 @@ func toPackageRevisionMeta(internalPkgRev *internalapi.PackageRev) PackageRevisi } } - return PackageRevisionMeta{ - Name: internalPkgRev.Name, - Namespace: internalPkgRev.Namespace, - Labels: labels, - Annotations: internalPkgRev.Annotations, - Finalizers: finalizers, - OwnerReferences: ownerReferences, - DeletionTimestamp: internalPkgRev.DeletionTimestamp, - } + internalPkgRev.Finalizers = finalizers + return internalPkgRev.ObjectMeta } func isPackageRevOwnerRef(or metav1.OwnerReference, pkgRevName string) bool { diff --git a/pkg/oci/oci.go b/pkg/oci/oci.go index 0983dbc5..a9c6be4e 100644 --- a/pkg/oci/oci.go +++ b/pkg/oci/oci.go @@ -31,7 +31,6 @@ import ( configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" "github.com/nephio-project/porch/internal/kpt/pkg" kptfile "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1" - "github.com/nephio-project/porch/pkg/meta" "github.com/nephio-project/porch/pkg/repository" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -262,7 +261,7 @@ type ociPackageRevision struct { created time.Time resourceVersion string uid types.UID - metadata meta.PackageRevisionMeta + metadata metav1.ObjectMeta parent *ociRepository tasks []v1alpha1.Task @@ -425,10 +424,10 @@ func (p *ociPackageRevision) UpdateLifecycle(ctx context.Context, new v1alpha1.P return nil } -func (p *ociPackageRevision) GetMeta() meta.PackageRevisionMeta { +func (p *ociPackageRevision) GetMeta() metav1.ObjectMeta { return p.metadata } -func (p *ociPackageRevision) SetMeta(metadata meta.PackageRevisionMeta) { +func (p *ociPackageRevision) SetMeta(metadata metav1.ObjectMeta) { p.metadata = metadata } diff --git a/pkg/repository/fake/packagerevision.go b/pkg/repository/fake/packagerevision.go index 8f962065..bd271d67 100644 --- a/pkg/repository/fake/packagerevision.go +++ b/pkg/repository/fake/packagerevision.go @@ -19,8 +19,8 @@ import ( "github.com/nephio-project/porch/api/porch/v1alpha1" kptfile "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1" - "github.com/nephio-project/porch/pkg/meta" "github.com/nephio-project/porch/pkg/repository" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) @@ -91,9 +91,9 @@ func (f *FakePackageRevision) UpdateLifecycle(context.Context, v1alpha1.PackageR return nil } -func (f *FakePackageRevision) GetMeta() meta.PackageRevisionMeta { - return meta.PackageRevisionMeta{} +func (f *FakePackageRevision) GetMeta() metav1.ObjectMeta { + return metav1.ObjectMeta{} } -func (f *FakePackageRevision) SetMeta(meta.PackageRevisionMeta) { +func (f *FakePackageRevision) SetMeta(metav1.ObjectMeta) { } diff --git a/pkg/repository/repository.go b/pkg/repository/repository.go index d73e90d7..154505f4 100644 --- a/pkg/repository/repository.go +++ b/pkg/repository/repository.go @@ -21,7 +21,7 @@ import ( "github.com/go-git/go-git/v5/plumbing/transport" "github.com/nephio-project/porch/api/porch/v1alpha1" kptfile "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1" - "github.com/nephio-project/porch/pkg/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) @@ -100,10 +100,10 @@ type PackageRevision interface { ToMainPackageRevision() PackageRevision // Get the Kubernetes metadata for the package revision - GetMeta() meta.PackageRevisionMeta + GetMeta() metav1.ObjectMeta // Set the Kubernetes metadata for the package revision - SetMeta(meta.PackageRevisionMeta) + SetMeta(metav1.ObjectMeta) } // Package is an abstract package. From 9e440382a5159cf57d39fa9852742fec1745a395 Mon Sep 17 00:00:00 2001 From: liamfallon Date: Tue, 10 Dec 2024 11:48:12 +0000 Subject: [PATCH 2/2] Removed unused PackageRevisionMeta struct --- pkg/meta/store.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/pkg/meta/store.go b/pkg/meta/store.go index d41805ce..6a172c78 100644 --- a/pkg/meta/store.go +++ b/pkg/meta/store.go @@ -52,18 +52,6 @@ type MetadataStore interface { Delete(ctx context.Context, namespacedName types.NamespacedName, clearFinalizer bool) (metav1.ObjectMeta, error) } -// PackageRevisionMeta contains metadata about a specific PackageRevision. The -// PackageRevision is identified by the name and namespace. -type PackageRevisionMeta struct { - Name string - Namespace string - Labels map[string]string - Annotations map[string]string - Finalizers []string - OwnerReferences []metav1.OwnerReference - DeletionTimestamp *metav1.Time -} - var _ MetadataStore = &crdMetadataStore{} func NewCrdMetadataStore(coreClient client.Client) *crdMetadataStore {