Skip to content

Commit ef4febc

Browse files
ankitathomasankitathomas
committed
fixing unit tests, removing unused files
1 parent 9fc371c commit ef4febc

File tree

16 files changed

+83
-955
lines changed

16 files changed

+83
-955
lines changed

‎controllers/catalogsource_controller.go‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package controllers
33
import (
44
"context"
55
"fmt"
6+
"reflect"
67
"sync"
78
"time"
89

@@ -90,10 +91,11 @@ func (r *CatalogSourceReconciler) Reconcile(ctx context.Context, req ctrl.Reques
9091
entities, err := r.registry.ListEntities(ctx, catalogSource)
9192
if err != nil {
9293
r.recorder.Event(catalogSource, eventTypeWarning, eventReasonCacheUpdateFailed, fmt.Sprintf("Failed to update bundle cache from %s/%s: %v", catalogSource.GetNamespace(), catalogSource.GetName(), err))
93-
return ctrl.Result{Requeue: true}, err
94+
return ctrl.Result{Requeue: !isManagedCatalogSource(*catalogSource)}, err
95+
}
96+
if updated := r.updateCache(req.String(), entities); updated {
97+
r.recorder.Event(catalogSource, eventTypeNormal, eventReasonCacheUpdated, fmt.Sprintf("Successfully updated bundle cache from %s/%s", catalogSource.GetNamespace(), catalogSource.GetName()))
9498
}
95-
r.updateCache(req.String(), entities)
96-
r.recorder.Event(catalogSource, eventTypeNormal, eventReasonCacheUpdated, fmt.Sprintf("Successfully updated bundle cache from %s/%s", catalogSource.GetNamespace(), catalogSource.GetName()))
9799

98100
if isManagedCatalogSource(*catalogSource) {
99101
return ctrl.Result{}, nil
@@ -113,14 +115,19 @@ func isManagedCatalogSource(catalogSource v1alpha1.CatalogSource) bool {
113115
return len(catalogSource.Spec.Address) == 0
114116
}
115117

116-
func (r *CatalogSourceReconciler) updateCache(sourceID string, entities []*input.Entity) {
117-
r.RWMutex.Lock()
118-
defer r.RWMutex.Unlock()
118+
func (r *CatalogSourceReconciler) updateCache(sourceID string, entities []*input.Entity) bool {
119119
newSourceCache := make(map[deppy.Identifier]*input.Entity)
120120
for _, entity := range entities {
121121
newSourceCache[entity.Identifier()] = entity
122122
}
123+
if _, ok := r.cache[sourceID]; ok && reflect.DeepEqual(r.cache[sourceID], newSourceCache) {
124+
return false
125+
}
126+
r.RWMutex.Lock()
127+
defer r.RWMutex.Unlock()
123128
r.cache[sourceID] = newSourceCache
129+
// return whether cache had updates
130+
return true
124131
}
125132

126133
func (r *CatalogSourceReconciler) dropSource(sourceID string) {

‎controllers/catalogsource_controller_test.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ var _ = Describe("CatalogSource Controller Test", func() {
141141
By("running re-reconcile")
142142
res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
143143
Expect(res).To(Equal(ctrl.Result{}))
144+
Expect(err).ToNot(HaveOccurred())
144145

145146
By("checking the cache is empty again")
146147
entities = nil
@@ -151,5 +152,63 @@ var _ = Describe("CatalogSource Controller Test", func() {
151152
Expect(err).ToNot(HaveOccurred())
152153
Expect(entities).To(BeEmpty())
153154
})
155+
156+
Describe("querying CatalogSource EntitySource", func() {
157+
BeforeEach(func() {
158+
registryEntities := []*input.Entity{
159+
input.NewEntity(deppy.Identifier(fmt.Sprintf("%s/%s/pkg1/chan1/0.1.0", catalogSourceName, namespace)), map[string]string{}),
160+
input.NewEntity(deppy.Identifier(fmt.Sprintf("%s/%s/pkg1/chan1/0.2.0", catalogSourceName, namespace)), map[string]string{"k": "v"}),
161+
}
162+
fakeRegistry.setEntitiesForSource(opKey.String(), registryEntities...)
163+
164+
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey})
165+
Expect(err).ToNot(HaveOccurred())
166+
Expect(res).To(Equal(ctrl.Result{}))
167+
})
168+
Describe("Get", func() {
169+
It("should fetch an entity by ID", func() {
170+
Expect(reconciler.Get(ctx, deppy.Identifier(fmt.Sprintf("%s/%s/pkg1/chan1/0.1.0", catalogSourceName, namespace)))).To(
171+
Equal(input.NewEntity(deppy.Identifier(fmt.Sprintf("%s/%s/pkg1/chan1/0.1.0", catalogSourceName, namespace)), map[string]string{})),
172+
)
173+
})
174+
It("should not fetch anything for nonexistent entity ID", func() {
175+
Expect(reconciler.Get(ctx, "non-existent")).To(BeNil())
176+
})
177+
})
178+
Describe("Filter", func() {
179+
It("should return entities that meet filter predicates", func() {
180+
actual, err := reconciler.Filter(ctx, func(e *input.Entity) bool {
181+
_, ok := e.Properties["k"]
182+
return ok
183+
})
184+
Expect(err).ToNot(HaveOccurred())
185+
Expect(actual).To(ConsistOf(input.EntityList{*input.NewEntity(deppy.Identifier(fmt.Sprintf("%s/%s/pkg1/chan1/0.2.0", catalogSourceName, namespace)), map[string]string{"k": "v"})}))
186+
})
187+
})
188+
Describe("GroupBy", func() {
189+
It("should group entities by the keys provided by the groupBy function", func() {
190+
actual, err := reconciler.GroupBy(ctx, func(e *input.Entity) []string {
191+
var keys []string
192+
for k := range e.Properties {
193+
keys = append(keys, k)
194+
}
195+
return keys
196+
})
197+
Expect(err).ToNot(HaveOccurred())
198+
Expect(actual).To(Equal(input.EntityListMap{"k": input.EntityList{*input.NewEntity(deppy.Identifier(fmt.Sprintf("%s/%s/pkg1/chan1/0.2.0", catalogSourceName, namespace)), map[string]string{"k": "v"})}}))
199+
})
200+
})
201+
Describe("Iterate", func() {
202+
It("should go through all entities", func() {
203+
var ids []string
204+
Expect(reconciler.Iterate(ctx, func(e *input.Entity) error {
205+
ids = append(ids, e.Identifier().String())
206+
return nil
207+
})).To(BeNil())
208+
Expect(ids).To(ConsistOf([]string{fmt.Sprintf("%s/%s/pkg1/chan1/0.1.0", catalogSourceName, namespace),
209+
fmt.Sprintf("%s/%s/pkg1/chan1/0.2.0", catalogSourceName, namespace)}))
210+
})
211+
})
212+
})
154213
})
155214
})

‎controllers/operator_controller_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ var testEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{
570570
"olm.gvk": `[]`,
571571
}),
572572
"operatorhub/badimage/0.1.0": *input.NewEntity("operatorhub/badimage/0.1.0", map[string]string{
573-
"olm.bundle.path": `{"name": "quay.io/operatorhubio/badimage:v0.1.0"}`,
573+
"olm.bundle.path": ``,
574574
"olm.package": `{"packageName":"badimage","version":"0.1.0"}`,
575575
"olm.gvk": `[]`,
576576
}),

‎go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ go 1.19
44

55
require (
66
github.com/blang/semver/v4 v4.0.0
7+
github.com/containerd/containerd v1.6.3
78
github.com/onsi/ginkgo/v2 v2.3.1
89
github.com/onsi/gomega v1.22.1
910
github.com/operator-framework/api v0.15.0

‎go.sum‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ github.com/cncf/xds/go v0.0.0-20210312221358-fbca930ec8ed/go.mod h1:eXthEFrGJvWH
9898
github.com/cncf/xds/go v0.0.0-20210922020428-25de7278fc84/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
9999
github.com/cncf/xds/go v0.0.0-20211001041855-01bcc9b48dfe/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
100100
github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
101+
github.com/containerd/containerd v1.6.3 h1:JfgUEIAH07xDWk6kqz0P3ArZt+KJ9YeihSC9uyFtSKg=
102+
github.com/containerd/containerd v1.6.3/go.mod h1:gCVGrYRYFm2E8GmuUIbj/NGD7DLZQLzSJQazjVKDOig=
101103
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
102104
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
103105
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=

‎internal/resolution/variable_sources/bundles_and_dependencies/bundles_and_dependencies_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ import (
88
. "github.com/onsi/gomega"
99
"github.com/operator-framework/deppy/pkg/deppy"
1010
"github.com/operator-framework/deppy/pkg/deppy/input"
11-
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/required_package"
1211
"github.com/operator-framework/operator-registry/alpha/property"
1312

1413
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundles_and_dependencies"
1514
olmentity "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity"
15+
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/required_package"
1616
)
1717

1818
func TestBundlesAndDeps(t *testing.T) {

‎internal/resolution/variable_sources/crd_constraints/crd_constraints_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@ import (
1010
"github.com/operator-framework/deppy/pkg/deppy"
1111
"github.com/operator-framework/deppy/pkg/deppy/constraint"
1212
"github.com/operator-framework/deppy/pkg/deppy/input"
13-
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/required_package"
1413
"github.com/operator-framework/operator-registry/alpha/property"
1514

1615
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundles_and_dependencies"
1716
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/crd_constraints"
1817
olmentity "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity"
18+
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/required_package"
1919
)
2020

2121
func TestGlobalConstraints(t *testing.T) {

‎internal/resolution/variable_sources/olm/olm.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ import (
55

66
"github.com/operator-framework/deppy/pkg/deppy"
77
"github.com/operator-framework/deppy/pkg/deppy/input"
8-
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/required_package"
98

109
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundles_and_dependencies"
1110
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/crd_constraints"
11+
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/required_package"
1212
)
1313

1414
var _ input.VariableSource = &OLMVariableSource{}

‎internal/resolution/variable_sources/required_package/required_package_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ import (
99
. "github.com/onsi/gomega"
1010
"github.com/operator-framework/deppy/pkg/deppy"
1111
"github.com/operator-framework/deppy/pkg/deppy/input"
12-
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/required_package"
1312
"github.com/operator-framework/operator-registry/alpha/property"
1413

1514
olmentity "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity"
15+
"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/required_package"
1616
)
1717

1818
func TestRequiredPackage(t *testing.T) {

‎test/e2e/catalogsource_install_test.go‎

Lines changed: 1 addition & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -27,84 +27,6 @@ import (
2727
"sigs.k8s.io/controller-runtime/pkg/client/config"
2828
)
2929

30-
// e2e resolution from a catalogsource backed entitysource without olm
31-
//var _ = Describe("Deppy", func() {
32-
// var kubeClient *kubernetes.Clientset
33-
// var err error
34-
// var testNamespace string
35-
// cleanup := func() {}
36-
// var ctx = context.TODO()
37-
// BeforeEach(func() {
38-
// kubeClient, err = kubernetes.NewForConfig(config.GetConfigOrDie())
39-
// Expect(err).To(BeNil())
40-
// testNamespace = createTestNamespace(ctx, kubeClient, "registry-grpc-")
41-
// cleanup = applyCRDifNotPresent(ctx)
42-
// })
43-
// AfterEach(func() {
44-
// deleteTestNamespace(ctx, kubeClient, testNamespace)
45-
// cleanup()
46-
// })
47-
// It("can install an operator from a catalogSource", func() {
48-
// testPrefix := "registry-grpc-"
49-
//
50-
// serviceAccountName := createTestServiceAccount(ctx, kubeClient, testNamespace, testPrefix)
51-
// createTestRegistryPod(ctx, kubeClient, testNamespace, testPrefix, serviceAccountName)
52-
// serviceName := createTestRegistryService(ctx, kubeClient, testNamespace, testPrefix)
53-
// createTestCatalogSource(ctx, kubeClient, testNamespace, "prometheus-index", serviceName)
54-
//
55-
// scheme := runtime.NewScheme()
56-
// // Add catalogSources
57-
// err = v1alpha1.AddToScheme(scheme)
58-
// Expect(err).To(BeNil())
59-
//
60-
// c := config.GetConfigOrDie()
61-
// ctrlCli, err := controllerClient.NewWithWatch(c, controllerClient.Options{
62-
// Scheme: scheme,
63-
// })
64-
// Expect(err).To(BeNil())
65-
//
66-
// logger := zap.New()
67-
// cacheCli := catalogsource.NewCachedRegistryQuerier(ctrlCli, catalogsource.NewRegistryGRPCClient(0), &logger)
68-
//
69-
// go cacheCli.Start(ctx)
70-
//
71-
// defer cacheCli.Stop()
72-
//
73-
// Eventually(func(g Gomega) {
74-
// // wait till cache is populated
75-
// var entityIDs []deppy.Identifier
76-
// err := cacheCli.Iterate(ctx, func(entity *input.Entity) error {
77-
// entityIDs = append(entityIDs, entity.Identifier())
78-
// return nil
79-
// })
80-
// g.Expect(err).To(BeNil())
81-
// g.Expect(entityIDs).To(ConsistOf([]deppy.Identifier{
82-
// deppy.IdentifierFromString(testNamespace + "/prometheus-index/prometheus/beta/0.14.0"),
83-
// deppy.IdentifierFromString(testNamespace + "/prometheus-index/prometheus/beta/0.15.0"),
84-
// deppy.IdentifierFromString(testNamespace + "/prometheus-index/prometheus/beta/0.22.2"),
85-
// deppy.IdentifierFromString(testNamespace + "/prometheus-index/prometheus/beta/0.27.0"),
86-
// deppy.IdentifierFromString(testNamespace + "/prometheus-index/prometheus/beta/0.32.0"),
87-
// deppy.IdentifierFromString(testNamespace + "/prometheus-index/prometheus/beta/0.37.0"),
88-
// }))
89-
// }).Should(Succeed())
90-
//
91-
// s, err := solver.NewDeppySolver(cacheCli, olm.NewOLMVariableSource("prometheus"))
92-
//
93-
// //catalogsource.WithDependencies([]*input.Entity{{ID: deppy.IdentifierFromString("with package prometheus"), Properties: map[string]string{
94-
// // property.TypePackageRequired: `[{"packageName":"prometheus","version":">=0.37.0"}]`,
95-
// //}}})
96-
// Expect(err).To(BeNil())
97-
// solutionSet, err := s.Solve(ctx, solver.AddAllVariablesToSolution())
98-
// Expect(err).To(BeNil())
99-
//
100-
// Expect(solutionSet.Error()).To(BeNil())
101-
// Expect(solutionSet.SelectedVariables()).To(HaveKey(deppy.IdentifierFromString(testNamespace + "/prometheus-index/prometheus/beta/0.37.0")))
102-
// Expect(solutionSet)
103-
//
104-
// })
105-
//
106-
//})
107-
10830
// ListEntities
10931
func createTestNamespace(ctx context.Context, c *kubernetes.Clientset, prefix string) string {
11032
ns, err := c.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{
@@ -280,7 +202,7 @@ func applyCRDifNotPresent(ctx context.Context) func() {
280202
err = ctrlCli.Get(ctx, types.NamespacedName{Name: "catalogsources.operators.coreos.com"}, &catalogSourceCRD)
281203
if err != nil {
282204
Expect(errors.IsNotFound(err)).To(BeTrue())
283-
crdContents, err := os.ReadFile("../testdata/operators.coreos.com_catalogsources.crd.yaml")
205+
crdContents, err := os.ReadFile("../../testdata/crds/operators.coreos.com_catalogsources.yaml")
284206
Expect(err).To(BeNil())
285207

286208
err = yaml2.Unmarshal(crdContents, &catalogSourceCRD)

0 commit comments

Comments
 (0)