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.

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

chore: Refactor NewCachedDescriptorProvider #1695

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ issues:
linters: [ gci ] # Disable gci due to the test utilities dot import.
- path: tests/integration/declarative/declarative_test.go
linters: [ gci ] # Disable gci due to the test utilities dot import.
- path: tests/integration/controller/(controlplane|eventfilters|kyma|withwatcher|purge|mandatorymodule)/(.*)_test.go
- path: tests/integration/controller/(eventfilters|kyma|withwatcher|purge|mandatorymodule|kcp)/(.*)_test.go
linters: [ gci ] # Disable gci due to the test utilities dot import.
- linters:
- importas
Expand Down
9 changes: 6 additions & 3 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,11 @@ func setupManager(flagVar *flags.FlagVar, cacheOptions cache.Options, scheme *ma
}

sharedMetrics := metrics.NewSharedMetrics()
descriptorProvider := provider.NewCachedDescriptorProvider(nil)
descriptorProvider := provider.NewCachedDescriptorProvider()
kymaMetrics := metrics.NewKymaMetrics(sharedMetrics)
mandatoryModulesMetrics := metrics.NewMandatoryModulesMetrics()
setupKymaReconciler(mgr, descriptorProvider, skrContextProvider, eventRecorder, flagVar, options, skrWebhookManager, kymaMetrics,
setupKymaReconciler(mgr, descriptorProvider, skrContextProvider, eventRecorder, flagVar, options, skrWebhookManager,
kymaMetrics,
setupLog)
setupManifestReconciler(mgr, flagVar, options, sharedMetrics, mandatoryModulesMetrics, setupLog)
setupMandatoryModuleReconciler(mgr, descriptorProvider, flagVar, options, mandatoryModulesMetrics, setupLog)
Expand Down Expand Up @@ -315,7 +316,9 @@ func setupKymaReconciler(mgr ctrl.Manager,
}
}

func createSkrWebhookManager(mgr ctrl.Manager, skrContextFactory remote.SkrContextProvider, flagVar *flags.FlagVar) (*watcher.SKRWebhookManifestManager, error) {
func createSkrWebhookManager(mgr ctrl.Manager, skrContextFactory remote.SkrContextProvider,
flagVar *flags.FlagVar,
) (*watcher.SKRWebhookManifestManager, error) {
caCertificateCache := watcher.NewCACertificateCache(flagVar.CaCertCacheTTL)
config := watcher.SkrWebhookManagerConfig{
SKRWatcherPath: flagVar.WatcherResourcesPath,
Expand Down
4 changes: 3 additions & 1 deletion docs/user-tutorials/01-10-control-plane-quick-start.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ To use Lifecycle Manager in a local setup, you need the following prerequisites:
kubectl apply -f https://raw.githubusercontent.com/prometheus-community/helm-charts/main/charts/kube-prometheus-stack/charts/crds/crds/crd-servicemonitors.yaml
```

You can also follow the official [Prometheus Operator quick start](https://prometheus-operator.dev/docs/getting-started/quick-start/) guide to deploy a full set of Prometheus Operator features into your cluster.
You can also follow the
official [Prometheus Operator quick start](https://prometheus-operator.dev/docs/getting-started/) guide to deploy a full
set of Prometheus Operator features into your cluster.

### Deploy Lifecycle Manager

Expand Down
19 changes: 7 additions & 12 deletions internal/descriptor/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,12 @@ var (
)

type CachedDescriptorProvider struct {
descriptorCache *cache.DescriptorCache
DescriptorCache *cache.DescriptorCache
}

func NewCachedDescriptorProvider(descriptorCache *cache.DescriptorCache) *CachedDescriptorProvider {
if descriptorCache != nil {
return &CachedDescriptorProvider{
descriptorCache: descriptorCache,
}
}
func NewCachedDescriptorProvider() *CachedDescriptorProvider {
return &CachedDescriptorProvider{
descriptorCache: cache.NewDescriptorCache(),
DescriptorCache: cache.NewDescriptorCache(),
}
}

Expand All @@ -49,7 +44,7 @@ func (c *CachedDescriptorProvider) GetDescriptor(template *v1beta2.ModuleTemplat
return desc, nil
}
key := cache.GenerateDescriptorKey(template)
descriptor := c.descriptorCache.Get(key)
descriptor := c.DescriptorCache.Get(key)
if descriptor != nil {
return descriptor, nil
}
Expand All @@ -75,15 +70,15 @@ func (c *CachedDescriptorProvider) Add(template *v1beta2.ModuleTemplate) error {
return ErrTemplateNil
}
key := cache.GenerateDescriptorKey(template)
descriptor := c.descriptorCache.Get(key)
descriptor := c.DescriptorCache.Get(key)
if descriptor != nil {
return nil
}

if template.Spec.Descriptor.Object != nil {
desc, ok := template.Spec.Descriptor.Object.(*v1beta2.Descriptor)
if ok && desc != nil {
c.descriptorCache.Set(key, desc)
c.DescriptorCache.Set(key, desc)
return nil
}
}
Expand All @@ -101,6 +96,6 @@ func (c *CachedDescriptorProvider) Add(template *v1beta2.ModuleTemplate) error {
return ErrTypeAssert
}

c.descriptorCache.Set(key, descriptor)
c.DescriptorCache.Set(key, descriptor)
return nil
}
25 changes: 13 additions & 12 deletions internal/descriptor/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

func TestGetDescriptor_OnEmptySpec_ReturnsErrDecode(t *testing.T) {
descriptorProvider := provider.NewCachedDescriptorProvider(nil) // assuming it handles nil cache internally
descriptorProvider := provider.NewCachedDescriptorProvider()
template := &v1beta2.ModuleTemplate{}

_, err := descriptorProvider.GetDescriptor(template)
Expand All @@ -24,7 +24,7 @@ func TestGetDescriptor_OnEmptySpec_ReturnsErrDecode(t *testing.T) {
}

func TestAdd_OnNilTemplate_ReturnsErrTemplateNil(t *testing.T) {
descriptorProvider := provider.NewCachedDescriptorProvider(nil)
descriptorProvider := provider.NewCachedDescriptorProvider()

err := descriptorProvider.Add(nil)

Expand All @@ -33,7 +33,7 @@ func TestAdd_OnNilTemplate_ReturnsErrTemplateNil(t *testing.T) {
}

func TestGetDescriptor_OnNilTemplate_ReturnsErrTemplateNil(t *testing.T) {
descriptorProvider := provider.NewCachedDescriptorProvider(nil)
descriptorProvider := provider.NewCachedDescriptorProvider()

_, err := descriptorProvider.GetDescriptor(nil)

Expand All @@ -42,7 +42,7 @@ func TestGetDescriptor_OnNilTemplate_ReturnsErrTemplateNil(t *testing.T) {
}

func TestGetDescriptor_OnInvalidRawDescriptor_ReturnsErrDescriptorNil(t *testing.T) {
descriptorProvider := provider.NewCachedDescriptorProvider(nil)
descriptorProvider := provider.NewCachedDescriptorProvider()
template := builder.NewModuleTemplateBuilder().WithRawDescriptor([]byte("invalid descriptor")).WithDescriptor(nil).Build()

_, err := descriptorProvider.GetDescriptor(template)
Expand All @@ -52,8 +52,7 @@ func TestGetDescriptor_OnInvalidRawDescriptor_ReturnsErrDescriptorNil(t *testing
}

func TestGetDescriptor_OnEmptyCache_ReturnsParsedDescriptor(t *testing.T) {
descriptorCache := cache.NewDescriptorCache()
descriptorProvider := provider.NewCachedDescriptorProvider(descriptorCache)
descriptorProvider := provider.NewCachedDescriptorProvider()
template := builder.NewModuleTemplateBuilder().Build()

_, err := descriptorProvider.GetDescriptor(template)
Expand All @@ -62,7 +61,7 @@ func TestGetDescriptor_OnEmptyCache_ReturnsParsedDescriptor(t *testing.T) {
}

func TestAdd_OnInvalidRawDescriptor_ReturnsErrDecode(t *testing.T) {
descriptorProvider := provider.NewCachedDescriptorProvider(nil)
descriptorProvider := provider.NewCachedDescriptorProvider()
template := builder.NewModuleTemplateBuilder().WithRawDescriptor([]byte("invalid descriptor")).WithDescriptor(nil).Build()

err := descriptorProvider.Add(template)
Expand All @@ -72,7 +71,7 @@ func TestAdd_OnInvalidRawDescriptor_ReturnsErrDecode(t *testing.T) {
}

func TestAdd_OnDescriptorTypeButNull_ReturnsNoError(t *testing.T) {
descriptorProvider := provider.NewCachedDescriptorProvider(nil)
descriptorProvider := provider.NewCachedDescriptorProvider()
template := builder.NewModuleTemplateBuilder().WithDescriptor(&v1beta2.Descriptor{}).Build()

err := descriptorProvider.Add(template)
Expand All @@ -82,12 +81,14 @@ func TestAdd_OnDescriptorTypeButNull_ReturnsNoError(t *testing.T) {

func TestGetDescriptor_OnEmptyCache_AddsDescriptorFromTemplate(t *testing.T) {
descriptorCache := cache.NewDescriptorCache()
descriptorProvider := provider.NewCachedDescriptorProvider(descriptorCache)
descriptorProvider := provider.CachedDescriptorProvider{DescriptorCache: descriptorCache}

expected := &v1beta2.Descriptor{
ComponentDescriptor: &compdesc.ComponentDescriptor{Metadata: compdesc.Metadata{
ConfiguredVersion: "v2",
}},
ComponentDescriptor: &compdesc.ComponentDescriptor{
Metadata: compdesc.Metadata{
ConfiguredVersion: "v2",
},
},
}
template := builder.NewModuleTemplateBuilder().WithDescriptor(expected).Build()

Expand Down
5 changes: 5 additions & 0 deletions pkg/testutils/kyma.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ func EnableModule(ctx context.Context,
if err != nil {
return err
}
for _, enabledModule := range kyma.Spec.Modules {
if enabledModule.Name == module.Name {
return nil
}
}
kyma.Spec.Modules = append(
kyma.Spec.Modules, module)
err = clnt.Update(ctx, kyma)
Expand Down
4 changes: 2 additions & 2 deletions pkg/testutils/moduletemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func GetModuleTemplate(ctx context.Context,
module v1beta2.Module,
defaultChannel string,
) (*v1beta2.ModuleTemplate, error) {
descriptorProvider := provider.NewCachedDescriptorProvider(nil)
descriptorProvider := provider.NewCachedDescriptorProvider()
templateLookup := templatelookup.NewTemplateLookup(clnt, descriptorProvider)
templateInfo := templateLookup.GetAndValidate(ctx, module.Name, module.Channel, defaultChannel)
if templateInfo.Err != nil {
Expand Down Expand Up @@ -94,7 +94,7 @@ func ReadModuleVersionFromModuleTemplate(ctx context.Context, clnt client.Client
return "", fmt.Errorf("failed to fetch ModuleTemplate: %w", err)
}

descriptorProvider := provider.NewCachedDescriptorProvider(nil)
descriptorProvider := provider.NewCachedDescriptorProvider()
ocmDesc, err := descriptorProvider.GetDescriptor(moduleTemplate)
if err != nil {
return "", fmt.Errorf("failed to get descriptor: %w", err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ var _ = Describe("Webhook ValidationCreate Strict", Ordered, func() {
WithChannel(v1beta2.DefaultChannel).
WithOCM(compdescv2.SchemaVersion).Build()
Expect(k8sClient.Create(webhookServerContext, template)).Should(Succeed())
descriptorProvider := provider.NewCachedDescriptorProvider(nil)
descriptorProvider := provider.NewCachedDescriptorProvider()
descriptor, err := descriptorProvider.GetDescriptor(template)
Expect(err).ToNot(HaveOccurred())
version, err := semver.NewVersion(descriptor.Version)
Expand Down
5 changes: 3 additions & 2 deletions tests/integration/controller/eventfilters/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/manager"
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"

"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kyma-project/lifecycle-manager/api"
"github.com/kyma-project/lifecycle-manager/api/shared"
"github.com/kyma-project/lifecycle-manager/internal"
Expand All @@ -47,7 +49,6 @@ import (
"github.com/kyma-project/lifecycle-manager/pkg/queue"
testskrcontext "github.com/kyma-project/lifecycle-manager/pkg/testutils/skrcontextimpl"
"github.com/kyma-project/lifecycle-manager/tests/integration"
"sigs.k8s.io/controller-runtime/pkg/client"

_ "github.com/open-component-model/ocm/pkg/contexts/ocm"

Expand Down Expand Up @@ -142,7 +143,7 @@ var _ = BeforeSuite(func() {
SkrContextFactory: testSkrContextFactory,
Event: testEventRec,
RequeueIntervals: intervals,
DescriptorProvider: provider.NewCachedDescriptorProvider(nil),
DescriptorProvider: provider.NewCachedDescriptorProvider(),
SyncRemoteCrds: remote.NewSyncCrdsUseCase(kcpClient, testSkrContextFactory, nil),
InKCPMode: false,
RemoteSyncNamespace: flags.DefaultRemoteSyncNamespace,
Expand Down
5 changes: 3 additions & 2 deletions tests/integration/controller/kcp/remote_sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ import (
"github.com/kyma-project/lifecycle-manager/internal/pkg/flags"
"github.com/kyma-project/lifecycle-manager/pkg/testutils/builder"

. "github.com/kyma-project/lifecycle-manager/pkg/testutils"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

. "github.com/kyma-project/lifecycle-manager/pkg/testutils"
)

var (
Expand Down Expand Up @@ -223,7 +224,7 @@ func buildSkrKyma() *v1beta2.Kyma {

func IsDescriptorCached(template *v1beta2.ModuleTemplate) bool {
key := cache.GenerateDescriptorKey(template)
result := descriptorCache.Get(key)
result := descriptorProvider.DescriptorCache.Get(key)
return result != nil
}

Expand Down
9 changes: 3 additions & 6 deletions tests/integration/controller/kcp/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,19 @@ import (
"github.com/kyma-project/lifecycle-manager/internal"
"github.com/kyma-project/lifecycle-manager/internal/controller/kyma"
"github.com/kyma-project/lifecycle-manager/internal/crd"
"github.com/kyma-project/lifecycle-manager/internal/descriptor/cache"
"github.com/kyma-project/lifecycle-manager/internal/descriptor/provider"
"github.com/kyma-project/lifecycle-manager/internal/event"
"github.com/kyma-project/lifecycle-manager/internal/pkg/flags"
"github.com/kyma-project/lifecycle-manager/internal/pkg/metrics"
"github.com/kyma-project/lifecycle-manager/internal/remote"
"github.com/kyma-project/lifecycle-manager/pkg/log"
"github.com/kyma-project/lifecycle-manager/pkg/queue"
"github.com/kyma-project/lifecycle-manager/pkg/testutils"
testskrcontext "github.com/kyma-project/lifecycle-manager/pkg/testutils/skrcontextimpl"
"github.com/kyma-project/lifecycle-manager/tests/integration"

_ "github.com/open-component-model/ocm/pkg/contexts/ocm"

. "github.com/kyma-project/lifecycle-manager/pkg/testutils"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
Expand All @@ -71,7 +70,6 @@ var (
ctx context.Context
cancel context.CancelFunc
cfg *rest.Config
descriptorCache *cache.DescriptorCache
descriptorProvider *provider.CachedDescriptorProvider
crdCache *crd.Cache
)
Expand All @@ -88,7 +86,7 @@ var _ = BeforeSuite(func() {
var err error
By("bootstrapping test environment")

externalCRDs, err := AppendExternalCRDs(
externalCRDs, err := testutils.AppendExternalCRDs(
filepath.Join(integration.GetProjectRoot(), "config", "samples", "tests", "crds"),
"cert-manager-v1.10.1.crds.yaml",
"istio-v1.17.1.crds.yaml")
Expand Down Expand Up @@ -140,8 +138,7 @@ var _ = BeforeSuite(func() {

testEventRec := event.NewRecorderWrapper(mgr.GetEventRecorderFor(shared.OperatorName))
testSkrContextFactory = testskrcontext.NewDualClusterFactory(kcpClient.Scheme(), testEventRec)
descriptorCache = cache.NewDescriptorCache()
descriptorProvider = provider.NewCachedDescriptorProvider(descriptorCache)
descriptorProvider = provider.NewCachedDescriptorProvider()
crdCache = crd.NewCache(nil)
err = (&kyma.Reconciler{
Client: kcpClient,
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/controller/kyma/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ var _ = BeforeSuite(func() {
Warning: 100 * time.Millisecond,
}

descriptorProvider = provider.NewCachedDescriptorProvider(nil)
descriptorProvider = provider.NewCachedDescriptorProvider()
kcpClient = mgr.GetClient()
testEventRec := event.NewRecorderWrapper(mgr.GetEventRecorderFor(shared.OperatorName))
testSkrContextFactory := testskrcontext.NewSingleClusterFactory(kcpClient, mgr.GetConfig(), testEventRec)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ var _ = BeforeSuite(func() {
Warning: 100 * time.Millisecond,
}

descriptorProvider := provider.NewCachedDescriptorProvider(nil)
descriptorProvider := provider.NewCachedDescriptorProvider()
reconciler = &mandatorymodule.DeletionReconciler{
Client: mgr.GetClient(),
Event: event.NewRecorderWrapper(mgr.GetEventRecorderFor(shared.OperatorName)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ var _ = BeforeSuite(func() {
Warning: 100 * time.Millisecond,
}

descriptorProvider := provider.NewCachedDescriptorProvider(nil)
descriptorProvider := provider.NewCachedDescriptorProvider()
reconciler = &mandatorymodule.InstallationReconciler{
Client: mgr.GetClient(),
DescriptorProvider: descriptorProvider,
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/controller/withwatcher/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ var _ = BeforeSuite(func() {
Event: testEventRec,
RequeueIntervals: intervals,
SKRWebhookManager: skrWebhookChartManager,
DescriptorProvider: provider.NewCachedDescriptorProvider(nil),
DescriptorProvider: provider.NewCachedDescriptorProvider(),
SyncRemoteCrds: remote.NewSyncCrdsUseCase(kcpClient, testSkrContextFactory, nil),
RemoteSyncNamespace: flags.DefaultRemoteSyncNamespace,
InKCPMode: true,
Expand Down
2 changes: 1 addition & 1 deletion unit-test-coverage.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
packages:
internal/crd: 92
internal/descriptor/cache: 93
internal/descriptor/provider: 68
internal/descriptor/provider: 66
internal/event: 100
internal/manifest/filemutex: 100
internal/istio: 63
Expand Down
Loading