Skip to content

Commit

Permalink
Adjust tracker to take structured keys as well. (knative#706)
Browse files Browse the repository at this point in the history
* Use structured keys in the tracker.

* Adjust the addressable resolver too.
  • Loading branch information
markusthoemmes authored and knative-prow-robot committed Sep 20, 2019
1 parent 27ef42f commit b55b842
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 10 deletions.
3 changes: 2 additions & 1 deletion resolver/addressable_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/types"

"knative.dev/pkg/apis"
pkgapisduck "knative.dev/pkg/apis/duck"
Expand All @@ -43,7 +44,7 @@ type URIResolver struct {
}

// NewURIResolver constructs a new URIResolver with context and a callback passed to the URIResolver's tracker.
func NewURIResolver(ctx context.Context, callback func(string)) *URIResolver {
func NewURIResolver(ctx context.Context, callback func(types.NamespacedName)) *URIResolver {
ret := &URIResolver{}

ret.tracker = tracker.New(callback, controller.GetTrackerLease(ctx))
Expand Down
3 changes: 2 additions & 1 deletion resolver/addressable_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
"knative.dev/pkg/apis"
duckv1alpha1 "knative.dev/pkg/apis/duck/v1alpha1"
Expand Down Expand Up @@ -160,7 +161,7 @@ func TestGetURI_ObjectReference(t *testing.T) {
for n, tc := range tests {
t.Run(n, func(t *testing.T) {
ctx, _ := fakedynamicclient.With(context.Background(), scheme.Scheme, tc.objects...)
r := resolver.NewURIResolver(ctx, func(string) {})
r := resolver.NewURIResolver(ctx, func(types.NamespacedName) {})

// Run it twice since this should be idempotent. URI Resolver should
// not modify the cache's copy.
Expand Down
12 changes: 7 additions & 5 deletions tracker/enqueue.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import (
"time"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/client-go/tools/cache"

"knative.dev/pkg/kmeta"
)
Expand All @@ -38,7 +38,7 @@ import (
// When OnChanged is called by the informer for a particular
// GroupVersionKind, the provided callback is called with the "key"
// of each object actively watching the changed object.
func New(callback func(string), lease time.Duration) Interface {
func New(callback func(types.NamespacedName), lease time.Duration) Interface {
return &impl{
leaseDuration: lease,
cb: callback,
Expand All @@ -55,14 +55,14 @@ type impl struct {
// before having to renew the lease.
leaseDuration time.Duration

cb func(string)
cb func(types.NamespacedName)
}

// Check that impl implements Interface.
var _ Interface = (*impl)(nil)

// set is a map from keys to expirations
type set map[string]time.Time
type set map[types.NamespacedName]time.Time

// Track implements Interface.
func (i *impl) Track(ref corev1.ObjectReference, obj interface{}) error {
Expand All @@ -83,11 +83,13 @@ func (i *impl) Track(ref corev1.ObjectReference, obj interface{}) error {
return fmt.Errorf("invalid ObjectReference:\n%s", strings.Join(fieldErrors, "\n"))
}

key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(obj)
object, err := kmeta.DeletionHandlingAccessor(obj)
if err != nil {
return err
}

key := types.NamespacedName{Namespace: object.GetNamespace(), Name: object.GetName()}

i.m.Lock()
defer i.m.Unlock()
if i.mapping == nil {
Expand Down
7 changes: 4 additions & 3 deletions tracker/enqueue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@ import (

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/cache"

. "knative.dev/pkg/testing"
)

func TestHappyPaths(t *testing.T) {
calls := 0
f := func(key string) {
f := func(key types.NamespacedName) {
calls = calls + 1
}

Expand Down Expand Up @@ -165,7 +166,7 @@ func TestHappyPaths(t *testing.T) {
}

func TestAllowedObjectReferences(t *testing.T) {
trk := New(func(key string) {}, 10*time.Millisecond)
trk := New(func(key types.NamespacedName) {}, 10*time.Millisecond)
thing1 := &Resource{
TypeMeta: metav1.TypeMeta{
APIVersion: "ref.knative.dev/v1alpha1",
Expand Down Expand Up @@ -231,7 +232,7 @@ func TestAllowedObjectReferences(t *testing.T) {
}

func TestBadObjectReferences(t *testing.T) {
trk := New(func(key string) {}, 10*time.Millisecond)
trk := New(func(key types.NamespacedName) {}, 10*time.Millisecond)
thing1 := &Resource{
TypeMeta: metav1.TypeMeta{
APIVersion: "ref.knative.dev/v1alpha1",
Expand Down

0 comments on commit b55b842

Please sign in to comment.