From 462bd849e9bdf0d2b1cd4df4614a501491ff9e05 Mon Sep 17 00:00:00 2001 From: Ce Gao Date: Wed, 5 Jun 2019 02:13:53 -0500 Subject: [PATCH] fix: Remove dup code (#1022) * fix: Remove dup code Signed-off-by: Ce Gao * fix: Return if completed Signed-off-by: Ce Gao --- pkg/controller.v1/tensorflow/status.go | 35 ++++++++------------- pkg/controller.v1beta2/tensorflow/status.go | 35 ++++++++------------- 2 files changed, 26 insertions(+), 44 deletions(-) diff --git a/pkg/controller.v1/tensorflow/status.go b/pkg/controller.v1/tensorflow/status.go index bcd793840f..972c1775c1 100644 --- a/pkg/controller.v1/tensorflow/status.go +++ b/pkg/controller.v1/tensorflow/status.go @@ -178,17 +178,6 @@ func (tc *TFController) updateTFJobStatus(tfjob *tfv1.TFJob) error { // updateTFJobConditions updates the conditions of the given tfjob. func updateTFJobConditions(tfjob *tfv1.TFJob, conditionType common.JobConditionType, reason, message string) error { - // Check if the condition exists in the conditions. - // See https://github.com/kubeflow/pytorch-operator/issues/88 - for i, c := range tfjob.Status.Conditions { - // Found the condition, thus no need to update the LastTransitionTime. - if c.Type == conditionType && c.Status == v1.ConditionTrue { - tfjob.Status.Conditions[i].LastUpdateTime = metav1.Now() - tfjob.Status.Conditions[i].Reason = reason - tfjob.Status.Conditions[i].Message = message - return nil - } - } condition := newCondition(conditionType, reason, message) setCondition(&tfjob.Status, condition) return nil @@ -260,24 +249,26 @@ func isFailed(status common.JobStatus) bool { // If the condition that we are about to add already exists // and has the same status and reason then we are not going to update. func setCondition(status *common.JobStatus, condition common.JobCondition) { - // Do nothing if TFJobStatus have failed condition - if isFailed(*status) { + // Do nothing if TFJobStatus is completed. + if isFailed(*status) || isSucceeded(*status) { return } currentCond := getCondition(*status, condition.Type) - // Do nothing if condition doesn't change - if currentCond != nil && currentCond.Status == condition.Status && currentCond.Reason == condition.Reason { - return - } - - // Do not update lastTransitionTime if the status of the condition doesn't change. - if currentCond != nil && currentCond.Status == condition.Status { - condition.LastTransitionTime = currentCond.LastTransitionTime + if currentCond != nil { + if currentCond.Status == condition.Status && + currentCond.Reason == condition.Reason && + currentCond.Message == condition.Message { + // Do nothing if the condition does not change. + return + } else if currentCond.Status == condition.Status { + // Do not update lastTransitionTime if the status of the condition doesn't change. + condition.LastTransitionTime = currentCond.LastTransitionTime + } } - // Append the updated condition to the + // Append the updated condition to the status.Conditions. newConditions := filterOutCondition(status.Conditions, condition.Type) status.Conditions = append(newConditions, condition) } diff --git a/pkg/controller.v1beta2/tensorflow/status.go b/pkg/controller.v1beta2/tensorflow/status.go index 18763f94f9..dd7d1ace43 100644 --- a/pkg/controller.v1beta2/tensorflow/status.go +++ b/pkg/controller.v1beta2/tensorflow/status.go @@ -156,17 +156,6 @@ func (tc *TFController) updateTFJobStatus(tfjob *tfv1beta2.TFJob) error { // updateTFJobConditions updates the conditions of the given tfjob. func updateTFJobConditions(tfjob *tfv1beta2.TFJob, conditionType common.JobConditionType, reason, message string) error { - // Check if the condition exists in the conditions. - // See https://github.com/kubeflow/pytorch-operator/issues/88 - for i, c := range tfjob.Status.Conditions { - // Found the condition, thus no need to update the LastTransitionTime. - if c.Type == conditionType && c.Status == v1.ConditionTrue { - tfjob.Status.Conditions[i].LastUpdateTime = metav1.Now() - tfjob.Status.Conditions[i].Reason = reason - tfjob.Status.Conditions[i].Message = message - return nil - } - } condition := newCondition(conditionType, reason, message) setCondition(&tfjob.Status, condition) return nil @@ -238,24 +227,26 @@ func isFailed(status common.JobStatus) bool { // If the condition that we are about to add already exists // and has the same status and reason then we are not going to update. func setCondition(status *common.JobStatus, condition common.JobCondition) { - // Do nothing if TFJobStatus have failed condition - if isFailed(*status) { + // Do nothing if TFJobStatus is completed. + if isFailed(*status) || isSucceeded(*status) { return } currentCond := getCondition(*status, condition.Type) - // Do nothing if condition doesn't change - if currentCond != nil && currentCond.Status == condition.Status && currentCond.Reason == condition.Reason { - return - } - - // Do not update lastTransitionTime if the status of the condition doesn't change. - if currentCond != nil && currentCond.Status == condition.Status { - condition.LastTransitionTime = currentCond.LastTransitionTime + if currentCond != nil { + if currentCond.Status == condition.Status && + currentCond.Reason == condition.Reason && + currentCond.Message == condition.Message { + // Do nothing if the condition does not change. + return + } else if currentCond.Status == condition.Status { + // Do not update lastTransitionTime if the status of the condition doesn't change. + condition.LastTransitionTime = currentCond.LastTransitionTime + } } - // Append the updated condition to the + // Append the updated condition to the status.Conditions. newConditions := filterOutCondition(status.Conditions, condition.Type) status.Conditions = append(newConditions, condition) }