Skip to content

Commit

Permalink
feat: Refactor template lookup to use strategy pattern
Browse files Browse the repository at this point in the history
  • Loading branch information
c-pius committed Jan 21, 2025
1 parent 90b9197 commit 883e804
Show file tree
Hide file tree
Showing 27 changed files with 928 additions and 376 deletions.
12 changes: 9 additions & 3 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import (
"github.com/kyma-project/lifecycle-manager/pkg/matcher"
"github.com/kyma-project/lifecycle-manager/pkg/queue"
"github.com/kyma-project/lifecycle-manager/pkg/templatelookup"
"github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup"
"github.com/kyma-project/lifecycle-manager/pkg/watcher"

_ "k8s.io/client-go/plugin/pkg/client/auth"
Expand Down Expand Up @@ -280,13 +281,18 @@ func scheduleMetricsCleanup(kymaMetrics *metrics.KymaMetrics, cleanupIntervalInM
func setupKymaReconciler(mgr ctrl.Manager, descriptorProvider *provider.CachedDescriptorProvider,
skrContextFactory remote.SkrContextProvider, event event.Event, flagVar *flags.FlagVar, options ctrlruntime.Options,
skrWebhookManager *watcher.SKRWebhookManifestManager, kymaMetrics *metrics.KymaMetrics,
setupLog logr.Logger, maintenanceWindow templatelookup.MaintenanceWindow,
) {
setupLog logr.Logger, _ *maintenancewindows.MaintenanceWindow) {
options.RateLimiter = internal.RateLimiter(flagVar.FailureBaseDelay,
flagVar.FailureMaxDelay, flagVar.RateLimiterFrequency, flagVar.RateLimiterBurst)
options.CacheSyncTimeout = flagVar.CacheSyncTimeout
options.MaxConcurrentReconciles = flagVar.MaxConcurrentKymaReconciles

moduleTemplateInfoLookupStrategy := moduletemplateinfolookup.NewAggregatedModuleTemplateInfoLookupStrategy([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{
moduletemplateinfolookup.NewByVersionStrategy(mgr.GetClient()),
moduletemplateinfolookup.NewByChannelStrategy(mgr.GetClient()),
moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(mgr.GetClient()),
})

if err := (&kyma.Reconciler{
Client: mgr.GetClient(),
SkrContextFactory: skrContextFactory,
Expand All @@ -306,7 +312,7 @@ func setupKymaReconciler(mgr ctrl.Manager, descriptorProvider *provider.CachedDe
Metrics: kymaMetrics,
RemoteCatalog: remote.NewRemoteCatalogFromKyma(mgr.GetClient(), skrContextFactory,
flagVar.RemoteSyncNamespace),
TemplateLookup: templatelookup.NewTemplateLookup(mgr.GetClient(), descriptorProvider, maintenanceWindow),
TemplateLookup: templatelookup.NewTemplateLookup(mgr.GetClient(), descriptorProvider, moduleTemplateInfoLookupStrategy),
}).SetupWithManager(
mgr, options, kyma.SetupOptions{
ListenerAddr: flagVar.KymaListenerAddr,
Expand Down
3 changes: 2 additions & 1 deletion pkg/module/sync/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/kyma-project/lifecycle-manager/pkg/log"
"github.com/kyma-project/lifecycle-manager/pkg/module/common"
"github.com/kyma-project/lifecycle-manager/pkg/templatelookup"
"github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup"
"github.com/kyma-project/lifecycle-manager/pkg/util"
)

Expand Down Expand Up @@ -275,7 +276,7 @@ func generateModuleStatus(module *common.Module, existStatus *v1beta2.ModuleStat
newModuleStatus.Message = module.Template.Err.Error()
return *newModuleStatus
}
if errors.Is(module.Template.Err, templatelookup.ErrNoTemplatesInListResult) {
if errors.Is(module.Template.Err, moduletemplateinfolookup.ErrNoTemplatesInListResult) {
return v1beta2.ModuleStatus{
Name: module.ModuleName,
Channel: module.Template.DesiredChannel,
Expand Down
47 changes: 47 additions & 0 deletions pkg/templatelookup/moduletemplateinfolookup/aggregated.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package moduletemplateinfolookup

import (
"context"
"errors"

"github.com/kyma-project/lifecycle-manager/api/v1beta2"
"github.com/kyma-project/lifecycle-manager/pkg/templatelookup"
)

var ErrNoResponsibleStrategy = errors.New("failed to find responsible module template lookup strategy")

type ModuleTemplateInfoLookupStrategy interface {
Lookup(ctx context.Context,
moduleInfo *templatelookup.ModuleInfo,
kyma *v1beta2.Kyma,
moduleReleaseMeta *v1beta2.ModuleReleaseMeta,
) templatelookup.ModuleTemplateInfo
IsResponsible(moduleInfo *templatelookup.ModuleInfo,
kyma *v1beta2.Kyma,
moduleReleaseMeta *v1beta2.ModuleReleaseMeta,
) bool
}

type AggregatedModuleTemplateInfoLookupStrategy struct {
strategies []ModuleTemplateInfoLookupStrategy
}

func NewAggregatedModuleTemplateInfoLookupStrategy(strategies []ModuleTemplateInfoLookupStrategy) AggregatedModuleTemplateInfoLookupStrategy {
return AggregatedModuleTemplateInfoLookupStrategy{strategies: strategies}
}

func (s AggregatedModuleTemplateInfoLookupStrategy) Lookup(ctx context.Context,
moduleInfo *templatelookup.ModuleInfo,
kyma *v1beta2.Kyma,
moduleReleaseMeta *v1beta2.ModuleReleaseMeta,
) templatelookup.ModuleTemplateInfo {
for _, strategy := range s.strategies {
if strategy.IsResponsible(moduleInfo, kyma, moduleReleaseMeta) {
return strategy.Lookup(ctx, moduleInfo, kyma, moduleReleaseMeta)
}
}

return templatelookup.ModuleTemplateInfo{
Err: ErrNoResponsibleStrategy,
}
}
66 changes: 66 additions & 0 deletions pkg/templatelookup/moduletemplateinfolookup/aggregated_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package moduletemplateinfolookup_test

import (
"context"
"testing"

"github.com/kyma-project/lifecycle-manager/api/v1beta2"
"github.com/kyma-project/lifecycle-manager/pkg/templatelookup"
"github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func Test_AggregatedModuleTemplateInfoLookupStrategy_Lookup_CallsResponsibleStrategy(t *testing.T) {
nonResponsibleStrategy := newLookupStrategyStub(false)
responsibleStrategy := newLookupStrategyStub(true)
aggregatedStrategy := moduletemplateinfolookup.NewAggregatedModuleTemplateInfoLookupStrategy([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{
&nonResponsibleStrategy,
&responsibleStrategy,
})

moduleTemplateInfo := aggregatedStrategy.Lookup(context.Background(), nil, nil, nil)

assert.True(t, responsibleStrategy.called)
assert.False(t, nonResponsibleStrategy.called)
require.NoError(t, moduleTemplateInfo.Err)
}

func Test_AggregatedModuleTemplateInfoLookupStrategy_Lookup_ReturnsFailureWhenNoStrategyResponsible(t *testing.T) {
nonResponsibleStrategy := newLookupStrategyStub(false)
aggregatedStrategy := moduletemplateinfolookup.NewAggregatedModuleTemplateInfoLookupStrategy([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{
&nonResponsibleStrategy,
})

moduleTemplateInfo := aggregatedStrategy.Lookup(context.Background(), nil, nil, nil)

assert.False(t, nonResponsibleStrategy.called)
require.ErrorIs(t, moduleTemplateInfo.Err, moduletemplateinfolookup.ErrNoResponsibleStrategy)
}

func newLookupStrategyStub(responsible bool) LookupStrategyStub {
return LookupStrategyStub{
responsible: responsible,
}
}

type LookupStrategyStub struct {
responsible bool
called bool
}

func (s *LookupStrategyStub) Lookup(ctx context.Context,
moduleInfo *templatelookup.ModuleInfo,
kyma *v1beta2.Kyma,
moduleReleaseMeta *v1beta2.ModuleReleaseMeta,
) templatelookup.ModuleTemplateInfo {
s.called = true
return templatelookup.ModuleTemplateInfo{}
}

func (s *LookupStrategyStub) IsResponsible(moduleInfo *templatelookup.ModuleInfo,
kyma *v1beta2.Kyma,
moduleReleaseMeta *v1beta2.ModuleReleaseMeta,
) bool {
return s.responsible
}
138 changes: 138 additions & 0 deletions pkg/templatelookup/moduletemplateinfolookup/by_channel.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package moduletemplateinfolookup

import (
"context"
"errors"
"fmt"

"github.com/kyma-project/lifecycle-manager/api/v1beta2"
"github.com/kyma-project/lifecycle-manager/pkg/log"
"github.com/kyma-project/lifecycle-manager/pkg/templatelookup"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"
)

var (
ErrNotDefaultChannelAllowed = errors.New("specifying no default channel is not allowed")
)

type ByChannelStrategy struct {
client client.Reader
}

func NewByChannelStrategy(client client.Reader) ByChannelStrategy {
return ByChannelStrategy{client: client}
}

func (ByChannelStrategy) IsResponsible(moduleInfo *templatelookup.ModuleInfo,

Check failure on line 27 in pkg/templatelookup/moduletemplateinfolookup/by_channel.go

View workflow job for this annotation

GitHub Actions / lint

ST1016: methods on the same type should have the same receiver name (seen 1x "p", 1x "s") (stylecheck)
_ *v1beta2.Kyma,
moduleReleaseMeta *v1beta2.ModuleReleaseMeta,
) bool {
if moduleReleaseMeta != nil {
return false
}

if moduleInfo.IsInstalledByVersion() {
return false
}

return true
}

func (s ByChannelStrategy) Lookup(ctx context.Context,
moduleInfo *templatelookup.ModuleInfo,
kyma *v1beta2.Kyma,
_ *v1beta2.ModuleReleaseMeta,
) templatelookup.ModuleTemplateInfo {
desiredChannel := getDesiredChannel(moduleInfo.Channel, kyma.Spec.Channel)
info := templatelookup.ModuleTemplateInfo{
DesiredChannel: desiredChannel,
}

template, err := s.filterTemplatesByChannel(ctx, moduleInfo.Name, desiredChannel)
if err != nil {
info.Err = err
return info
}

actualChannel := template.Spec.Channel
if actualChannel == "" {
info.Err = fmt.Errorf(
"no channel found on template for module: %s: %w",
moduleInfo.Name, ErrNotDefaultChannelAllowed,
)
return info
}

logUsedChannel(ctx, moduleInfo.Name, actualChannel, kyma.Spec.Channel)
info.ModuleTemplate = template
return info
}

func (p ByChannelStrategy) filterTemplatesByChannel(ctx context.Context, name, desiredChannel string) (
*v1beta2.ModuleTemplate, error,
) {
templateList := &v1beta2.ModuleTemplateList{}
err := p.client.List(ctx, templateList)
if err != nil {
return nil, fmt.Errorf("failed to list module templates on lookup: %w", err)
}

var filteredTemplates []*v1beta2.ModuleTemplate
for _, template := range templateList.Items {
if TemplateNameMatch(&template, name) && template.Spec.Channel == desiredChannel {
filteredTemplates = append(filteredTemplates, &template)
continue
}
}

if len(filteredTemplates) > 1 {
return nil, newMoreThanOneTemplateCandidateErr(name, templateList.Items)
}

if len(filteredTemplates) == 0 {
return nil, fmt.Errorf("%w: for module %s in channel %s ",
ErrNoTemplatesInListResult, name, desiredChannel)
}

if filteredTemplates[0].Spec.Mandatory {
return nil, fmt.Errorf("%w: for module %s in channel %s",
ErrTemplateMarkedAsMandatory, name, desiredChannel)
}

return filteredTemplates[0], nil
}

func getDesiredChannel(moduleChannel, globalChannel string) string {
var desiredChannel string

switch {
case moduleChannel != "":
desiredChannel = moduleChannel
case globalChannel != "":
desiredChannel = globalChannel
default:
desiredChannel = v1beta2.DefaultChannel
}

return desiredChannel
}

func logUsedChannel(ctx context.Context, name string, actualChannel string, defaultChannel string) {
logger := logf.FromContext(ctx)
if actualChannel != defaultChannel {
logger.V(log.DebugLevel).Info(
fmt.Sprintf(
"using %s (instead of %s) for module %s",
actualChannel, defaultChannel, name,
),
)
} else {
logger.V(log.DebugLevel).Info(
fmt.Sprintf(
"using %s for module %s",
actualChannel, name,
),
)
}
}
71 changes: 71 additions & 0 deletions pkg/templatelookup/moduletemplateinfolookup/by_channel_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package moduletemplateinfolookup_test

import (
"testing"

"github.com/stretchr/testify/assert"

"github.com/kyma-project/lifecycle-manager/api/v1beta2"
"github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup"
"github.com/kyma-project/lifecycle-manager/pkg/testutils/builder"
)

func Test_ByChannelStrategy_IsResponsible_ReturnsTrue(t *testing.T) {
moduleInfo := newModuleInfoBuilder().WithChannel("regular").Enabled().Build()
var kyma *v1beta2.Kyma = nil
var moduleReleaseMeta *v1beta2.ModuleReleaseMeta = nil
byChannelStrategy := moduletemplateinfolookup.NewByChannelStrategy(nil)

responsible := byChannelStrategy.IsResponsible(moduleInfo, kyma, moduleReleaseMeta)

assert.True(t, responsible)
}

func Test_ByChannelStrategy_IsResponsible_ReturnsFalse_WhenModuleReleaseMetaIsNotNil(t *testing.T) {
moduleInfo := newModuleInfoBuilder().WithChannel("regular").Enabled().Build()
var kyma *v1beta2.Kyma = nil
var moduleReleaseMeta *v1beta2.ModuleReleaseMeta = builder.NewModuleReleaseMetaBuilder().Build()

Check failure on line 27 in pkg/templatelookup/moduletemplateinfolookup/by_channel_test.go

View workflow job for this annotation

GitHub Actions / lint

ST1023: should omit type *v1beta2.ModuleReleaseMeta from declaration; it will be inferred from the right-hand side (stylecheck)
byChannelStrategy := moduletemplateinfolookup.NewByChannelStrategy(nil)

responsible := byChannelStrategy.IsResponsible(moduleInfo, kyma, moduleReleaseMeta)

assert.False(t, responsible)
}

func Test_ByChannelStrategy_IsResponsible_ReturnsFalse_WhenInstalledByVersion(t *testing.T) {
moduleInfo := newModuleInfoBuilder().WithVersion("1.0.0").Enabled().Build()
var kyma *v1beta2.Kyma = nil
var moduleReleaseMeta *v1beta2.ModuleReleaseMeta = nil
byChannelStrategy := moduletemplateinfolookup.NewByChannelStrategy(nil)

responsible := byChannelStrategy.IsResponsible(moduleInfo, kyma, moduleReleaseMeta)

assert.False(t, responsible)
}

func Test_ByChannelStrategy_Lookup_ReturnsModuleTemplateInfo(t *testing.T) {
moduleInfo := newModuleInfoBuilder().WithName("test-module").WithChannel("regular").Enabled().Build()
var kyma *v1beta2.Kyma = builder.NewKymaBuilder().Build()

Check failure on line 48 in pkg/templatelookup/moduletemplateinfolookup/by_channel_test.go

View workflow job for this annotation

GitHub Actions / lint

ST1023: should omit type *v1beta2.Kyma from declaration; it will be inferred from the right-hand side (stylecheck)
var moduleReleaseMeta *v1beta2.ModuleReleaseMeta = nil
moduleTemplate := builder.NewModuleTemplateBuilder().
WithName("test-module-regular").
WithModuleName("test-module").
WithVersion("").
WithChannel("regular").
Build()
byChannelStrategy := moduletemplateinfolookup.NewByChannelStrategy(fakeClient(
&v1beta2.ModuleTemplateList{
Items: []v1beta2.ModuleTemplate{
*moduleTemplate,
},
},
))

moduleTemplateInfo := byChannelStrategy.Lookup(nil, moduleInfo, kyma, moduleReleaseMeta)

Check failure on line 64 in pkg/templatelookup/moduletemplateinfolookup/by_channel_test.go

View workflow job for this annotation

GitHub Actions / lint

SA1012: do not pass a nil Context, even if a function permits it; pass context.TODO if you are unsure about which Context to use (staticcheck)

assert.NotNil(t, moduleTemplateInfo)
assert.Equal(t, moduleTemplate.Name, moduleTemplateInfo.ModuleTemplate.Name)
assert.Equal(t, moduleTemplate.Spec.ModuleName, moduleTemplateInfo.ModuleTemplate.Spec.ModuleName)
assert.Equal(t, moduleTemplate.Spec.Version, moduleTemplateInfo.ModuleTemplate.Spec.Version)
assert.Equal(t, moduleTemplate.Spec.Channel, moduleTemplateInfo.ModuleTemplate.Spec.Channel)
}
Loading

0 comments on commit 883e804

Please sign in to comment.