Skip to content

Commit

Permalink
Add context parameter to Reconcile function of the Reconciler interfa…
Browse files Browse the repository at this point in the history 
…ce (#2146)

* Reconciler objects don't have context field

* context.Background used in tests to start a known context
  • Loading branch information
@aorcholski
aorcholski committed Sep 29, 2023
1 parent 547f5e6 commit f1e141d
Show file tree
Hide file tree
Showing 24 changed files with 330 additions and 340 deletions.
46 changes: 22 additions & 24 deletions src/controllers/certificates/webhook_cert_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ func newWebhookCertificateController(mgr manager.Manager, cancelMgr context.Canc
}

type WebhookCertificateController struct {
ctx context.Context
client client.Client
apiReader client.Reader
namespace string
Expand All @@ -64,28 +63,27 @@ func (controller *WebhookCertificateController) Reconcile(ctx context.Context, r
log.Info("reconciling webhook certificates",
"namespace", request.Namespace, "name", request.Name)
controller.namespace = request.Namespace
controller.ctx = ctx

webhookDeployment := appsv1.Deployment{}
err := controller.apiReader.Get(controller.ctx, types.NamespacedName{Name: webhook.DeploymentName, Namespace: controller.namespace}, &webhookDeployment)
err := controller.apiReader.Get(ctx, types.NamespacedName{Name: webhook.DeploymentName, Namespace: controller.namespace}, &webhookDeployment)
if k8serrors.IsNotFound(err) {
log.Info("no webhook deployment found, skipping webhook certificate generation")
return reconcile.Result{}, nil
} else if err != nil {
return reconcile.Result{}, errors.WithStack(err)
}

mutatingWebhookConfiguration, validatingWebhookConfiguration := controller.getWebhooksConfigurations()
mutatingWebhookConfiguration, validatingWebhookConfiguration := controller.getWebhooksConfigurations(ctx)

crd, err := controller.getCrd()
crd, err := controller.getCrd(ctx)
if err != nil {
log.Info("could not find CRD configuration")
return reconcile.Result{}, errors.WithStack(err)
}

certSecret := newCertificateSecret(controller.scheme, &webhookDeployment)

err = certSecret.setSecretFromReader(controller.ctx, controller.apiReader, controller.namespace)
err = certSecret.setSecretFromReader(ctx, controller.apiReader, controller.namespace)
if err != nil {
return reconcile.Result{}, errors.WithStack(err)
}
Expand All @@ -104,7 +102,7 @@ func (controller *WebhookCertificateController) Reconcile(ctx context.Context, r
return reconcile.Result{RequeueAfter: SuccessDuration}, nil
}

if err = certSecret.createOrUpdateIfNecessary(controller.ctx, controller.client); err != nil {
if err = certSecret.createOrUpdateIfNecessary(ctx, controller.client); err != nil {
return reconcile.Result{}, errors.WithStack(err)
}

Expand All @@ -113,17 +111,17 @@ func (controller *WebhookCertificateController) Reconcile(ctx context.Context, r
return reconcile.Result{}, errors.WithStack(err)
}

err = controller.updateClientConfigurations(bundle, mutatingWebhookClientConfigs, mutatingWebhookConfiguration)
err = controller.updateClientConfigurations(ctx, bundle, mutatingWebhookClientConfigs, mutatingWebhookConfiguration)
if err != nil {
return reconcile.Result{}, errors.WithStack(err)
}

err = controller.updateClientConfigurations(bundle, validatingWebhookConfigConfigs, validatingWebhookConfiguration)
err = controller.updateClientConfigurations(ctx, bundle, validatingWebhookConfigConfigs, validatingWebhookConfiguration)
if err != nil {
return reconcile.Result{}, errors.WithStack(err)
}

if err = controller.updateCRDConfiguration(bundle); err != nil {
if err = controller.updateCRDConfiguration(ctx, bundle); err != nil {
return reconcile.Result{}, errors.WithStack(err)
}

Expand All @@ -143,15 +141,15 @@ func (controller *WebhookCertificateController) isUpToDate(certSecret *certifica
return isUpToDate
}

func (controller *WebhookCertificateController) getWebhooksConfigurations() (*admissionregistrationv1.MutatingWebhookConfiguration, *admissionregistrationv1.ValidatingWebhookConfiguration) {
mutatingWebhookConfiguration, err := controller.getMutatingWebhookConfiguration()
func (controller *WebhookCertificateController) getWebhooksConfigurations(ctx context.Context) (*admissionregistrationv1.MutatingWebhookConfiguration, *admissionregistrationv1.ValidatingWebhookConfiguration) {
mutatingWebhookConfiguration, err := controller.getMutatingWebhookConfiguration(ctx)
if err != nil {
// Generation must not be skipped because webhook startup routine listens for the secret
// See cmd/operator/manager.go and cmd/operator/watcher.go
log.Info("could not find mutating webhook configuration, this is normal when deployed using OLM")
}

validatingWebhookConfiguration, err := controller.getValidatingWebhookConfiguration()
validatingWebhookConfiguration, err := controller.getValidatingWebhookConfiguration(ctx)
if err != nil {
// Generation must not be skipped because webhook startup routine listens for the secret
// See cmd/operator/manager.go and cmd/operator/watcher.go
Expand All @@ -167,9 +165,9 @@ func (controller *WebhookCertificateController) cancelMgr() {
}
}

func (controller *WebhookCertificateController) getMutatingWebhookConfiguration() (*admissionregistrationv1.MutatingWebhookConfiguration, error) {
func (controller *WebhookCertificateController) getMutatingWebhookConfiguration(ctx context.Context) (*admissionregistrationv1.MutatingWebhookConfiguration, error) {
var mutatingWebhook admissionregistrationv1.MutatingWebhookConfiguration
err := controller.apiReader.Get(controller.ctx, client.ObjectKey{
err := controller.apiReader.Get(ctx, client.ObjectKey{
Name: webhook.DeploymentName,
}, &mutatingWebhook)
if err != nil {
Expand All @@ -182,9 +180,9 @@ func (controller *WebhookCertificateController) getMutatingWebhookConfiguration(
return &mutatingWebhook, nil
}

func (controller *WebhookCertificateController) getValidatingWebhookConfiguration() (*admissionregistrationv1.ValidatingWebhookConfiguration, error) {
func (controller *WebhookCertificateController) getValidatingWebhookConfiguration(ctx context.Context) (*admissionregistrationv1.ValidatingWebhookConfiguration, error) {
var mutatingWebhook admissionregistrationv1.ValidatingWebhookConfiguration
err := controller.apiReader.Get(controller.ctx, client.ObjectKey{
err := controller.apiReader.Get(ctx, client.ObjectKey{
Name: webhook.DeploymentName,
}, &mutatingWebhook)
if err != nil {
Expand All @@ -197,16 +195,16 @@ func (controller *WebhookCertificateController) getValidatingWebhookConfiguratio
return &mutatingWebhook, nil
}

func (controller *WebhookCertificateController) getCrd() (*apiv1.CustomResourceDefinition, error) {
func (controller *WebhookCertificateController) getCrd(ctx context.Context) (*apiv1.CustomResourceDefinition, error) {
var crd apiv1.CustomResourceDefinition
if err := controller.apiReader.Get(controller.ctx, types.NamespacedName{Name: crdName}, &crd); err != nil {
if err := controller.apiReader.Get(ctx, types.NamespacedName{Name: crdName}, &crd); err != nil {
return nil, err
}

return &crd, nil
}

func (controller *WebhookCertificateController) updateClientConfigurations(bundle []byte,
func (controller *WebhookCertificateController) updateClientConfigurations(ctx context.Context, bundle []byte,
webhookClientConfigs []*admissionregistrationv1.WebhookClientConfig, webhookConfig client.Object) error {
if webhookConfig == nil || reflect.ValueOf(webhookConfig).IsNil() {
return nil
Expand All @@ -216,15 +214,15 @@ func (controller *WebhookCertificateController) updateClientConfigurations(bundl
webhookClientConfigs[i].CABundle = bundle
}

if err := controller.client.Update(controller.ctx, webhookConfig); err != nil {
if err := controller.client.Update(ctx, webhookConfig); err != nil {
return err
}
return nil
}

func (controller *WebhookCertificateController) updateCRDConfiguration(bundle []byte) error {
func (controller *WebhookCertificateController) updateCRDConfiguration(ctx context.Context, bundle []byte) error {
var crd apiv1.CustomResourceDefinition
if err := controller.apiReader.Get(controller.ctx, types.NamespacedName{Name: crdName}, &crd); err != nil {
if err := controller.apiReader.Get(ctx, types.NamespacedName{Name: crdName}, &crd); err != nil {
return err
}

Expand All @@ -235,7 +233,7 @@ func (controller *WebhookCertificateController) updateCRDConfiguration(bundle []

// update crd
crd.Spec.Conversion.Webhook.ClientConfig.CABundle = bundle
if err := controller.client.Update(controller.ctx, &crd); err != nil {
if err := controller.client.Update(ctx, &crd); err != nil {
return err
}
return nil
Expand Down
1 change: 0 additions & 1 deletion src/controllers/certificates/webhook_cert_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ func createTestSecret(_ *testing.T, certData map[string][]byte) *corev1.Secret {

func prepareController(clt client.Client) (*WebhookCertificateController, reconcile.Request) {
rec := &WebhookCertificateController{
ctx: context.TODO(),
client: clt,
apiReader: clt,
namespace: testNamespace,
Expand Down
26 changes: 13 additions & 13 deletions src/controllers/dynakube/activegate/internal/authtoken/reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,43 +43,43 @@ func NewReconciler(clt client.Client, apiReader client.Reader, scheme *runtime.S
}
}

func (r *Reconciler) Reconcile() error {
err := r.reconcileAuthTokenSecret()
func (r *Reconciler) Reconcile(ctx context.Context) error {
err := r.reconcileAuthTokenSecret(ctx)
if err != nil {
return errors.WithMessage(err, "failed to create activeGateAuthToken secret")
}
return nil
}

func (r *Reconciler) reconcileAuthTokenSecret() error {
func (r *Reconciler) reconcileAuthTokenSecret(ctx context.Context) error {
var secret corev1.Secret
err := r.apiReader.Get(context.TODO(),
err := r.apiReader.Get(ctx,
client.ObjectKey{Name: r.dynakube.ActiveGateAuthTokenSecret(), Namespace: r.dynakube.Namespace},
&secret)
if err != nil {
if k8serrors.IsNotFound(err) {
log.Info("creating activeGateAuthToken secret")
return r.ensureAuthTokenSecret()
return r.ensureAuthTokenSecret(ctx)
}
return errors.WithStack(err)
}
if isSecretOutdated(&secret) {
log.Info("activeGateAuthToken is outdated, creating new one")
if err := r.deleteSecret(&secret); err != nil {
if err := r.deleteSecret(ctx, &secret); err != nil {
return errors.WithStack(err)
}
return r.ensureAuthTokenSecret()
return r.ensureAuthTokenSecret(ctx)
}

return nil
}

func (r *Reconciler) ensureAuthTokenSecret() error {
func (r *Reconciler) ensureAuthTokenSecret(ctx context.Context) error {
agSecretData, err := r.getActiveGateAuthToken()
if err != nil {
return errors.WithMessagef(err, "failed to create secret '%s'", r.dynakube.ActiveGateAuthTokenSecret())
}
return r.createSecret(agSecretData)
return r.createSecret(ctx, agSecretData)
}

func (r *Reconciler) getActiveGateAuthToken() (map[string][]byte, error) {
Expand All @@ -92,7 +92,7 @@ func (r *Reconciler) getActiveGateAuthToken() (map[string][]byte, error) {
}, nil
}

func (r *Reconciler) createSecret(secretData map[string][]byte) error {
func (r *Reconciler) createSecret(ctx context.Context, secretData map[string][]byte) error {
secretName := r.dynakube.ActiveGateAuthTokenSecret()
secret, err := kubeobjects.CreateSecret(r.scheme, r.dynakube,
kubeobjects.NewSecretNameModifier(secretName),
Expand All @@ -102,15 +102,15 @@ func (r *Reconciler) createSecret(secretData map[string][]byte) error {
return errors.WithStack(err)
}

err = r.client.Create(context.TODO(), secret)
err = r.client.Create(ctx, secret)
if err != nil {
return errors.Errorf("failed to create secret '%s': %v", secretName, err)
}
return nil
}

func (r *Reconciler) deleteSecret(secret *corev1.Secret) error {
if err := r.client.Delete(context.TODO(), secret); err != nil && !k8serrors.IsNotFound(err) {
func (r *Reconciler) deleteSecret(ctx context.Context, secret *corev1.Secret) error {
if err := r.client.Delete(ctx, secret); err != nil && !k8serrors.IsNotFound(err) {
return err
}
return nil
Expand Down
12 changes: 6 additions & 6 deletions src/controllers/dynakube/activegate/internal/authtoken/reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ func newTestReconcilerWithInstance(client client.Client) *Reconciler {
func TestReconcile(t *testing.T) {
t.Run(`reconcile auth token for first time`, func(t *testing.T) {
r := newTestReconcilerWithInstance(fake.NewClientBuilder().Build())
err := r.Reconcile()
err := r.Reconcile(context.Background())
require.NoError(t, err)

var authToken corev1.Secret
_ = r.client.Get(context.TODO(), client.ObjectKey{Name: r.dynakube.ActiveGateAuthTokenSecret(), Namespace: testNamespace}, &authToken)
_ = r.client.Get(context.Background(), client.ObjectKey{Name: r.dynakube.ActiveGateAuthTokenSecret(), Namespace: testNamespace}, &authToken)

assert.NotEmpty(t, authToken.Data[ActiveGateAuthTokenName])
})
Expand All @@ -73,11 +73,11 @@ func TestReconcile(t *testing.T) {
Build()

r := newTestReconcilerWithInstance(clt)
err := r.Reconcile()
err := r.Reconcile(context.Background())
require.NoError(t, err)

var authToken corev1.Secret
_ = r.client.Get(context.TODO(), client.ObjectKey{Name: r.dynakube.ActiveGateAuthTokenSecret(), Namespace: testNamespace}, &authToken)
_ = r.client.Get(context.Background(), client.ObjectKey{Name: r.dynakube.ActiveGateAuthTokenSecret(), Namespace: testNamespace}, &authToken)

assert.NotEqual(t, authToken.Data[ActiveGateAuthTokenName], []byte(testToken))
})
Expand All @@ -95,11 +95,11 @@ func TestReconcile(t *testing.T) {
Build()
r := newTestReconcilerWithInstance(clt)

err := r.Reconcile()
err := r.Reconcile(context.Background())
require.NoError(t, err)

var authToken corev1.Secret
_ = r.client.Get(context.TODO(), client.ObjectKey{Name: r.dynakube.ActiveGateAuthTokenSecret(), Namespace: testNamespace}, &authToken)
_ = r.client.Get(context.Background(), client.ObjectKey{Name: r.dynakube.ActiveGateAuthTokenSecret(), Namespace: testNamespace}, &authToken)

assert.Equal(t, authToken.Data[ActiveGateAuthTokenName], []byte(testToken))
})
Expand Down
7 changes: 5 additions & 2 deletions src/controllers/dynakube/activegate/internal/capability/mock.go
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package capability

import "github.com/stretchr/testify/mock"
import (
"github.com/stretchr/testify/mock"
"golang.org/x/net/context"
)

type MockReconciler struct {
mock.Mock
}

func (m *MockReconciler) Reconcile() error {
func (m *MockReconciler) Reconcile(_ context.Context) error {
args := m.Called()
return args.Error(0)
}
16 changes: 8 additions & 8 deletions src/controllers/dynakube/activegate/internal/capability/reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,26 +37,26 @@ func NewReconciler(clt client.Client, capability capability.Capability, dynakube

type NewReconcilerFunc = func(clt client.Client, capability capability.Capability, dynakube *dynatracev1beta1.DynaKube, statefulsetReconciler controllers.Reconciler, customPropertiesReconciler controllers.Reconciler) *Reconciler

func (r *Reconciler) Reconcile() error {
err := r.customPropertiesReconciler.Reconcile()
func (r *Reconciler) Reconcile(ctx context.Context) error {
err := r.customPropertiesReconciler.Reconcile(ctx)
if err != nil {
return errors.WithStack(err)
}
if r.dynakube.NeedsActiveGateService() {
err = r.createOrUpdateService()
err = r.createOrUpdateService(ctx)
if err != nil {
return errors.WithStack(err)
}
}

err = r.statefulsetReconciler.Reconcile()
err = r.statefulsetReconciler.Reconcile(ctx)
return errors.WithStack(err)
}

func (r *Reconciler) createOrUpdateService() error {
func (r *Reconciler) createOrUpdateService(ctx context.Context) error {
desired := CreateService(r.dynakube, r.capability.ShortName())
installed := &corev1.Service{}
err := r.client.Get(context.TODO(), kubeobjects.Key(desired), installed)
err := r.client.Get(ctx, kubeobjects.Key(desired), installed)

if k8serrors.IsNotFound(err) {
log.Info("creating AG service", "module", r.capability.ShortName())
Expand All @@ -66,7 +66,7 @@ func (r *Reconciler) createOrUpdateService() error {
return errors.WithStack(err)
}

err = r.client.Create(context.TODO(), desired)
err = r.client.Create(ctx, desired)
return errors.WithStack(err)
}

Expand All @@ -77,7 +77,7 @@ func (r *Reconciler) createOrUpdateService() error {
if r.portsAreOutdated(installed, desired) || r.labelsAreOutdated(installed, desired) {
desired.Spec.ClusterIP = installed.Spec.ClusterIP
desired.ObjectMeta.ResourceVersion = installed.ObjectMeta.ResourceVersion
err = r.client.Update(context.TODO(), desired)
err = r.client.Update(ctx, desired)

if err != nil {
return err
Expand Down

0 comments on commit f1e141d

Please sign in to comment.