Skip to content

Commit

Permalink
add the unparam linter to golangci and fix those issues flagged
Browse files Browse the repository at this point in the history
  • Loading branch information
kannon92 committed Aug 9, 2024
1 parent a96000a commit f1fc56f
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 20 deletions.
1 change: 1 addition & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ linters-settings:
linters:
enable:
- goimports
- unparam
19 changes: 6 additions & 13 deletions pkg/controllers/jobset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -489,11 +489,7 @@ func (r *JobSetReconciler) reconcileReplicatedJobs(ctx context.Context, js *jobs
startupPolicy := js.Spec.StartupPolicy

for _, replicatedJob := range js.Spec.ReplicatedJobs {
jobs, err := constructJobsFromTemplate(js, &replicatedJob, ownedJobs)
if err != nil {
return err
}

jobs := constructJobsFromTemplate(js, &replicatedJob, ownedJobs)
status := findReplicatedJobStatus(replicatedJobStatus, replicatedJob.Name)

// For startup policy, if the replicatedJob is started we can skip this loop.
Expand Down Expand Up @@ -639,23 +635,20 @@ func executeSuccessPolicy(js *jobset.JobSet, ownedJobs *childJobs, updateStatusO
return false
}

func constructJobsFromTemplate(js *jobset.JobSet, rjob *jobset.ReplicatedJob, ownedJobs *childJobs) ([]*batchv1.Job, error) {
func constructJobsFromTemplate(js *jobset.JobSet, rjob *jobset.ReplicatedJob, ownedJobs *childJobs) []*batchv1.Job {
var jobs []*batchv1.Job
for jobIdx := 0; jobIdx < int(rjob.Replicas); jobIdx++ {
jobName := placement.GenJobName(js.Name, rjob.Name, jobIdx)
if create := shouldCreateJob(jobName, ownedJobs); !create {
continue
}
job, err := constructJob(js, rjob, jobIdx)
if err != nil {
return nil, err
}
job := constructJob(js, rjob, jobIdx)
jobs = append(jobs, job)
}
return jobs, nil
return jobs
}

func constructJob(js *jobset.JobSet, rjob *jobset.ReplicatedJob, jobIdx int) (*batchv1.Job, error) {
func constructJob(js *jobset.JobSet, rjob *jobset.ReplicatedJob, jobIdx int) *batchv1.Job {
job := &batchv1.Job{
ObjectMeta: metav1.ObjectMeta{
Labels: collections.CloneMap(rjob.Template.Labels),
Expand Down Expand Up @@ -689,7 +682,7 @@ func constructJob(js *jobset.JobSet, rjob *jobset.ReplicatedJob, jobIdx int) (*b
jobsetSuspended := jobSetSuspended(js)
job.Spec.Suspend = ptr.To(jobsetSuspended)

return job, nil
return job
}

func addTaintToleration(job *batchv1.Job) {
Expand Down
6 changes: 1 addition & 5 deletions pkg/controllers/jobset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,11 +688,7 @@ func TestConstructJobsFromTemplate(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
var got []*batchv1.Job
for _, rjob := range tc.js.Spec.ReplicatedJobs {
jobs, err := constructJobsFromTemplate(tc.js, &rjob, tc.ownedJobs)
if err != nil {
t.Errorf("constructJobsFromTemplate() error = %v", err)
return
}
jobs := constructJobsFromTemplate(tc.js, &rjob, tc.ownedJobs)
got = append(got, jobs...)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/webhooks/jobset_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func replicatedJobNamesFromSpec(js *jobset.JobSet) []string {
// 3. coordinator podIndex is a valid pod index for the job.
func validateCoordinator(js *jobset.JobSet) error {
// Validate replicatedJob.
replicatedJob := replicatedJobByName(js, js.Spec.Coordinator.ReplicatedJob)
replicatedJob := replicatedJobByName(js)
if replicatedJob == nil {
return fmt.Errorf("coordinator replicatedJob %s does not exist", js.Spec.Coordinator.ReplicatedJob)
}
Expand All @@ -382,7 +382,7 @@ func validateCoordinator(js *jobset.JobSet) error {

// replicatedJobByName fetches the replicatedJob spec from the JobSet by name.
// Returns nil if no replicatedJob with the given name exists.
func replicatedJobByName(js *jobset.JobSet, replicatedJob string) *jobset.ReplicatedJob {
func replicatedJobByName(js *jobset.JobSet) *jobset.ReplicatedJob {
for _, rjob := range js.Spec.ReplicatedJobs {
if rjob.Name == js.Spec.Coordinator.ReplicatedJob {
return &rjob
Expand Down

0 comments on commit f1fc56f

Please sign in to comment.