Skip to content

Commit

Permalink
Deflake controllerregistration controller integration test (gardene…
Browse files Browse the repository at this point in the history 
…r#8336)

* Improvements in controller

Use fieldselector for controllerinstallation SeedRefName
Use fieldselector for controllerinstallation RegistrationRefName

* Deflake integration test

* Rework unit tests using fakeClient

* Address PR review feedback

* Cleanup stuff related to local setup

Leftover from gardener#8194

* Improvements for envtest setup

* Create seed only once in the test

* Skip `ControllerRegistrationResources` plugin

It's not relevant in this test
  • Loading branch information
@shafeeqes
shafeeqes authored Aug 21, 2023
1 parent f6ad0ab commit d26b000
Show file tree
Hide file tree
Showing 13 changed files with 317 additions and 472 deletions.
2 changes: 1 addition & 1 deletion docs/development/testing.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ k get managedresource -A -w
Run a `gardenerenvtest` suite (using `gardener-apiserver`) against an existing test environment:

```bash
# modify test/start-envtest to disable admission plugins and enable feature gates like in test suite...
# modify GardenerTestEnvironment{} in test/start-envtest to disable admission plugins and enable feature gates like in test suite...
make start-envtest ENVTEST_TYPE=gardener
Expand Down
6 changes: 0 additions & 6 deletions example/provider-local/seed-kind-ha-multi-zone/local/kustomization.yaml
View file

This file was deleted.

47 changes: 0 additions & 47 deletions example/provider-local/seed-kind-ha-multi-zone/local/seed.yaml
View file

This file was deleted.

17 changes: 10 additions & 7 deletions pkg/api/indexer/gardener_core.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@ func ControllerInstallationSeedRefNameIndexerFunc(obj client.Object) []string {
return []string{controllerInstallation.Spec.SeedRef.Name}
}

// ControllerInstallationRegistrationRefNameIndexerFunc extracts the .spec.registrationRef.name field of a ControllerInstallation.
func ControllerInstallationRegistrationRefNameIndexerFunc(obj client.Object) []string {
controllerInstallation, ok := obj.(*gardencorev1beta1.ControllerInstallation)
if !ok {
return []string{""}
}
return []string{controllerInstallation.Spec.RegistrationRef.Name}
}

// InternalSecretTypeIndexerFunc extracts the .type field of an InternalSecret.
func InternalSecretTypeIndexerFunc(obj client.Object) []string {
internalSecret, ok := obj.(*gardencorev1beta1.InternalSecret)
Expand Down Expand Up @@ -146,13 +155,7 @@ func AddControllerInstallationSeedRefName(ctx context.Context, indexer client.Fi

// AddControllerInstallationRegistrationRefName adds an index for core.ControllerInstallationRegistrationRefName to the given indexer.
func AddControllerInstallationRegistrationRefName(ctx context.Context, indexer client.FieldIndexer) error {
if err := indexer.IndexField(ctx, &gardencorev1beta1.ControllerInstallation{}, core.RegistrationRefName, func(obj client.Object) []string {
controllerInstallation, ok := obj.(*gardencorev1beta1.ControllerInstallation)
if !ok {
return []string{""}
}
return []string{controllerInstallation.Spec.RegistrationRef.Name}
}); err != nil {
if err := indexer.IndexField(ctx, &gardencorev1beta1.ControllerInstallation{}, core.RegistrationRefName, ControllerInstallationRegistrationRefNameIndexerFunc); err != nil {
return fmt.Errorf("failed to add indexer for %s to ControllerInstallation Informer: %w", core.RegistrationRefName, err)
}
return nil
Expand Down
17 changes: 7 additions & 10 deletions ...ermanager/controller/controllerregistration/controllerregistrationfinalizer/reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

gardencore "github.com/gardener/gardener/pkg/apis/core"
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
"github.com/gardener/gardener/pkg/controllerutils"
)
Expand Down Expand Up @@ -61,21 +62,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
}

controllerInstallationList := &gardencorev1beta1.ControllerInstallationList{}
if err := r.Client.List(ctx, controllerInstallationList); err != nil {
if err := r.Client.List(ctx, controllerInstallationList, client.MatchingFields{gardencore.RegistrationRefName: controllerRegistration.Name}); err != nil {
return reconcile.Result{}, err
}

for _, controllerInstallation := range controllerInstallationList.Items {
if controllerInstallation.Spec.RegistrationRef.Name == controllerRegistration.Name {
return reconcile.Result{}, fmt.Errorf("cannot remove finalizer of ControllerRegistration %q because still found at least one ControllerInstallation", controllerRegistration.Name)
}
if len(controllerInstallationList.Items) > 0 {
return reconcile.Result{}, fmt.Errorf("cannot remove finalizer of ControllerRegistration %q because still found ControllerInstallations: %s", controllerRegistration.Name, controllerutils.GetControllerInstallationNames(controllerInstallationList.Items))
}

if controllerutil.ContainsFinalizer(controllerRegistration, FinalizerName) {
log.Info("Removing finalizer")
if err := controllerutils.RemoveFinalizers(ctx, r.Client, controllerRegistration, FinalizerName); err != nil {
return reconcile.Result{}, fmt.Errorf("failed to remove finalizer: %w", err)
}
log.Info("Removing finalizer")
if err := controllerutils.RemoveFinalizers(ctx, r.Client, controllerRegistration, FinalizerName); err != nil {
return reconcile.Result{}, fmt.Errorf("failed to remove finalizer: %w", err)
}

return reconcile.Result{}, nil
Expand Down
147 changes: 38 additions & 109 deletions ...ager/controller/controllerregistration/controllerregistrationfinalizer/reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,189 +16,118 @@ package controllerregistrationfinalizer_test

import (
"context"
"fmt"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"go.uber.org/mock/gomock"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/gardener/gardener/pkg/api/indexer"
"github.com/gardener/gardener/pkg/apis/core"
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
"github.com/gardener/gardener/pkg/client/kubernetes"
. "github.com/gardener/gardener/pkg/controllermanager/controller/controllerregistration/controllerregistrationfinalizer"
mockclient "github.com/gardener/gardener/pkg/mock/controller-runtime/client"
kubernetesutils "github.com/gardener/gardener/pkg/utils/kubernetes"
. "github.com/gardener/gardener/pkg/utils/test/matchers"
)

var _ = Describe("ControllerRegistration", func() {
var (
ctx = context.TODO()
fakeErr = fmt.Errorf("fake err")
ctx = context.TODO()

ctrl *gomock.Controller
c *mockclient.MockClient
c client.Client

reconciler *Reconciler
controllerRegistration *gardencorev1beta1.ControllerRegistration

controllerRegistrationName = "controllerRegistration"
)

BeforeEach(func() {
ctrl = gomock.NewController(GinkgoT())
c = mockclient.NewMockClient(ctrl)
c = fakeclient.NewClientBuilder().
WithScheme(kubernetes.GardenScheme).
WithIndex(&gardencorev1beta1.ControllerInstallation{}, core.RegistrationRefName, indexer.ControllerInstallationRegistrationRefNameIndexerFunc).
Build()
})

AfterEach(func() {
ctrl.Finish()
})

Describe("Reconciler", func() {
const finalizerName = "core.gardener.cloud/controllerregistration"

var (
reconciler *Reconciler
controllerRegistration *gardencorev1beta1.ControllerRegistration
)

BeforeEach(func() {
reconciler = &Reconciler{Client: c}
controllerRegistration = &gardencorev1beta1.ControllerRegistration{
ObjectMeta: metav1.ObjectMeta{
Name: controllerRegistrationName,
ResourceVersion: "42",
Name: controllerRegistrationName,
},
}
})

It("should return nil because object not found", func() {
c.EXPECT().Get(gomock.Any(), kubernetesutils.Key(controllerRegistrationName), gomock.AssignableToTypeOf(&gardencorev1beta1.ControllerRegistration{})).Return(apierrors.NewNotFound(schema.GroupResource{}, ""))

result, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: controllerRegistrationName}})
Expect(result).To(Equal(reconcile.Result{}))
Expect(err).NotTo(HaveOccurred())
})

It("should return err because object reading failed", func() {
c.EXPECT().Get(gomock.Any(), kubernetesutils.Key(controllerRegistrationName), gomock.AssignableToTypeOf(&gardencorev1beta1.ControllerRegistration{})).Return(fakeErr)

result, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: controllerRegistrationName}})
Expect(result).To(Equal(reconcile.Result{}))
Expect(err).To(MatchError(fakeErr))
})

Context("deletion timestamp not set", func() {
BeforeEach(func() {
c.EXPECT().Get(gomock.Any(), kubernetesutils.Key(controllerRegistrationName), gomock.AssignableToTypeOf(&gardencorev1beta1.ControllerRegistration{})).DoAndReturn(func(_ context.Context, _ client.ObjectKey, obj *gardencorev1beta1.ControllerRegistration, _ ...client.GetOption) error {
*obj = *controllerRegistration
return nil
})
})

It("should ensure the finalizer (error)", func() {
errToReturn := apierrors.NewNotFound(schema.GroupResource{}, controllerRegistrationName)

c.EXPECT().Patch(gomock.Any(), gomock.AssignableToTypeOf(&gardencorev1beta1.ControllerRegistration{}), gomock.Any()).DoAndReturn(func(_ context.Context, o client.Object, patch client.Patch, opts ...client.PatchOption) error {
Expect(patch.Data(o)).To(BeEquivalentTo(fmt.Sprintf(`{"metadata":{"finalizers":["%s"],"resourceVersion":"42"}}`, finalizerName)))
return errToReturn
})

result, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: controllerRegistrationName}})
Expect(result).To(Equal(reconcile.Result{}))
Expect(err).To(MatchError(err))
Expect(c.Create(ctx, controllerRegistration)).To(Succeed())
})

It("should ensure the finalizer (no error)", func() {
c.EXPECT().Patch(gomock.Any(), gomock.AssignableToTypeOf(&gardencorev1beta1.ControllerRegistration{}), gomock.Any()).DoAndReturn(func(_ context.Context, o client.Object, patch client.Patch, opts ...client.PatchOption) error {
Expect(patch.Data(o)).To(BeEquivalentTo(fmt.Sprintf(`{"metadata":{"finalizers":["%s"],"resourceVersion":"42"}}`, finalizerName)))
return nil
})

It("should ensure the finalizer", func() {
result, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: controllerRegistrationName}})
Expect(result).To(Equal(reconcile.Result{}))
Expect(err).NotTo(HaveOccurred())

Expect(c.Get(ctx, client.ObjectKeyFromObject(controllerRegistration), controllerRegistration)).To(Succeed())
Expect(controllerRegistration.Finalizers).To(ConsistOf("core.gardener.cloud/controllerregistration"))
})
})

Context("deletion timestamp set", func() {
BeforeEach(func() {
now := metav1.Now()
controllerRegistration.DeletionTimestamp = &now
controllerRegistration.Finalizers = []string{FinalizerName}

c.EXPECT().Get(gomock.Any(), kubernetesutils.Key(controllerRegistrationName), gomock.AssignableToTypeOf(&gardencorev1beta1.ControllerRegistration{})).DoAndReturn(func(_ context.Context, _ client.ObjectKey, obj *gardencorev1beta1.ControllerRegistration, _ ...client.GetOption) error {
*obj = *controllerRegistration
return nil
})
})

It("should do nothing because finalizer is not present", func() {
controllerRegistration.Finalizers = nil

Expect(c.Create(ctx, controllerRegistration)).To(Succeed())
result, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: controllerRegistrationName}})
Expect(result).To(Equal(reconcile.Result{}))
Expect(err).NotTo(HaveOccurred())
})

It("should return an error because installation list failed", func() {
c.EXPECT().List(gomock.Any(), gomock.AssignableToTypeOf(&gardencorev1beta1.ControllerInstallationList{})).Return(fakeErr)
Expect(c.Get(ctx, client.ObjectKeyFromObject(controllerRegistration), controllerRegistration)).To(Succeed())
Expect(controllerRegistration.Finalizers).To(ConsistOf("core.gardener.cloud/controllerregistration"))

result, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: controllerRegistrationName}})
Expect(result).To(Equal(reconcile.Result{}))
Expect(err).To(MatchError(fakeErr))
Expect(c.Delete(ctx, controllerRegistration)).To(Succeed())
})

It("should return an error because installation referencing controllerRegistration exists", func() {
c.EXPECT().List(gomock.Any(), gomock.AssignableToTypeOf(&gardencorev1beta1.ControllerInstallationList{})).DoAndReturn(func(_ context.Context, obj *gardencorev1beta1.ControllerInstallationList, _ ...client.ListOption) error {
(&gardencorev1beta1.ControllerInstallationList{Items: []gardencorev1beta1.ControllerInstallation{
{
Spec: gardencorev1beta1.ControllerInstallationSpec{
RegistrationRef: corev1.ObjectReference{
Name: controllerRegistrationName,
},
},
controllerInstallation := &gardencorev1beta1.ControllerInstallation{
ObjectMeta: metav1.ObjectMeta{
Name: "controllerInstallation",
},
Spec: gardencorev1beta1.ControllerInstallationSpec{
RegistrationRef: corev1.ObjectReference{
Name: controllerRegistrationName,
},
}}).DeepCopyInto(obj)
return nil
})
},
}

result, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: controllerRegistrationName}})
Expect(result).To(Equal(reconcile.Result{}))
Expect(err).To(MatchError(ContainSubstring("cannot remove finalizer")))
})

It("should remove the finalizer (error)", func() {
c.EXPECT().List(gomock.Any(), gomock.AssignableToTypeOf(&gardencorev1beta1.ControllerInstallationList{})).DoAndReturn(func(_ context.Context, obj *gardencorev1beta1.ControllerInstallationList, _ ...client.ListOption) error {
(&gardencorev1beta1.ControllerInstallationList{}).DeepCopyInto(obj)
return nil
})

c.EXPECT().Patch(gomock.Any(), gomock.AssignableToTypeOf(&gardencorev1beta1.ControllerRegistration{}), gomock.Any()).DoAndReturn(func(_ context.Context, o client.Object, patch client.Patch, opts ...client.PatchOption) error {
Expect(patch.Data(o)).To(BeEquivalentTo(`{"metadata":{"finalizers":null,"resourceVersion":"42"}}`))
return fakeErr
})
Expect(c.Create(ctx, controllerInstallation)).To(Succeed())

result, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: controllerRegistrationName}})
Expect(result).To(Equal(reconcile.Result{}))
Expect(err).To(MatchError(fakeErr))
Expect(err).To(MatchError(ContainSubstring("cannot remove finalizer of ControllerRegistration %q because still found ControllerInstallations: [%s]", controllerRegistration.Name, controllerInstallation.Name)))
})

It("should remove the finalizer (no error)", func() {
c.EXPECT().List(gomock.Any(), gomock.AssignableToTypeOf(&gardencorev1beta1.ControllerInstallationList{})).DoAndReturn(func(_ context.Context, obj *gardencorev1beta1.ControllerInstallationList, _ ...client.ListOption) error {
(&gardencorev1beta1.ControllerInstallationList{}).DeepCopyInto(obj)
return nil
})

c.EXPECT().Patch(gomock.Any(), gomock.AssignableToTypeOf(&gardencorev1beta1.ControllerRegistration{}), gomock.Any()).DoAndReturn(func(_ context.Context, o client.Object, patch client.Patch, opts ...client.PatchOption) error {
Expect(patch.Data(o)).To(BeEquivalentTo(`{"metadata":{"finalizers":null,"resourceVersion":"42"}}`))
return nil
})

It("should remove the finalizer", func() {
result, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: controllerRegistrationName}})
Expect(result).To(Equal(reconcile.Result{}))
Expect(err).NotTo(HaveOccurred())

Expect(c.Get(ctx, client.ObjectKeyFromObject(controllerRegistration), controllerRegistration)).To(BeNotFoundError())
})
})
})
Expand Down
4 changes: 2 additions & 2 deletions pkg/controllermanager/controller/controllerregistration/seed/add.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ func (r *Reconciler) AddToManager(ctx context.Context, mgr manager.Manager) erro
)
}

// SeedPredicate returns true for all Seed events except for updates. Here, it only returns true when there is a change
// in the .spec.dns.provider field or when the deletion timestamp is set.
// SeedPredicate returns true for all Seed events except for updates. Here, it returns true when there is a change
// in the spec or labels or annotations or when the deletion timestamp is set.
func (r *Reconciler) SeedPredicate() predicate.Predicate {
return predicate.Funcs{
UpdateFunc: func(e event.UpdateEvent) bool {
Expand Down
Loading

0 comments on commit d26b000

Please sign in to comment.