From 9a9f915098e3d428963e327611d7e25ae43eff92 Mon Sep 17 00:00:00 2001 From: Francesco Ilario Date: Mon, 13 Nov 2023 21:00:37 +0100 Subject: [PATCH] fix: forward context in common pkg/client (#340) Signed-off-by: Francesco Ilario Co-authored-by: Alexey Kazakov --- pkg/client/client.go | 24 +++++++------- pkg/client/client_test.go | 66 +++++++++++++++++++-------------------- 2 files changed, 45 insertions(+), 45 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 94a3a630..3ac5c9b2 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -79,12 +79,12 @@ func SaveConfiguration(saveConfiguration bool) ApplyObjectOption { } // ApplyRuntimeObject casts the provided object to client.Object and calls ApplyClient.ApplyObject method -func (c ApplyClient) ApplyRuntimeObject(obj runtime.Object, options ...ApplyObjectOption) (bool, error) { +func (c ApplyClient) ApplyRuntimeObject(ctx context.Context, obj runtime.Object, options ...ApplyObjectOption) (bool, error) { clientObj, ok := obj.(client.Object) if !ok { return false, fmt.Errorf("unable to cast of the object to client.Object: %+v", obj) } - return c.applyObject(clientObj, options...) + return c.applyObject(ctx, clientObj, options...) } // ApplyObject creates the object if is missing and if the owner object is provided, then it's set as a controller reference. @@ -92,16 +92,16 @@ func (c ApplyClient) ApplyRuntimeObject(obj runtime.Object, options ...ApplyObje // is automatically updated. If it looks to be same then based on the value of forceUpdate param it updates the object or not. // The return boolean says if the object was either created or updated (`true`). If nothing changed (ie, the generation was not // incremented by the server), then it returns `false`. -func (c ApplyClient) ApplyObject(obj client.Object, options ...ApplyObjectOption) (bool, error) { +func (c ApplyClient) ApplyObject(ctx context.Context, obj client.Object, options ...ApplyObjectOption) (bool, error) { gvk := obj.GetObjectKind().GroupVersionKind() - createdOrUpdated, err := c.applyObject(obj, options...) + createdOrUpdated, err := c.applyObject(ctx, obj, options...) if err != nil { return createdOrUpdated, errors.Wrapf(err, "unable to create resource of kind: %s, version: %s", gvk.Kind, gvk.Version) } return createdOrUpdated, nil } -func (c ApplyClient) applyObject(obj client.Object, options ...ApplyObjectOption) (bool, error) { +func (c ApplyClient) applyObject(ctx context.Context, obj client.Object, options ...ApplyObjectOption) (bool, error) { // gets the meta accessor to the new resource // gets the meta accessor to the new resource config := newApplyObjectConfiguration(options...) @@ -122,9 +122,9 @@ func (c ApplyClient) applyObject(obj client.Object, options ...ApplyObjectOption } // gets current object (if exists) namespacedName := types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()} - if err := c.Client.Get(context.TODO(), namespacedName, existing); err != nil { + if err := c.Client.Get(ctx, namespacedName, existing); err != nil { if apierrors.IsNotFound(err) { - return true, c.createObj(obj, config.owner) + return true, c.createObj(ctx, obj, config.owner) } return false, errors.Wrapf(err, "unable to get the resource '%v'", existing) } @@ -151,7 +151,7 @@ func (c ApplyClient) applyObject(obj client.Object, options ...ApplyObjectOption if err := RetainClusterIP(obj, existing); err != nil { return false, err } - if err := c.Client.Update(context.TODO(), obj); err != nil { + if err := c.Client.Update(ctx, obj); err != nil { return false, errors.Wrapf(err, "unable to update the resource '%v'", obj) } @@ -211,25 +211,25 @@ func marshalObjectContent(newResource runtime.Object) ([]byte, error) { return json.Marshal(newResource) } -func (c ApplyClient) createObj(newResource client.Object, owner v1.Object) error { +func (c ApplyClient) createObj(ctx context.Context, newResource client.Object, owner v1.Object) error { if owner != nil { err := controllerutil.SetControllerReference(owner, newResource, c.Client.Scheme()) if err != nil { return errors.Wrap(err, "unable to set controller references") } } - return c.Client.Create(context.TODO(), newResource) + return c.Client.Create(ctx, newResource) } // Apply applies the objects, ie, creates or updates them on the cluster // returns `true, nil` if at least one of the objects was created or modified, // `false, nil` if nothing changed, and `false, err` if an error occurred -func (c ApplyClient) Apply(toolchainObjects []client.Object, newLabels map[string]string) (bool, error) { +func (c ApplyClient) Apply(ctx context.Context, toolchainObjects []client.Object, newLabels map[string]string) (bool, error) { createdOrUpdated := false for _, toolchainObject := range toolchainObjects { MergeLabels(toolchainObject, newLabels) - result, err := c.ApplyObject(toolchainObject, ForceUpdate(true)) + result, err := c.ApplyObject(ctx, toolchainObject, ForceUpdate(true)) if err != nil { return false, errors.Wrapf(err, "unable to create resource of kind: %s, version: %s", toolchainObject.GetObjectKind().GroupVersionKind().Kind, toolchainObject.GetObjectKind().GroupVersionKind().Version) } diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index a6009907..34a43f08 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -79,12 +79,12 @@ func TestApplySingle(t *testing.T) { // given cl, _ := newClient(t) obj := defaultService.DeepCopy() - _, err := cl.ApplyObject(obj, client.ForceUpdate(true)) + _, err := cl.ApplyObject(context.TODO(), obj, client.ForceUpdate(true)) require.NoError(t, err) originalGeneration := obj.GetGeneration() // when updating with the same obj again - createdOrChanged, err := cl.ApplyObject(obj, client.ForceUpdate(true)) + createdOrChanged, err := cl.ApplyObject(context.TODO(), obj, client.ForceUpdate(true)) // then require.NoError(t, err) @@ -97,12 +97,12 @@ func TestApplySingle(t *testing.T) { // given cl, _ := newClient(t) obj := defaultService.DeepCopy() - _, err := cl.ApplyObject(obj, client.ForceUpdate(true)) + _, err := cl.ApplyObject(context.TODO(), obj, client.ForceUpdate(true)) require.NoError(t, err) originalGeneration := obj.GetGeneration() obj.Spec.ClusterIP = "" // modify for version to update // when updating with the same obj again - createdOrChanged, err := cl.ApplyObject(obj, client.ForceUpdate(true)) + createdOrChanged, err := cl.ApplyObject(context.TODO(), obj, client.ForceUpdate(true)) // then require.NoError(t, err) @@ -116,14 +116,14 @@ func TestApplySingle(t *testing.T) { // given cl, _ := newClient(t) obj := defaultService.DeepCopy() - _, err := cl.ApplyObject(obj, client.ForceUpdate(true)) + _, err := cl.ApplyObject(context.TODO(), obj, client.ForceUpdate(true)) require.NoError(t, err) originalGeneration := obj.GetGeneration() // when updating with the modified obj modifiedObj := modifiedService.DeepCopy() modifiedObj.Spec.ClusterIP = "" - createdOrChanged, err := cl.ApplyObject(modifiedObj, client.ForceUpdate(true)) + createdOrChanged, err := cl.ApplyObject(context.TODO(), modifiedObj, client.ForceUpdate(true)) // then require.NoError(t, err) @@ -137,14 +137,14 @@ func TestApplySingle(t *testing.T) { // given cl, _ := newClient(t) obj := defaultService.DeepCopy() - _, err := cl.ApplyObject(obj, client.ForceUpdate(true)) + _, err := cl.ApplyObject(context.TODO(), obj, client.ForceUpdate(true)) require.NoError(t, err) originalGeneration := obj.GetGeneration() // when updating with the modified obj modifiedObj := modifiedService.DeepCopy() modifiedObj.Spec.ClusterIP = "" - createdOrChanged, err := cl.ApplyObject(modifiedObj, client.ForceUpdate(true)) + createdOrChanged, err := cl.ApplyObject(context.TODO(), modifiedObj, client.ForceUpdate(true)) // then require.NoError(t, err) @@ -159,7 +159,7 @@ func TestApplySingle(t *testing.T) { cl, cli := newClient(t) // when - createdOrChanged, err := cl.ApplyRuntimeObject(modifiedService.DeepCopyObject(), client.ForceUpdate(true), client.SetOwner(&appsv1.Deployment{})) + createdOrChanged, err := cl.ApplyRuntimeObject(context.TODO(), modifiedService.DeepCopyObject(), client.ForceUpdate(true), client.SetOwner(&appsv1.Deployment{})) // then require.NoError(t, err) @@ -178,11 +178,11 @@ func TestApplySingle(t *testing.T) { t.Run("it should update when spec is different", func(t *testing.T) { // given cl, cli := newClient(t) - _, err := cl.ApplyRuntimeObject(defaultService.DeepCopyObject(), client.ForceUpdate(true)) + _, err := cl.ApplyRuntimeObject(context.TODO(), defaultService.DeepCopyObject(), client.ForceUpdate(true)) require.NoError(t, err) // when - createdOrChanged, err := cl.ApplyRuntimeObject(modifiedService.DeepCopyObject()) + createdOrChanged, err := cl.ApplyRuntimeObject(context.TODO(), modifiedService.DeepCopyObject()) // then require.NoError(t, err) @@ -196,11 +196,11 @@ func TestApplySingle(t *testing.T) { t.Run("it should not update when using same object", func(t *testing.T) { // given cl, _ := newClient(t) - _, err := cl.ApplyRuntimeObject(defaultService.DeepCopyObject(), client.ForceUpdate(true)) + _, err := cl.ApplyRuntimeObject(context.TODO(), defaultService.DeepCopyObject(), client.ForceUpdate(true)) require.NoError(t, err) // when - createdOrChanged, err := cl.ApplyRuntimeObject(defaultService.DeepCopyObject()) + createdOrChanged, err := cl.ApplyRuntimeObject(context.TODO(), defaultService.DeepCopyObject()) // then require.NoError(t, err) @@ -212,7 +212,7 @@ func TestApplySingle(t *testing.T) { cl, cli := newClient(t) // when - createdOrChanged, err := cl.ApplyRuntimeObject(modifiedService.DeepCopyObject(), client.SetOwner(&appsv1.Deployment{})) + createdOrChanged, err := cl.ApplyRuntimeObject(context.TODO(), modifiedService.DeepCopyObject(), client.SetOwner(&appsv1.Deployment{})) // then require.NoError(t, err) @@ -232,7 +232,7 @@ func TestApplySingle(t *testing.T) { cl, cli := newClient(t) // when - createdOrChanged, err := cl.ApplyRuntimeObject(modifiedService.DeepCopyObject(), client.SaveConfiguration(false)) + createdOrChanged, err := cl.ApplyRuntimeObject(context.TODO(), modifiedService.DeepCopyObject(), client.SaveConfiguration(false)) // then require.NoError(t, err) @@ -247,11 +247,11 @@ func TestApplySingle(t *testing.T) { t.Run("it should update when spec is different", func(t *testing.T) { // given cl, cli := newClient(t) - _, err := cl.ApplyRuntimeObject(defaultService.DeepCopyObject(), client.SaveConfiguration(false)) + _, err := cl.ApplyRuntimeObject(context.TODO(), defaultService.DeepCopyObject(), client.SaveConfiguration(false)) require.NoError(t, err) // when - createdOrChanged, err := cl.ApplyRuntimeObject(modifiedService.DeepCopyObject(), client.SaveConfiguration(false)) + createdOrChanged, err := cl.ApplyRuntimeObject(context.TODO(), modifiedService.DeepCopyObject(), client.SaveConfiguration(false)) // then require.NoError(t, err) @@ -272,7 +272,7 @@ func TestApplySingle(t *testing.T) { } // when - createdOrChanged, err := cl.ApplyRuntimeObject(modifiedService.DeepCopyObject()) + createdOrChanged, err := cl.ApplyRuntimeObject(context.TODO(), modifiedService.DeepCopyObject()) // then require.Error(t, err) @@ -295,14 +295,14 @@ func TestApplySingle(t *testing.T) { obj, err := toUnstructured(defaultService.DeepCopy()) require.NoError(t, err) - _, err = cl.ApplyObject(obj, client.ForceUpdate(true)) + _, err = cl.ApplyObject(context.TODO(), obj, client.ForceUpdate(true)) require.NoError(t, err) modifiedObj := obj.DeepCopy() err = unstructured.SetNestedField(modifiedObj.Object, "", "spec", "clusterIP") // modify for version to update require.NoError(t, err) // when updating with the same obj again - createdOrChanged, err := cl.ApplyObject(modifiedObj, client.ForceUpdate(true)) + createdOrChanged, err := cl.ApplyObject(context.TODO(), modifiedObj, client.ForceUpdate(true)) // then require.NoError(t, err) @@ -322,11 +322,11 @@ func TestApplySingle(t *testing.T) { t.Run("it should update ConfigMap when data field is different and forceUpdate=false", func(t *testing.T) { // given cl, cli := newClient(t) - _, err := cl.ApplyRuntimeObject(defaultCm.DeepCopyObject(), client.ForceUpdate(true)) + _, err := cl.ApplyRuntimeObject(context.TODO(), defaultCm.DeepCopyObject(), client.ForceUpdate(true)) require.NoError(t, err) // when - createdOrChanged, err := cl.ApplyRuntimeObject(modifiedCm.DeepCopyObject()) + createdOrChanged, err := cl.ApplyRuntimeObject(context.TODO(), modifiedCm.DeepCopyObject()) // then require.NoError(t, err) @@ -533,7 +533,7 @@ func TestProcessAndApply(t *testing.T) { labels := newLabels("", "john", "") // when - createdOrUpdated, err := client.NewApplyClient(cl).Apply(objs, labels) + createdOrUpdated, err := client.NewApplyClient(cl).Apply(context.TODO(), objs, labels) // then require.NoError(t, err) @@ -553,7 +553,7 @@ func TestProcessAndApply(t *testing.T) { labels := newLabels("base1ns", "john", "dev") // when - createdOrUpdated, err := client.NewApplyClient(cl).Apply(objs, labels) + createdOrUpdated, err := client.NewApplyClient(cl).Apply(context.TODO(), objs, labels) // then require.NoError(t, err) @@ -573,7 +573,7 @@ func TestProcessAndApply(t *testing.T) { labels := newLabels("", "john", "dev") // when - createdOrUpdated, err := client.NewApplyClient(cl).Apply(objs, labels) + createdOrUpdated, err := client.NewApplyClient(cl).Apply(context.TODO(), objs, labels) // then require.NoError(t, err) @@ -600,7 +600,7 @@ func TestProcessAndApply(t *testing.T) { require.NoError(t, err) witoutType := newLabels("base1ns", "john", "") - createdOrUpdated, err := client.NewApplyClient(cl).Apply(objs, witoutType) + createdOrUpdated, err := client.NewApplyClient(cl).Apply(context.TODO(), objs, witoutType) require.NoError(t, err) assert.True(t, createdOrUpdated) assertRoleBindingExists(t, cl, user, witoutType) @@ -612,7 +612,7 @@ func TestProcessAndApply(t *testing.T) { objs, err = p.Process(tmpl, values) require.NoError(t, err) complete := newLabels("advanced", "john", "dev") - createdOrUpdated, err = client.NewApplyClient(cl).Apply(objs, complete) + createdOrUpdated, err = client.NewApplyClient(cl).Apply(context.TODO(), objs, complete) // then require.NoError(t, err) @@ -635,14 +635,14 @@ func TestProcessAndApply(t *testing.T) { objs, err := p.Process(tmpl, values) require.NoError(t, err) labels := newLabels("base1ns", "john", "dev") - created, err := client.NewApplyClient(cl).Apply(objs, labels) + created, err := client.NewApplyClient(cl).Apply(context.TODO(), objs, labels) require.NoError(t, err) assert.True(t, created) assertNamespaceExists(t, cl, user, labels, commit) assertRoleBindingExists(t, cl, user, labels) // when apply the same template again - updated, err := client.NewApplyClient(cl).Apply(objs, labels) + updated, err := client.NewApplyClient(cl).Apply(context.TODO(), objs, labels) // then require.NoError(t, err) @@ -665,7 +665,7 @@ func TestProcessAndApply(t *testing.T) { // when objs, err := p.Process(tmpl, values) require.NoError(t, err) - createdOrUpdated, err := client.NewApplyClient(cl).Apply(objs, newLabels("", "", "")) + createdOrUpdated, err := client.NewApplyClient(cl).Apply(context.TODO(), objs, newLabels("", "", "")) // then require.Error(t, err) @@ -685,7 +685,7 @@ func TestProcessAndApply(t *testing.T) { objs, err := p.Process(tmpl, values) require.NoError(t, err) labels := newLabels("", "", "") - createdOrUpdated, err := client.NewApplyClient(cl).Apply(objs, labels) + createdOrUpdated, err := client.NewApplyClient(cl).Apply(context.TODO(), objs, labels) require.NoError(t, err) assert.True(t, createdOrUpdated) @@ -695,7 +695,7 @@ func TestProcessAndApply(t *testing.T) { require.NoError(t, err) objs, err = p.Process(tmpl, values) require.NoError(t, err) - createdOrUpdated, err = client.NewApplyClient(cl).Apply(objs, newLabels("advanced", "john", "dev")) + createdOrUpdated, err = client.NewApplyClient(cl).Apply(context.TODO(), objs, newLabels("advanced", "john", "dev")) // then assert.Error(t, err) @@ -727,7 +727,7 @@ func TestProcessAndApply(t *testing.T) { }, }) labels := newLabels("base1ns", "john", "dev") - createdOrUpdated, err := client.NewApplyClient(cl).Apply(objs, labels) + createdOrUpdated, err := client.NewApplyClient(cl).Apply(context.TODO(), objs, labels) // then require.NoError(t, err)