Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix make staticcheck, (use NewInformerWithOptions and TypedRateLimitingInterface) #6647

Merged
merged 4 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 19 additions & 20 deletions internal/externaldns/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
extdnslisters "github.com/nginxinc/kubernetes-ingress/pkg/client/listers/externaldns/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"
Expand All @@ -31,7 +32,7 @@
type ExtDNSController struct {
sync SyncFn
ctx context.Context
queue workqueue.RateLimitingInterface
queue workqueue.TypedRateLimitingInterface[types.NamespacedName]
recorder record.EventRecorder
client k8s_nginx.Interface
informerGroup map[string]*namespacedInformer
Expand Down Expand Up @@ -60,9 +61,14 @@
// NewController takes external dns config and return a new External DNS Controller.
func NewController(opts *ExtDNSOpts) *ExtDNSController {
ig := make(map[string]*namespacedInformer)

rateLimiter := workqueue.DefaultTypedControllerRateLimiter[types.NamespacedName]()

Check warning on line 65 in internal/externaldns/controller.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/controller.go#L65

Added line #L65 was not covered by tests

queue := workqueue.NewTypedRateLimitingQueueWithConfig(rateLimiter, workqueue.TypedRateLimitingQueueConfig[types.NamespacedName]{Name: ControllerName})

Check warning on line 67 in internal/externaldns/controller.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/controller.go#L67

Added line #L67 was not covered by tests

c := &ExtDNSController{
ctx: opts.context,
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), ControllerName),
queue: queue,

Check warning on line 71 in internal/externaldns/controller.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/controller.go#L71

Added line #L71 was not covered by tests
informerGroup: ig,
recorder: opts.eventRecorder,
client: opts.client,
Expand Down Expand Up @@ -155,39 +161,31 @@
l := nl.LoggerFromContext(ctx)
nl.Debugf(l, "processing items on the workqueue")
for {
obj, shutdown := c.queue.Get()
key, shutdown := c.queue.Get()

Check warning on line 164 in internal/externaldns/controller.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/controller.go#L164

Added line #L164 was not covered by tests
if shutdown {
break
}

func() {
defer c.queue.Done(obj)
key, ok := obj.(string)
if !ok {
return
}

defer c.queue.Done(key)

Check warning on line 170 in internal/externaldns/controller.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/controller.go#L170

Added line #L170 was not covered by tests
if err := c.processItem(ctx, key); err != nil {
nl.Debugf(l, "Re-queuing item due to error processing: %v", err)
c.queue.AddRateLimited(obj)
c.queue.AddRateLimited(key)

Check warning on line 173 in internal/externaldns/controller.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/controller.go#L173

Added line #L173 was not covered by tests
return
}
nl.Debugf(l, "finished processing work item")
c.queue.Forget(obj)
c.queue.Forget(key)

Check warning on line 177 in internal/externaldns/controller.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/controller.go#L177

Added line #L177 was not covered by tests
}()
}
}

func (c *ExtDNSController) processItem(ctx context.Context, key string) error {
namespace, name, err := cache.SplitMetaNamespaceKey(key)
if err != nil {
runtime.HandleError(fmt.Errorf("invalid resource key: %s", key))
return err
}
func (c *ExtDNSController) processItem(ctx context.Context, key types.NamespacedName) error {
namespace := key.Namespace
name := key.Name

Check warning on line 184 in internal/externaldns/controller.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/controller.go#L182-L184

Added lines #L182 - L184 were not covered by tests
l := nl.LoggerFromContext(ctx)
var vs *conf_v1.VirtualServer
nsi := getNamespacedInformer(namespace, c.informerGroup)
vs, err = nsi.vsLister.VirtualServers(namespace).Get(name)
vs, err := nsi.vsLister.VirtualServers(namespace).Get(name)

Check warning on line 188 in internal/externaldns/controller.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/controller.go#L188

Added line #L188 was not covered by tests

// VS has been deleted
if apierrors.IsNotFound(err) {
Expand All @@ -201,7 +199,7 @@
return c.sync(ctx, vs)
}

func externalDNSHandler(queue workqueue.RateLimitingInterface) func(obj interface{}) {
func externalDNSHandler(queue workqueue.TypedRateLimitingInterface[types.NamespacedName]) func(obj interface{}) {

Check warning on line 202 in internal/externaldns/controller.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/controller.go#L202

Added line #L202 was not covered by tests
return func(obj interface{}) {
ep, ok := obj.(*extdns_v1.DNSEndpoint)
if !ok {
Expand All @@ -223,7 +221,8 @@
return
}

queue.Add(ep.Namespace + "/" + ref.Name)
key := types.NamespacedName{Namespace: ep.Namespace, Name: ref.Name}
queue.Add(key)

Check warning on line 225 in internal/externaldns/controller.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/controller.go#L224-L225

Added lines #L224 - L225 were not covered by tests
}
}

Expand Down
14 changes: 10 additions & 4 deletions internal/externaldns/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
"reflect"
"time"

"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/util/workqueue"
Expand All @@ -16,23 +18,27 @@

// DefaultItemBasedRateLimiter returns a new rate limiter with base delay of 5
// seconds, max delay of 5 minutes.
func DefaultItemBasedRateLimiter() workqueue.RateLimiter {
return workqueue.NewItemExponentialFailureRateLimiter(time.Second*5, time.Minute*5)
func DefaultItemBasedRateLimiter() workqueue.TypedRateLimiter[types.NamespacedName] {
return workqueue.NewTypedItemExponentialFailureRateLimiter[types.NamespacedName](5*time.Second, 5*time.Minute)

Check warning on line 22 in internal/externaldns/handlers.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/handlers.go#L21-L22

Added lines #L21 - L22 were not covered by tests
}

// QueuingEventHandler is an implementation of cache.ResourceEventHandler that
// simply queues objects that are added/updated/deleted.
type QueuingEventHandler struct {
Queue workqueue.RateLimitingInterface
Queue workqueue.TypedRateLimitingInterface[types.NamespacedName]
}

// Enqueue adds a key for an object to the workqueue.
func (q *QueuingEventHandler) Enqueue(obj interface{}) {
key, err := KeyFunc(obj)
accessor, err := meta.Accessor(obj)

Check warning on line 33 in internal/externaldns/handlers.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/handlers.go#L33

Added line #L33 was not covered by tests
if err != nil {
runtime.HandleError(err)
return
}
key := types.NamespacedName{
Namespace: accessor.GetNamespace(),
Name: accessor.GetName(),

Check warning on line 40 in internal/externaldns/handlers.go

View check run for this annotation

Codecov / codecov/patch

internal/externaldns/handlers.go#L38-L40

Added lines #L38 - L40 were not covered by tests
}
q.Queue.Add(key)
}

Expand Down
77 changes: 51 additions & 26 deletions internal/k8s/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"log/slog"
"reflect"
"testing"
"time"

"github.com/google/go-cmp/cmp"
nic_glog "github.com/nginxinc/kubernetes-ingress/internal/logger/glog"
Expand Down Expand Up @@ -94,16 +95,18 @@ func TestUpdateTransportServerStatusIgnoreNoChange(t *testing.T) {
},
})

tsLister, _ := cache.NewInformer(
cache.NewListWatchFromClient(
fakeClient.K8sV1().RESTClient(),
"transportservers",
"nginx-ingress",
fields.Everything(),
),
&conf_v1.TransportServer{},
2,
nil,
tsLister, _ := cache.NewInformerWithOptions(
cache.InformerOptions{
ListerWatcher: cache.NewListWatchFromClient(
fakeClient.K8sV1().RESTClient(),
"transportservers",
"nginx-ingress",
fields.Everything(),
),
ObjectType: &conf_v1.TransportServer{},
ResyncPeriod: 2 * time.Second,
Handler: nil,
},
)

err := tsLister.Add(ts)
Expand Down Expand Up @@ -156,16 +159,18 @@ func TestUpdateTransportServerStatusMissingTransportServer(t *testing.T) {
Items: []conf_v1.TransportServer{},
})

tsLister, _ := cache.NewInformer(
cache.NewListWatchFromClient(
fakeClient.K8sV1().RESTClient(),
"transportservers",
"nginx-ingress",
fields.Everything(),
),
&conf_v1.TransportServer{},
2,
nil,
tsLister, _ := cache.NewInformerWithOptions(
cache.InformerOptions{
ListerWatcher: cache.NewListWatchFromClient(
fakeClient.K8sV1().RESTClient(),
"transportservers",
"nginx-ingress",
fields.Everything(),
),
ObjectType: &conf_v1.TransportServer{},
ResyncPeriod: 2 * time.Second,
Handler: nil,
},
)

nsi := make(map[string]*namespacedInformer)
Expand Down Expand Up @@ -219,9 +224,19 @@ func TestStatusUpdateWithExternalStatusAndExternalService(t *testing.T) {
}},
)
ingLister := storeToIngressLister{}
ingLister.Store, _ = cache.NewInformer(
cache.NewListWatchFromClient(fakeClient.NetworkingV1().RESTClient(), "ingresses", "nginx-ingress", fields.Everything()),
&networking.Ingress{}, 2, nil)
ingLister.Store, _ = cache.NewInformerWithOptions(
cache.InformerOptions{
ListerWatcher: cache.NewListWatchFromClient(
fakeClient.NetworkingV1().RESTClient(),
"ingresses",
"nginx-ingress",
fields.Everything(),
),
ObjectType: &networking.Ingress{},
ResyncPeriod: 2 * time.Second,
Handler: nil,
},
)

err := ingLister.Store.Add(&ing)
if err != nil {
Expand Down Expand Up @@ -328,9 +343,19 @@ func TestStatusUpdateWithExternalStatusAndIngressLink(t *testing.T) {
}},
)
ingLister := storeToIngressLister{}
ingLister.Store, _ = cache.NewInformer(
cache.NewListWatchFromClient(fakeClient.NetworkingV1().RESTClient(), "ingresses", "nginx-ingress", fields.Everything()),
&networking.Ingress{}, 2, nil)
ingLister.Store, _ = cache.NewInformerWithOptions(
cache.InformerOptions{
ListerWatcher: cache.NewListWatchFromClient(
fakeClient.NetworkingV1().RESTClient(),
"ingresses",
"nginx-ingress",
fields.Everything(),
),
ObjectType: &networking.Ingress{},
ResyncPeriod: 2 * time.Second,
Handler: nil,
},
)

err := ingLister.Store.Add(&ing)
if err != nil {
Expand Down
Loading