Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: kubernetes-sigs/controller-runtime
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v0.20.3
Choose a base ref
...
head repository: kubernetes-sigs/controller-runtime
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v0.20.4
Choose a head ref
  • 15 commits
  • 19 files changed
  • 4 contributors

Commits on Mar 15, 2025

  1. bug: Restmapper: Respect preferred version

    When requesting a resource without version through methods
    such as `RESTMapping`, the mapper would previously return a random
    version rather than the preferred one, this change fixes that.
    alvaroaleman authored and k8s-infra-cherrypick-robot committed Mar 15, 2025
    Copy the full SHA
    dfd1239 View commit details

Commits on Mar 16, 2025

  1. Merge pull request #3159 from k8s-infra-cherrypick-robot/cherry-pick-…

    …3151-to-release-0.20
    
    [release-0.20] 🐛 Restmapper: Respect preferred version
    k8s-ci-robot authored Mar 16, 2025

    Verified

    This commit was signed with the committer’s verified signature.
    FrozenPandaz Jason Jean
    Copy the full SHA
    50ae490 View commit details

Commits on Mar 21, 2025

  1. Mention the SkipNameValidation option in the name validation error

    Signed-off-by: Stefan Büringer buringerst@vmware.com
    sbueringer authored and k8s-infra-cherrypick-robot committed Mar 21, 2025
    Copy the full SHA
    e813e08 View commit details
  2. sparkles: Controller: Retain the priority

    This change makes the controller retain the priority if a priority queue
    is used. The priority queue will still bump the priority if the item
    gets re-added to it with a higher priority.
    alvaroaleman authored and k8s-infra-cherrypick-robot committed Mar 21, 2025
    Copy the full SHA
    2510686 View commit details
  3. Merge pull request #3172 from k8s-infra-cherrypick-robot/cherry-pick-…

    …3170-to-release-0.20
    
    [release-0.20] 🌱 Mention the SkipNameValidation option in the name validation error
    k8s-ci-robot authored Mar 21, 2025
    Copy the full SHA
    23975bc View commit details
  4. Merge pull request #3173 from k8s-infra-cherrypick-robot/cherry-pick-…

    …3167-to-release-0.20
    
    [release-0.20] ✨ Controller: Retain the priority
    k8s-ci-robot authored Mar 21, 2025
    Copy the full SHA
    833f208 View commit details

Commits on Mar 23, 2025

  1. add version.version to tools/setup-envtest to show installed version

    Signed-off-by: Troy Connor <troy0820@users.noreply.github.com>
    troy0820 authored and k8s-infra-cherrypick-robot committed Mar 23, 2025
    Copy the full SHA
    4ae5f39 View commit details
  2. Merge pull request #3175 from k8s-infra-cherrypick-robot/cherry-pick-…

    …3166-to-release-0.20
    
    [release-0.20] ✨Add RELEASE_TAG to tools/setup-envtest to show binary version with setup-envtest version
    k8s-ci-robot authored Mar 23, 2025
    Copy the full SHA
    3156ace View commit details

Commits on Mar 24, 2025

  1. 🐛Implement priorityqueue as default on handlers if using priorityqueu…

    …e interface (#3111)
    
    * implement priority queue for handlers
    
    Signed-off-by: Troy Connor <troy0820@users.noreply.github.com>
    
    * check object before placing in priorityqueue
    
    Signed-off-by: Troy Connor <troy0820@users.noreply.github.com>
    
    * implement priority queue
    
    Signed-off-by: Troy Connor <troy0820@users.noreply.github.com>
    
    ---------
    
    Signed-off-by: Troy Connor <troy0820@users.noreply.github.com>
    troy0820 authored and alvaroaleman committed Mar 24, 2025
    Copy the full SHA
    5355658 View commit details
  2. 🌱 Handlers: Use low priority when object is unchanged and priority q

    This change makes the `TypedFuncs` and `enqueueRequestsFromMapFunc` set
    a low priority when the object is unchanged by default, as well as
    extend the test coverage to validate this behavior for `EnqueueRequestForOwner`.
    alvaroaleman committed Mar 24, 2025
    Copy the full SHA
    29debb1 View commit details
  3. 🌱 Followups to default low priority in mappers

    Addresses Stefans comments
    alvaroaleman committed Mar 24, 2025
    Copy the full SHA
    2af3164 View commit details
  4. 🌱 Remove redundant WithLowPriorityWhenUnchanged in builder

    The handlers themselves already support this, so there is no need to
    also do it in the builder.
    alvaroaleman committed Mar 24, 2025
    Copy the full SHA
    2062f3a View commit details
  5. Fix godoc of TypedEventHandler

    Signed-off-by: Stefan Büringer buringerst@vmware.com
    sbueringer authored and alvaroaleman committed Mar 24, 2025
    Copy the full SHA
    c7d5d83 View commit details
  6. 🌱 TypedRequestForOwner: Decrease priority when unchanged

    We already did this for RequestForOwner, but not the typed variation,
    this change adds that.
    alvaroaleman committed Mar 24, 2025
    Copy the full SHA
    9951869 View commit details
  7. Merge pull request #3179 from alvaroaleman/lowpdefault

    [release-0.20] 🌱 Handlers: Default to LowPriorityWhenUnchanged without a wrapper
    k8s-ci-robot authored Mar 24, 2025
    Copy the full SHA
    0f7927c View commit details
2 changes: 2 additions & 0 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
@@ -14,6 +14,8 @@ jobs:
name: Upload binaries to release
runs-on: ubuntu-latest
steps:
- name: Set env
run: echo "RELEASE_TAG=${GITHUB_REF:10}" >> $GITHUB_ENV
- name: Check out code
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=v4.2.2
- name: Calculate go version
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -174,7 +174,7 @@ release-binary: $(RELEASE_DIR)
-v "$$(pwd):/workspace$(DOCKER_VOL_OPTS)" \
-w /workspace/tools/setup-envtest \
golang:$(GO_VERSION) \
go build -a -trimpath -ldflags "-extldflags '-static'" \
go build -a -trimpath -ldflags "-X 'sigs.k8s.io/controller-runtime/tools/setup-envtest/version.version=$(RELEASE_TAG)' -extldflags '-static'" \
-o ./out/$(RELEASE_BINARY) ./

## --------------------------------------
8 changes: 4 additions & 4 deletions pkg/builder/controller.go
Original file line number Diff line number Diff line change
@@ -163,7 +163,7 @@ func (blder *TypedBuilder[request]) Watches(
) *TypedBuilder[request] {
input := WatchesInput[request]{
obj: object,
handler: handler.WithLowPriorityWhenUnchanged(eventHandler),
handler: eventHandler,
}
for _, opt := range opts {
opt.ApplyToWatches(&input)
@@ -317,7 +317,7 @@ func (blder *TypedBuilder[request]) doWatch() error {
}

var hdler handler.TypedEventHandler[client.Object, request]
reflect.ValueOf(&hdler).Elem().Set(reflect.ValueOf(handler.WithLowPriorityWhenUnchanged(&handler.EnqueueRequestForObject{})))
reflect.ValueOf(&hdler).Elem().Set(reflect.ValueOf(&handler.EnqueueRequestForObject{}))
allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...)
allPredicates = append(allPredicates, blder.forInput.predicates...)
src := source.TypedKind(blder.mgr.GetCache(), obj, hdler, allPredicates...)
@@ -341,11 +341,11 @@ func (blder *TypedBuilder[request]) doWatch() error {
}

var hdler handler.TypedEventHandler[client.Object, request]
reflect.ValueOf(&hdler).Elem().Set(reflect.ValueOf(handler.WithLowPriorityWhenUnchanged(handler.EnqueueRequestForOwner(
reflect.ValueOf(&hdler).Elem().Set(reflect.ValueOf(handler.EnqueueRequestForOwner(
blder.mgr.GetScheme(), blder.mgr.GetRESTMapper(),
blder.forInput.object,
opts...,
))))
)))
allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...)
allPredicates = append(allPredicates, own.predicates...)
src := source.TypedKind(blder.mgr.GetCache(), obj, hdler, allPredicates...)
20 changes: 14 additions & 6 deletions pkg/client/apiutil/restmapper.go
Original file line number Diff line number Diff line change
@@ -246,10 +246,18 @@ func (m *mapper) addGroupVersionResourcesToCacheAndReloadLocked(gvr map[schema.G
}

if !found {
groupResources.Group.Versions = append(groupResources.Group.Versions, metav1.GroupVersionForDiscovery{
gv := metav1.GroupVersionForDiscovery{
GroupVersion: metav1.GroupVersion{Group: groupVersion.Group, Version: version}.String(),
Version: version,
})
}

// Prepend if preferred version, else append. The upstream DiscoveryRestMappper assumes
// the first version is the preferred one: https://github.com/kubernetes/kubernetes/blob/ef54ac803b712137871c1a1f8d635d50e69ffa6c/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper.go#L458-L461
if group, ok := m.apiGroups[groupVersion.Group]; ok && group.PreferredVersion.Version == version {
groupResources.Group.Versions = append([]metav1.GroupVersionForDiscovery{gv}, groupResources.Group.Versions...)
} else {
groupResources.Group.Versions = append(groupResources.Group.Versions, gv)
}
}

// Update data in the cache.
@@ -284,14 +292,14 @@ func (m *mapper) findAPIGroupByNameAndMaybeAggregatedDiscoveryLocked(groupName s
}

m.initialDiscoveryDone = true
if len(maybeResources) > 0 {
didAggregatedDiscovery = true
m.addGroupVersionResourcesToCacheAndReloadLocked(maybeResources)
}
for i := range apiGroups.Groups {
group := &apiGroups.Groups[i]
m.apiGroups[group.Name] = group
}
if len(maybeResources) > 0 {
didAggregatedDiscovery = true
m.addGroupVersionResourcesToCacheAndReloadLocked(maybeResources)
}

// Looking in the cache again.
// Don't return an error here if the API group is not present.
28 changes: 28 additions & 0 deletions pkg/client/apiutil/restmapper_test.go
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ import (
"fmt"
"net/http"
"strconv"
"sync"
"testing"

_ "github.com/onsi/ginkgo/v2"
@@ -735,6 +736,33 @@ func TestLazyRestMapperProvider(t *testing.T) {
g.Expect(err).NotTo(gmg.HaveOccurred())
g.Expect(mapping.Resource.Version).To(gmg.Equal("v1"))
})

t.Run("Restmapper should consistently return the preferred version", func(t *testing.T) {
g := gmg.NewWithT(t)

wg := sync.WaitGroup{}
wg.Add(50)
for i := 0; i < 50; i++ {
go func() {
defer wg.Done()
httpClient, err := rest.HTTPClientFor(restCfg)
g.Expect(err).NotTo(gmg.HaveOccurred())

mapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient)
g.Expect(err).NotTo(gmg.HaveOccurred())

mapping, err := mapper.RESTMapping(schema.GroupKind{
Group: "crew.example.com",
Kind: "Driver",
})
g.Expect(err).NotTo(gmg.HaveOccurred())
// APIServer seems to have a heuristic to prefer the higher
// version number.
g.Expect(mapping.GroupVersionKind.Version).To(gmg.Equal("v2"))
}()
}
wg.Wait()
})
})
}
}
2 changes: 1 addition & 1 deletion pkg/controller/name.go
Original file line number Diff line number Diff line change
@@ -34,7 +34,7 @@ func checkName(name string) error {
}

if usedNames.Has(name) {
return fmt.Errorf("controller with name %s already exists. Controller names must be unique to avoid multiple controllers reporting to the same metric", name)
return fmt.Errorf("controller with name %s already exists. Controller names must be unique to avoid multiple controllers reporting the same metric. This validation can be disabled via the SkipNameValidation option", name)
}

usedNames.Insert(name)
19 changes: 13 additions & 6 deletions pkg/handler/enqueue.go
Original file line number Diff line number Diff line change
@@ -52,25 +52,32 @@ func (e *TypedEnqueueRequestForObject[T]) Create(ctx context.Context, evt event.
enqueueLog.Error(nil, "CreateEvent received with no metadata", "event", evt)
return
}
q.Add(reconcile.Request{NamespacedName: types.NamespacedName{

item := reconcile.Request{NamespacedName: types.NamespacedName{
Name: evt.Object.GetName(),
Namespace: evt.Object.GetNamespace(),
}})
}}

addToQueueCreate(q, evt, item)
}

// Update implements EventHandler.
func (e *TypedEnqueueRequestForObject[T]) Update(ctx context.Context, evt event.TypedUpdateEvent[T], q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
switch {
case !isNil(evt.ObjectNew):
q.Add(reconcile.Request{NamespacedName: types.NamespacedName{
item := reconcile.Request{NamespacedName: types.NamespacedName{
Name: evt.ObjectNew.GetName(),
Namespace: evt.ObjectNew.GetNamespace(),
}})
}}

addToQueueUpdate(q, evt, item)
case !isNil(evt.ObjectOld):
q.Add(reconcile.Request{NamespacedName: types.NamespacedName{
item := reconcile.Request{NamespacedName: types.NamespacedName{
Name: evt.ObjectOld.GetName(),
Namespace: evt.ObjectOld.GetNamespace(),
}})
}}

addToQueueUpdate(q, evt, item)
default:
enqueueLog.Error(nil, "UpdateEvent received with no metadata", "event", evt)
}
45 changes: 36 additions & 9 deletions pkg/handler/enqueue_mapped.go
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ import (

"k8s.io/client-go/util/workqueue"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/priorityqueue"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)
@@ -63,15 +64,17 @@ func EnqueueRequestsFromMapFunc(fn MapFunc) EventHandler {
// TypedEnqueueRequestsFromMapFunc is experimental and subject to future change.
func TypedEnqueueRequestsFromMapFunc[object any, request comparable](fn TypedMapFunc[object, request]) TypedEventHandler[object, request] {
return &enqueueRequestsFromMapFunc[object, request]{
toRequests: fn,
toRequests: fn,
objectImplementsClientObject: implementsClientObject[object](),
}
}

var _ EventHandler = &enqueueRequestsFromMapFunc[client.Object, reconcile.Request]{}

type enqueueRequestsFromMapFunc[object any, request comparable] struct {
// Mapper transforms the argument into a slice of keys to be reconciled
toRequests TypedMapFunc[object, request]
toRequests TypedMapFunc[object, request]
objectImplementsClientObject bool
}

// Create implements EventHandler.
@@ -81,7 +84,15 @@ func (e *enqueueRequestsFromMapFunc[object, request]) Create(
q workqueue.TypedRateLimitingInterface[request],
) {
reqs := map[request]empty{}
e.mapAndEnqueue(ctx, q, evt.Object, reqs)

var lowPriority bool
if e.objectImplementsClientObject && isPriorityQueue(q) && !isNil(evt.Object) {
clientObjectEvent := event.CreateEvent{Object: any(evt.Object).(client.Object)}
if isObjectUnchanged(clientObjectEvent) {
lowPriority = true
}
}
e.mapAndEnqueue(ctx, q, evt.Object, reqs, lowPriority)
}

// Update implements EventHandler.
@@ -90,9 +101,13 @@ func (e *enqueueRequestsFromMapFunc[object, request]) Update(
evt event.TypedUpdateEvent[object],
q workqueue.TypedRateLimitingInterface[request],
) {
var lowPriority bool
if e.objectImplementsClientObject && isPriorityQueue(q) && !isNil(evt.ObjectOld) && !isNil(evt.ObjectNew) {
lowPriority = any(evt.ObjectOld).(client.Object).GetResourceVersion() == any(evt.ObjectNew).(client.Object).GetResourceVersion()
}
reqs := map[request]empty{}
e.mapAndEnqueue(ctx, q, evt.ObjectOld, reqs)
e.mapAndEnqueue(ctx, q, evt.ObjectNew, reqs)
e.mapAndEnqueue(ctx, q, evt.ObjectOld, reqs, lowPriority)
e.mapAndEnqueue(ctx, q, evt.ObjectNew, reqs, lowPriority)
}

// Delete implements EventHandler.
@@ -102,7 +117,7 @@ func (e *enqueueRequestsFromMapFunc[object, request]) Delete(
q workqueue.TypedRateLimitingInterface[request],
) {
reqs := map[request]empty{}
e.mapAndEnqueue(ctx, q, evt.Object, reqs)
e.mapAndEnqueue(ctx, q, evt.Object, reqs, false)
}

// Generic implements EventHandler.
@@ -112,14 +127,26 @@ func (e *enqueueRequestsFromMapFunc[object, request]) Generic(
q workqueue.TypedRateLimitingInterface[request],
) {
reqs := map[request]empty{}
e.mapAndEnqueue(ctx, q, evt.Object, reqs)
e.mapAndEnqueue(ctx, q, evt.Object, reqs, false)
}

func (e *enqueueRequestsFromMapFunc[object, request]) mapAndEnqueue(ctx context.Context, q workqueue.TypedRateLimitingInterface[request], o object, reqs map[request]empty) {
func (e *enqueueRequestsFromMapFunc[object, request]) mapAndEnqueue(
ctx context.Context,
q workqueue.TypedRateLimitingInterface[request],
o object,
reqs map[request]empty,
lowPriority bool,
) {
for _, req := range e.toRequests(ctx, o) {
_, ok := reqs[req]
if !ok {
q.Add(req)
if lowPriority {
q.(priorityqueue.PriorityQueue[request]).AddWithOpts(priorityqueue.AddOpts{
Priority: LowPriority,
}, req)
} else {
q.Add(req)
}
reqs[req] = empty{}
}
}
2 changes: 1 addition & 1 deletion pkg/handler/enqueue_owner.go
Original file line number Diff line number Diff line change
@@ -72,7 +72,7 @@ func TypedEnqueueRequestForOwner[object client.Object](scheme *runtime.Scheme, m
for _, opt := range opts {
opt(e)
}
return e
return WithLowPriorityWhenUnchanged(e)
}

// OnlyControllerOwner if provided will only look at the first OwnerReference with Controller: true.
Loading