Skip to content

Commit

Permalink
Add ctx parameter to Federator methods
Browse files Browse the repository at this point in the history
Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
  • Loading branch information
tpantelis committed Oct 30, 2023
1 parent 545d893 commit dd8dc39
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 44 deletions.
4 changes: 2 additions & 2 deletions pkg/federate/base_federator.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ func newBaseFederator(dynClient dynamic.Interface, restMapper meta.RESTMapper, t
return b
}

func (f *baseFederator) Delete(obj runtime.Object) error {
func (f *baseFederator) Delete(ctx context.Context, obj runtime.Object) error {
toDelete, resourceClient, err := f.toUnstructured(obj)
if err != nil {
return err
}

logger.V(log.LIBTRACE).Infof("Deleting resource: %#v", toDelete)

err = resourceClient.Delete(context.TODO(), toDelete.GetName(), metav1.DeleteOptions{})
err = resourceClient.Delete(ctx, toDelete.GetName(), metav1.DeleteOptions{})

if f.eventLogName != "" && err == nil {
logger.Infof("%s: Deleted %s \"%s/%s\" ", f.eventLogName, toDelete.GetKind(), toDelete.GetNamespace(), toDelete.GetName())
Expand Down
4 changes: 2 additions & 2 deletions pkg/federate/create_federator.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func NewCreateFederator(dynClient dynamic.Interface, restMapper meta.RESTMapper,
}

//nolint:wrapcheck // This function is effectively a wrapper so no need to wrap errors.
func (f *createFederator) Distribute(obj runtime.Object) error {
func (f *createFederator) Distribute(ctx context.Context, obj runtime.Object) error {
logger.V(log.LIBTRACE).Infof("In Distribute for %#v", obj)

toDistribute, resourceClient, err := f.toUnstructured(obj)
Expand All @@ -50,7 +50,7 @@ func (f *createFederator) Distribute(obj runtime.Object) error {

f.prepareResourceForSync(toDistribute)

_, err = resourceClient.Create(context.TODO(), toDistribute, metav1.CreateOptions{})
_, err = resourceClient.Create(ctx, toDistribute, metav1.CreateOptions{})
if apierrors.IsAlreadyExists(err) {
return nil
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/federate/create_or_update_federator.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NewCreateOrUpdateFederator(dynClient dynamic.Interface, restMapper meta.RES
}
}

func (f *createOrUpdateFederator) Distribute(obj runtime.Object) error {
func (f *createOrUpdateFederator) Distribute(ctx context.Context, obj runtime.Object) error {
logger.V(log.LIBTRACE).Infof("In Distribute for %#v", obj)

toDistribute, resourceClient, err := f.toUnstructured(obj)
Expand All @@ -58,7 +58,7 @@ func (f *createOrUpdateFederator) Distribute(obj runtime.Object) error {

f.prepareResourceForSync(toDistribute)

result, err := util.CreateOrUpdate[runtime.Object](context.TODO(), resource.ForDynamic(resourceClient), toDistribute,
result, err := util.CreateOrUpdate[runtime.Object](ctx, resource.ForDynamic(resourceClient), toDistribute,
func(obj runtime.Object) (runtime.Object, error) {
return util.CopyImmutableMetadata(obj.(*unstructured.Unstructured), toDistribute), nil
})
Expand Down
5 changes: 3 additions & 2 deletions pkg/federate/fake/federator.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ limitations under the License.
package fake

import (
"context"
"time"

. "github.com/onsi/gomega"
Expand All @@ -42,7 +43,7 @@ func New() *Federator {
}
}

func (f *Federator) Distribute(resource runtime.Object) error {
func (f *Federator) Distribute(_ context.Context, resource runtime.Object) error {
err := f.FailOnDistribute
if err != nil {
if f.ResetOnFailure {
Expand All @@ -57,7 +58,7 @@ func (f *Federator) Distribute(resource runtime.Object) error {
return nil
}

func (f *Federator) Delete(resource runtime.Object) error {
func (f *Federator) Delete(_ context.Context, resource runtime.Object) error {
err := f.FailOnDelete
if err != nil {
if f.ResetOnFailure {
Expand Down
10 changes: 6 additions & 4 deletions pkg/federate/federator.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ limitations under the License.
package federate

import (
"context"

"k8s.io/apimachinery/pkg/runtime"
)

Expand All @@ -34,12 +36,12 @@ type Federator interface {
//
// If the resource was previously distributed and the given resource differs, each previous cluster will receive the
// updated resource.
Distribute(resource runtime.Object) error
Distribute(ctx context.Context, resource runtime.Object) error

// Delete stops distributing the given resource and deletes it from all clusters to which it was distributed.
// The actual deletion may occur asynchronously in which any returned error only indicates that the request
// failed.
Delete(resource runtime.Object) error
Delete(ctx context.Context, resource runtime.Object) error
}

type FederatorExt interface {
Expand All @@ -54,10 +56,10 @@ func NewNoopFederator() Federator {
return &noopFederator{}
}

func (n noopFederator) Distribute(_ runtime.Object) error {
func (n noopFederator) Distribute(_ context.Context, _ runtime.Object) error {
return nil
}

func (n noopFederator) Delete(_ runtime.Object) error {
func (n noopFederator) Delete(_ context.Context, _ runtime.Object) error {
return nil
}
55 changes: 29 additions & 26 deletions pkg/federate/federator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ limitations under the License.
package federate_test

import (
"context"
"errors"

. "github.com/onsi/ginkgo/v2"
Expand All @@ -36,6 +37,8 @@ import (
)

var (
ctx = context.Background()

_ = Describe("CreateOrUpdate Federator", testCreateOrUpdateFederator)
_ = Describe("Create Federator", testCreateFederator)
_ = Describe("Update Federator", testUpdateFederator)
Expand All @@ -61,7 +64,7 @@ func testCreateOrUpdateFederator() {
When("the resource does not already exist in the datastore", func() {
Context("and a local cluster ID is specified", func() {
It("should create the resource with the cluster ID label", func() {
Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})
})
Expand All @@ -72,7 +75,7 @@ func testCreateOrUpdateFederator() {
})

It("should create the resource without the cluster ID label", func() {
Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})
})
Expand All @@ -86,7 +89,7 @@ func testCreateOrUpdateFederator() {
})

It("should create the resource with the Status data", func() {
Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})
})
Expand All @@ -103,7 +106,7 @@ func testCreateOrUpdateFederator() {
})

It("should create the resource with the OwnerReferences", func() {
Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})
})
Expand All @@ -114,7 +117,7 @@ func testCreateOrUpdateFederator() {
})

It("should return an error", func() {
Expect(f.Distribute(t.resource)).ToNot(Succeed())
Expect(f.Distribute(ctx, t.resource)).ToNot(Succeed())
})
})

Expand All @@ -129,7 +132,7 @@ func testCreateOrUpdateFederator() {
})

It("should update the resource", func() {
Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})
})
Expand All @@ -143,7 +146,7 @@ func testCreateOrUpdateFederator() {
})

It("should update the resource", func() {
Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})

Expand All @@ -153,7 +156,7 @@ func testCreateOrUpdateFederator() {
})

It("should retry until it succeeds", func() {
Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})
})
Expand All @@ -164,7 +167,7 @@ func testCreateOrUpdateFederator() {
})

It("should return an error", func() {
Expect(f.Distribute(t.resource)).ToNot(Succeed())
Expect(f.Distribute(ctx, t.resource)).ToNot(Succeed())
})
})
})
Expand All @@ -175,7 +178,7 @@ func testCreateOrUpdateFederator() {
})

It("should return an error", func() {
Expect(f.Distribute(t.resource)).ToNot(Succeed())
Expect(f.Distribute(ctx, t.resource)).ToNot(Succeed())
})
})

Expand All @@ -186,7 +189,7 @@ func testCreateOrUpdateFederator() {
})

It("should create the resource in the source namespace", func() {
Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})
})
Expand All @@ -210,7 +213,7 @@ func testCreateFederator() {

When("the resource does not already exist in the datastore", func() {
It("create the resource", func() {
Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})

Expand All @@ -220,7 +223,7 @@ func testCreateFederator() {
})

It("should return an error", func() {
Expect(f.Distribute(t.resource)).ToNot(Succeed())
Expect(f.Distribute(ctx, t.resource)).ToNot(Succeed())
})
})
})
Expand All @@ -232,7 +235,7 @@ func testCreateFederator() {
})

It("should succeed and not update the resource", func() {
Expect(f.Distribute(test.NewPodWithImage(test.LocalNamespace, "apache"))).To(Succeed())
Expect(f.Distribute(ctx, test.NewPodWithImage(test.LocalNamespace, "apache"))).To(Succeed())
t.verifyResource()
})
})
Expand Down Expand Up @@ -263,7 +266,7 @@ func testUpdateFederator() {
})

It("should update the resource", func() {
Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})

Expand All @@ -273,7 +276,7 @@ func testUpdateFederator() {
})

It("should retry until it succeeds", func() {
Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})

Expand All @@ -283,7 +286,7 @@ func testUpdateFederator() {
})

It("should return an error", func() {
Expect(f.Distribute(t.resource)).ToNot(Succeed())
Expect(f.Distribute(ctx, t.resource)).ToNot(Succeed())
})
})
})
Expand All @@ -294,14 +297,14 @@ func testUpdateFederator() {
})

It("should return an error", func() {
Expect(f.Distribute(t.resource)).ToNot(Succeed())
Expect(f.Distribute(ctx, t.resource)).ToNot(Succeed())
})
})
})

When("the resource does not exist in the datastore", func() {
It("should succeed", func() {
Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
})
})
}
Expand Down Expand Up @@ -337,7 +340,7 @@ func testUpdateStatusFederator() {
},
}

Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})
})
Expand All @@ -355,7 +358,7 @@ func testUpdateStatusFederator() {
PodIP: "1.2.3.4",
}

Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())
t.verifyResource()
})
})
Expand All @@ -376,7 +379,7 @@ func testUpdateStatusFederator() {
t.resource.Annotations = map[string]string{"key1": "abc"}
t.resource.Labels = map[string]string{"key2": "def"}

Expect(f.Distribute(t.resource)).To(Succeed())
Expect(f.Distribute(ctx, t.resource)).To(Succeed())

t.resource = prev

Expand Down Expand Up @@ -408,7 +411,7 @@ func testDelete() {
})

It("should delete the resource", func() {
Expect(f.Delete(t.resource)).To(Succeed())
Expect(f.Delete(ctx, t.resource)).To(Succeed())

_, err := test.GetResourceAndError(t.resourceClient, t.resource)
Expect(apierrors.IsNotFound(err)).To(BeTrue())
Expand All @@ -420,7 +423,7 @@ func testDelete() {
})

It("should return an error", func() {
Expect(f.Delete(t.resource)).ToNot(Succeed())
Expect(f.Delete(ctx, t.resource)).ToNot(Succeed())
})
})

Expand All @@ -431,7 +434,7 @@ func testDelete() {
})

It("should delete the resource from the source namespace", func() {
Expect(f.Delete(t.resource)).To(Succeed())
Expect(f.Delete(ctx, t.resource)).To(Succeed())

_, err := test.GetResourceAndError(t.resourceClient, t.resource)
Expect(apierrors.IsNotFound(err)).To(BeTrue())
Expand All @@ -441,7 +444,7 @@ func testDelete() {

When("the resource does not exist in the datastore", func() {
It("should return NotFound error", func() {
Expect(apierrors.IsNotFound(f.Delete(t.resource))).To(BeTrue())
Expect(apierrors.IsNotFound(f.Delete(ctx, t.resource))).To(BeTrue())
})
})
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/federate/update_federator.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewUpdateStatusFederator(dynClient dynamic.Interface, restMapper meta.RESTM
})
}

func (f *updateFederator) Distribute(obj runtime.Object) error {
func (f *updateFederator) Distribute(ctx context.Context, obj runtime.Object) error {
logger.V(log.LIBTRACE).Infof("In Distribute for %#v", obj)

toUpdate, resourceClient, err := f.toUnstructured(obj)
Expand All @@ -62,7 +62,7 @@ func (f *updateFederator) Distribute(obj runtime.Object) error {

f.prepareResourceForSync(toUpdate)

return util.Update[runtime.Object](context.TODO(), resource.ForDynamic(resourceClient), toUpdate,
return util.Update[runtime.Object](ctx, resource.ForDynamic(resourceClient), toUpdate,
func(obj runtime.Object) (runtime.Object, error) {
return f.update(obj.(*unstructured.Unstructured), toUpdate), nil
})
Expand Down
Loading

0 comments on commit dd8dc39

Please sign in to comment.