kubernetes-sigs / kubernetes-sigs/controller-runtime

Lossy map functions

Open
#1,996 34 comments 6 reactions 0 assignees View on GitHub
lifecycle/frozen
Dominant language
Go
Stars
3k
Forks
1.3k
Avg merge
2d 5h
Merged PRs (30d)
14

Description

### Preface

This relates to a conversation I brought up at the community meeting on 2022-09-08, @camilamacedo86 and @jmrodri suggested I create an issue so we can discuss it further.

### Background

Currently in `controller-runtime` we have the following tools:

```go
type MapFunc func(client.Object) []reconcile.Request

func EnqueueRequestsFromMapFunc(fn MapFunc) EventHandler {
return &enqueueRequestsFromMapFunc{
toRequests: fn,
}
}
```

With these it's possible to express a `Watch()` on one `source.Kind` which enqueues objects of another kind by some arbitrary relation:

```go
ctrl.NewControllerManagedBy(mgr).Watches(
&source.Kind{Type: &customapi.Tree},
handler.EnqueueRequestsFromMapFunc(r.mapFruitsFromTree),
)
```

Now ideally you use `metav1.ObjectReference` when dealing with relationships between objects, however it can sometimes be the case that you need to use the API client or some other utility **that may produce an error** to map the related objects, e.g.:

```go
func (r *myReconciler) mapFruitsFromTree(tree client.Object) []reconcile.Request {
var fruits customapi.FruitList
if err := r.client.List(context.Background(), &fruits); err != nil {
r.log.WithError(err).Error("error listing fruits")
return nil
}

var req []reconcile.Request
for _, fruit := range fruits.Items {
if isRelated(tree, fruit) {
req = append(req, reconcile.Request{
NamespacedName: types.NamespacedName{
Namespace: fruit.Namespace,
Name: fruit.Name,
},
})
}
}

return req
}
```

In these cases it would appear there is effectively _no provisions to handle error conditions_.

This kind of implementation and use case is _not uncommon_. I was able to find several examples of notable open source projects which are using it this way:

- https://github.com/projectcontour/contour/blob/2e758465348d8ebc32b037fbc5ce899e36cf036f/internal/controller/gateway.go#L124
- https://github.com/kumahq/kuma/blob/abd89b44c5ee3bf5b57c13d7379b3c2b15ed5aba/pkg/plugins/runtime/k8s/controllers/configmap_controller.go#L100
- https://github.com/cert-manager/cert-manager/blob/e82c72cff03dda703ca918d32ab11f719edfcd88/pkg/controller/cainjector/indexers.go#L40
- https://github.com/Kong/kubernetes-ingress-controller/blob/185f9d7c842869179dda844e6a28926468657b7f/internal/controllers/gateway/gateway_controller.go#L178

### Problem

The `EnqueueRequestsFromMapFunc` generator for `EventHandlers` has no concept of handling errors that may occur when making the mapping. As such in the example above and all the links provided for projects which are doing this that process is currently "lossy": that is to say, it appears the machinery could effectively end up _dropping_ an enqueued object from the queue if a failure occurred during the mapping process.

For a specific example using the `Tree` and `Fruit` sample above: if the object listing produces an error (e.g. performed at a time when the API server is having trouble and cache can't be used) this failure means that the `Tree` object is consumed from the queue but the related objects don't get enqueued. This would cause the resource state to become "stuck" and this wont heal without adding some other mechanism to re-queue it, or some unrelated action to re-trigger it.

### Exploring Solutions

The purpose of this issue is to ask for community guidance on an existing or new provision to support this kind of mapping in a manner that is fault tolerant and can heal from an error.

Some of the workarounds that I have conceived of are:

- using object references as mentioned above (wont work in all cases)
- some fallback process which would actively re-enqueue all relevant objects (messy, costly)
- watching a custom backup `source.Channel` where objects are re-enqueued in an implementation-specific manner (clunky)

So far I _think_ the third option there might be the most effective strategy with what we have _today_, but _ideally_ what I would like to see is the ability to cleanly trigger a re-queue from within the mapping function as part of the function return. I would very much appreciate community feedback on this.

Contributor guide

Open the contributing guide

Research direction

Start by reading the MapFunc and EnqueueRequestsFromMapFunc definitions and the surrounding EventHandler behavior in controller-runtime. Review the linked usage examples and the discussion for an agreed fault-tolerant mapping design. Done means the project has a decided approach for mapping errors and corresponding implementation and tests, rather than only a proposed workaround.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.