diff --git a/.golangci.yml b/.golangci.yml index de12a0fab3..500a4fd677 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -21,7 +21,6 @@ linters: - dupl - errcheck - funlen - - gochecknoinits - goconst - gocritic - gocyclo @@ -103,3 +102,10 @@ issues: - golint - deadcode - godot + +linters-settings: + lll: + line-length: 256 + funlen: + lines: 100 + statements: 60 diff --git a/cmd/cli/util/util.go b/cmd/cli/util/util.go index a1058841d9..d0e9b011f1 100644 --- a/cmd/cli/util/util.go +++ b/cmd/cli/util/util.go @@ -30,7 +30,7 @@ func CheckError(cmd *cobra.Command, err error) { // Ignore the root command. for cur := cmd; cur.Parent() != nil; cur = cur.Parent() { - msg = msg + fmt.Sprintf(" %s", cur.Name()) + msg += fmt.Sprintf(" %s", cur.Name()) } fmt.Printf("%s: %v\n", msg, err) diff --git a/example/kubecon-2019-china/scripts/node-info.go b/example/kubecon-2019-china/scripts/node-info.go index 869207605e..a530a5c337 100644 --- a/example/kubecon-2019-china/scripts/node-info.go +++ b/example/kubecon-2019-china/scripts/node-info.go @@ -2,6 +2,7 @@ package main import ( "fmt" + v1 "k8s.io/api/core/v1" "volcano.sh/volcano/pkg/scheduler/api" @@ -89,5 +90,4 @@ func main() { fmt.Printf(" %-20s |", res) } fmt.Print("\n--------------------------------------------------------------------------------------------------------------\n") - } diff --git a/pkg/cli/job/util.go b/pkg/cli/job/util.go index 202bbdaf4f..f1e1630434 100644 --- a/pkg/cli/job/util.go +++ b/pkg/cli/job/util.go @@ -141,5 +141,5 @@ func HumanDuration(d time.Duration) string { } else if hours < 24*365*8 { return fmt.Sprintf("%dy%dd", hours/24/365, (hours/24)%365) } - return fmt.Sprintf("%dy", int(hours/24/365)) + return fmt.Sprintf("%dy", hours/24/365) } diff --git a/pkg/cli/queue/create.go b/pkg/cli/queue/create.go index c2babd2248..b77b5a434f 100644 --- a/pkg/cli/queue/create.go +++ b/pkg/cli/queue/create.go @@ -58,7 +58,7 @@ func CreateQueue() error { Name: createQueueFlags.Name, }, Spec: schedulingv1beta1.QueueSpec{ - Weight: int32(createQueueFlags.Weight), + Weight: createQueueFlags.Weight, }, Status: schedulingv1beta1.QueueStatus{ State: schedulingv1beta1.QueueState(createQueueFlags.State), diff --git a/pkg/cli/util/util.go b/pkg/cli/util/util.go index 15ae87467a..1254ef8661 100644 --- a/pkg/cli/util/util.go +++ b/pkg/cli/util/util.go @@ -170,5 +170,5 @@ func HumanDuration(d time.Duration) string { } else if hours < 24*365*8 { return fmt.Sprintf("%dy%dd", hours/24/365, (hours/24)%365) } - return fmt.Sprintf("%dy", int(hours/24/365)) + return fmt.Sprintf("%dy", hours/24/365) } diff --git a/pkg/controllers/cache/cache_test.go b/pkg/controllers/cache/cache_test.go index 8b240bd28b..517cb185d3 100644 --- a/pkg/controllers/cache/cache_test.go +++ b/pkg/controllers/cache/cache_test.go @@ -469,7 +469,7 @@ func TestJobCache_AddPod(t *testing.T) { if err == nil { job, err := jobCache.Get(fmt.Sprintf("%s/%s", testcase.JobsInCache["job1"].Namespace, testcase.JobsInCache["job1"].Name)) if err != nil { - t.Errorf("Expected Error not to occur while retriving job from cache in case %d", i) + t.Errorf("Expected Error not to occur while retrieving job from cache in case %d", i) } if err == nil { if len(job.Pods) != 1 { diff --git a/pkg/controllers/job/job_controller_actions.go b/pkg/controllers/job/job_controller_actions.go index a4c41803e8..a739038492 100644 --- a/pkg/controllers/job/job_controller_actions.go +++ b/pkg/controllers/job/job_controller_actions.go @@ -88,7 +88,7 @@ func (cc *Controller) killJob(jobInfo *apis.JobInfo, podRetainPhase state.PhaseM job = job.DeepCopy() // Job version is bumped only when job is killed - job.Status.Version = job.Status.Version + 1 + job.Status.Version++ job.Status = batch.JobStatus{ State: job.Status.State, diff --git a/pkg/scheduler/actions/preempt/preempt.go b/pkg/scheduler/actions/preempt/preempt.go index b8ae9bc4c8..04eb89b725 100644 --- a/pkg/scheduler/actions/preempt/preempt.go +++ b/pkg/scheduler/actions/preempt/preempt.go @@ -108,7 +108,7 @@ func (alloc *Action) Execute(ssn *framework.Session) { preemptor := preemptorTasks[preemptorJob.UID].Pop().(*api.TaskInfo) - if preempted, _ := preempt(ssn, stmt, preemptor, func(task *api.TaskInfo) bool { + if preempted := preempt(ssn, stmt, preemptor, func(task *api.TaskInfo) bool { // Ignore non running task. if task.Status != api.Running { return false @@ -155,7 +155,7 @@ func (alloc *Action) Execute(ssn *framework.Session) { preemptor := preemptorTasks[job.UID].Pop().(*api.TaskInfo) stmt := ssn.Statement() - assigned, _ := preempt(ssn, stmt, preemptor, func(task *api.TaskInfo) bool { + assigned := preempt(ssn, stmt, preemptor, func(task *api.TaskInfo) bool { // Ignore non running task. if task.Status != api.Running { return false @@ -185,7 +185,7 @@ func preempt( stmt *framework.Statement, preemptor *api.TaskInfo, filter func(*api.TaskInfo) bool, -) (bool, error) { +) bool { assigned := false allNodes := util.GetNodeList(ssn.Nodes) @@ -258,5 +258,5 @@ func preempt( } } - return assigned, nil + return assigned } diff --git a/pkg/scheduler/actions/reclaim/reclaim.go b/pkg/scheduler/actions/reclaim/reclaim.go index a9680cbfba..af4a342c7b 100644 --- a/pkg/scheduler/actions/reclaim/reclaim.go +++ b/pkg/scheduler/actions/reclaim/reclaim.go @@ -50,7 +50,7 @@ func (ra *Action) Execute(ssn *framework.Session) { klog.V(3).Infof("There are <%d> Jobs and <%d> Queues in total for scheduling.", len(ssn.Jobs), len(ssn.Queues)) - var underRequest []*api.JobInfo + // var underRequest []*api.JobInfo for _, job := range ssn.Jobs { if job.PodGroup.Status.Phase == scheduling.PodGroupPending { continue @@ -64,14 +64,10 @@ func (ra *Action) Execute(ssn *framework.Session) { klog.Errorf("Failed to find Queue <%s> for Job <%s/%s>", job.Queue, job.Namespace, job.Name) continue - } else { - if _, existed := queueMap[queue.UID]; !existed { - klog.V(4).Infof("Added Queue <%s> for Job <%s/%s>", - queue.Name, job.Namespace, job.Name) - - queueMap[queue.UID] = queue - queues.Push(queue) - } + } else if _, existed := queueMap[queue.UID]; !existed { + klog.V(4).Infof("Added Queue <%s> for Job <%s/%s>", queue.Name, job.Namespace, job.Name) + queueMap[queue.UID] = queue + queues.Push(queue) } if len(job.TaskStatusIndex[api.Pending]) != 0 { @@ -79,7 +75,7 @@ func (ra *Action) Execute(ssn *framework.Session) { preemptorsMap[job.Queue] = util.NewPriorityQueue(ssn.JobOrderFn) } preemptorsMap[job.Queue].Push(job) - underRequest = append(underRequest, job) + // underRequest = append(underRequest, job) preemptorTasks[job.UID] = util.NewPriorityQueue(ssn.TaskOrderFn) for _, task := range job.TaskStatusIndex[api.Pending] { preemptorTasks[job.UID].Push(task) @@ -192,7 +188,6 @@ func (ra *Action) Execute(ssn *framework.Session) { } queues.Push(queue) } - } func (ra *Action) UnInitialize() { diff --git a/pkg/scheduler/cache/cache.go b/pkg/scheduler/cache/cache.go index ce719e57d1..bae6e55182 100644 --- a/pkg/scheduler/cache/cache.go +++ b/pkg/scheduler/cache/cache.go @@ -18,7 +18,10 @@ package cache import ( "fmt" - "k8s.io/api/core/v1" + "sync" + "time" + + v1 "k8s.io/api/core/v1" "k8s.io/api/scheduling/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -38,8 +41,6 @@ import ( "k8s.io/klog" podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/scheduler/volumebinder" - "sync" - "time" vcv1beta1 "volcano.sh/volcano/pkg/apis/scheduling/v1beta1" "volcano.sh/volcano/cmd/scheduler/app/options" @@ -395,7 +396,6 @@ func (sc *SchedulerCache) Run(stopCh <-chan struct{}) { // WaitForCacheSync sync the cache with the api server func (sc *SchedulerCache) WaitForCacheSync(stopCh <-chan struct{}) bool { - return cache.WaitForCacheSync(stopCh, func() []cache.InformerSynced { informerSynced := []cache.InformerSynced{ @@ -737,31 +737,31 @@ func (sc *SchedulerCache) String() string { str := "Cache:\n" if len(sc.Nodes) != 0 { - str = str + "Nodes:\n" + str += "Nodes:\n" for _, n := range sc.Nodes { - str = str + fmt.Sprintf("\t %s: idle(%v) used(%v) allocatable(%v) pods(%d)\n", + str += fmt.Sprintf("\t %s: idle(%v) used(%v) allocatable(%v) pods(%d)\n", n.Name, n.Idle, n.Used, n.Allocatable, len(n.Tasks)) i := 0 for _, p := range n.Tasks { - str = str + fmt.Sprintf("\t\t %d: %v\n", i, p) + str += fmt.Sprintf("\t\t %d: %v\n", i, p) i++ } } } if len(sc.Jobs) != 0 { - str = str + "Jobs:\n" + str += "Jobs:\n" for _, job := range sc.Jobs { - str = str + fmt.Sprintf("\t %s\n", job) + str += fmt.Sprintf("\t %s\n", job) } } if len(sc.NamespaceCollection) != 0 { - str = str + "Namespaces:\n" + str += "Namespaces:\n" for _, ns := range sc.NamespaceCollection { info := ns.Snapshot() - str = str + fmt.Sprintf("\t Namespace(%s) Weight(%v)\n", + str += fmt.Sprintf("\t Namespace(%s) Weight(%v)\n", info.Name, info.Weight) } } @@ -831,6 +831,5 @@ func (sc *SchedulerCache) recordPodGroupEvent(podGroup *schedulingapi.PodGroup, klog.Errorf("Error while converting PodGroup to v1alpha1.PodGroup with error: %v", err) return } - sc.Recorder.Eventf(pg, eventType, reason, msg) } diff --git a/pkg/scheduler/cache/event_handlers.go b/pkg/scheduler/cache/event_handlers.go index e4a7aa69d2..59e5a1f474 100644 --- a/pkg/scheduler/cache/event_handlers.go +++ b/pkg/scheduler/cache/event_handlers.go @@ -19,7 +19,7 @@ package cache import ( "fmt" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/api/scheduling/v1beta1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -246,18 +246,16 @@ func (sc *SchedulerCache) DeletePod(obj interface{}) { } // Assumes that lock is already acquired. -func (sc *SchedulerCache) addNode(node *v1.Node) error { +func (sc *SchedulerCache) addNode(node *v1.Node) { if sc.Nodes[node.Name] != nil { sc.Nodes[node.Name].SetNode(node) } else { sc.Nodes[node.Name] = schedulingapi.NewNodeInfo(node) } - - return nil } // Assumes that lock is already acquired. -func (sc *SchedulerCache) updateNode(oldNode, newNode *v1.Node) error { +func (sc *SchedulerCache) updateNode(newNode *v1.Node) error { if sc.Nodes[newNode.Name] != nil { sc.Nodes[newNode.Name].SetNode(newNode) return nil @@ -286,11 +284,7 @@ func (sc *SchedulerCache) AddNode(obj interface{}) { sc.Mutex.Lock() defer sc.Mutex.Unlock() - err := sc.addNode(node) - if err != nil { - klog.Errorf("Failed to add node %s into cache: %v", node.Name, err) - return - } + sc.addNode(node) } // UpdateNode update node to scheduler cache @@ -309,7 +303,7 @@ func (sc *SchedulerCache) UpdateNode(oldObj, newObj interface{}) { sc.Mutex.Lock() defer sc.Mutex.Unlock() - err := sc.updateNode(oldNode, newNode) + err := sc.updateNode(newNode) if err != nil { klog.Errorf("Failed to update node %v in cache: %v", oldNode.Name, err) return @@ -596,9 +590,7 @@ func (sc *SchedulerCache) UpdatePriorityClass(oldObj, newObj interface{}) { newSS, ok := newObj.(*v1beta1.PriorityClass) if !ok { klog.Errorf("Cannot convert newObj to *v1beta1.PriorityClass: %v", newObj) - return - } sc.Mutex.Lock() diff --git a/pkg/scheduler/framework/session_plugins.go b/pkg/scheduler/framework/session_plugins.go index 120aebe7d3..96ba58dadd 100644 --- a/pkg/scheduler/framework/session_plugins.go +++ b/pkg/scheduler/framework/session_plugins.go @@ -460,7 +460,7 @@ func (ssn *Session) NodeOrderFn(task *api.TaskInfo, node *api.NodeInfo) (float64 if err != nil { return 0, err } - priorityScore = priorityScore + score + priorityScore += score } } @@ -509,7 +509,7 @@ func (ssn *Session) NodeOrderMapFn(task *api.TaskInfo, node *api.NodeInfo) (map[ if err != nil { return nodeScoreMap, priorityScore, err } - priorityScore = priorityScore + score + priorityScore += score } if pfn, found := ssn.nodeMapFns[plugin.Name]; found { score, err := pfn(task, node) @@ -540,7 +540,7 @@ func (ssn *Session) NodeOrderReduceFn(task *api.TaskInfo, pluginNodeScoreMap map return nodeScoreMap, err } for _, hp := range pluginNodeScoreMap[plugin.Name] { - nodeScoreMap[hp.Host] = nodeScoreMap[hp.Host] + float64(hp.Score) + nodeScoreMap[hp.Host] += float64(hp.Score) } } } diff --git a/pkg/scheduler/framework/statement.go b/pkg/scheduler/framework/statement.go index fdacf38ff7..192760c36e 100644 --- a/pkg/scheduler/framework/statement.go +++ b/pkg/scheduler/framework/statement.go @@ -73,7 +73,7 @@ func (s *Statement) Evict(reclaimee *api.TaskInfo, reason string) error { func (s *Statement) evict(reclaimee *api.TaskInfo, reason string) error { if err := s.ssn.cache.Evict(reclaimee, reason); err != nil { - if e := s.unevict(reclaimee, reason); e != nil { + if e := s.unevict(reclaimee); e != nil { klog.Errorf("Faled to unevict task <%v/%v>: %v.", reclaimee.Namespace, reclaimee.Name, e) } @@ -83,7 +83,7 @@ func (s *Statement) evict(reclaimee *api.TaskInfo, reason string) error { return nil } -func (s *Statement) unevict(reclaimee *api.TaskInfo, reason string) error { +func (s *Statement) unevict(reclaimee *api.TaskInfo) error { // Update status in session job, found := s.ssn.Jobs[reclaimee.Job] if found { @@ -251,7 +251,7 @@ func (s *Statement) Allocate(task *api.TaskInfo, hostname string) error { return nil } -func (s *Statement) allocate(task *api.TaskInfo, hostname string) error { +func (s *Statement) allocate(task *api.TaskInfo) error { if err := s.ssn.cache.BindVolumes(task); err != nil { return err } @@ -278,7 +278,7 @@ func (s *Statement) allocate(task *api.TaskInfo, hostname string) error { } // unallocate the pod for task -func (s *Statement) unallocate(task *api.TaskInfo, reason string) error { +func (s *Statement) unallocate(task *api.TaskInfo) error { // Update status in session job, found := s.ssn.Jobs[task.Job] if found { @@ -314,11 +314,11 @@ func (s *Statement) Discard() { op := s.operations[i] switch op.name { case "evict": - s.unevict(op.args[0].(*api.TaskInfo), op.args[1].(string)) + s.unevict(op.args[0].(*api.TaskInfo)) case "pipeline": s.unpipeline(op.args[0].(*api.TaskInfo)) case "allocate": - s.unallocate(op.args[0].(*api.TaskInfo), op.args[1].(string)) + s.unallocate(op.args[0].(*api.TaskInfo)) } } } @@ -333,7 +333,7 @@ func (s *Statement) Commit() { case "pipeline": s.pipeline(op.args[0].(*api.TaskInfo)) case "allocate": - s.allocate(op.args[0].(*api.TaskInfo), op.args[1].(string)) + s.allocate(op.args[0].(*api.TaskInfo)) } } } diff --git a/pkg/scheduler/plugins/binpack/binpack.go b/pkg/scheduler/plugins/binpack/binpack.go index f54709c32e..56e3b0a6a7 100644 --- a/pkg/scheduler/plugins/binpack/binpack.go +++ b/pkg/scheduler/plugins/binpack/binpack.go @@ -20,7 +20,7 @@ import ( "fmt" "strings" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/klog" schedulerapi "k8s.io/kubernetes/pkg/scheduler/api" @@ -237,7 +237,7 @@ func BinPackingScore(task *api.TaskInfo, node *api.NodeInfo, weight priorityWeig // mapping the result from [0, weightSum] to [0, 10(MaxPriority)] if weightSum > 0 { - score = score / float64(weightSum) + score /= float64(weightSum) } score *= schedulerapi.MaxPriority * float64(weight.BinPackingWeight) diff --git a/pkg/scheduler/plugins/nodeorder/nodeorder.go b/pkg/scheduler/plugins/nodeorder/nodeorder.go index 52abc6b362..c75c16202b 100644 --- a/pkg/scheduler/plugins/nodeorder/nodeorder.go +++ b/pkg/scheduler/plugins/nodeorder/nodeorder.go @@ -183,7 +183,7 @@ func (pp *nodeOrderPlugin) OnSessionOpen(ssn *framework.Session) { return 0, err } // If leastReqWeight in provided, host.Score is multiplied with weight, if not, host.Score is added to total score. - score = score + float64(host.Score*weight.leastReqWeight) + score += float64(host.Score * weight.leastReqWeight) host, err = priorities.BalancedResourceAllocationMap(task.Pod, nil, nodeInfo) if err != nil { @@ -191,7 +191,7 @@ func (pp *nodeOrderPlugin) OnSessionOpen(ssn *framework.Session) { return 0, err } // If balancedRescourceWeight in provided, host.Score is multiplied with weight, if not, host.Score is added to total score. - score = score + float64(host.Score*weight.balancedRescourceWeight) + score += float64(host.Score * weight.balancedRescourceWeight) host, err = priorities.CalculateNodeAffinityPriorityMap(task.Pod, nil, nodeInfo) if err != nil { @@ -199,7 +199,7 @@ func (pp *nodeOrderPlugin) OnSessionOpen(ssn *framework.Session) { return 0, err } // If nodeAffinityWeight in provided, host.Score is multiplied with weight, if not, host.Score is added to total score. - score = score + float64(host.Score*weight.nodeAffinityWeight) + score += float64(host.Score * weight.nodeAffinityWeight) klog.V(4).Infof("Total Score for task %s/%s on node %s is: %f", task.Namespace, task.Name, node.Name, score) return score, nil diff --git a/pkg/scheduler/util/scheduler_helper.go b/pkg/scheduler/util/scheduler_helper.go index ab1d277f9c..8f600b55bd 100644 --- a/pkg/scheduler/util/scheduler_helper.go +++ b/pkg/scheduler/util/scheduler_helper.go @@ -161,20 +161,20 @@ func PrioritizeNodes(task *api.TaskInfo, nodes []*api.NodeInfo, batchFn api.Batc for _, node := range nodes { if score, found := reduceScores[node.Name]; found { if orderScore, ok := nodeOrderScoreMap[node.Name]; ok { - score = score + orderScore + score += orderScore } if batchScore, ok := batchNodeScore[node.Name]; ok { - score = score + batchScore + score += batchScore } nodeScores[score] = append(nodeScores[score], node) } else { // If no plugin is applied to this node, the default is 0.0 score = 0.0 if orderScore, ok := nodeOrderScoreMap[node.Name]; ok { - score = score + orderScore + score += orderScore } if batchScore, ok := batchNodeScore[node.Name]; ok { - score = score + batchScore + score += batchScore } nodeScores[score] = append(nodeScores[score], node) } diff --git a/test/e2e/command.go b/test/e2e/command.go index eb6cca4177..b95a08e981 100644 --- a/test/e2e/command.go +++ b/test/e2e/command.go @@ -59,7 +59,8 @@ var _ = Describe("Job E2E Test: Test Job Command", func() { Expect(err).NotTo(HaveOccurred()) //Command outputs are identical outputs := ListJobs(ctx.namespace) - jobs, _ := ctx.vcclient.BatchV1alpha1().Jobs(ctx.namespace).List(metav1.ListOptions{}) + jobs, err := ctx.vcclient.BatchV1alpha1().Jobs(ctx.namespace).List(metav1.ListOptions{}) + Expect(err).NotTo(HaveOccurred()) ctlJob.PrintJobs(jobs, &outBuffer) Expect(outputs).To(Equal(outBuffer.String()), "List command result should be:\n %s", outBuffer.String())