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
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 25, 2024
1 parent b3707b1 commit e729d79
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 76 deletions.
2 changes: 1 addition & 1 deletion api/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/openstack-k8s-operators/cinder-operator/api
go 1.20

require (
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240325131352-06e67bf6100d
github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b
k8s.io/api v0.28.7
k8s.io/apimachinery v0.28.7
Expand Down
4 changes: 2 additions & 2 deletions api/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/onsi/ginkgo/v2 v2.16.0 h1:7q1w9frJDzninhXxjZd+Y/x54XNjG/UlRLIYPZafsPM=
github.com/onsi/gomega v1.31.1 h1:KYppCUK+bUgAZwHOu7EXVBKyQA6ILvOESHkn/tgoqvo=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b h1:5EzrrjcGziV69MsEgoBwPdsggY56M6jUxGBP9pp+hwo=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b/go.mod h1:DL+Ts0k+fzgZmx0XxWArIeAmdKuTkPa1I5DThdybfmE=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240325131352-06e67bf6100d h1:zGZaCNuUtQqrg86bX/BPNiLMplRx0ERjVuiJGn2SRDE=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240325131352-06e67bf6100d/go.mod h1:DL+Ts0k+fzgZmx0XxWArIeAmdKuTkPa1I5DThdybfmE=
github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b h1:lygG1KiF5d9HpKpGAl5fa8JVlC9j5VFvC4iKvJkJslA=
github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b/go.mod h1:O5Cc9+++JnKewv8VWtTQeH5r2gPLy0lhdECfmjy7mF0=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
Expand Down
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 := instance.Status.Conditions.DeepCopy()

// 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 := instance.Status.Conditions.DeepCopy()

// 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 := instance.Status.Conditions.DeepCopy()

// 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 := instance.Status.Conditions.DeepCopy()

// 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 := instance.Status.Conditions.DeepCopy()

// 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
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/onsi/gomega v1.31.1
github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240313161042-88282483a04f
github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240325131352-06e67bf6100d
github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b
github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240314165949-fec16b14c33b
github.com/openstack-k8s-operators/mariadb-operator/api v0.3.1-0.20240314113200-40cf3e6aa38e
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240313161042-8
github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240313161042-88282483a04f/go.mod h1:qKuzDDDMlAmJn4JWPoUeBEzpAia7J17++hhzR0oPv88=
github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a h1:XcUHh0j65hm8/4orLTH6aRTv3Ah4rGP1rA4yu7G0fR0=
github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a/go.mod h1:8C7VPKXAxiwB5Z4Kwn12VL0guW6onIG0Ayxiio5Vyu0=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b h1:5EzrrjcGziV69MsEgoBwPdsggY56M6jUxGBP9pp+hwo=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b/go.mod h1:DL+Ts0k+fzgZmx0XxWArIeAmdKuTkPa1I5DThdybfmE=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240325131352-06e67bf6100d h1:zGZaCNuUtQqrg86bX/BPNiLMplRx0ERjVuiJGn2SRDE=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240325131352-06e67bf6100d/go.mod h1:DL+Ts0k+fzgZmx0XxWArIeAmdKuTkPa1I5DThdybfmE=
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b h1:FEbadtLx4+ktxf79ZJoKZmfMNsQyqqgL5T9NXWc3i/k=
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b/go.mod h1:ghnFgNIzj4amS897wEto+L+jYzDSg2cJ6y32RNfFGhk=
github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b h1:lygG1KiF5d9HpKpGAl5fa8JVlC9j5VFvC4iKvJkJslA=
Expand Down
12 changes: 12 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,14 @@ func GetPodAffinity(componentName string) *corev1.Affinity {
corev1.LabelHostname,
)
}

// 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)
if savedCond != nil && condition.HasSameState(&cond, savedCond) {
(*conditions)[idx].LastTransitionTime = savedCond.LastTransitionTime
}
}
}

0 comments on commit e729d79

Please sign in to comment.