From 8b6f6117cb0ab6ab9c64000db38a9c0736f6ad93 Mon Sep 17 00:00:00 2001 From: Francesco Ilario Date: Wed, 8 Nov 2023 15:57:34 +0100 Subject: [PATCH] fix: forward context into pkgs (#499) Signed-off-by: Francesco Ilario --- .../memberoperatorconfig_controller.go | 14 +++++----- .../memberoperatorconfig_controller_test.go | 27 ++++++++++++------- pkg/autoscaler/autoscaler.go | 6 ++--- pkg/autoscaler/autoscaler_test.go | 8 +++--- pkg/cert/secret.go | 8 +++--- pkg/cert/secret_test.go | 12 ++++----- pkg/utils/route/route.go | 4 +-- pkg/utils/route/route_test.go | 9 ++++--- pkg/webhook/deploy/deployment.go | 5 ++-- pkg/webhook/deploy/deployment_test.go | 6 ++--- 10 files changed, 55 insertions(+), 44 deletions(-) diff --git a/controllers/memberoperatorconfig/memberoperatorconfig_controller.go b/controllers/memberoperatorconfig/memberoperatorconfig_controller.go index ba4380e8..ecc67646 100644 --- a/controllers/memberoperatorconfig/memberoperatorconfig_controller.go +++ b/controllers/memberoperatorconfig/memberoperatorconfig_controller.go @@ -57,11 +57,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. return reconcile.Result{}, err } - if err := r.handleAutoscalerDeploy(reqLogger, crtConfig, request.Namespace); err != nil { + if err := r.handleAutoscalerDeploy(ctx, crtConfig, request.Namespace); err != nil { return reconcile.Result{}, err } - if err := r.handleWebhookDeploy(reqLogger, crtConfig, request.Namespace); err != nil { + if err := r.handleWebhookDeploy(ctx, crtConfig, request.Namespace); err != nil { return reconcile.Result{}, err } @@ -72,7 +72,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl. return reconcile.Result{}, nil } -func (r *Reconciler) handleAutoscalerDeploy(logger logr.Logger, cfg Configuration, namespace string) error { +func (r *Reconciler) handleAutoscalerDeploy(ctx context.Context, cfg Configuration, namespace string) error { + logger := log.FromContext(ctx) if cfg.Autoscaler().Deploy() { logger.Info("(Re)Deploying autoscaling buffer") if err := autoscaler.Deploy(r.Client, r.Client.Scheme(), namespace, cfg.Autoscaler().BufferMemory(), cfg.Autoscaler().BufferReplicas()); err != nil { @@ -80,7 +81,7 @@ func (r *Reconciler) handleAutoscalerDeploy(logger logr.Logger, cfg Configuratio } logger.Info("(Re)Deployed autoscaling buffer") } else { - deleted, err := autoscaler.Delete(r.Client, r.Client.Scheme(), namespace) + deleted, err := autoscaler.Delete(ctx, r.Client, r.Client.Scheme(), namespace) if err != nil { return err } @@ -93,13 +94,14 @@ func (r *Reconciler) handleAutoscalerDeploy(logger logr.Logger, cfg Configuratio return nil } -func (r *Reconciler) handleWebhookDeploy(logger logr.Logger, cfg Configuration, namespace string) error { +func (r *Reconciler) handleWebhookDeploy(ctx context.Context, cfg Configuration, namespace string) error { + logger := log.FromContext(ctx) // By default the users' pods webhook will be deployed, however in some cases (eg. e2e tests) there can be multiple member operators // installed in the same cluster. In those cases only 1 webhook is needed because the MutatingWebhookConfiguration is a cluster-scoped resource and naming can conflict. if cfg.Webhook().Deploy() { webhookImage := os.Getenv("MEMBER_OPERATOR_WEBHOOK_IMAGE") logger.Info("(Re)Deploying users' pods webhook") - if err := deploy.Webhook(r.Client, r.Client.Scheme(), namespace, webhookImage); err != nil { + if err := deploy.Webhook(ctx, r.Client, r.Client.Scheme(), namespace, webhookImage); err != nil { return err } logger.Info("(Re)Deployed users' pods webhook") diff --git a/controllers/memberoperatorconfig/memberoperatorconfig_controller_test.go b/controllers/memberoperatorconfig/memberoperatorconfig_controller_test.go index 6ae59f05..fb2168c2 100644 --- a/controllers/memberoperatorconfig/memberoperatorconfig_controller_test.go +++ b/controllers/memberoperatorconfig/memberoperatorconfig_controller_test.go @@ -24,6 +24,7 @@ import ( "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -124,9 +125,10 @@ func TestHandleAutoscalerDeploy(t *testing.T) { controller, cl := prepareReconcile(t, config) actualConfig, err := GetConfiguration(cl) require.NoError(t, err) + ctx := log.IntoContext(context.TODO(), controller.Log) // when - err = controller.handleAutoscalerDeploy(controller.Log, actualConfig, test.MemberOperatorNs) + err = controller.handleAutoscalerDeploy(ctx, actualConfig, test.MemberOperatorNs) // then require.NoError(t, err) @@ -140,12 +142,12 @@ func TestHandleAutoscalerDeploy(t *testing.T) { // given config := commonconfig.NewMemberOperatorConfigWithReset(t, testconfig.Autoscaler().Deploy(true)) controller, cl := prepareReconcile(t, config) - actualConfig, err := GetConfiguration(cl) require.NoError(t, err) + ctx := log.IntoContext(context.TODO(), controller.Log) // when - err = controller.handleAutoscalerDeploy(controller.Log, actualConfig, test.MemberOperatorNs) + err = controller.handleAutoscalerDeploy(ctx, actualConfig, test.MemberOperatorNs) // then require.NoError(t, err) @@ -160,9 +162,10 @@ func TestHandleAutoscalerDeploy(t *testing.T) { updatedConfig, err := ForceLoadConfiguration(cl) require.NoError(t, err) require.False(t, updatedConfig.Autoscaler().Deploy()) + ctx := log.IntoContext(context.TODO(), controller.Log) // when - err = controller.handleAutoscalerDeploy(controller.Log, updatedConfig, test.MemberOperatorNs) + err = controller.handleAutoscalerDeploy(ctx, updatedConfig, test.MemberOperatorNs) // then require.NoError(t, err) @@ -179,12 +182,13 @@ func TestHandleAutoscalerDeploy(t *testing.T) { controller, cl := prepareReconcile(t, config) actualConfig, err := GetConfiguration(cl) require.NoError(t, err) + ctx := log.IntoContext(context.TODO(), controller.Log) // when cl.(*test.FakeClient).MockGet = func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { return fmt.Errorf("client error") } - err = controller.handleAutoscalerDeploy(controller.Log, actualConfig, test.MemberOperatorNs) + err = controller.handleAutoscalerDeploy(ctx, actualConfig, test.MemberOperatorNs) // then require.NotNil(t, err) @@ -202,12 +206,13 @@ func TestHandleAutoscalerDeploy(t *testing.T) { controller, cl := prepareReconcile(t, config, actualPrioClass) actualConfig, err := GetConfiguration(cl) require.NoError(t, err) + ctx := log.IntoContext(context.TODO(), controller.Log) // when cl.(*test.FakeClient).MockDelete = func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { return fmt.Errorf("client error") } - err = controller.handleAutoscalerDeploy(controller.Log, actualConfig, test.MemberOperatorNs) + err = controller.handleAutoscalerDeploy(ctx, actualConfig, test.MemberOperatorNs) // then require.EqualError(t, err, "cannot delete autoscaling buffer object: client error") @@ -219,12 +224,12 @@ func TestHandleWebhookDeploy(t *testing.T) { // given config := commonconfig.NewMemberOperatorConfigWithReset(t, testconfig.Webhook().Deploy(false)) controller, cl := prepareReconcile(t, config) - actualConfig, err := GetConfiguration(cl) require.NoError(t, err) + ctx := log.IntoContext(context.TODO(), controller.Log) // when - err = controller.handleWebhookDeploy(controller.Log, actualConfig, test.MemberOperatorNs) + err = controller.handleWebhookDeploy(ctx, actualConfig, test.MemberOperatorNs) // then require.NoError(t, err) @@ -240,9 +245,10 @@ func TestHandleWebhookDeploy(t *testing.T) { controller, cl := prepareReconcile(t, config) actualConfig, err := GetConfiguration(cl) require.NoError(t, err) + ctx := log.IntoContext(context.TODO(), controller.Log) // when - err = controller.handleWebhookDeploy(controller.Log, actualConfig, test.MemberOperatorNs) + err = controller.handleWebhookDeploy(ctx, actualConfig, test.MemberOperatorNs) // then require.NoError(t, err) @@ -257,12 +263,13 @@ func TestHandleWebhookDeploy(t *testing.T) { controller, cl := prepareReconcile(t, config) actualConfig, err := GetConfiguration(cl) require.NoError(t, err) + ctx := log.IntoContext(context.TODO(), controller.Log) // when cl.(*test.FakeClient).MockGet = func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { return fmt.Errorf("client error") } - err = controller.handleWebhookDeploy(controller.Log, actualConfig, test.MemberOperatorNs) + err = controller.handleWebhookDeploy(ctx, actualConfig, test.MemberOperatorNs) // then require.EqualError(t, err, "cannot deploy webhook template: client error") diff --git a/pkg/autoscaler/autoscaler.go b/pkg/autoscaler/autoscaler.go index c7581be0..c46e24c9 100644 --- a/pkg/autoscaler/autoscaler.go +++ b/pkg/autoscaler/autoscaler.go @@ -36,7 +36,7 @@ func Deploy(cl runtimeclient.Client, s *runtime.Scheme, namespace, requestsMemor // Delete deletes the autoscaling buffer app if it's deployed. Does nothing if it's not. // Returns true if the app was deleted. -func Delete(cl client.Client, s *runtime.Scheme, namespace string) (bool, error) { +func Delete(ctx context.Context, cl client.Client, s *runtime.Scheme, namespace string) (bool, error) { objs, err := getTemplateObjects(s, namespace, "0", 0) if err != nil { return false, err @@ -46,12 +46,12 @@ func Delete(cl client.Client, s *runtime.Scheme, namespace string) (bool, error) for _, obj := range objs { unst := &unstructured.Unstructured{} unst.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind()) - if err := cl.Get(context.TODO(), types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}, unst); err != nil { + if err := cl.Get(ctx, types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}, unst); err != nil { if !errors.IsNotFound(err) { // Ignore not found return false, errs.Wrap(err, "cannot get autoscaling buffer object") } } else { - if err := cl.Delete(context.TODO(), unst); err != nil { + if err := cl.Delete(ctx, unst); err != nil { return false, errs.Wrap(err, "cannot delete autoscaling buffer object") } deleted = true diff --git a/pkg/autoscaler/autoscaler_test.go b/pkg/autoscaler/autoscaler_test.go index b5660df9..89f6e349 100644 --- a/pkg/autoscaler/autoscaler_test.go +++ b/pkg/autoscaler/autoscaler_test.go @@ -99,7 +99,7 @@ func TestDelete(t *testing.T) { AssertThatNamespace(t, test.MemberOperatorNs, fakeClient).HasResource(dm.Name, &appsv1.Deployment{}) // when - deleted, err := Delete(fakeClient, s, test.MemberOperatorNs) + deleted, err := Delete(context.TODO(), fakeClient, s, test.MemberOperatorNs) // then require.NoError(t, err) @@ -113,7 +113,7 @@ func TestDelete(t *testing.T) { fakeClient := test.NewFakeClient(t) // when - deleted, err := Delete(fakeClient, s, test.MemberOperatorNs) + deleted, err := Delete(context.TODO(), fakeClient, s, test.MemberOperatorNs) // then require.NoError(t, err) @@ -128,7 +128,7 @@ func TestDelete(t *testing.T) { } // when - deleted, err := Delete(fakeClient, s, test.MemberOperatorNs) + deleted, err := Delete(context.TODO(), fakeClient, s, test.MemberOperatorNs) // then assert.EqualError(t, err, "cannot get autoscaling buffer object: some error") @@ -143,7 +143,7 @@ func TestDelete(t *testing.T) { } // when - deleted, err := Delete(fakeClient, s, test.MemberOperatorNs) + deleted, err := Delete(context.TODO(), fakeClient, s, test.MemberOperatorNs) // then assert.EqualError(t, err, "cannot delete autoscaling buffer object: some error") diff --git a/pkg/cert/secret.go b/pkg/cert/secret.go index eca6d34d..1ed33b0c 100644 --- a/pkg/cert/secret.go +++ b/pkg/cert/secret.go @@ -28,9 +28,9 @@ const ( Expiration = 365 * 24 * time.Hour ) -func EnsureSecret(cl client.Client, namespace, certSecretName, serviceName string, expiration time.Duration) ([]byte, error) { +func EnsureSecret(ctx context.Context, cl client.Client, namespace, certSecretName, serviceName string, expiration time.Duration) ([]byte, error) { certSecret := &corev1.Secret{} - if err := cl.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: certSecretName}, certSecret); err != nil && !errors.IsNotFound(err) { + if err := cl.Get(ctx, types.NamespacedName{Namespace: namespace, Name: certSecretName}, certSecret); err != nil && !errors.IsNotFound(err) { return nil, err } else if err != nil { // does not exist, so let's create it @@ -38,7 +38,7 @@ func EnsureSecret(cl client.Client, namespace, certSecretName, serviceName strin if err != nil { return nil, err } - if err := cl.Create(context.TODO(), certSecret); err != nil { + if err := cl.Create(ctx, certSecret); err != nil { return nil, err } return certSecret.Data[CACert], nil @@ -64,7 +64,7 @@ func EnsureSecret(cl client.Client, namespace, certSecretName, serviceName strin return nil, err } newSecret.SetResourceVersion(certSecret.GetResourceVersion()) - if err := cl.Update(context.TODO(), newSecret); err != nil { + if err := cl.Update(ctx, newSecret); err != nil { return nil, err } return newSecret.Data[CACert], nil diff --git a/pkg/cert/secret_test.go b/pkg/cert/secret_test.go index cece7585..4bf9b3ed 100644 --- a/pkg/cert/secret_test.go +++ b/pkg/cert/secret_test.go @@ -46,7 +46,7 @@ func TestEnsureCertSecret(t *testing.T) { fakeClient := test.NewFakeClient(t) // when - caCert, err := EnsureSecret(fakeClient, test.MemberOperatorNs, certSecretName, serviceName, Expiration) + caCert, err := EnsureSecret(context.TODO(), fakeClient, test.MemberOperatorNs, certSecretName, serviceName, Expiration) // then require.NoError(t, err) @@ -74,7 +74,7 @@ func TestEnsureCertSecret(t *testing.T) { fakeClient := test.NewFakeClient(t, secret) // when - caCert, err := EnsureSecret(fakeClient, test.MemberOperatorNs, certSecretName, serviceName, Expiration) + caCert, err := EnsureSecret(context.TODO(), fakeClient, test.MemberOperatorNs, certSecretName, serviceName, Expiration) // then require.NoError(t, err) @@ -100,7 +100,7 @@ func TestEnsureCertSecret(t *testing.T) { time.Sleep(shortExpiration / 2) // when - caCert, err := EnsureSecret(fakeClient, test.MemberOperatorNs, certSecretName, serviceName, shortExpiration) + caCert, err := EnsureSecret(context.TODO(), fakeClient, test.MemberOperatorNs, certSecretName, serviceName, shortExpiration) // then require.NoError(t, err) @@ -124,7 +124,7 @@ func TestEnsureCertSecret(t *testing.T) { fakeClient := test.NewFakeClient(t, secret) // when - caCert, err := EnsureSecret(fakeClient, test.MemberOperatorNs, certSecretName, serviceName, Expiration) + caCert, err := EnsureSecret(context.TODO(), fakeClient, test.MemberOperatorNs, certSecretName, serviceName, Expiration) // then require.NoError(t, err) @@ -145,7 +145,7 @@ func TestEnsureCertSecret(t *testing.T) { } // when - caCert, err := EnsureSecret(fakeClient, test.MemberOperatorNs, certSecretName, serviceName, Expiration) + caCert, err := EnsureSecret(context.TODO(), fakeClient, test.MemberOperatorNs, certSecretName, serviceName, Expiration) fmt.Println() // then @@ -164,7 +164,7 @@ func TestEnsureCertSecret(t *testing.T) { } // when - caCert, err := EnsureSecret(fakeClient, test.MemberOperatorNs, certSecretName, serviceName, Expiration) + caCert, err := EnsureSecret(context.TODO(), fakeClient, test.MemberOperatorNs, certSecretName, serviceName, Expiration) // then require.Error(t, err) diff --git a/pkg/utils/route/route.go b/pkg/utils/route/route.go index 6379673f..263f9003 100644 --- a/pkg/utils/route/route.go +++ b/pkg/utils/route/route.go @@ -11,10 +11,10 @@ import ( ) // GetRouteURL gets the URL of the route with the given name and namespace using the given client -func GetRouteURL(cl client.Client, namespace, name string) (string, error) { +func GetRouteURL(ctx context.Context, cl client.Client, namespace, name string) (string, error) { route := &routev1.Route{} namespacedName := types.NamespacedName{Namespace: namespace, Name: name} - err := cl.Get(context.TODO(), namespacedName, route) + err := cl.Get(ctx, namespacedName, route) if err != nil { return "", err } diff --git a/pkg/utils/route/route_test.go b/pkg/utils/route/route_test.go index 1c832aa5..1cfe7d22 100644 --- a/pkg/utils/route/route_test.go +++ b/pkg/utils/route/route_test.go @@ -1,6 +1,7 @@ package route import ( + "context" "fmt" "testing" @@ -36,7 +37,7 @@ func TestGetRouteURL(t *testing.T) { cl := test.NewFakeClient(t, route) // when - routeURL, err := GetRouteURL(cl, ns, name) + routeURL, err := GetRouteURL(context.TODO(), cl, ns, name) // then require.NoError(t, err) @@ -51,7 +52,7 @@ func TestGetRouteURL(t *testing.T) { cl := test.NewFakeClient(t, r) // when - routeURL, err := GetRouteURL(cl, ns, name) + routeURL, err := GetRouteURL(context.TODO(), cl, ns, name) // then require.NoError(t, err) @@ -66,7 +67,7 @@ func TestGetRouteURL(t *testing.T) { cl := test.NewFakeClient(t, r) // when - routeURL, err := GetRouteURL(cl, ns, name) + routeURL, err := GetRouteURL(context.TODO(), cl, ns, name) // then require.NoError(t, err) @@ -81,7 +82,7 @@ func TestGetRouteURL(t *testing.T) { cl := test.NewFakeClient(t, r) // when - routeURL, err := GetRouteURL(cl, ns, name) + routeURL, err := GetRouteURL(context.TODO(), cl, ns, name) // then require.NoError(t, err) diff --git a/pkg/webhook/deploy/deployment.go b/pkg/webhook/deploy/deployment.go index e2bf3d04..59d6749a 100644 --- a/pkg/webhook/deploy/deployment.go +++ b/pkg/webhook/deploy/deployment.go @@ -1,6 +1,7 @@ package deploy import ( + "context" "encoding/base64" "github.com/codeready-toolchain/member-operator/pkg/cert" @@ -23,8 +24,8 @@ const ( serviceName = "member-operator-webhook" ) -func Webhook(cl runtimeclient.Client, s *runtime.Scheme, namespace, image string) error { - caBundle, err := cert.EnsureSecret(cl, namespace, certSecretName, serviceName, cert.Expiration) +func Webhook(ctx context.Context, cl runtimeclient.Client, s *runtime.Scheme, namespace, image string) error { + caBundle, err := cert.EnsureSecret(ctx, cl, namespace, certSecretName, serviceName, cert.Expiration) if err != nil { return errs.Wrap(err, "cannot deploy webhook template") } diff --git a/pkg/webhook/deploy/deployment_test.go b/pkg/webhook/deploy/deployment_test.go index deea813a..19e47730 100644 --- a/pkg/webhook/deploy/deployment_test.go +++ b/pkg/webhook/deploy/deployment_test.go @@ -59,7 +59,7 @@ func TestDeployWebhook(t *testing.T) { fakeClient := test.NewFakeClient(t) // when - err := Webhook(fakeClient, s, test.MemberOperatorNs, imgLoc) + err := Webhook(context.TODO(), fakeClient, s, test.MemberOperatorNs, imgLoc) // then require.NoError(t, err) @@ -90,7 +90,7 @@ func TestDeployWebhook(t *testing.T) { fakeClient := test.NewFakeClient(t, prioClass, serviceObj, deploymentObj, mutWbhConf) // when - err := Webhook(fakeClient, s, test.MemberOperatorNs, imgLoc) + err := Webhook(context.TODO(), fakeClient, s, test.MemberOperatorNs, imgLoc) // then require.NoError(t, err) @@ -105,7 +105,7 @@ func TestDeployWebhook(t *testing.T) { } // when - err := Webhook(fakeClient, s, test.MemberOperatorNs, imgLoc) + err := Webhook(context.TODO(), fakeClient, s, test.MemberOperatorNs, imgLoc) // then require.Error(t, err)