Skip to content

Commit

Permalink
Prepare context with cluster
Browse files Browse the repository at this point in the history
  • Loading branch information
@aaronjwood
aaronjwood committed Apr 30, 2024
1 parent 607a854 commit 73900a3
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 26 deletions.
3 changes: 3 additions & 0 deletions pkg/internal/controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ import (
"time"

"github.com/go-logr/logr"
"github.com/kcp-dev/logicalcluster/v3"
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/client-go/util/workqueue"

"sigs.k8s.io/controller-runtime/pkg/handler"
ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/internal/controller/metrics"
"sigs.k8s.io/controller-runtime/pkg/kontext"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand Down Expand Up @@ -309,6 +311,7 @@ func (c *Controller) reconcileHandler(ctx context.Context, obj interface{}) {
log = log.WithValues("reconcileID", reconcileID)
ctx = logf.IntoContext(ctx, log)
ctx = addReconcileID(ctx, reconcileID)
ctx = kontext.WithCluster(ctx, logicalcluster.Name(req.ClusterName))

// RunInformersAndControllers the syncHandler, passing it the Namespace/Name string of the
// resource to be synced.
Expand Down
66 changes: 40 additions & 26 deletions pkg/internal/controller/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/handler"
ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/internal/controller/metrics"
"sigs.k8s.io/controller-runtime/pkg/internal/log"
"sigs.k8s.io/controller-runtime/pkg/kontext"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"
Expand All @@ -51,13 +52,13 @@ var _ = Describe("controller", func() {
var fakeReconcile *fakeReconciler
var ctrl *Controller
var queue *controllertest.Queue
var reconciled chan reconcile.Request
var reconciled chan fakeReconcilerRequest
var request = reconcile.Request{
NamespacedName: types.NamespacedName{Namespace: "foo", Name: "bar"},
}

BeforeEach(func() {
reconciled = make(chan reconcile.Request)
reconciled = make(chan fakeReconcilerRequest)
fakeReconcile = &fakeReconciler{
Requests: reconciled,
results: make(chan fakeReconcileResultPair, 10 /* chosen by the completely scientific approach of guessing */),
Expand All @@ -80,7 +81,7 @@ var _ = Describe("controller", func() {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

ctrl.Do = reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
ctrl.Do = reconcile.Func(func(doCtx context.Context, doReq reconcile.Request) (reconcile.Result, error) {
return reconcile.Result{Requeue: true}, nil
})
result, err := ctrl.Reconcile(ctx,
Expand Down Expand Up @@ -325,7 +326,7 @@ var _ = Describe("controller", func() {

By("Invoking Reconciler")
fakeReconcile.AddResult(reconcile.Result{}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)

By("Removing the item from the queue")
Eventually(queue.Len).Should(Equal(0))
Expand Down Expand Up @@ -370,14 +371,14 @@ var _ = Describe("controller", func() {

By("Invoking Reconciler which will give an error")
fakeReconcile.AddResult(reconcile.Result{}, fmt.Errorf("expected error: reconcile"))
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)
queue.AddedRateLimitedLock.Lock()
Expect(queue.AddedRatelimited).To(Equal([]any{request}))
queue.AddedRateLimitedLock.Unlock()

By("Invoking Reconciler a second time without error")
fakeReconcile.AddResult(reconcile.Result{}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)

By("Removing the item from the queue")
Eventually(queue.Len).Should(Equal(0))
Expand All @@ -396,7 +397,7 @@ var _ = Describe("controller", func() {

By("Invoking Reconciler which will give an error")
fakeReconcile.AddResult(reconcile.Result{}, reconcile.TerminalError(fmt.Errorf("expected error: reconcile")))
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)

queue.AddedRateLimitedLock.Lock()
Expect(queue.AddedRatelimited).To(BeEmpty())
Expand All @@ -423,18 +424,18 @@ var _ = Describe("controller", func() {

By("Invoking Reconciler which returns an error")
fakeReconcile.AddResult(reconcile.Result{}, fmt.Errorf("something's wrong"))
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)
Eventually(dq.getCounts).Should(Equal(countInfo{Trying: 1, AddRateLimited: 1}))

By("Invoking Reconciler a second time with an error")
fakeReconcile.AddResult(reconcile.Result{}, fmt.Errorf("another thing's wrong"))
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)

Eventually(dq.getCounts).Should(Equal(countInfo{Trying: 1, AddRateLimited: 2}))

By("Invoking Reconciler a third time, where it finally does not return an error")
fakeReconcile.AddResult(reconcile.Result{}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)

Eventually(dq.getCounts).Should(Equal(countInfo{Trying: 0, AddRateLimited: 2}))

Expand All @@ -459,12 +460,12 @@ var _ = Describe("controller", func() {

By("Invoking Reconciler which will ask for requeue")
fakeReconcile.AddResult(reconcile.Result{Requeue: true}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)
Eventually(dq.getCounts).Should(Equal(countInfo{Trying: 1, AddRateLimited: 1}))

By("Invoking Reconciler a second time without asking for requeue")
fakeReconcile.AddResult(reconcile.Result{Requeue: false}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)

Eventually(dq.getCounts).Should(Equal(countInfo{Trying: 0, AddRateLimited: 1}))

Expand All @@ -489,12 +490,12 @@ var _ = Describe("controller", func() {

By("Invoking Reconciler which will ask for requeue & requeueafter")
fakeReconcile.AddResult(reconcile.Result{RequeueAfter: time.Millisecond * 100, Requeue: true}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)
Eventually(dq.getCounts).Should(Equal(countInfo{Trying: 0, AddAfter: 1}))

By("Invoking Reconciler a second time asking for a requeueafter only")
fakeReconcile.AddResult(reconcile.Result{RequeueAfter: time.Millisecond * 100}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)

Eventually(dq.getCounts).Should(Equal(countInfo{Trying: -1 /* we don't increment the count in addafter */, AddAfter: 2}))

Expand All @@ -519,12 +520,12 @@ var _ = Describe("controller", func() {

By("Invoking Reconciler which will ask for requeueafter with an error")
fakeReconcile.AddResult(reconcile.Result{RequeueAfter: time.Millisecond * 100}, fmt.Errorf("expected error: reconcile"))
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)
Eventually(dq.getCounts).Should(Equal(countInfo{Trying: 1, AddRateLimited: 1}))

By("Invoking Reconciler a second time asking for requeueafter without errors")
fakeReconcile.AddResult(reconcile.Result{RequeueAfter: time.Millisecond * 100}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)
Eventually(dq.getCounts).Should(Equal(countInfo{AddAfter: 1, AddRateLimited: 1}))

By("Removing the item from the queue")
Expand Down Expand Up @@ -571,7 +572,7 @@ var _ = Describe("controller", func() {
queue.Add(request)

fakeReconcile.AddResult(reconcile.Result{}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)
Eventually(func() error {
Expect(ctrlmetrics.ReconcileTotal.WithLabelValues(ctrl.Name, "success").Write(&reconcileTotal)).To(Succeed())
if actual := reconcileTotal.GetCounter().GetValue(); actual != 1.0 {
Expand Down Expand Up @@ -600,7 +601,7 @@ var _ = Describe("controller", func() {
queue.Add(request)

fakeReconcile.AddResult(reconcile.Result{}, fmt.Errorf("expected error: reconcile"))
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)
Eventually(func() error {
Expect(ctrlmetrics.ReconcileTotal.WithLabelValues(ctrl.Name, "error").Write(&reconcileTotal)).To(Succeed())
if actual := reconcileTotal.GetCounter().GetValue(); actual != 1.0 {
Expand Down Expand Up @@ -630,7 +631,7 @@ var _ = Describe("controller", func() {
queue.Add(request)

fakeReconcile.AddResult(reconcile.Result{Requeue: true}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)
Eventually(func() error {
Expect(ctrlmetrics.ReconcileTotal.WithLabelValues(ctrl.Name, "requeue").Write(&reconcileTotal)).To(Succeed())
if actual := reconcileTotal.GetCounter().GetValue(); actual != 1.0 {
Expand Down Expand Up @@ -659,7 +660,7 @@ var _ = Describe("controller", func() {
queue.Add(request)

fakeReconcile.AddResult(reconcile.Result{RequeueAfter: 5 * time.Hour}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)
Eventually(func() error {
Expect(ctrlmetrics.ReconcileTotal.WithLabelValues(ctrl.Name, "requeue_after").Write(&reconcileTotal)).To(Succeed())
if actual := reconcileTotal.GetCounter().GetValue(); actual != 1.0 {
Expand Down Expand Up @@ -692,7 +693,7 @@ var _ = Describe("controller", func() {

By("Invoking Reconciler which will give an error")
fakeReconcile.AddResult(reconcile.Result{}, fmt.Errorf("expected error: reconcile"))
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)
Eventually(func() error {
Expect(ctrlmetrics.ReconcileErrors.WithLabelValues(ctrl.Name).Write(&reconcileErrs)).To(Succeed())
if reconcileErrs.GetCounter().GetValue() != 1.0 {
Expand All @@ -703,7 +704,7 @@ var _ = Describe("controller", func() {

By("Invoking Reconciler a second time without error")
fakeReconcile.AddResult(reconcile.Result{}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)

By("Removing the item from the queue")
Eventually(queue.Len).Should(Equal(0))
Expand Down Expand Up @@ -734,7 +735,7 @@ var _ = Describe("controller", func() {

By("Invoking Reconciler")
fakeReconcile.AddResult(reconcile.Result{}, nil)
Expect(<-reconciled).To(Equal(request))
checkReconcile(<-reconciled, request)

By("Removing the item from the queue")
Eventually(queue.Len).Should(Equal(0))
Expand Down Expand Up @@ -832,20 +833,26 @@ type fakeReconcileResultPair struct {
Err error
}

type fakeReconcilerRequest struct {
reconcile.Request
ctx context.Context
}

type fakeReconciler struct {
Requests chan reconcile.Request
Requests chan fakeReconcilerRequest
results chan fakeReconcileResultPair
}

func (f *fakeReconciler) AddResult(res reconcile.Result, err error) {
f.results <- fakeReconcileResultPair{Result: res, Err: err}
}

func (f *fakeReconciler) Reconcile(_ context.Context, r reconcile.Request) (reconcile.Result, error) {
func (f *fakeReconciler) Reconcile(ctx context.Context, r reconcile.Request) (reconcile.Result, error) {
res := <-f.results
if f.Requests != nil {
f.Requests <- r
f.Requests <- fakeReconcilerRequest{ctx: ctx, Request: r}
}

return res.Result, res.Err
}

Expand Down Expand Up @@ -877,3 +884,10 @@ func (c *cacheWithIndefinitelyBlockingGetInformer) GetInformer(ctx context.Conte
<-ctx.Done()
return nil, errors.New("GetInformer timed out")
}

func checkReconcile(rec fakeReconcilerRequest, request reconcile.Request) {
Expect(rec.Request).To(Equal(request))

_, ok := kontext.ClusterFrom(rec.ctx)
Expect(ok).To(BeTrue())
}

0 comments on commit 73900a3

Please sign in to comment.