Skip to content

Commit

Permalink
♻️ Refactor from deepsource feedback (#2222)
Browse files Browse the repository at this point in the history
Signed-off-by: vankichi <[email protected]>
  • Loading branch information
vankichi authored Oct 30, 2023
1 parent 036ec9a commit 3ef37a4
Show file tree
Hide file tree
Showing 15 changed files with 39 additions and 55 deletions.
1 change: 0 additions & 1 deletion internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1434,7 +1434,6 @@ func TestToRawYaml(t *testing.T) {

func TestMerge(t *testing.T) {
type config struct {
// Config GlobalConfig
Discoverer *Discoverer
}

Expand Down
2 changes: 1 addition & 1 deletion internal/k8s/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
13 changes: 7 additions & 6 deletions internal/k8s/job/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand All @@ -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(), "-")
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
10 changes: 2 additions & 8 deletions internal/k8s/vald/benchmark/api/v1/job_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
}
10 changes: 2 additions & 8 deletions internal/k8s/vald/benchmark/api/v1/scenario_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions internal/k8s/vald/benchmark/job/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
4 changes: 2 additions & 2 deletions internal/k8s/vald/benchmark/scenario/scenario.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions internal/sync/errgroup/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down
2 changes: 1 addition & 1 deletion pkg/tools/benchmark/job/handler/rest/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
type Handler interface{}

type handler struct {
js benchmark.JobServer
benchmark.JobServer
}

func New(opts ...Option) Handler {
Expand Down
5 changes: 2 additions & 3 deletions pkg/tools/benchmark/operator/handler/grpc/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
}
2 changes: 1 addition & 1 deletion pkg/tools/benchmark/operator/handler/rest/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
type Handler interface{}

type handler struct {
js benchmark.JobServer
benchmark.JobServer
}

func New(opts ...Option) Handler {
Expand Down
20 changes: 9 additions & 11 deletions pkg/tools/benchmark/operator/service/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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())
}

Check warning on line 269 in pkg/tools/benchmark/operator/service/operator.go

View check run for this annotation

Codecov / codecov/patch

pkg/tools/benchmark/operator/service/operator.go#L268-L269

Added lines #L268 - L269 were not covered by tests
Expand Down Expand Up @@ -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
}
}
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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

Check warning on line 642 in pkg/tools/benchmark/operator/service/operator.go

View check run for this annotation

Codecov / codecov/patch

pkg/tools/benchmark/operator/service/operator.go#L640-L642

Added lines #L640 - L642 were not covered by tests
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/tools/benchmark/operator/service/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 3 additions & 3 deletions pkg/tools/benchmark/operator/service/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/tools/benchmark/operator/usecase/benchmarkd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}

0 comments on commit 3ef37a4

Please sign in to comment.