From b4c2a8501a7d97e1c9362b6ce60ab2817e030724 Mon Sep 17 00:00:00 2001 From: xuzhonghu Date: Mon, 13 May 2019 11:03:20 +0800 Subject: [PATCH] fix comments --- pkg/controllers/job/job_controller_actions.go | 12 ++++-- pkg/controllers/job/state/aborted.go | 6 +-- pkg/controllers/job/state/aborting.go | 10 ++--- pkg/controllers/job/state/completing.go | 8 ++-- pkg/controllers/job/state/factory.go | 2 +- pkg/controllers/job/state/inqueue.go | 25 +++++------ pkg/controllers/job/state/pending.go | 22 +++++----- pkg/controllers/job/state/restarting.go | 16 +++---- pkg/controllers/job/state/running.go | 43 ++++++++----------- pkg/controllers/job/state/terminating.go | 8 ++-- 10 files changed, 67 insertions(+), 85 deletions(-) diff --git a/pkg/controllers/job/job_controller_actions.go b/pkg/controllers/job/job_controller_actions.go index 129524bc46..94017c11b6 100644 --- a/pkg/controllers/job/job_controller_actions.go +++ b/pkg/controllers/job/job_controller_actions.go @@ -104,7 +104,9 @@ func (cc *Controller) killJob(jobInfo *apis.JobInfo, updateStatus state.UpdateSt } if updateStatus != nil { - updateStatus(&job.Status) + if updateStatus(&job.Status) { + job.Status.State.LastTransitionTime = metav1.Now() + } } // Update Job status @@ -165,7 +167,9 @@ func (cc *Controller) createJob(jobInfo *apis.JobInfo, updateStatus state.Update } if updateStatus != nil { - updateStatus(&job.Status) + if updateStatus(&job.Status) { + job.Status.State.LastTransitionTime = metav1.Now() + } } if job, err := cc.vkClients.BatchV1alpha1().Jobs(job.Namespace).UpdateStatus(job); err != nil { @@ -316,7 +320,9 @@ func (cc *Controller) syncJob(jobInfo *apis.JobInfo, updateStatus state.UpdateSt } if updateStatus != nil { - updateStatus(&job.Status) + if updateStatus(&job.Status) { + job.Status.State.LastTransitionTime = metav1.Now() + } } if job, err := cc.vkClients.BatchV1alpha1().Jobs(job.Namespace).UpdateStatus(job); err != nil { diff --git a/pkg/controllers/job/state/aborted.go b/pkg/controllers/job/state/aborted.go index 9952bff23c..953ee392d5 100644 --- a/pkg/controllers/job/state/aborted.go +++ b/pkg/controllers/job/state/aborted.go @@ -17,8 +17,6 @@ limitations under the License. package state import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - vkv1 "volcano.sh/volcano/pkg/apis/batch/v1alpha1" "volcano.sh/volcano/pkg/controllers/apis" ) @@ -30,10 +28,10 @@ type abortedState struct { func (as *abortedState) Execute(action vkv1.Action) error { switch action { case vkv1.ResumeJobAction: - return KillJob(as.job, func(status *vkv1.JobStatus) { + return KillJob(as.job, func(status *vkv1.JobStatus) bool { status.State.Phase = vkv1.Restarting - status.State.LastTransitionTime = metav1.Now() status.RetryCount++ + return true }) default: return KillJob(as.job, nil) diff --git a/pkg/controllers/job/state/aborting.go b/pkg/controllers/job/state/aborting.go index 338a86533f..8d123b6ada 100644 --- a/pkg/controllers/job/state/aborting.go +++ b/pkg/controllers/job/state/aborting.go @@ -31,19 +31,19 @@ func (ps *abortingState) Execute(action vkv1.Action) error { switch action { case vkv1.ResumeJobAction: // Already in Restarting phase, just sync it - return KillJob(ps.job, func(status *vkv1.JobStatus) { - status.State.Phase = vkv1.Restarting - status.State.LastTransitionTime = metav1.Now() + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { status.RetryCount++ + return false }) default: - return KillJob(ps.job, func(status *vkv1.JobStatus) { + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { // If any "alive" pods, still in Aborting phase if status.Terminating != 0 || status.Pending != 0 || status.Running != 0 { - status.State.Phase = vkv1.Aborting + return false } else { status.State.Phase = vkv1.Aborted status.State.LastTransitionTime = metav1.Now() + return true } }) } diff --git a/pkg/controllers/job/state/completing.go b/pkg/controllers/job/state/completing.go index 27ce1a6f23..cb9f7074bd 100644 --- a/pkg/controllers/job/state/completing.go +++ b/pkg/controllers/job/state/completing.go @@ -17,8 +17,6 @@ limitations under the License. package state import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - vkv1 "volcano.sh/volcano/pkg/apis/batch/v1alpha1" "volcano.sh/volcano/pkg/controllers/apis" ) @@ -28,13 +26,13 @@ type completingState struct { } func (ps *completingState) Execute(action vkv1.Action) error { - return KillJob(ps.job, func(status *vkv1.JobStatus) { + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { // If any "alive" pods, still in Completing phase if status.Terminating != 0 || status.Pending != 0 || status.Running != 0 { - status.State.Phase = vkv1.Completing + return false } else { status.State.Phase = vkv1.Completed - status.State.LastTransitionTime = metav1.Now() + return true } }) } diff --git a/pkg/controllers/job/state/factory.go b/pkg/controllers/job/state/factory.go index a24f605543..f33cb753f7 100644 --- a/pkg/controllers/job/state/factory.go +++ b/pkg/controllers/job/state/factory.go @@ -21,7 +21,7 @@ import ( "volcano.sh/volcano/pkg/controllers/apis" ) -type UpdateStatusFn func(status *vkv1.JobStatus) +type UpdateStatusFn func(status *vkv1.JobStatus) (jobPhaseChanged bool) type ActionFn func(job *apis.JobInfo, fn UpdateStatusFn) error var ( diff --git a/pkg/controllers/job/state/inqueue.go b/pkg/controllers/job/state/inqueue.go index a7a917e2c9..44cbfa514f 100644 --- a/pkg/controllers/job/state/inqueue.go +++ b/pkg/controllers/job/state/inqueue.go @@ -17,8 +17,6 @@ limitations under the License. package state import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - vkv1 "volcano.sh/volcano/pkg/apis/batch/v1alpha1" "volcano.sh/volcano/pkg/controllers/apis" ) @@ -30,44 +28,41 @@ type inqueueState struct { func (ps *inqueueState) Execute(action vkv1.Action) error { switch action { case vkv1.RestartJobAction: - return KillJob(ps.job, func(status *vkv1.JobStatus) { + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { phase := vkv1.Pending if status.Terminating != 0 { phase = vkv1.Restarting status.RetryCount++ } - status.State.LastTransitionTime = metav1.Now() status.State.Phase = phase + return true }) case vkv1.AbortJobAction: - return KillJob(ps.job, func(status *vkv1.JobStatus) { + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { phase := vkv1.Pending if status.Terminating != 0 { phase = vkv1.Aborting } - status.State.LastTransitionTime = metav1.Now() status.State.Phase = phase + return true }) case vkv1.CompleteJobAction: - return KillJob(ps.job, func(status *vkv1.JobStatus) { + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { phase := vkv1.Completed if status.Terminating != 0 { phase = vkv1.Completing } - status.State.LastTransitionTime = metav1.Now() status.State.Phase = phase + return true }) default: - return SyncJob(ps.job, func(status *vkv1.JobStatus) { - phase := vkv1.Inqueue - + return SyncJob(ps.job, func(status *vkv1.JobStatus) bool { if ps.job.Job.Spec.MinAvailable <= status.Running+status.Succeeded+status.Failed { - status.State.LastTransitionTime = metav1.Now() - phase = vkv1.Running + status.State.Phase = vkv1.Running + return true } - - status.State.Phase = phase + return false }) } return nil diff --git a/pkg/controllers/job/state/pending.go b/pkg/controllers/job/state/pending.go index 0ff1a3a3c1..9cead263a5 100644 --- a/pkg/controllers/job/state/pending.go +++ b/pkg/controllers/job/state/pending.go @@ -17,8 +17,6 @@ limitations under the License. package state import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - vkv1 "volcano.sh/volcano/pkg/apis/batch/v1alpha1" "volcano.sh/volcano/pkg/controllers/apis" ) @@ -30,36 +28,36 @@ type pendingState struct { func (ps *pendingState) Execute(action vkv1.Action) error { switch action { case vkv1.RestartJobAction: - return KillJob(ps.job, func(status *vkv1.JobStatus) { + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { phase := vkv1.Pending if status.Terminating != 0 { phase = vkv1.Restarting status.RetryCount++ } status.State.Phase = phase - status.State.LastTransitionTime = metav1.Now() + return true }) case vkv1.AbortJobAction: - return KillJob(ps.job, func(status *vkv1.JobStatus) { + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { phase := vkv1.Pending if status.Terminating != 0 { phase = vkv1.Aborting } status.State.Phase = phase - status.State.LastTransitionTime = metav1.Now() + return true }) case vkv1.CompleteJobAction: - return KillJob(ps.job, func(status *vkv1.JobStatus) { + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { phase := vkv1.Completed if status.Terminating != 0 { phase = vkv1.Completing } status.State.Phase = phase - status.State.LastTransitionTime = metav1.Now() + return true }) case vkv1.EnqueueAction: - return SyncJob(ps.job, func(status *vkv1.JobStatus) { + return SyncJob(ps.job, func(status *vkv1.JobStatus) bool { phase := vkv1.Inqueue if ps.job.Job.Spec.MinAvailable <= status.Running+status.Succeeded+status.Failed { @@ -67,12 +65,12 @@ func (ps *pendingState) Execute(action vkv1.Action) error { } status.State.Phase = phase - status.State.LastTransitionTime = metav1.Now() + return true }) default: - return CreateJob(ps.job, func(status *vkv1.JobStatus) { + return CreateJob(ps.job, func(status *vkv1.JobStatus) bool { status.State.Phase = vkv1.Pending - status.State.LastTransitionTime = metav1.Now() + return true }) } } diff --git a/pkg/controllers/job/state/restarting.go b/pkg/controllers/job/state/restarting.go index 55ffd54019..b11fb460a2 100644 --- a/pkg/controllers/job/state/restarting.go +++ b/pkg/controllers/job/state/restarting.go @@ -17,8 +17,6 @@ limitations under the License. package state import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - vkv1 "volcano.sh/volcano/pkg/apis/batch/v1alpha1" "volcano.sh/volcano/pkg/controllers/apis" ) @@ -28,9 +26,7 @@ type restartingState struct { } func (ps *restartingState) Execute(action vkv1.Action) error { - return KillJob(ps.job, func(status *vkv1.JobStatus) { - phase := vkv1.Restarting - + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { // Get the maximum number of retries. maxRetry := DefaultMaxRetry if ps.job.Job.Spec.MaxRetry != 0 { @@ -39,8 +35,8 @@ func (ps *restartingState) Execute(action vkv1.Action) error { if status.RetryCount >= maxRetry { // Failed is the phase that the job is restarted failed reached the maximum number of retries. - phase = vkv1.Failed - status.State.LastTransitionTime = metav1.Now() + status.State.Phase = vkv1.Failed + return true } else { total := int32(0) for _, task := range ps.job.Job.Spec.Tasks { @@ -48,12 +44,12 @@ func (ps *restartingState) Execute(action vkv1.Action) error { } if total-status.Terminating >= status.MinAvailable { - phase = vkv1.Pending - status.State.LastTransitionTime = metav1.Now() + status.State.Phase = vkv1.Pending + return true } } - status.State.Phase = phase + return false }) } diff --git a/pkg/controllers/job/state/running.go b/pkg/controllers/job/state/running.go index e1c920bcf6..e19fecc151 100644 --- a/pkg/controllers/job/state/running.go +++ b/pkg/controllers/job/state/running.go @@ -17,8 +17,6 @@ limitations under the License. package state import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - vkv1 "volcano.sh/volcano/pkg/apis/batch/v1alpha1" "volcano.sh/volcano/pkg/controllers/apis" ) @@ -30,55 +28,50 @@ type runningState struct { func (ps *runningState) Execute(action vkv1.Action) error { switch action { case vkv1.RestartJobAction: - return KillJob(ps.job, func(status *vkv1.JobStatus) { - phase := vkv1.Running + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { if status.Terminating != 0 { - phase = vkv1.Restarting - status.State.LastTransitionTime = metav1.Now() + status.State.Phase = vkv1.Restarting status.RetryCount++ + return true } - - status.State.Phase = phase + return false }) case vkv1.AbortJobAction: - return KillJob(ps.job, func(status *vkv1.JobStatus) { - phase := vkv1.Running + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { if status.Terminating != 0 { - phase = vkv1.Aborting - status.State.LastTransitionTime = metav1.Now() + status.State.Phase = vkv1.Aborting + return true } - status.State.Phase = phase + return false }) case vkv1.TerminateJobAction: - return KillJob(ps.job, func(status *vkv1.JobStatus) { - phase := vkv1.Running + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { if status.Terminating != 0 { - phase = vkv1.Terminating - status.State.LastTransitionTime = metav1.Now() + status.State.Phase = vkv1.Terminating + return true } - status.State.Phase = phase + return false }) case vkv1.CompleteJobAction: - return KillJob(ps.job, func(status *vkv1.JobStatus) { + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { phase := vkv1.Completed if status.Terminating != 0 { phase = vkv1.Completing } status.State.Phase = phase - status.State.LastTransitionTime = metav1.Now() + return true }) default: - return SyncJob(ps.job, func(status *vkv1.JobStatus) { - phase := vkv1.Running + return SyncJob(ps.job, func(status *vkv1.JobStatus) bool { if status.Succeeded+status.Failed == TotalTasks(ps.job.Job) { - phase = vkv1.Completed - status.State.LastTransitionTime = metav1.Now() + status.State.Phase = vkv1.Completed + return true } - status.State.Phase = phase + return false }) } } diff --git a/pkg/controllers/job/state/terminating.go b/pkg/controllers/job/state/terminating.go index 5372865196..5ceddecb78 100644 --- a/pkg/controllers/job/state/terminating.go +++ b/pkg/controllers/job/state/terminating.go @@ -17,8 +17,6 @@ limitations under the License. package state import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - vkv1 "volcano.sh/volcano/pkg/apis/batch/v1alpha1" "volcano.sh/volcano/pkg/controllers/apis" ) @@ -28,13 +26,13 @@ type terminatingState struct { } func (ps *terminatingState) Execute(action vkv1.Action) error { - return KillJob(ps.job, func(status *vkv1.JobStatus) { + return KillJob(ps.job, func(status *vkv1.JobStatus) bool { // If any "alive" pods, still in Terminating phase if status.Terminating != 0 || status.Pending != 0 || status.Running != 0 { - status.State.Phase = vkv1.Terminating + return false } else { status.State.Phase = vkv1.Terminated - status.State.LastTransitionTime = metav1.Now() + return true } }) }