From 6b377ca7f27ff97d21187da45c4c7c37790858c2 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Thu, 13 Jul 2023 12:56:07 +0200 Subject: [PATCH] Address golangci-lint comments Signed-off-by: Per Goncalves da Silva Signed-off-by: Todd Short --- pkg/controller/install/rule_checker_test.go | 6 ++- pkg/controller/operators/olm/operator_test.go | 8 ++-- .../registry/resolver/resolver_test.go | 7 +--- .../registry/resolver/solver/bench_test.go | 17 ++++---- pkg/lib/operatorstatus/builder_test.go | 40 +++++++++---------- pkg/lib/operatorstatus/monitor_test.go | 6 +-- pkg/lib/proxy/envvar.go | 6 +-- pkg/lib/proxy/envvar_test.go | 12 +++--- pkg/lib/proxy/overridden_test.go | 16 ++++---- pkg/package-server/provider/registry.go | 2 + pkg/package-server/server/server.go | 8 ++-- 11 files changed, 65 insertions(+), 63 deletions(-) diff --git a/pkg/controller/install/rule_checker_test.go b/pkg/controller/install/rule_checker_test.go index 4a01abfbca..50ae8733d4 100644 --- a/pkg/controller/install/rule_checker_test.go +++ b/pkg/controller/install/rule_checker_test.go @@ -594,12 +594,14 @@ func NewFakeCSVRuleChecker(k8sObjs []runtime.Object, csv *operatorsv1alpha1.Clus for _, informer := range []cache.SharedIndexInformer{roleInformer.Informer(), roleBindingInformer.Informer(), clusterRoleInformer.Informer(), clusterRoleBindingInformer.Informer()} { go informer.Run(stopCh) - synced := func() (bool, error) { + synced := func(_ context.Context) (done bool, err error) { return informer.HasSynced(), nil } // wait until the informer has synced to continue - wait.PollUntil(500*time.Millisecond, synced, stopCh) + if err := wait.PollUntilContextTimeout(context.Background(), 500*time.Millisecond, 5*time.Second, true, synced); err != nil { + return nil, err + } } ruleChecker := NewCSVRuleChecker(roleInformer.Lister(), roleBindingInformer.Lister(), clusterRoleInformer.Lister(), clusterRoleBindingInformer.Lister(), csv) diff --git a/pkg/controller/operators/olm/operator_test.go b/pkg/controller/operators/olm/operator_test.go index 8377073c23..5cac02b6cf 100644 --- a/pkg/controller/operators/olm/operator_test.go +++ b/pkg/controller/operators/olm/operator_test.go @@ -4953,7 +4953,7 @@ func TestSyncOperatorGroups(t *testing.T) { require.NoError(t, err) // Wait for the lister cache to catch up - err = wait.PollImmediateWithContext(ctx, tick, timeout, func(ctx context.Context) (bool, error) { + err = wait.PollUntilContextTimeout(ctx, tick, timeout, true, func(ctx context.Context) (bool, error) { deployment, err := op.lister.AppsV1().DeploymentLister().Deployments(namespace).Get(dep.GetName()) if err != nil || deployment == nil { return false, err @@ -4974,7 +4974,7 @@ func TestSyncOperatorGroups(t *testing.T) { require.NoError(t, err) // Wait on operator group updated status to be in the cache as it is required for later CSV operations - err = wait.PollImmediateWithContext(ctx, tick, timeout, func(ctx context.Context) (bool, error) { + err = wait.PollUntilContextTimeout(ctx, tick, timeout, true, func(ctx context.Context) (bool, error) { og, err := op.lister.OperatorsV1().OperatorGroupLister().OperatorGroups(operatorGroup.GetNamespace()).Get(operatorGroup.GetName()) if err != nil { return false, err @@ -4993,14 +4993,14 @@ func TestSyncOperatorGroups(t *testing.T) { // This must be done (at least) twice to have annotateCSVs run in syncOperatorGroups and to catch provided API changes // syncOperatorGroups is eventually consistent and may return errors until the cache has caught up with the cluster (fake client here) - wait.PollImmediateWithContext(ctx, tick, timeout, func(ctx context.Context) (bool, error) { // Throw away timeout errors since any timeout will coincide with err != nil anyway + err = wait.PollUntilContextTimeout(ctx, tick, timeout, true, func(ctx context.Context) (bool, error) { // Throw away timeout errors since any timeout will coincide with err != nil anyway err = op.syncOperatorGroups(operatorGroup) return err == nil, nil }) require.NoError(t, err) // Sync csvs enough to get them back to a succeeded state - err = wait.PollImmediateWithContext(ctx, tick, timeout, func(ctx context.Context) (bool, error) { + err = wait.PollUntilContextTimeout(ctx, tick, timeout, true, func(ctx context.Context) (bool, error) { csvs, err := op.client.OperatorsV1alpha1().ClusterServiceVersions(operatorNamespace).List(ctx, metav1.ListOptions{}) if err != nil { return false, err diff --git a/pkg/controller/registry/resolver/resolver_test.go b/pkg/controller/registry/resolver/resolver_test.go index 86bfaf282a..d1bb8d3aa8 100644 --- a/pkg/controller/registry/resolver/resolver_test.go +++ b/pkg/controller/registry/resolver/resolver_test.go @@ -24,10 +24,7 @@ import ( ) var testGVKKey = opregistry.APIKey{Group: "g", Version: "v", Kind: "k", Plural: "ks"} - -func init() { - rand.Seed(time.Now().UnixNano()) -} +var rnd = rand.New(rand.NewSource(time.Now().UnixNano())) // tests can directly specify fixtures as cache entries instead of depending on this translation func csvSnapshotOrPanic(ns string, subs []*v1alpha1.Subscription, csvs ...*v1alpha1.ClusterServiceVersion) *cache.Snapshot { @@ -778,7 +775,7 @@ func (g entryGenerator) gen() *cache.Entry { func genEntriesRandom(ops ...entryGenerator) []*cache.Entry { entries := make([]*cache.Entry, len(ops)) // Randomize entry order to fuzz input operators over time. - idxs := rand.Perm(len(ops)) + idxs := rnd.Perm(len(ops)) for destIdx, srcIdx := range idxs { entries[destIdx] = ops[srcIdx].gen() } diff --git a/pkg/controller/registry/resolver/solver/bench_test.go b/pkg/controller/registry/resolver/solver/bench_test.go index 0296b85f10..b9b9c95ba5 100644 --- a/pkg/controller/registry/resolver/solver/bench_test.go +++ b/pkg/controller/registry/resolver/solver/bench_test.go @@ -18,33 +18,35 @@ var BenchmarkInput = func() []Variable { nConflict = 3 ) + rnd := rand.New(rand.NewSource(seed)) + id := func(i int) Identifier { return Identifier(strconv.Itoa(i)) } variable := func(i int) TestVariable { var c []Constraint - if rand.Float64() < pMandatory { + if rnd.Float64() < pMandatory { c = append(c, Mandatory()) } - if rand.Float64() < pDependency { - n := rand.Intn(nDependency-1) + 1 + if rnd.Float64() < pDependency { + n := rnd.Intn(nDependency-1) + 1 var d []Identifier for x := 0; x < n; x++ { y := i for y == i { - y = rand.Intn(length) + y = rnd.Intn(length) } d = append(d, id(y)) } c = append(c, Dependency(d...)) } - if rand.Float64() < pConflict { - n := rand.Intn(nConflict-1) + 1 + if rnd.Float64() < pConflict { + n := rnd.Intn(nConflict-1) + 1 for x := 0; x < n; x++ { y := i for y == i { - y = rand.Intn(length) + y = rnd.Intn(length) } c = append(c, Conflict(id(y))) } @@ -55,7 +57,6 @@ var BenchmarkInput = func() []Variable { } } - rand.Seed(seed) result := make([]Variable, length) for i := range result { result[i] = variable(i) diff --git a/pkg/lib/operatorstatus/builder_test.go b/pkg/lib/operatorstatus/builder_test.go index b9a9e585f8..b5b2ff5563 100644 --- a/pkg/lib/operatorstatus/builder_test.go +++ b/pkg/lib/operatorstatus/builder_test.go @@ -30,7 +30,7 @@ func TestBuilder(t *testing.T) { }, expected: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{ - configv1.ClusterOperatorStatusCondition{ + { Type: configv1.OperatorProgressing, Status: configv1.ConditionTrue, Message: "message", @@ -51,7 +51,7 @@ func TestBuilder(t *testing.T) { }, existing: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{ - configv1.ClusterOperatorStatusCondition{ + { Type: configv1.OperatorProgressing, Status: configv1.ConditionFalse, }, @@ -61,7 +61,7 @@ func TestBuilder(t *testing.T) { }, expected: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{ - configv1.ClusterOperatorStatusCondition{ + { Type: configv1.OperatorProgressing, Status: configv1.ConditionTrue, Message: "message", @@ -82,7 +82,7 @@ func TestBuilder(t *testing.T) { }, existing: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{ - configv1.ClusterOperatorStatusCondition{ + { Type: configv1.OperatorProgressing, Status: configv1.ConditionTrue, LastTransitionTime: minuteAgo, @@ -93,7 +93,7 @@ func TestBuilder(t *testing.T) { }, expected: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{ - configv1.ClusterOperatorStatusCondition{ + { Type: configv1.OperatorProgressing, Status: configv1.ConditionTrue, Message: "message", @@ -199,7 +199,7 @@ func TestBuilder(t *testing.T) { expected: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{ - configv1.OperandVersion{ + { Name: "foo", Version: "1.00", }, @@ -218,7 +218,7 @@ func TestBuilder(t *testing.T) { existing: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{ - configv1.OperandVersion{ + { Name: "foo", Version: "1.00", }, @@ -228,7 +228,7 @@ func TestBuilder(t *testing.T) { expected: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{ - configv1.OperandVersion{ + { Name: "foo", Version: "1.00", }, @@ -247,7 +247,7 @@ func TestBuilder(t *testing.T) { existing: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{ - configv1.OperandVersion{ + { Name: "foo", Version: "1.00", }, @@ -257,7 +257,7 @@ func TestBuilder(t *testing.T) { expected: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{ - configv1.OperandVersion{ + { Name: "foo", Version: "2.00", }, @@ -275,7 +275,7 @@ func TestBuilder(t *testing.T) { existing: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{ - configv1.OperandVersion{ + { Name: "foo", Version: "1.00", }, @@ -285,11 +285,11 @@ func TestBuilder(t *testing.T) { expected: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{ - configv1.OperandVersion{ + { Name: "foo", Version: "2.00", }, - configv1.OperandVersion{ + { Name: "bar", Version: "1.00", }, @@ -308,11 +308,11 @@ func TestBuilder(t *testing.T) { existing: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{ - configv1.OperandVersion{ + { Name: "foo", Version: "1.00", }, - configv1.OperandVersion{ + { Name: "bar", Version: "1.00", }, @@ -322,7 +322,7 @@ func TestBuilder(t *testing.T) { expected: &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{ - configv1.OperandVersion{ + { Name: "bar", Version: "1.00", }, @@ -342,7 +342,7 @@ func TestBuilder(t *testing.T) { Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{}, RelatedObjects: []configv1.ObjectReference{ - configv1.ObjectReference{ + { Group: "group", Resource: "resources", Namespace: "namespace", @@ -363,7 +363,7 @@ func TestBuilder(t *testing.T) { Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{}, RelatedObjects: []configv1.ObjectReference{ - configv1.ObjectReference{ + { Group: "group", Resource: "resources", Namespace: "namespace", @@ -375,7 +375,7 @@ func TestBuilder(t *testing.T) { Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{}, RelatedObjects: []configv1.ObjectReference{ - configv1.ObjectReference{ + { Group: "group", Resource: "resources", Namespace: "namespace", @@ -396,7 +396,7 @@ func TestBuilder(t *testing.T) { Conditions: []configv1.ClusterOperatorStatusCondition{}, Versions: []configv1.OperandVersion{}, RelatedObjects: []configv1.ObjectReference{ - configv1.ObjectReference{ + { Group: "group", Resource: "resources", Namespace: "namespace", diff --git a/pkg/lib/operatorstatus/monitor_test.go b/pkg/lib/operatorstatus/monitor_test.go index 072d17fae6..f375a36ba9 100644 --- a/pkg/lib/operatorstatus/monitor_test.go +++ b/pkg/lib/operatorstatus/monitor_test.go @@ -17,17 +17,17 @@ func TestMonitorWaiting(t *testing.T) { statusWant := &configv1.ClusterOperatorStatus{ Conditions: []configv1.ClusterOperatorStatusCondition{ - configv1.ClusterOperatorStatusCondition{ + { Type: configv1.OperatorDegraded, Status: configv1.ConditionFalse, LastTransitionTime: metav1.NewTime(fakeClock.Now()), }, - configv1.ClusterOperatorStatusCondition{ + { Type: configv1.OperatorAvailable, Status: configv1.ConditionFalse, LastTransitionTime: metav1.NewTime(fakeClock.Now()), }, - configv1.ClusterOperatorStatusCondition{ + { Type: configv1.OperatorProgressing, Status: configv1.ConditionTrue, Message: fmt.Sprintf("waiting for events - source=%s", name), diff --git a/pkg/lib/proxy/envvar.go b/pkg/lib/proxy/envvar.go index d603207199..12766011b0 100644 --- a/pkg/lib/proxy/envvar.go +++ b/pkg/lib/proxy/envvar.go @@ -36,15 +36,15 @@ var ( // As a consumer we should be reading off of proxy.status. func ToEnvVar(proxy *apiconfigv1.Proxy) []corev1.EnvVar { return []corev1.EnvVar{ - corev1.EnvVar{ + { Name: envHTTPProxyName, Value: proxy.Status.HTTPProxy, }, - corev1.EnvVar{ + { Name: envHTTPSProxyName, Value: proxy.Status.HTTPSProxy, }, - corev1.EnvVar{ + { Name: envNoProxyName, Value: proxy.Status.NoProxy, }, diff --git a/pkg/lib/proxy/envvar_test.go b/pkg/lib/proxy/envvar_test.go index e46dd949bd..8dcb595fc4 100644 --- a/pkg/lib/proxy/envvar_test.go +++ b/pkg/lib/proxy/envvar_test.go @@ -31,15 +31,15 @@ func TestToEnvVar(t *testing.T) { }, }, envVarWant: []corev1.EnvVar{ - corev1.EnvVar{ + { Name: envHTTPProxyName, Value: "http://", }, - corev1.EnvVar{ + { Name: envHTTPSProxyName, Value: "https://", }, - corev1.EnvVar{ + { Name: envNoProxyName, Value: "foo,bar", }, @@ -56,15 +56,15 @@ func TestToEnvVar(t *testing.T) { }, }, envVarWant: []corev1.EnvVar{ - corev1.EnvVar{ + { Name: envHTTPProxyName, Value: "http://", }, - corev1.EnvVar{ + { Name: envHTTPSProxyName, Value: "", }, - corev1.EnvVar{ + { Name: envNoProxyName, Value: "", }, diff --git a/pkg/lib/proxy/overridden_test.go b/pkg/lib/proxy/overridden_test.go index 6a0e86b1fa..c720926adc 100644 --- a/pkg/lib/proxy/overridden_test.go +++ b/pkg/lib/proxy/overridden_test.go @@ -12,15 +12,15 @@ import ( var ( globalProxyConfig = []corev1.EnvVar{ - corev1.EnvVar{ + { Name: "HTTP_PROXY", Value: "http://foo.com:8080", }, - corev1.EnvVar{ + { Name: "HTTPS_PROXY", Value: "https://foo.com:443", }, - corev1.EnvVar{ + { Name: "NO_PROXY", Value: "a.com,b.com", }, @@ -46,7 +46,7 @@ func TestIsOverridden(t *testing.T) { { name: "WithUnrelatedEnvVar", envVar: []corev1.EnvVar{ - corev1.EnvVar{ + { Name: "foo", Value: "foo_value", }, @@ -56,7 +56,7 @@ func TestIsOverridden(t *testing.T) { { name: "WithHTTP_PROXY", envVar: []corev1.EnvVar{ - corev1.EnvVar{ + { Name: envHTTPProxyName, Value: "http://", }, @@ -66,7 +66,7 @@ func TestIsOverridden(t *testing.T) { { name: "WithHTTPS_PROXY", envVar: []corev1.EnvVar{ - corev1.EnvVar{ + { Name: envHTTPSProxyName, Value: "https://", }, @@ -76,7 +76,7 @@ func TestIsOverridden(t *testing.T) { { name: "WithNO_PROXY", envVar: []corev1.EnvVar{ - corev1.EnvVar{ + { Name: envNoProxyName, Value: "https://", }, @@ -86,7 +86,7 @@ func TestIsOverridden(t *testing.T) { { name: "WithCaseSensitive", envVar: []corev1.EnvVar{ - corev1.EnvVar{ + { Name: strings.ToLower(envHTTPSProxyName), Value: "http://", }, diff --git a/pkg/package-server/provider/registry.go b/pkg/package-server/provider/registry.go index cb48c6efa6..37deaf84df 100644 --- a/pkg/package-server/provider/registry.go +++ b/pkg/package-server/provider/registry.go @@ -541,6 +541,8 @@ func newPackageManifest(ctx context.Context, logger *logrus.Entry, pkg *api.Pack defaultCsv *operatorsv1alpha1.ClusterServiceVersion ) for _, pkgChannel := range pkgChannels { + // TODO: replace with non-deprecated API (e.g. client.GetBundle()) + // nolint:staticcheck bundle, err := client.GetBundleForChannel(ctx, &api.GetBundleInChannelRequest{PkgName: pkg.GetName(), ChannelName: pkgChannel.GetName()}) if err != nil { logger.WithError(err).WithField("channel", pkgChannel.GetName()).Warn("error getting bundle, eliding channel") diff --git a/pkg/package-server/server/server.go b/pkg/package-server/server/server.go index ff7d23cb81..a281c97149 100644 --- a/pkg/package-server/server/server.go +++ b/pkg/package-server/server/server.go @@ -127,14 +127,14 @@ func (o *PackageServerOptions) Config(ctx context.Context) (*apiserver.Config, e // In some cases the API server can return connection refused when getting the "extension-apiserver-authentication" config map var lastApplyErr error - err := wait.PollImmediateUntil(1*time.Second, func() (done bool, err error) { + err := wait.PollUntilContextCancel(pollCtx, 1*time.Second, true, func(_ context.Context) (done bool, err error) { lastApplyErr := authenticationOptions.ApplyTo(&config.Authentication, config.SecureServing, config.OpenAPIConfig) if lastApplyErr != nil { log.WithError(lastApplyErr).Warn("Error initializing delegating authentication (will retry)") return false, nil } return true, nil - }, pollCtx.Done()) + }) if err != nil { return nil, lastApplyErr @@ -153,14 +153,14 @@ func (o *PackageServerOptions) Config(ctx context.Context) (*apiserver.Config, e authorizationOptions.AllowCacheTTL = 35 * time.Second // In some cases the API server can return connection refused when getting the "extension-apiserver-authentication" config map - err = wait.PollImmediateUntil(1*time.Second, func() (done bool, err error) { + err = wait.PollUntilContextCancel(ctx, 1*time.Second, true, func(_ context.Context) (done bool, err error) { lastApplyErr = authorizationOptions.ApplyTo(&config.Authorization) if lastApplyErr != nil { log.WithError(lastApplyErr).Warn("Error initializing delegating authorization (will retry)") return false, nil } return true, nil - }, pollCtx.Done()) + }) if err != nil { return nil, lastApplyErr