From 1e1f618cd23761569bf70d1672af5c0437ca74f5 Mon Sep 17 00:00:00 2001 From: Dave Protasowski Date: Thu, 9 Jan 2025 23:10:51 -0500 Subject: [PATCH] address remaining linter changes --- .golangci.yaml | 2 -- .../networking/v1alpha1/ingress_helpers.go | 4 +-- pkg/deprecated_header.go | 2 +- pkg/http/header/header.go | 2 +- pkg/status/status.go | 31 ++++++++++--------- pkg/status/status_test.go | 2 +- test/conformance/ingress/grpc.go | 4 +-- test/conformance/ingress/headers.go | 2 -- test/conformance/ingress/path.go | 1 - test/conformance/ingress/timeout.go | 1 - test/prober.go | 8 ++--- 11 files changed, 27 insertions(+), 32 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 5fbed11c9..9c69b83ac 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -30,8 +30,6 @@ issues: text: "ifElseChain" linters-settings: - intrange: - severity: warning gomodguard: blocked: modules: diff --git a/pkg/apis/networking/v1alpha1/ingress_helpers.go b/pkg/apis/networking/v1alpha1/ingress_helpers.go index f3e015b05..fa77c3c6e 100644 --- a/pkg/apis/networking/v1alpha1/ingress_helpers.go +++ b/pkg/apis/networking/v1alpha1/ingress_helpers.go @@ -27,13 +27,13 @@ import ( func (i *Ingress) GetIngressTLSForVisibility(visibility IngressVisibility) []IngressTLS { ingressTLS := make([]IngressTLS, 0, len(i.Spec.TLS)) - if i.Spec.TLS == nil || len(i.Spec.TLS) == 0 { + if len(i.Spec.TLS) == 0 { return ingressTLS } for _, rule := range i.Spec.Rules { if rule.Visibility == visibility { - if rule.Hosts == nil || len(rule.Hosts) == 0 { + if len(rule.Hosts) == 0 { return ingressTLS } diff --git a/pkg/deprecated_header.go b/pkg/deprecated_header.go index c2e98c2c3..ce7d3059d 100644 --- a/pkg/deprecated_header.go +++ b/pkg/deprecated_header.go @@ -67,7 +67,7 @@ const ( // different user-agent. So we augment the probes with this header. // // Deprecated: use knative.dev/networking/pkg/http/header.KubeletProbeKey - KubeletProbeHeaderName = header.KubeletProbeKey + KubeletProbeHeaderName = header.KubeletProbeKey //nolint:staticheck // UserAgentKey is the constant for header "User-Agent". // diff --git a/pkg/http/header/header.go b/pkg/http/header/header.go index 143f8bedb..6a8265d6a 100644 --- a/pkg/http/header/header.go +++ b/pkg/http/header/header.go @@ -59,7 +59,7 @@ const ( // Prior to this deprecation, Istio with mTLS rewrote probes and their probes passed a // different user-agent. Therefore, this header was added to augment the probes. // - // Deprecated: this custom request header is no longer necessary since Istio now propagates the + // This custom request header is no longer necessary since Istio now propagates the // original request header `User-Agent` sent by the kubelet (e.g., User-Agent: kube-probe/1.29). // For updated usage, please utilize knative.dev/networking/pkg/http/header.UserAgentKey and // knative.dev/networking/pkg/http/header.KubeProbeUAPrefix diff --git a/pkg/status/status.go b/pkg/status/status.go index ca0315632..4d7ebe515 100644 --- a/pkg/status/status.go +++ b/pkg/status/status.go @@ -29,9 +29,9 @@ import ( "reflect" "strconv" "sync" + "sync/atomic" "time" - "go.uber.org/atomic" "go.uber.org/zap" "golang.org/x/time/rate" @@ -78,7 +78,7 @@ type ingressState struct { ing *v1alpha1.Ingress // pendingCount is the number of pods that haven't been successfully probed yet - pendingCount atomic.Int32 + pendingCount atomic.Int64 lastAccessed time.Time cancel func() @@ -87,7 +87,7 @@ type ingressState struct { // podState represents the probing state of a Pod (for a specific Ingress) type podState struct { // pendingCount is the number of probes for the Pod - pendingCount atomic.Int32 + pendingCount atomic.Int64 cancel func() } @@ -137,7 +137,7 @@ type Prober struct { ingressStates map[types.NamespacedName]*ingressState podContexts map[string]cancelContext - workQueue workqueue.RateLimitingInterface + workQueue workqueue.TypedRateLimitingInterface[any] targetLister ProbeTargetLister @@ -157,11 +157,11 @@ func NewProber( ingressStates: make(map[types.NamespacedName]*ingressState), podContexts: make(map[string]cancelContext), workQueue: workqueue.NewNamedRateLimitingQueue( - workqueue.NewMaxOfRateLimiter( + workqueue.NewTypedMaxOfRateLimiter( // Per item exponential backoff - workqueue.NewItemExponentialFailureRateLimiter(50*time.Millisecond, probeMaxRetryDelay), + workqueue.NewTypedItemExponentialFailureRateLimiter[any](50*time.Millisecond, probeMaxRetryDelay), // Global rate limiter - &workqueue.BucketRateLimiter{Limiter: rate.NewLimiter(rate.Limit(50), 100)}, + &workqueue.TypedBucketRateLimiter[any]{Limiter: rate.NewLimiter(rate.Limit(50), 100)}, ), "ProbingQueue"), targetLister: targetLister, @@ -231,7 +231,7 @@ func (m *Prober) IsReady(ctx context.Context, ing *v1alpha1.Ingress) (bool, erro } } - ingressState.pendingCount.Store(int32(len(workItems))) + ingressState.pendingCount.Store(int64(len(workItems))) for ip, ipWorkItems := range workItems { // Get or create the context for that IP @@ -252,10 +252,11 @@ func (m *Prober) IsReady(ctx context.Context, ing *v1alpha1.Ingress) (bool, erro podCtx, cancel := context.WithCancel(ingCtx) podState := &podState{ - pendingCount: *atomic.NewInt32(int32(len(ipWorkItems))), - cancel: cancel, + cancel: cancel, } + podState.pendingCount.Store(int64(len(ipWorkItems))) + // Quick and dirty way to join two contexts (i.e. podCtx is cancelled when either ingCtx or ipCtx are cancelled) go func() { select { @@ -277,7 +278,7 @@ func (m *Prober) IsReady(ctx context.Context, ing *v1alpha1.Ingress) (bool, erro for _, wi := range ipWorkItems { wi.podState = podState - wi.context = podCtx + wi.context = podCtx //nolint:fatcontext m.workQueue.AddAfter(wi, initialDelay) logger.Infof("Queuing probe for %s, IP: %s:%s (depth: %d)", wi.url, wi.podIP, wi.podPort, m.workQueue.Len()) @@ -427,12 +428,12 @@ func (m *Prober) processWorkItem() bool { func (m *Prober) onProbingSuccess(ingressState *ingressState, podState *podState) { // The last probe call for the Pod succeeded, the Pod is ready - if podState.pendingCount.Dec() == 0 { + if podState.pendingCount.Add(-1) == 0 { // Unlock the goroutine blocked on <-podCtx.Done() podState.cancel() // This is the last pod being successfully probed, the Ingress is ready - if ingressState.pendingCount.Dec() == 0 { + if ingressState.pendingCount.Add(-1) == 0 { m.readyCallback(ingressState.ing) } } @@ -447,9 +448,9 @@ func (m *Prober) onProbingCancellation(ingressState *ingressState, podState *pod } // Attempt to set pendingCount to 0. - if podState.pendingCount.CAS(pendingCount, 0) { + if podState.pendingCount.CompareAndSwap(pendingCount, 0) { // This is the last pod being successfully probed, the Ingress is ready - if ingressState.pendingCount.Dec() == 0 { + if ingressState.pendingCount.Add(-1) == 0 { m.readyCallback(ingressState.ing) } return diff --git a/pkg/status/status_test.go b/pkg/status/status_test.go index 9051446fa..c8d89592e 100644 --- a/pkg/status/status_test.go +++ b/pkg/status/status_test.go @@ -24,6 +24,7 @@ import ( "net/url" "strconv" "strings" + "sync/atomic" "testing" "time" @@ -33,7 +34,6 @@ import ( "knative.dev/networking/pkg/http/probe" "knative.dev/networking/pkg/ingress" - "go.uber.org/atomic" "go.uber.org/zap/zaptest" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/test/conformance/ingress/grpc.go b/test/conformance/ingress/grpc.go index f95333953..4f0b51cc1 100644 --- a/test/conformance/ingress/grpc.go +++ b/test/conformance/ingress/grpc.go @@ -64,7 +64,7 @@ func TestGRPC(t *testing.T) { }}, }) - conn, err := grpc.Dial( + conn, err := grpc.NewClient( domain+":80", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { @@ -133,7 +133,7 @@ func TestGRPCSplit(t *testing.T) { }}, }) - conn, err := grpc.Dial( + conn, err := grpc.NewClient( domain+":80", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { diff --git a/test/conformance/ingress/headers.go b/test/conformance/ingress/headers.go index e1a90d323..762e93442 100644 --- a/test/conformance/ingress/headers.go +++ b/test/conformance/ingress/headers.go @@ -80,7 +80,6 @@ func TestProbeHeaders(t *testing.T) { }} for _, tt := range tests { - tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() @@ -187,7 +186,6 @@ func TestTagHeaders(t *testing.T) { }} for _, tt := range tests { - tt := tt t.Run(tt.Name, func(t *testing.T) { t.Parallel() diff --git a/test/conformance/ingress/path.go b/test/conformance/ingress/path.go index 32a6dc7d1..3f66e8631 100644 --- a/test/conformance/ingress/path.go +++ b/test/conformance/ingress/path.go @@ -127,7 +127,6 @@ func TestPath(t *testing.T) { } for path, want := range tests { - path, want := path, want t.Run(path, func(t *testing.T) { t.Parallel() diff --git a/test/conformance/ingress/timeout.go b/test/conformance/ingress/timeout.go index ddeff9b6a..6484ef0e2 100644 --- a/test/conformance/ingress/timeout.go +++ b/test/conformance/ingress/timeout.go @@ -75,7 +75,6 @@ func TestTimeout(t *testing.T) { }} for _, test := range tests { - test := test t.Run(test.name, func(t *testing.T) { t.Parallel() diff --git a/test/prober.go b/test/prober.go index 493271628..ef0206b8e 100644 --- a/test/prober.go +++ b/test/prober.go @@ -24,9 +24,9 @@ import ( "net/http" "net/url" "sync" + "sync/atomic" "testing" - "go.uber.org/atomic" "golang.org/x/sync/errgroup" pkgTest "knative.dev/pkg/test" @@ -158,16 +158,16 @@ func (m *manager) Spawn(url *url.URL) Prober { return nil default: res, err := client.Do(req) - if p.requests.Inc() == p.minimumProbes { + if p.requests.Add(1) == p.minimumProbes { close(p.minDoneCh) } if err != nil { p.logf("%q error: %v", p.url, err) - p.failures.Inc() + p.failures.Add(1) } else if res.StatusCode != http.StatusOK { p.logf("%q status = %d, want: %d", p.url, res.StatusCode, http.StatusOK) p.logf("Response: %s", res) - p.failures.Inc() + p.failures.Add(1) } } }