Skip to content
Open
2 changes: 2 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ linters:
alias: kymastatusrepo
- pkg: github.com/kyma-project/lifecycle-manager/internal/repository/skr/kyma
alias: skrkymarepo
- pkg: github.com/kyma-project/lifecycle-manager/internal/repository/skr/crd
alias: skrcrdrepo
- pkg: github.com/kyma-project/lifecycle-manager/internal/repository/skr/kyma/status
alias: skrkymastatusrepo
- pkg: github.com/kyma-project/lifecycle-manager/internal/repository/secret
Expand Down
24 changes: 23 additions & 1 deletion cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,11 @@
kymarepo "github.com/kyma-project/lifecycle-manager/internal/repository/kyma"
kymastatusrepo "github.com/kyma-project/lifecycle-manager/internal/repository/kyma/status"
secretrepo "github.com/kyma-project/lifecycle-manager/internal/repository/secret"
skrcrdrepo "github.com/kyma-project/lifecycle-manager/internal/repository/skr/crd"
skrkymarepo "github.com/kyma-project/lifecycle-manager/internal/repository/skr/kyma"
skrkymastatusrepo "github.com/kyma-project/lifecycle-manager/internal/repository/skr/kyma/status"
resultevent "github.com/kyma-project/lifecycle-manager/internal/result/event"
"github.com/kyma-project/lifecycle-manager/internal/result/kyma/usecase"
"github.com/kyma-project/lifecycle-manager/internal/service/accessmanager"
kymadeletionsvc "github.com/kyma-project/lifecycle-manager/internal/service/kyma/deletion"
"github.com/kyma-project/lifecycle-manager/internal/service/kyma/deletion/usecases"
Expand Down Expand Up @@ -420,7 +422,7 @@
setupLog.V(log.DebugLevel).Info("scheduled job for cleaning up metrics")
}

func setupKymaReconciler(mgr ctrl.Manager, descriptorProvider *provider.CachedDescriptorProvider,

Check failure on line 425 in cmd/main.go

View workflow job for this annotation

GitHub Actions / lint

Function 'setupKymaReconciler' is too long (96 > 80) (funlen)
skrContextFactory remote.SkrContextProvider, event event.Event, flagVar *flags.FlagVar, options ctrlruntime.Options,
skrWebhookManager *watcher.SkrWebhookManifestManager, kymaMetrics *metrics.KymaMetrics,
setupLog logr.Logger, maintenanceWindow maintenancewindows.MaintenanceWindow, ociRegistryHost string,
Expand Down Expand Up @@ -460,9 +462,29 @@
accessSecretRepository)
deleteSkrKyma := usecases.NewDeleteSkrKyma(skrkymarepo.NewRepository(skrClientCache),
accessSecretRepository)
deleteSkrMtCrd := usecases.NewDeleteSkrCrd(
skrcrdrepo.NewRepository(skrClientCache,
fmt.Sprintf("%s.%s", shared.ModuleTemplateKind.Plural(), shared.OperatorGroup)),
accessSecretRepository,
usecase.DeleteSkrModuleTemplateCrd)
deleteSkrMrmCrd := usecases.NewDeleteSkrCrd(
skrcrdrepo.NewRepository(skrClientCache,
fmt.Sprintf("%s.%s", shared.ModuleReleaseMetaKind.Plural(), shared.OperatorGroup)),
accessSecretRepository,
usecase.DeleteSkrModuleReleaseMetaCrd)
deleteSkrKymaCrd := usecases.NewDeleteSkrCrd(
skrcrdrepo.NewRepository(skrClientCache,
fmt.Sprintf("%s.%s", shared.KymaKind.Plural(), shared.OperatorGroup)),
accessSecretRepository,
usecase.DeleteSkrKymaCrd)

kymaDeletionService := kymadeletionsvc.NewService(setKcpKymaStateDeleting,
setSkrKymaStateDeleting,
deleteSkrKyma)
deleteSkrKyma,
deleteSkrMtCrd,
deleteSkrMrmCrd,
deleteSkrKymaCrd,
)

if err := (&kyma.Reconciler{
Client: kcpClient,
Expand Down
5 changes: 3 additions & 2 deletions internal/controller/kyma/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
SkrImagePullSecretName string
}

// TODO: make all fields private and provide constructor.

Check failure on line 105 in internal/controller/kyma/controller.go

View workflow job for this annotation

GitHub Actions / lint

Line contains TODO/BUG/FIXME: "TODO: make all fields private and provid..." (godox)
type Reconciler struct {
client.Client
event.Event
Expand Down Expand Up @@ -264,7 +264,7 @@
func (r *Reconciler) processDeletion(ctx context.Context, kyma *v1beta2.Kyma) (ctrl.Result, error) {
res := r.DeletionService.Delete(ctx, kyma)

// TODO revisit this, but probably necessary right now as we just re-use the client cache

Check failure on line 267 in internal/controller/kyma/controller.go

View workflow job for this annotation

GitHub Actions / lint

Line contains TODO/BUG/FIXME: "TODO revisit this, but probably necessar..." (godox)
if util.IsConnectionRelatedError(res.Err) {
r.SkrContextFactory.InvalidateCache(kyma.GetNamespacedName())
}
Expand All @@ -277,8 +277,9 @@
usecase.SetSkrKymaStateDeleting,
usecase.DeleteSkrKyma,
usecase.DeleteSkrWatcher,
usecase.DeleteSkrModuleMetadata,
usecase.DeleteSkrCrds,
usecase.DeleteSkrModuleTemplateCrd,
usecase.DeleteSkrModuleReleaseMetaCrd,
usecase.DeleteSkrKymaCrd,
usecase.DeleteWatcherCertificate,
usecase.DeleteManifests,
usecase.DeleteMetrics:
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/kyma/deletion/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ func (w *MetricWriter) Write(res result.Result) {
w.metrics.RecordRequeueReason(metrics.StatusSyncToRemote, requeueType)
case usecase.DeleteSkrKyma:
w.metrics.RecordRequeueReason(metrics.RemoteKymaDeletion, requeueType)
case usecase.DeleteSkrModuleMetadata:
case usecase.DeleteSkrModuleTemplateCrd, usecase.DeleteSkrModuleReleaseMetaCrd:
w.metrics.RecordRequeueReason(metrics.RemoteModuleCatalogDeletion, requeueType)
case usecase.DeleteManifests:
w.metrics.RecordRequeueReason(metrics.CleanupManifestCrs, requeueType)
case usecase.DeleteSkrWatcher,
usecase.DeleteSkrCrds,
usecase.DeleteSkrKymaCrd,
usecase.DeleteWatcherCertificate,
usecase.DeleteMetrics,
usecase.RemoveKymaFinalizers:
Expand Down
28 changes: 22 additions & 6 deletions internal/controller/kyma/deletion/metric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,16 @@ func TestMetricWriter_Write(t *testing.T) {
expectedRequeueType: queue.IntendedRequeue,
},
{
name: "RemoteModuleCatalogDeletion intended",
useCase: usecase.DeleteSkrModuleMetadata,
name: "RemoteModuleCatalogDeletion MT intended",
useCase: usecase.DeleteSkrModuleTemplateCrd,
err: nil,
expectedCall: true,
expectedReason: metrics.RemoteModuleCatalogDeletion,
expectedRequeueType: queue.IntendedRequeue,
},
{
name: "RemoteModuleCatalogDeletion MRM intended",
useCase: usecase.DeleteSkrModuleReleaseMetaCrd,
err: nil,
expectedCall: true,
expectedReason: metrics.RemoteModuleCatalogDeletion,
Expand Down Expand Up @@ -102,8 +110,16 @@ func TestMetricWriter_Write(t *testing.T) {
expectedRequeueType: queue.UnexpectedRequeue,
},
{
name: "RemoteModuleCatalogDeletion unexpected",
useCase: usecase.DeleteSkrModuleMetadata,
name: "RemoteModuleCatalogDeletion MT unexpected",
useCase: usecase.DeleteSkrModuleTemplateCrd,
err: assert.AnError,
expectedCall: true,
expectedReason: metrics.RemoteModuleCatalogDeletion,
expectedRequeueType: queue.UnexpectedRequeue,
},
{
name: "RemoteModuleCatalogDeletion MRM unexpected",
useCase: usecase.DeleteSkrModuleReleaseMetaCrd,
err: assert.AnError,
expectedCall: true,
expectedReason: metrics.RemoteModuleCatalogDeletion,
Expand All @@ -126,8 +142,8 @@ func TestMetricWriter_Write(t *testing.T) {
expectedCall: false,
},
{
name: "Nothing for UseCaseDeleteSkrCrds",
useCase: usecase.DeleteSkrCrds,
name: "Nothing for DeleteSkrKymaCrd",
useCase: usecase.DeleteSkrKymaCrd,
err: nil,
expectedCall: false,
},
Expand Down
86 changes: 86 additions & 0 deletions internal/repository/skr/crd/repo.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package crd

import (
"context"
"fmt"
"reflect"

apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1beta1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kyma-project/lifecycle-manager/internal/errors"
"github.com/kyma-project/lifecycle-manager/pkg/util"
)

type SkrClientCache interface {
Get(key client.ObjectKey) client.Client
}

type Repository struct {
skrClientCache SkrClientCache
crdName string
}

func NewRepository(skrClientCache SkrClientCache,
crdName string,
) *Repository {
return &Repository{
skrClientCache: skrClientCache,
crdName: crdName,
}
}

func (r *Repository) Exists(ctx context.Context, kymaName types.NamespacedName) (bool, error) {
skrClient, err := r.getSkrClient(kymaName)
if err != nil {
return false, err
}

err = skrClient.Get(ctx,
types.NamespacedName{
Name: r.crdName,
},
&v1beta1.PartialObjectMetadata{
TypeMeta: apimetav1.TypeMeta{
Kind: reflect.TypeOf(apiextensionsv1.CustomResourceDefinition{}).Name(),
APIVersion: apiextensionsv1.SchemeGroupVersion.String(),
},
},
)

// not found error => (false, nil)
// other error => (true, err)
// no error => (true, nil)
return !util.IsNotFound(err), client.IgnoreNotFound(err)
}

func (r *Repository) Delete(ctx context.Context, kymaName types.NamespacedName) error {
skrClient, err := r.getSkrClient(kymaName)
if err != nil {
return err
}

return client.IgnoreNotFound(
skrClient.Delete(ctx,
&apiextensionsv1.CustomResourceDefinition{
ObjectMeta: apimetav1.ObjectMeta{
Name: r.crdName,
},
}))
}

// TODO: this should work as long as we use the same client cache that we passed to KymaSkrContextProvider

Check failure on line 75 in internal/repository/skr/crd/repo.go

View workflow job for this annotation

GitHub Actions / lint

Line contains TODO/BUG/FIXME: "TODO: this should work as long as we use..." (godox)
// it however depends on KymaSkrContextProvider.Init being called. As of now, this is the case, but we
// should re-think how we use the client cache.
func (r *Repository) getSkrClient(kymaName types.NamespacedName) (client.Client, error) {
skrClient := r.skrClientCache.Get(kymaName)

if skrClient == nil {
return nil, fmt.Errorf("%w: Kyma %s", errors.ErrSkrClientNotFound, kymaName.String())
}

return skrClient, nil
}
108 changes: 108 additions & 0 deletions internal/repository/skr/crd/repo_delete_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package crd_test

import (
"context"
"reflect"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kyma-project/lifecycle-manager/internal/errors"
skrcrdrepo "github.com/kyma-project/lifecycle-manager/internal/repository/skr/crd"
"github.com/kyma-project/lifecycle-manager/pkg/testutils/random"
)

func TestDelete_ClientCallSucceeds(t *testing.T) {
kymaName := types.NamespacedName{Name: random.Name(), Namespace: random.Name()}
crdName := random.Name()

clientStub := &deleteClientStub{}
clientCacheStub := &skrClientCacheStub{
client: clientStub,
}

repo := skrcrdrepo.NewRepository(clientCacheStub, crdName)

err := repo.Delete(context.Background(), kymaName)

require.NoError(t, err)
assert.True(t, clientStub.called)
// kymaName used to get the client
assert.Equal(t, kymaName, clientCacheStub.receivedKey)
// CRD name used to delete the CRD
assert.Equal(t, crdName, clientStub.deletedObject.GetName())
}

func TestDelete_ClientReturnsAnError(t *testing.T) {
clientStub := &deleteClientStub{
err: assert.AnError,
}
repo := skrcrdrepo.NewRepository(&skrClientCacheStub{
client: clientStub,
}, random.Name())

err := repo.Delete(context.Background(),
types.NamespacedName{
Name: random.Name(),
Namespace: random.Name(),
})

require.ErrorIs(t, err, assert.AnError)
assert.True(t, clientStub.called)
}

func TestDelete_ClientIgnoresNotFoundError(t *testing.T) {
clientStub := &deleteClientStub{
err: apierrors.NewNotFound(schema.GroupResource{
Group: apiextensionsv1.SchemeGroupVersion.Group,
Resource: reflect.TypeOf(apiextensionsv1.CustomResourceDefinition{}).Name(),
}, random.Name()),
}
repo := skrcrdrepo.NewRepository(&skrClientCacheStub{
client: clientStub,
}, random.Name())

err := repo.Delete(context.Background(),
types.NamespacedName{
Name: random.Name(),
Namespace: random.Name(),
})

require.NoError(t, err)
assert.True(t, clientStub.called)
}

func TestDelete_ClientNotFound_ReturnsError(t *testing.T) {
repo := skrcrdrepo.NewRepository(&skrClientCacheStub{
client: nil, // No client available in the cache
}, random.Name())

err := repo.Delete(context.Background(),
types.NamespacedName{
Name: random.Name(),
Namespace: random.Name(),
})

require.Error(t, err)
require.ErrorIs(t, err, errors.ErrSkrClientNotFound)
}

type deleteClientStub struct {
client.Client

called bool
deletedObject client.Object
err error
}

func (c *deleteClientStub) Delete(_ context.Context, obj client.Object, _ ...client.DeleteOption) error {
c.called = true
c.deletedObject = obj
return c.err
}
Loading
Loading