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

Satisfy linter #3132

Merged
merged 10 commits into from
Jan 10, 2025
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
5 changes: 1 addition & 4 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ run:
- injection/clients

output:
uniq-by-line: true
sort-results: true
sort-order:
- linter
Expand All @@ -15,6 +14,7 @@ output:


issues:
uniq-by-line: true
max-issues-per-linter: 0
max-same-issues: 0
exclude-rules:
Expand Down Expand Up @@ -103,9 +103,6 @@ linters:
# problems with the error wrapping scheme introduced in Go 1.13.
- errorlint

# Checks for pointers to enclosing loop variables.
- exportloopref

Copy link
Contributor

@skonto skonto Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyloopvar could be a replacement. See jaegertracing/jaeger#5976. We also use that in the networking linter.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have it already -

pkg/.golangci.yaml

Lines 89 to 90 in b4ff2c1

# Copyloopvar is a linter detects places where loop variables are copied.
- copyloopvar

# Detects nested contexts in loops.
- fatcontext

Expand Down
2 changes: 1 addition & 1 deletion apis/testing/roundtrip/roundtrip.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func ExternalTypesViaHub(t *testing.T, scheme, hubs *runtime.Scheme, fuzzerFuncs
continue
}

if reflect.PtrTo(objType).AssignableTo(metaV1ListType) {
if reflect.PointerTo(objType).AssignableTo(metaV1ListType) {
continue
}

Expand Down
6 changes: 3 additions & 3 deletions controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,15 +228,15 @@ type ControllerOptions struct {
WorkQueueName string
Logger *zap.SugaredLogger
Reporter StatsReporter
RateLimiter workqueue.RateLimiter
RateLimiter workqueue.TypedRateLimiter[any]
Concurrency int
}

// NewContext instantiates an instance of our controller that will feed work to the
// provided Reconciler as it is enqueued.
func NewContext(ctx context.Context, r Reconciler, options ControllerOptions) *Impl {
if options.RateLimiter == nil {
options.RateLimiter = workqueue.DefaultControllerRateLimiter()
options.RateLimiter = workqueue.DefaultTypedControllerRateLimiter[any]()
}
if options.Reporter == nil {
options.Reporter = MustNewStatsReporter(options.WorkQueueName, options.Logger)
Expand All @@ -263,7 +263,7 @@ func NewContext(ctx context.Context, r Reconciler, options ControllerOptions) *I
}

// WorkQueue permits direct access to the work queue.
func (c *Impl) WorkQueue() workqueue.RateLimitingInterface {
func (c *Impl) WorkQueue() workqueue.TypedRateLimitingInterface[any] {
return c.workQueue
}

Expand Down
8 changes: 4 additions & 4 deletions controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ func (t testRateLimiter) When(interface{}) time.Duration { return t.delay }
func (t testRateLimiter) Forget(interface{}) {}
func (t testRateLimiter) NumRequeues(interface{}) int { return 0 }

var _ workqueue.RateLimiter = (*testRateLimiter)(nil)
var _ workqueue.TypedRateLimiter[any] = (*testRateLimiter)(nil)

func TestEnqueue(t *testing.T) {
tests := []struct {
Expand Down Expand Up @@ -715,7 +715,7 @@ func TestEnqueue(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
var rl workqueue.RateLimiter = testRateLimiter{t, 100 * time.Millisecond}
var rl workqueue.TypedRateLimiter[any] = testRateLimiter{t, 100 * time.Millisecond}
impl := NewContext(context.TODO(), &nopReconciler{}, ControllerOptions{WorkQueueName: "Testing", Logger: TestLogger(t), RateLimiter: rl})
test.work(impl)

Expand All @@ -741,7 +741,7 @@ const (
queueCheckTimeout = shortDelay + 500*time.Millisecond
)

func pollQ(q workqueue.RateLimitingInterface, sig chan int) func(context.Context) (bool, error) {
func pollQ(q workqueue.TypedRateLimitingInterface[any], sig chan int) func(context.Context) (bool, error) {
return func(context.Context) (bool, error) {
if ql := q.Len(); ql > 0 {
sig <- ql
Expand Down Expand Up @@ -1356,7 +1356,7 @@ func TestStartAndShutdownWithRequeuingWork(t *testing.T) {
}
}

func drainWorkQueue(wq workqueue.RateLimitingInterface) (hasQueue []types.NamespacedName) {
func drainWorkQueue(wq workqueue.TypedRateLimitingInterface[any]) (hasQueue []types.NamespacedName) {
for {
key, shutdown := wq.Get()
if key == nil && shutdown {
Expand Down
20 changes: 10 additions & 10 deletions controller/two_lane_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ import "k8s.io/client-go/util/workqueue"
// -- slow queue (slowLane queue), whose contents are processed if fast queue has no items.
// All the default methods operate on the fast queue, unless noted otherwise.
type twoLaneQueue struct {
workqueue.RateLimitingInterface
slowLane workqueue.RateLimitingInterface
workqueue.TypedRateLimitingInterface[any]
slowLane workqueue.TypedRateLimitingInterface[any]
// consumerQueue is necessary to ensure that we're not reconciling
// the same object at the exact same time (e.g. if it had been enqueued
// in both fast and slow and is the only object there).
consumerQueue workqueue.Interface
consumerQueue workqueue.TypedInterface[any]

name string

Expand All @@ -37,9 +37,9 @@ type twoLaneQueue struct {
}

// Creates a new twoLaneQueue.
func newTwoLaneWorkQueue(name string, rl workqueue.RateLimiter) *twoLaneQueue {
func newTwoLaneWorkQueue(name string, rl workqueue.TypedRateLimiter[any]) *twoLaneQueue {
tlq := &twoLaneQueue{
RateLimitingInterface: workqueue.NewNamedRateLimitingQueue(
TypedRateLimitingInterface: workqueue.NewNamedRateLimitingQueue(
rl,
name+"-fast",
),
Expand All @@ -55,12 +55,12 @@ func newTwoLaneWorkQueue(name string, rl workqueue.RateLimiter) *twoLaneQueue {
// Run consumer thread.
go tlq.runConsumer()
// Run producer threads.
go process(tlq.RateLimitingInterface, tlq.fastChan)
go process(tlq.TypedRateLimitingInterface, tlq.fastChan)
go process(tlq.slowLane, tlq.slowChan)
return tlq
}

func process(q workqueue.Interface, ch chan interface{}) {
func process(q workqueue.TypedInterface[any], ch chan interface{}) {
// Sender closes the channel
defer close(ch)
for {
Expand Down Expand Up @@ -125,7 +125,7 @@ func (tlq *twoLaneQueue) runConsumer() {
// Shutdown implements workqueue.Interface.
// Shutdown shuts down both queues.
func (tlq *twoLaneQueue) ShutDown() {
tlq.RateLimitingInterface.ShutDown()
tlq.TypedRateLimitingInterface.ShutDown()
tlq.slowLane.ShutDown()
}

Expand All @@ -147,10 +147,10 @@ func (tlq *twoLaneQueue) Get() (interface{}, bool) {
// Len returns the sum of lengths.
// NB: actual _number_ of unique object might be less than this sum.
func (tlq *twoLaneQueue) Len() int {
return tlq.RateLimitingInterface.Len() + tlq.slowLane.Len() + tlq.consumerQueue.Len()
return tlq.TypedRateLimitingInterface.Len() + tlq.slowLane.Len() + tlq.consumerQueue.Len()
}

// SlowLane gives direct access to the slow queue.
func (tlq *twoLaneQueue) SlowLane() workqueue.RateLimitingInterface {
func (tlq *twoLaneQueue) SlowLane() workqueue.TypedRateLimitingInterface[any] {
return tlq.slowLane
}
8 changes: 4 additions & 4 deletions controller/two_lane_queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (r *chanRateLimiter) NumRequeues(item interface{}) int {
return 0
}

var _ workqueue.RateLimiter = &chanRateLimiter{}
var _ workqueue.TypedRateLimiter[any] = &chanRateLimiter{}

func TestRateLimit(t *testing.T) {
// Verifies that we properly pass the rate limiter to the queue.
Expand Down Expand Up @@ -87,7 +87,7 @@ func TestRateLimit(t *testing.T) {
}

func TestSlowQueue(t *testing.T) {
q := newTwoLaneWorkQueue("live-in-the-fast-lane", workqueue.DefaultControllerRateLimiter())
q := newTwoLaneWorkQueue("live-in-the-fast-lane", workqueue.DefaultTypedControllerRateLimiter[any]())
q.SlowLane().Add("1")
// Queue has async moving parts so if we check at the wrong moment, this might still be 0.
if wait.PollUntilContextTimeout(context.Background(), 10*time.Millisecond, 250*time.Millisecond, true, func(ctx context.Context) (bool, error) {
Expand Down Expand Up @@ -115,7 +115,7 @@ func TestSlowQueue(t *testing.T) {

func TestDoubleKey(t *testing.T) {
// Verifies that we don't get double concurrent processing of the same key.
q := newTwoLaneWorkQueue("live-in-the-fast-lane", workqueue.DefaultControllerRateLimiter())
q := newTwoLaneWorkQueue("live-in-the-fast-lane", workqueue.DefaultTypedControllerRateLimiter[any]())
q.Add("1")
t.Cleanup(q.ShutDown)

Expand Down Expand Up @@ -159,7 +159,7 @@ func TestDoubleKey(t *testing.T) {

func TestOrder(t *testing.T) {
// Verifies that we read from the fast queue first.
q := newTwoLaneWorkQueue("live-in-the-fast-lane", workqueue.DefaultControllerRateLimiter())
q := newTwoLaneWorkQueue("live-in-the-fast-lane", workqueue.DefaultTypedControllerRateLimiter[any]())
stop := make(chan struct{})
t.Cleanup(func() {
close(stop)
Expand Down
22 changes: 11 additions & 11 deletions hash/hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const (

// universe represents the possible range of angles [0, universe).
// We want to have universe divide total range evenly to reduce bias.
universe = (1 << 11)
universe uint64 = (1 << 11)
)

// computeAngle returns a uint64 number which represents
Expand All @@ -51,13 +51,13 @@ func computeHash(n []byte, h hash.Hash64) uint64 {

type hashData struct {
// The set of all hashes for fast lookup and to name mapping
nameLookup map[int]string
nameLookup map[uint64]string
// Sorted set of hashes for selection algorithm.
hashPool []int
hashPool []uint64
// start angle
start int
start uint64
// step angle
step int
step uint64
}

func (hd *hashData) fromIndexSet(s sets.Set[int]) sets.Set[string] {
Expand Down Expand Up @@ -85,29 +85,29 @@ func buildHashes(in sets.Set[string], target string) *hashData {
buf.WriteString(startSalt)
hasher := fnv.New64a()
hd := &hashData{
nameLookup: make(map[int]string, len(from)),
hashPool: make([]int, len(from)),
start: int(computeHash(buf.Bytes(), hasher) % universe),
nameLookup: make(map[uint64]string, len(from)),
hashPool: make([]uint64, len(from)),
start: computeHash(buf.Bytes(), hasher) % universe,
}
buf.Truncate(len(target)) // Discard the angle salt.
buf.WriteString(stepSalt)
hd.step = int(computeHash(buf.Bytes(), hasher) % universe)
hd.step = computeHash(buf.Bytes(), hasher) % universe

for i, f := range from {
buf.Reset() // This retains the storage.
// Make unique sets for every target.
buf.WriteString(f)
buf.WriteString(target)
h := computeHash(buf.Bytes(), hasher)
hs := int(h % universe)
hs := h % universe
// Two values slotted to the same bucket.
// On average should happen with 1/universe probability.
_, ok := hd.nameLookup[hs]
for ok {
// Feed the hash as salt.
buf.WriteString(strconv.FormatUint(h, 16 /*append hex strings for shortness*/))
h = computeHash(buf.Bytes(), hasher)
hs = int(h % universe)
hs = h % universe
_, ok = hd.nameLookup[hs]
}

Expand Down
2 changes: 1 addition & 1 deletion hash/hash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func BenchmarkSelection(b *testing.B) {
b.Run(fmt.Sprintf("pool-%d-subset-%d", v, ss), func(b *testing.B) {
target := uuid.NewString()
in := sets.New[string](from[:v]...)
for i := 0; i < b.N; i++ {
for range b.N {
ChooseSubset(in, 10, target)
}
})
Expand Down
3 changes: 1 addition & 2 deletions kmeta/names_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the license.
package kmeta

import (
"fmt"
"strings"
"testing"

Expand Down Expand Up @@ -70,7 +69,7 @@ func TestChildName(t *testing.T) {
t.Run(test.parent+"-"+test.suffix, func(t *testing.T) {
got, want := ChildName(test.parent, test.suffix), test.want
if errs := validation.IsDNS1123Subdomain(got); len(errs) != 0 {
t.Errorf(fmt.Sprintf("Invalid DNS1123 Subdomain %63s\n\n Errors: %v", got, errs))
t.Errorf("Invalid DNS1123 Subdomain %63s\n\n Errors: %v", got, errs)
}
if got != want {
t.Errorf("%s-%s: got: %63s want: %63s\ndiff:%s", test.parent, test.suffix, got, want, cmp.Diff(want, got))
Expand Down
2 changes: 1 addition & 1 deletion metrics/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func prometheusPort() (int, error) {
return defaultPrometheusPort, nil
}

pp, err := strconv.ParseUint(ppStr, 10, 16)
pp, err := strconv.ParseInt(ppStr, 10, 16)
if err != nil {
return -1, fmt.Errorf("the environment variable %q could not be parsed as a port number: %w",
prometheusPortEnvName, err)
Expand Down
Loading
Loading