Skip to content

Commit

Permalink
Fix updates to a condition's LastTransitionTime
Browse files Browse the repository at this point in the history 
Do not update a condition's LastTransitionTime unless its state
actually changes. This ensures the LastTransitionTime isn't always
modified during every reconcilation. PR openstack-k8s-operators#364 reinitializes all
all conditions to Unknown prior to determining their actual state,
and the intent is to avoid modifying the LastTransitionTime unless
reconciliation modifies the condition's state.

Jira: OSPRH-5698
  • Loading branch information
@ASBishop
ASBishop committed Mar 22, 2024
1 parent 599109b commit 2ba8207
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 70 deletions.
29 changes: 15 additions & 14 deletions controllers/cinder_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,28 +149,28 @@ func (r *CinderReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
return ctrl.Result{}, err
}

//
// initialize status
//
isNewInstance := instance.Status.Conditions == nil
if isNewInstance {
instance.Status.Conditions = condition.Conditions{}
}

// Save a copy of the condtions so that we can restore the LastTransitionTime
// when a condition's state doesn't change.
savedConditions := cinder.SaveConditions(instance.Status.Conditions)

// Always patch the instance status when exiting this function so we can persist any changes.
defer func() {
cinder.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
err := helper.PatchInstance(ctx, instance)
if err != nil {
_err = err
return
}
}()

// If we're not deleting this and the service object doesn't have our finalizer, add it.
if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) {
return ctrl.Result{}, nil
}

//
// initialize status
//
isNewInstance := instance.Status.Conditions == nil
if isNewInstance {
instance.Status.Conditions = condition.Conditions{}
}

// Always initialize conditions used later as Status=Unknown
// except ReadyCondition which is False unless proven otherwise
cl := condition.CreateList(
Expand All @@ -194,7 +194,8 @@ func (r *CinderReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
)
instance.Status.Conditions.Init(&cl)

if isNewInstance {
// If we're not deleting this and the service object doesn't have our finalizer, add it.
if (instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer())) || isNewInstance {
// Register overall status immediately to have an early feedback e.g. in the cli
return ctrl.Result{}, nil
}
Expand Down
34 changes: 20 additions & 14 deletions controllers/cinderapi_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,28 +133,28 @@ func (r *CinderAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
return ctrl.Result{}, err
}

//
// initialize status
//
isNewInstance := instance.Status.Conditions == nil
if isNewInstance {
instance.Status.Conditions = condition.Conditions{}
}

// Save a copy of the condtions so that we can restore the LastTransitionTime
// when a condition's state doesn't change.
savedConditions := cinder.SaveConditions(instance.Status.Conditions)

// Always patch the instance status when exiting this function so we can persist any changes.
defer func() {
cinder.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
err := helper.PatchInstance(ctx, instance)
if err != nil {
_err = err
return
}
}()

// If we're not deleting this and the service object doesn't have our finalizer, add it.
if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) {
return ctrl.Result{}, nil
}

//
// initialize status
//
isNewInstance := instance.Status.Conditions == nil
if isNewInstance {
instance.Status.Conditions = condition.Conditions{}
}

// Always initialize conditions used later as Status=Unknown
cl := condition.CreateList(
condition.UnknownCondition(condition.ExposeServiceReadyCondition, condition.InitReason, condition.ExposeServiceReadyInitMessage),
Expand All @@ -169,7 +169,8 @@ func (r *CinderAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
)
instance.Status.Conditions.Init(&cl)

if isNewInstance {
// If we're not deleting this and the service object doesn't have our finalizer, add it.
if (instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer())) || isNewInstance {
// Register overall status immediately to have an early feedback e.g. in the cli
return ctrl.Result{}, nil
}
Expand Down Expand Up @@ -721,6 +722,11 @@ func (r *CinderAPIReconciler) reconcileNormal(ctx context.Context, instance *cin
err.Error()))
return ctrlResult, err
} else if (ctrlResult != ctrl.Result{}) {
instance.Status.Conditions.MarkFalse(
condition.TLSInputReadyCondition,
condition.InitReason,
condition.SeverityInfo,
condition.InputReadyInitMessage)
return ctrlResult, nil
}
configVars[tls.TLSHashName] = env.SetValue(certsHash)
Expand Down
29 changes: 15 additions & 14 deletions controllers/cinderbackup_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,28 +117,28 @@ func (r *CinderBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request
return ctrl.Result{}, err
}

//
// initialize status
//
isNewInstance := instance.Status.Conditions == nil
if isNewInstance {
instance.Status.Conditions = condition.Conditions{}
}

// Save a copy of the condtions so that we can restore the LastTransitionTime
// when a condition's state doesn't change.
savedConditions := cinder.SaveConditions(instance.Status.Conditions)

// Always patch the instance status when exiting this function so we can persist any changes.
defer func() {
cinder.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
err := helper.PatchInstance(ctx, instance)
if err != nil {
_err = err
return
}
}()

// If we're not deleting this and the service object doesn't have our finalizer, add it.
if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) {
return ctrl.Result{}, nil
}

//
// initialize status
//
isNewInstance := instance.Status.Conditions == nil
if isNewInstance {
instance.Status.Conditions = condition.Conditions{}
}

// Always initialize conditions used later as Status=Unknown
cl := condition.CreateList(
condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage),
Expand All @@ -149,7 +149,8 @@ func (r *CinderBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request
)
instance.Status.Conditions.Init(&cl)

if isNewInstance {
// If we're not deleting this and the service object doesn't have our finalizer, add it.
if (instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer())) || isNewInstance {
// Register overall status immediately to have an early feedback e.g. in the cli
return ctrl.Result{}, nil
}
Expand Down
29 changes: 15 additions & 14 deletions controllers/cinderscheduler_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,28 +117,28 @@ func (r *CinderSchedulerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{}, err
}

//
// initialize status
//
isNewInstance := instance.Status.Conditions == nil
if isNewInstance {
instance.Status.Conditions = condition.Conditions{}
}

// Save a copy of the condtions so that we can restore the LastTransitionTime
// when a condition's state doesn't change.
savedConditions := cinder.SaveConditions(instance.Status.Conditions)

// Always patch the instance status when exiting this function so we can persist any changes.
defer func() {
cinder.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
err := helper.PatchInstance(ctx, instance)
if err != nil {
_err = err
return
}
}()

// If we're not deleting this and the service object doesn't have our finalizer, add it.
if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) {
return ctrl.Result{}, nil
}

//
// initialize status
//
isNewInstance := instance.Status.Conditions == nil
if isNewInstance {
instance.Status.Conditions = condition.Conditions{}
}

// Always initialize conditions used later as Status=Unknown
cl := condition.CreateList(
condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage),
Expand All @@ -149,7 +149,8 @@ func (r *CinderSchedulerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
)
instance.Status.Conditions.Init(&cl)

if isNewInstance {
// If we're not deleting this and the service object doesn't have our finalizer, add it.
if (instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer())) || isNewInstance {
// Register overall status immediately to have an early feedback e.g. in the cli
return ctrl.Result{}, nil
}
Expand Down
29 changes: 15 additions & 14 deletions controllers/cindervolume_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,28 +119,28 @@ func (r *CinderVolumeReconciler) Reconcile(ctx context.Context, req ctrl.Request
return ctrl.Result{}, err
}

//
// initialize status
//
isNewInstance := instance.Status.Conditions == nil
if isNewInstance {
instance.Status.Conditions = condition.Conditions{}
}

// Save a copy of the condtions so that we can restore the LastTransitionTime
// when a condition's state doesn't change.
savedConditions := cinder.SaveConditions(instance.Status.Conditions)

// Always patch the instance status when exiting this function so we can persist any changes.
defer func() {
cinder.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
err := helper.PatchInstance(ctx, instance)
if err != nil {
_err = err
return
}
}()

// If we're not deleting this and the service object doesn't have our finalizer, add it.
if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) {
return ctrl.Result{}, nil
}

//
// initialize status
//
isNewInstance := instance.Status.Conditions == nil
if isNewInstance {
instance.Status.Conditions = condition.Conditions{}
}

// Always initialize conditions used later as Status=Unknown
cl := condition.CreateList(
condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage),
Expand All @@ -151,7 +151,8 @@ func (r *CinderVolumeReconciler) Reconcile(ctx context.Context, req ctrl.Request
)
instance.Status.Conditions.Init(&cl)

if isNewInstance {
// If we're not deleting this and the service object doesn't have our finalizer, add it.
if (instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer())) || isNewInstance {
// Register overall status immediately to have an early feedback e.g. in the cli
return ctrl.Result{}, nil
}
Expand Down
23 changes: 23 additions & 0 deletions pkg/cinder/funcs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cinder
import (
common "github.com/openstack-k8s-operators/lib-common/modules/common"
"github.com/openstack-k8s-operators/lib-common/modules/common/affinity"
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"

corev1 "k8s.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -47,3 +48,25 @@ func GetPodAffinity(componentName string) *corev1.Affinity {
corev1.LabelHostname,
)
}

// SaveConditions - Returns a deep copy of the conditions.
func SaveConditions(conditions condition.Conditions) condition.Conditions {
return append(condition.Conditions{}, conditions...)
}

// RestoreLastTransitionTimes - Updates each condition's LastTransitionTime when its state
// matches the one in a list of "saved" conditions.
func RestoreLastTransitionTimes(conditions *condition.Conditions, savedConditions condition.Conditions) {
for idx, cond := range *conditions {
savedCond := savedConditions.Get(cond.Type)
// condition.hasSameState() would be handy but it's a private function
if savedCond != nil &&
savedCond.Type == cond.Type &&
savedCond.Status == cond.Status &&
savedCond.Reason == cond.Reason &&
savedCond.Severity == cond.Severity &&
savedCond.Message == cond.Message {
(*conditions)[idx].LastTransitionTime = savedCond.LastTransitionTime
}
}
}

0 comments on commit 2ba8207

Please sign in to comment.