From 3ef37a498926f638deb74bab142bddb12b5c9282 Mon Sep 17 00:00:00 2001 From: Kiichiro YUKAWA Date: Mon, 30 Oct 2023 15:04:14 +0900 Subject: [PATCH] :recycle: Refactor from deepsource feedback (#2222) Signed-off-by: vankichi --- internal/config/config_test.go | 1 - internal/k8s/client/client.go | 2 +- internal/k8s/job/job.go | 13 ++++++------ .../k8s/vald/benchmark/api/v1/job_types.go | 10 ++-------- .../vald/benchmark/api/v1/scenario_types.go | 10 ++-------- internal/k8s/vald/benchmark/job/job.go | 10 +++++----- .../k8s/vald/benchmark/scenario/scenario.go | 4 ++-- internal/sync/errgroup/group.go | 3 +-- .../benchmark/job/handler/rest/handler.go | 2 +- .../operator/handler/grpc/handler.go | 5 ++--- .../operator/handler/rest/handler.go | 2 +- .../benchmark/operator/service/operator.go | 20 +++++++++---------- .../operator/service/operator_test.go | 2 +- .../benchmark/operator/service/option.go | 6 +++--- .../benchmark/operator/usecase/benchmarkd.go | 4 ++-- 15 files changed, 39 insertions(+), 55 deletions(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 7f931915a9c..8624fbe48c5 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1434,7 +1434,6 @@ func TestToRawYaml(t *testing.T) { func TestMerge(t *testing.T) { type config struct { - // Config GlobalConfig Discoverer *Discoverer } diff --git a/internal/k8s/client/client.go b/internal/k8s/client/client.go index 1ebb0134268..0f74b4c29cb 100644 --- a/internal/k8s/client/client.go +++ b/internal/k8s/client/client.go @@ -77,7 +77,7 @@ func New(opts ...Option) (Client, error) { return c, nil } -func (c *client) Get(ctx context.Context, name string, namespace string, obj cli.Object, opts ...cli.GetOption) error { +func (c *client) Get(ctx context.Context, name, namespace string, obj cli.Object, opts ...cli.GetOption) error { return c.reader.Get( ctx, cli.ObjectKey{ diff --git a/internal/k8s/job/job.go b/internal/k8s/job/job.go index 5c00db89a0d..ae5a902045a 100644 --- a/internal/k8s/job/job.go +++ b/internal/k8s/job/job.go @@ -78,7 +78,7 @@ func New(opts ...Option) (JobWatcher, error) { } // Reconcile implements k8s reconciliation loop to retrieve the Job information from k8s. -func (r *reconciler) Reconcile(ctx context.Context, req reconcile.Request) (res reconcile.Result, err error) { +func (r *reconciler) Reconcile(ctx context.Context, _ reconcile.Request) (res reconcile.Result, err error) { js := new(batchv1.JobList) err = r.mgr.GetClient().List(ctx, js, r.listOpts...) @@ -101,7 +101,8 @@ func (r *reconciler) Reconcile(ctx context.Context, req reconcile.Request) (res } jobs := r.jobsByAppNamePool.Get().(map[string][]Job) - for _, job := range js.Items { + for idx := range js.Items { + job := js.Items[idx] name, ok := job.GetObjectMeta().GetLabels()["app"] if !ok { jns := strings.Split(job.GetName(), "-") @@ -133,7 +134,7 @@ func (r *reconciler) GetName() string { } // NewReconciler returns the reconciler for the Job. -func (r *reconciler) NewReconciler(ctx context.Context, mgr manager.Manager) reconcile.Reconciler { +func (r *reconciler) NewReconciler(_ context.Context, mgr manager.Manager) reconcile.Reconciler { if r.mgr == nil && mgr != nil { r.mgr = mgr } @@ -143,19 +144,19 @@ func (r *reconciler) NewReconciler(ctx context.Context, mgr manager.Manager) rec } // For returns the runtime.Object which is job. -func (r *reconciler) For() (client.Object, []builder.ForOption) { +func (*reconciler) For() (client.Object, []builder.ForOption) { return new(batchv1.Job), nil } // Owns returns the owner of the job watcher. // It will always return nil. -func (r *reconciler) Owns() (client.Object, []builder.OwnsOption) { +func (*reconciler) Owns() (client.Object, []builder.OwnsOption) { return nil, nil } // Watches returns the kind of the job and the event handler. // It will always return nil. -func (r *reconciler) Watches() (client.Object, handler.EventHandler, []builder.WatchesOption) { +func (*reconciler) Watches() (client.Object, handler.EventHandler, []builder.WatchesOption) { // return &source.Kind{Type: new(corev1.Pod)}, &handler.EnqueueRequestForObject{} return nil, nil, nil } diff --git a/internal/k8s/vald/benchmark/api/v1/job_types.go b/internal/k8s/vald/benchmark/api/v1/job_types.go index 4de559b154e..90ae1033ead 100644 --- a/internal/k8s/vald/benchmark/api/v1/job_types.go +++ b/internal/k8s/vald/benchmark/api/v1/job_types.go @@ -201,10 +201,7 @@ func (in *ValdBenchmarkJob) DeepCopy() *ValdBenchmarkJob { // DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. func (in *ValdBenchmarkJob) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil + return in.DeepCopy() } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -233,8 +230,5 @@ func (in *ValdBenchmarkJobList) DeepCopy() *ValdBenchmarkJobList { // DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. func (in *ValdBenchmarkJobList) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil + return in.DeepCopy() } diff --git a/internal/k8s/vald/benchmark/api/v1/scenario_types.go b/internal/k8s/vald/benchmark/api/v1/scenario_types.go index a0d1ebc821c..a78a38f8632 100644 --- a/internal/k8s/vald/benchmark/api/v1/scenario_types.go +++ b/internal/k8s/vald/benchmark/api/v1/scenario_types.go @@ -70,10 +70,7 @@ func (in *ValdBenchmarkScenario) DeepCopy() *ValdBenchmarkScenario { // DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. func (in *ValdBenchmarkScenario) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil + return in.DeepCopy() } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -102,10 +99,7 @@ func (in *ValdBenchmarkScenarioList) DeepCopy() *ValdBenchmarkScenarioList { // DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. func (in *ValdBenchmarkScenarioList) DeepCopyObject() runtime.Object { - if c := in.DeepCopy(); c != nil { - return c - } - return nil + return in.DeepCopy() } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. diff --git a/internal/k8s/vald/benchmark/job/job.go b/internal/k8s/vald/benchmark/job/job.go index 466d27107b3..02c86486f1e 100644 --- a/internal/k8s/vald/benchmark/job/job.go +++ b/internal/k8s/vald/benchmark/job/job.go @@ -72,7 +72,7 @@ func (r *reconciler) AddListOpts(opt client.ListOption) { r.lopts = append(r.lopts, opt) } -func (r *reconciler) Reconcile(ctx context.Context, req reconcile.Request) (res reconcile.Result, err error) { +func (r *reconciler) Reconcile(ctx context.Context, _ reconcile.Request) (res reconcile.Result, err error) { bj := new(v1.ValdBenchmarkJobList) if r.lopts == nil { @@ -115,7 +115,7 @@ func (r *reconciler) GetName() string { return r.name } -func (r *reconciler) NewReconciler(ctx context.Context, mgr manager.Manager) reconcile.Reconciler { +func (r *reconciler) NewReconciler(_ context.Context, mgr manager.Manager) reconcile.Reconciler { if r.mgr == nil && mgr != nil { r.mgr = mgr } @@ -125,15 +125,15 @@ func (r *reconciler) NewReconciler(ctx context.Context, mgr manager.Manager) rec return r } -func (r *reconciler) For() (client.Object, []builder.ForOption) { +func (*reconciler) For() (client.Object, []builder.ForOption) { return new(v1.ValdBenchmarkJob), nil } -func (r *reconciler) Owns() (client.Object, []builder.OwnsOption) { +func (*reconciler) Owns() (client.Object, []builder.OwnsOption) { return nil, nil } -func (r *reconciler) Watches() (client.Object, handler.EventHandler, []builder.WatchesOption) { +func (*reconciler) Watches() (client.Object, handler.EventHandler, []builder.WatchesOption) { // return &source.Kind{Type: new(corev1.Pod)}, &handler.EnqueueRequestForObject{} return nil, nil, nil } diff --git a/internal/k8s/vald/benchmark/scenario/scenario.go b/internal/k8s/vald/benchmark/scenario/scenario.go index 0ef74d4b6f9..68ec91a8581 100644 --- a/internal/k8s/vald/benchmark/scenario/scenario.go +++ b/internal/k8s/vald/benchmark/scenario/scenario.go @@ -61,7 +61,7 @@ func (r *reconciler) AddListOpts(opt client.ListOption) { r.lopts = append(r.lopts, opt) } -func (r *reconciler) Reconcile(ctx context.Context, req reconcile.Request) (res reconcile.Result, err error) { +func (r *reconciler) Reconcile(ctx context.Context, _ reconcile.Request) (res reconcile.Result, err error) { bs := new(v1.ValdBenchmarkScenarioList) if r.lopts == nil { @@ -104,7 +104,7 @@ func (r *reconciler) GetName() string { return r.name } -func (r *reconciler) NewReconciler(ctx context.Context, mgr manager.Manager) reconcile.Reconciler { +func (r *reconciler) NewReconciler(_ context.Context, mgr manager.Manager) reconcile.Reconciler { if r.mgr == nil && mgr != nil { r.mgr = mgr } diff --git a/internal/sync/errgroup/group.go b/internal/sync/errgroup/group.go index 1cc5345124f..93aee65edc9 100644 --- a/internal/sync/errgroup/group.go +++ b/internal/sync/errgroup/group.go @@ -21,9 +21,8 @@ import ( "context" "runtime" - "github.com/vdaas/vald/internal/sync" - "github.com/vdaas/vald/internal/errors" + "github.com/vdaas/vald/internal/sync" "github.com/vdaas/vald/internal/sync/semaphore" ) diff --git a/pkg/tools/benchmark/job/handler/rest/handler.go b/pkg/tools/benchmark/job/handler/rest/handler.go index e1e68671929..fea77e7e7c7 100644 --- a/pkg/tools/benchmark/job/handler/rest/handler.go +++ b/pkg/tools/benchmark/job/handler/rest/handler.go @@ -24,7 +24,7 @@ import ( type Handler interface{} type handler struct { - js benchmark.JobServer + benchmark.JobServer } func New(opts ...Option) Handler { diff --git a/pkg/tools/benchmark/operator/handler/grpc/handler.go b/pkg/tools/benchmark/operator/handler/grpc/handler.go index 06fce336f55..3cad884097c 100644 --- a/pkg/tools/benchmark/operator/handler/grpc/handler.go +++ b/pkg/tools/benchmark/operator/handler/grpc/handler.go @@ -31,8 +31,7 @@ type Benchmark interface { type server struct { benchmark.UnimplementedJobServer - - operator service.Operator + service.Operator } func New(opts ...Option) (bm Benchmark, err error) { @@ -48,5 +47,5 @@ func New(opts ...Option) (bm Benchmark, err error) { return b, nil } -func (s *server) Start(ctx context.Context) { +func (s *server) Start(_ context.Context) { } diff --git a/pkg/tools/benchmark/operator/handler/rest/handler.go b/pkg/tools/benchmark/operator/handler/rest/handler.go index e1e68671929..fea77e7e7c7 100644 --- a/pkg/tools/benchmark/operator/handler/rest/handler.go +++ b/pkg/tools/benchmark/operator/handler/rest/handler.go @@ -24,7 +24,7 @@ import ( type Handler interface{} type handler struct { - js benchmark.JobServer + benchmark.JobServer } func New(opts ...Option) Handler { diff --git a/pkg/tools/benchmark/operator/service/operator.go b/pkg/tools/benchmark/operator/service/operator.go index 867541fe213..13ee6ac4986 100644 --- a/pkg/tools/benchmark/operator/service/operator.go +++ b/pkg/tools/benchmark/operator/service/operator.go @@ -191,7 +191,8 @@ func (o *operator) jobReconcile(ctx context.Context, jobList map[string][]job.Jo for _, jobs := range jobList { cnt := len(jobs) var name string - for _, job := range jobs { + for idx := range jobs { + job := jobs[idx] if job.GetNamespace() != o.jobNamespace { continue } @@ -200,14 +201,13 @@ func (o *operator) jobReconcile(ctx context.Context, jobList map[string][]job.Jo cjobs[job.GetName()] = job.Namespace benchmarkJobStatus[job.GetName()] = v1.BenchmarkJobAvailable continue - } else { - name = job.GetName() } + name = job.GetName() if job.Status.Active == 0 && job.Status.Succeeded != 0 { cnt-- } } - if cnt == 0 && len(name) != 0 { + if cnt == 0 && name != "" { benchmarkJobStatus[name] = v1.BenchmarkJobCompleted } } @@ -263,7 +263,7 @@ func (o *operator) benchJobReconcile(ctx context.Context, benchJobList map[strin if oldJob.GetGeneration() != job.GetGeneration() { if job.Status != "" && oldJob.Status != v1.BenchmarkJobCompleted { // delete old version job - err := o.deleteJob(ctx, oldJob.GetName(), oldJob.GetGeneration()) + err := o.deleteJob(ctx, oldJob.GetName()) if err != nil { log.Warnf("[reconcile benchmark job resource] failed to delete old version job: job name=%s, version=%d\t%s", oldJob.GetName(), oldJob.GetGeneration(), err.Error()) } @@ -359,11 +359,9 @@ func (o *operator) benchScenarioReconcile(ctx context.Context, scenarioList map[ return s }(), } - } else { + } else if oldScenario.Crd.Status != sc.Status { // only update status - if oldScenario.Crd.Status != sc.Status { - cbsl[name].Crd.Status = sc.Status - } + cbsl[name].Crd.Status = sc.Status } } } @@ -393,7 +391,7 @@ func (o *operator) deleteBenchmarkJob(ctx context.Context, name string, generati } // deleteJob deletes job resource according to given benchmark job name and generation. -func (o *operator) deleteJob(ctx context.Context, name string, generation int64) error { +func (o *operator) deleteJob(ctx context.Context, name string) error { cj := new(job.Job) err := o.ctrl.GetManager().GetClient().Get(ctx, client.ObjectKey{ Namespace: o.jobNamespace, @@ -639,7 +637,7 @@ func (o *operator) checkAtomics() error { return nil } -func (o *operator) PreStart(ctx context.Context) error { +func (o *operator) PreStart(_ context.Context) error { log.Infof("[benchmark scenario operator] start vald benchmark scenario operator") return nil } diff --git a/pkg/tools/benchmark/operator/service/operator_test.go b/pkg/tools/benchmark/operator/service/operator_test.go index f7b49866d46..743e1984f54 100644 --- a/pkg/tools/benchmark/operator/service/operator_test.go +++ b/pkg/tools/benchmark/operator/service/operator_test.go @@ -3039,7 +3039,7 @@ func Test_operator_checkAtomics(t *testing.T) { val = *v benchJobMap[k] = &val } - ors := []metav1.OwnerReference{} + var ors []metav1.OwnerReference for _, v := range benchJobMap["scenario-search"].OwnerReferences { or := v.DeepCopy() or.Name = "incorrectName" diff --git a/pkg/tools/benchmark/operator/service/option.go b/pkg/tools/benchmark/operator/service/option.go index 6a4629ee6c5..4fd7203fa0e 100644 --- a/pkg/tools/benchmark/operator/service/option.go +++ b/pkg/tools/benchmark/operator/service/option.go @@ -60,7 +60,7 @@ func WithReconcileCheckDuration(ts string) Option { // WithJobNamespace sets the namespace for running benchmark job. func WithJobNamespace(ns string) Option { return func(o *operator) error { - if len(ns) == 0 { + if ns == "" { o.jobNamespace = "default" } else { o.jobNamespace = ns @@ -72,7 +72,7 @@ func WithJobNamespace(ns string) Option { // WithJobImage sets the benchmark job docker image info. func WithJobImage(image string) Option { return func(o *operator) error { - if len(image) > 0 { + if image != "" { o.jobImage = image } return nil @@ -82,7 +82,7 @@ func WithJobImage(image string) Option { // WithJobImagePullPolicy sets the benchmark job docker image pullPolicy. func WithJobImagePullPolicy(p string) Option { return func(o *operator) error { - if len(p) > 0 { + if p != "" { o.jobImagePullPolicy = p } return nil diff --git a/pkg/tools/benchmark/operator/usecase/benchmarkd.go b/pkg/tools/benchmark/operator/usecase/benchmarkd.go index 918dc8ea9eb..f6c4949ea4c 100644 --- a/pkg/tools/benchmark/operator/usecase/benchmarkd.go +++ b/pkg/tools/benchmark/operator/usecase/benchmarkd.go @@ -189,7 +189,7 @@ func (r *run) Start(ctx context.Context) (<-chan error, error) { return ech, nil } -func (r *run) PreStop(ctx context.Context) error { +func (r *run) PreStop(_ context.Context) error { return nil } @@ -200,6 +200,6 @@ func (r *run) Stop(ctx context.Context) error { return r.server.Shutdown(ctx) } -func (r *run) PostStop(ctx context.Context) error { +func (r *run) PostStop(_ context.Context) error { return nil }