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.

Sign up for GitHub

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

Jump to bottom

[release-v0.4] (bugfix): add label selector to sync list queries #123

Merged
merged 7 commits into from
Jul 18, 2023
Merged

[release-v0.4] (bugfix): add label selector to sync list queries #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
eaca4f4
add label selector to sync list queries
everettraven Jul 13, 2023
617174a
add unit test
everettraven Jul 13, 2023
d0be287
fix lint issues
everettraven Jul 14, 2023
0434e04
harden the sync test case by using multiple catalogs
everettraven Jul 14, 2023
f6fe216
clean up test implementation
everettraven Jul 14, 2023
7b408ef
address review comments
everettraven Jul 14, 2023
098fbaa
remove all label len checks
everettraven Jul 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/controllers/core/catalog_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ func (r *CatalogReconciler) syncBundleMetadata(ctx context.Context, declCfg *dec
}

var existingBundles v1alpha1.BundleMetadataList
if err := r.List(ctx, &existingBundles); err != nil {
if err := r.List(ctx, &existingBundles, &client.MatchingLabels{"catalog": catalog.Name}); err != nil {
return fmt.Errorf("list existing bundle metadatas: %v", err)
}
for i := range existingBundles.Items {
Expand Down Expand Up @@ -360,7 +360,7 @@ func (r *CatalogReconciler) syncPackages(ctx context.Context, declCfg *declcfg.D
}

var existingPkgs v1alpha1.PackageList
if err := r.List(ctx, &existingPkgs); err != nil {
if err := r.List(ctx, &existingPkgs, &client.MatchingLabels{"catalog": catalog.Name}); err != nil {
return fmt.Errorf("list existing packages: %v", err)
}
for i := range existingPkgs.Items {
Expand Down Expand Up @@ -445,7 +445,7 @@ func (r *CatalogReconciler) syncCatalogMetadata(ctx context.Context, fsys fs.FS,
}

var existingCatalogMetadataObjs v1alpha1.CatalogMetadataList
if err := r.List(ctx, &existingCatalogMetadataObjs); err != nil {
if err := r.List(ctx, &existingCatalogMetadataObjs, &client.MatchingLabels{"catalog": catalog.Name}); err != nil {
return fmt.Errorf("list existing catalog metadata: %v", err)
}
for i, existingCatalogMetadata := range existingCatalogMetadataObjs.Items {
Expand Down
211 changes: 186 additions & 25 deletions pkg/controllers/core/catalog_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import (

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/onsi/gomega/format"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/rand"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/manager"

"github.com/operator-framework/catalogd/api/core/v1alpha1"
Expand Down Expand Up @@ -42,15 +44,14 @@ func (ms *MockSource) Unpack(_ context.Context, _ *v1alpha1.Catalog) (*source.Re
}

var _ = Describe("Catalogd Controller Test", func() {
format.MaxLength = 0
var (
ctx context.Context
reconciler *core.CatalogReconciler
mockSource *MockSource
)
BeforeEach(func() {
ctx = context.Background()
Expect(features.CatalogdFeatureGate.Set("PackagesBundleMetadataAPIs=true")).To(Succeed())
Expect(features.CatalogdFeatureGate.Set("CatalogMetadataAPI=true")).To(Succeed())
mockSource = &MockSource{}
reconciler = &core.CatalogReconciler{
Client: cl,
Expand Down Expand Up @@ -252,15 +253,39 @@ var _ = Describe("Catalogd Controller Test", func() {
State: source.StateUnpacked,
FS: filesys,
}
})

It("should set unpacking status to 'unpacked'", func() {
// reconcile
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey})
Expect(res).To(Equal(ctrl.Result{}))
Expect(err).ToNot(HaveOccurred())

// get the catalog and ensure status is set properly
cat := &v1alpha1.Catalog{}
Expect(cl.Get(ctx, catalogKey, cat)).To(Succeed())
Expect(cat.Status.ResolvedSource).ToNot(BeNil())
Expect(cat.Status.Phase).To(Equal(v1alpha1.PhaseUnpacked))
cond := meta.FindStatusCondition(cat.Status.Conditions, v1alpha1.TypeUnpacked)
Expect(cond).ToNot(BeNil())
Expect(cond.Reason).To(Equal(v1alpha1.ReasonUnpackSuccessful))
Expect(cond.Status).To(Equal(metav1.ConditionTrue))
})

AfterEach(func() {
if features.CatalogdFeatureGate.Enabled(features.PackagesBundleMetadataAPIs) {
When("PackagesBundleMetadataAPIs feature gate is enabled", func() {
BeforeEach(func() {
Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{
string(features.PackagesBundleMetadataAPIs): true,
string(features.CatalogMetadataAPI): false,
})).NotTo(HaveOccurred())

// reconcile
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey})
Expect(res).To(Equal(ctrl.Result{}))
Expect(err).ToNot(HaveOccurred())
})

AfterEach(func() {
// clean up package
pkg := &v1alpha1.Package{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -276,23 +301,12 @@ var _ = Describe("Catalogd Controller Test", func() {
},
}
Expect(cl.Delete(ctx, bm)).To(Succeed())
}
})

It("should set unpacking status to 'unpacked'", func() {
// get the catalog and ensure status is set properly
cat := &v1alpha1.Catalog{}
Expect(cl.Get(ctx, catalogKey, cat)).To(Succeed())
Expect(cat.Status.ResolvedSource).ToNot(BeNil())
Expect(cat.Status.Phase).To(Equal(v1alpha1.PhaseUnpacked))
cond := meta.FindStatusCondition(cat.Status.Conditions, v1alpha1.TypeUnpacked)
Expect(cond).ToNot(BeNil())
Expect(cond.Reason).To(Equal(v1alpha1.ReasonUnpackSuccessful))
Expect(cond.Status).To(Equal(metav1.ConditionTrue))
})
Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{
string(features.PackagesBundleMetadataAPIs): false,
string(features.CatalogMetadataAPI): false,
})).NotTo(HaveOccurred())
})

if features.CatalogdFeatureGate.Enabled(features.PackagesBundleMetadataAPIs) {
// TODO (rashmigottipati): Add testing of BundleMetadata sync process.
It("should create BundleMetadata resources", func() {
// validate bundlemetadata resources
bundlemetadatas := &v1alpha1.BundleMetadataList{}
Expand Down Expand Up @@ -324,9 +338,108 @@ var _ = Describe("Catalogd Controller Test", func() {
Expect(pack.Spec.Channels[0].Entries).To(HaveLen(1))
Expect(pack.Spec.Channels[0].Entries[0].Name).To(Equal(testBundleName))
})
}

if features.CatalogdFeatureGate.Enabled(features.CatalogMetadataAPI) {
When("creating another Catalog", func() {
var (
tempCatalog *v1alpha1.Catalog
tempTestBundleMetaName string
tempTestPackageMetaName string
)
BeforeEach(func() {
tempCatalog = &v1alpha1.Catalog{
ObjectMeta: metav1.ObjectMeta{Name: "tempedout"},
Spec: v1alpha1.CatalogSpec{
Source: v1alpha1.CatalogSource{
Type: "image",
Image: &v1alpha1.ImageSource{
Ref: "somecatalog:latest",
},
},
},
}

tempTestBundleMetaName = fmt.Sprintf("%s-%s", tempCatalog.Name, testBundleName)
tempTestPackageMetaName = fmt.Sprintf("%s-%s", tempCatalog.Name, testPackageName)

Expect(cl.Create(ctx, tempCatalog)).To(Succeed())
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "tempedout"}})
Expect(res).To(Equal(ctrl.Result{}))
Expect(err).ToNot(HaveOccurred())
})

AfterEach(func() {
Expect(cl.Delete(ctx, tempCatalog)).NotTo(HaveOccurred())
})

It("should not delete BundleMetadata belonging to a different catalog", func() {
bundlemetadata := &v1alpha1.BundleMetadata{}
Expect(cl.Get(ctx, client.ObjectKey{Name: testBundleMetaName}, bundlemetadata)).To(Succeed())
Expect(bundlemetadata.Name).To(Equal(testBundleMetaName))
Expect(bundlemetadata.Spec.Image).To(Equal(testBundleImage))
Expect(bundlemetadata.Spec.Catalog.Name).To(Equal(catalog.Name))
Expect(bundlemetadata.Spec.Package).To(Equal(testPackageName))
Expect(bundlemetadata.Spec.RelatedImages).To(HaveLen(1))
Expect(bundlemetadata.Spec.RelatedImages[0].Name).To(Equal(testBundleRelatedImageName))
Expect(bundlemetadata.Spec.RelatedImages[0].Image).To(Equal(testBundleRelatedImageImage))
Expect(bundlemetadata.Spec.Properties).To(HaveLen(1))

bundlemetadata = &v1alpha1.BundleMetadata{}
Expect(cl.Get(ctx, client.ObjectKey{Name: tempTestBundleMetaName}, bundlemetadata)).To(Succeed())
Expect(bundlemetadata.Name).To(Equal(tempTestBundleMetaName))
Expect(bundlemetadata.Spec.Image).To(Equal(testBundleImage))
Expect(bundlemetadata.Spec.Catalog.Name).To(Equal(tempCatalog.Name))
Expect(bundlemetadata.Spec.Package).To(Equal(testPackageName))
Expect(bundlemetadata.Spec.RelatedImages).To(HaveLen(1))
Expect(bundlemetadata.Spec.RelatedImages[0].Name).To(Equal(testBundleRelatedImageName))
Expect(bundlemetadata.Spec.RelatedImages[0].Image).To(Equal(testBundleRelatedImageImage))
Expect(bundlemetadata.Spec.Properties).To(HaveLen(1))
})

It("should not delete Packages belonging to a different catalog", func() {
// validate package resources
pack := &v1alpha1.Package{}
Expect(cl.Get(ctx, client.ObjectKey{Name: testPackageMetaName}, pack)).To(Succeed())
Expect(pack.Name).To(Equal(testPackageMetaName))
Expect(pack.Spec.DefaultChannel).To(Equal(testPackageDefaultChannel))
Expect(pack.Spec.Catalog.Name).To(Equal(catalog.Name))
Expect(pack.Spec.Channels).To(HaveLen(1))
Expect(pack.Spec.Channels[0].Name).To(Equal(testChannelName))
Expect(pack.Spec.Channels[0].Entries).To(HaveLen(1))
Expect(pack.Spec.Channels[0].Entries[0].Name).To(Equal(testBundleName))

pack = &v1alpha1.Package{}
Expect(cl.Get(ctx, client.ObjectKey{Name: tempTestPackageMetaName}, pack)).To(Succeed())
Expect(pack.Name).To(Equal(tempTestPackageMetaName))
Expect(pack.Spec.DefaultChannel).To(Equal(testPackageDefaultChannel))
Expect(pack.Spec.Catalog.Name).To(Equal(tempCatalog.Name))
Expect(pack.Spec.Channels).To(HaveLen(1))
Expect(pack.Spec.Channels[0].Name).To(Equal(testChannelName))
Expect(pack.Spec.Channels[0].Entries).To(HaveLen(1))
Expect(pack.Spec.Channels[0].Entries[0].Name).To(Equal(testBundleName))
})
})
})

When("the CatalogMetadataAPI feature gate is enabled", func() {
BeforeEach(func() {
Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{
string(features.CatalogMetadataAPI): true,
})).NotTo(HaveOccurred())

// reconcile
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey})
Expect(res).To(Equal(ctrl.Result{}))
Expect(err).ToNot(HaveOccurred())
})

AfterEach(func() {
// clean up catalogmetadata
Expect(cl.DeleteAllOf(ctx, &v1alpha1.CatalogMetadata{})).To(Succeed())
Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{
string(features.CatalogMetadataAPI): false,
})).NotTo(HaveOccurred())
})

// TODO (rashmigottipati): Add testing of CatalogMetadata sync process.
It("should create CatalogMetadata resources", func() {
catalogMetadatas := &v1alpha1.CatalogMetadataList{}
Expand All @@ -335,17 +448,65 @@ var _ = Describe("Catalogd Controller Test", func() {
for _, catalogMetadata := range catalogMetadatas.Items {
Expect(catalogMetadata.Name).To(ContainSubstring(catalogKey.Name))
Expect(catalogMetadata.Kind).To(Equal("CatalogMetadata"))
Expect(catalogMetadata.GetLabels()).To(HaveLen(5))
Expect(catalogMetadata.OwnerReferences).To(HaveLen(1))
Expect(catalogMetadata.OwnerReferences[0].Name).To(Equal(catalogKey.Name))
Expect(catalogMetadata.Spec.Catalog.Name).To(Equal(catalogKey.Name))
}
})
}

When("creating another Catalog", func() {
var (
tempCatalog *v1alpha1.Catalog
)
BeforeEach(func() {
tempCatalog = &v1alpha1.Catalog{
ObjectMeta: metav1.ObjectMeta{Name: "tempedout"},
Spec: v1alpha1.CatalogSpec{
Source: v1alpha1.CatalogSource{
Type: "image",
Image: &v1alpha1.ImageSource{
Ref: "somecatalog:latest",
},
},
},
}

Expect(cl.Create(ctx, tempCatalog)).To(Succeed())
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "tempedout"}})
Expect(res).To(Equal(ctrl.Result{}))
Expect(err).ToNot(HaveOccurred())
})

AfterEach(func() {
Expect(cl.Delete(ctx, tempCatalog)).NotTo(HaveOccurred())
})

It("should not delete CatalogMetadata belonging to a different catalog", func() {
catalogMetadatas := &v1alpha1.CatalogMetadataList{}
Expect(cl.List(ctx, catalogMetadatas)).To(Succeed())
Expect(catalogMetadatas.Items).To(HaveLen(6))
for _, catalogMetadata := range catalogMetadatas.Items {
for _, or := range catalogMetadata.GetOwnerReferences() {
if or.Kind == "Catalog" {
if or.Name == catalogKey.Name {
Expect(catalogMetadata.Name).To(ContainSubstring(catalogKey.Name))
Expect(catalogMetadata.Kind).To(Equal("CatalogMetadata"))
Expect(catalogMetadata.Spec.Catalog.Name).To(Equal(catalogKey.Name))
break
} else if or.Name == tempCatalog.Name {
Expect(catalogMetadata.Name).To(ContainSubstring(tempCatalog.Name))
Expect(catalogMetadata.Kind).To(Equal("CatalogMetadata"))
Expect(catalogMetadata.Spec.Catalog.Name).To(Equal(tempCatalog.Name))
break
}
}
}
}
})
})
})
})
})

})
})

Expand Down