Skip to content

Commit 817980d

Browse files
fmountfmount
committed
Fix updates to a condition's LastTransitionTime
Do not update a condition's LastTransitionTime unless its state actually changes. This ensures the LastTransitionTime isn't always modified during every reconcilation. In general, when a reconciliation starts, it keeps track of the previous conditions, and at the end of the loop the conditions array is updated. However, if a condition didn't change during the last reconciliation (condition.HasSameState() from lib-common helps on that front), we shouldn't update the LastTransitionTime, and we can keep a consistent view of the conditions at each reconcile. Signed-off-by: Francesco Pantano <[email protected]>
1 parent f8ed170 commit 817980d

File tree

6 files changed

+38
-34
lines changed

6 files changed

+38
-34
lines changed

‎api/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ go 1.20
55
require (
66
github.com/gophercloud/gophercloud v1.11.0
77
github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a
8-
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b
8+
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6
99
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b
1010
github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b
1111
k8s.io/api v0.28.7

‎api/go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 h1:rncLxJBpFGqBztyxC
6969
github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4=
7070
github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a h1:XcUHh0j65hm8/4orLTH6aRTv3Ah4rGP1rA4yu7G0fR0=
7171
github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a/go.mod h1:8C7VPKXAxiwB5Z4Kwn12VL0guW6onIG0Ayxiio5Vyu0=
72-
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b h1:5EzrrjcGziV69MsEgoBwPdsggY56M6jUxGBP9pp+hwo=
73-
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b/go.mod h1:DL+Ts0k+fzgZmx0XxWArIeAmdKuTkPa1I5DThdybfmE=
72+
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6 h1:4Z7LjnjEF82XiusXJTI/4TqgwnJas3cdvg/qEgkrW8Q=
73+
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6/go.mod h1:DL+Ts0k+fzgZmx0XxWArIeAmdKuTkPa1I5DThdybfmE=
7474
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b h1:FEbadtLx4+ktxf79ZJoKZmfMNsQyqqgL5T9NXWc3i/k=
7575
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b/go.mod h1:ghnFgNIzj4amS897wEto+L+jYzDSg2cJ6y32RNfFGhk=
7676
github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b h1:lygG1KiF5d9HpKpGAl5fa8JVlC9j5VFvC4iKvJkJslA=

‎controllers/glance_controller.go‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,17 @@ func (r *GlanceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
123123
return ctrl.Result{}, err
124124
}
125125

126+
// initialize status if Conditions is nil, but do not reset if it already
127+
// exists
128+
isNewInstance := instance.Status.Conditions == nil
129+
if isNewInstance {
130+
instance.Status.Conditions = condition.Conditions{}
131+
}
132+
133+
// Save a copy of the condtions so that we can restore the LastTransitionTime
134+
// when a condition's state doesn't change.
135+
savedConditions := instance.Status.Conditions.DeepCopy()
136+
126137
// Always patch the instance status when exiting this function so we can persist any changes.
127138
defer func() {
128139
// update the Ready condition based on the sub conditions
@@ -137,25 +148,14 @@ func (r *GlanceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
137148
instance.Status.Conditions.Set(
138149
instance.Status.Conditions.Mirror(condition.ReadyCondition))
139150
}
151+
condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
140152
err := helper.PatchInstance(ctx, instance)
141153
if err != nil {
142154
_err = err
143155
return
144156
}
145157
}()
146158

147-
// If we're not deleting this and the service object doesn't have our finalizer, add it.
148-
if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) {
149-
return ctrl.Result{}, nil
150-
}
151-
152-
// initialize status if Conditions is nil, but do not reset if it already
153-
// exists
154-
isNewInstance := instance.Status.Conditions == nil
155-
if isNewInstance {
156-
instance.Status.Conditions = condition.Conditions{}
157-
}
158-
159159
// initialize conditions used later as Status=Unknown, except the ReadyCondition
160160
// that should be False when we start
161161
cl := condition.CreateList(
@@ -176,7 +176,8 @@ func (r *GlanceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
176176
)
177177
instance.Status.Conditions.Init(&cl)
178178

179-
if isNewInstance {
179+
// If we're not deleting this and the service object doesn't have our finalizer, add it.
180+
if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) || isNewInstance {
180181
// Register overall status immediately to have an early feedback e.g. in the cli
181182
return ctrl.Result{}, nil
182183
}

‎controllers/glanceapi_controller.go‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,20 @@ func (r *GlanceAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
118118
return ctrl.Result{}, err
119119
}
120120

121+
//
122+
// initialize status
123+
//
124+
// initialize status if Conditions is nil, but do not reset if it
125+
// already exists
126+
isNewInstance := instance.Status.Conditions == nil
127+
if isNewInstance {
128+
instance.Status.Conditions = condition.Conditions{}
129+
}
130+
131+
// Save a copy of the condtions so that we can restore the LastTransitionTime
132+
// when a condition's state doesn't change.
133+
savedConditions := instance.Status.Conditions.DeepCopy()
134+
121135
// Always patch the instance status when exiting this function so we can persist any changes.
122136
defer func() {
123137
// update the Ready condition based on the sub conditions
@@ -132,27 +146,14 @@ func (r *GlanceAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
132146
instance.Status.Conditions.Set(
133147
instance.Status.Conditions.Mirror(condition.ReadyCondition))
134148
}
149+
condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
135150
err := helper.PatchInstance(ctx, instance)
136151
if err != nil {
137152
_err = err
138153
return
139154
}
140155
}()
141156

142-
// If we're not deleting this and the service object doesn't have our finalizer, add it.
143-
if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) {
144-
return ctrl.Result{}, nil
145-
}
146-
147-
//
148-
// initialize status
149-
//
150-
// initialize status if Conditions is nil, but do not reset if it
151-
// already exists
152-
isNewInstance := instance.Status.Conditions == nil
153-
if isNewInstance {
154-
instance.Status.Conditions = condition.Conditions{}
155-
}
156157
// initialize conditions used later as Status=Unknown
157158
cl := condition.CreateList(
158159
condition.UnknownCondition(glancev1.CinderCondition, condition.InitReason, glancev1.CinderInitMessage),
@@ -168,7 +169,9 @@ func (r *GlanceAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
168169
)
169170

170171
instance.Status.Conditions.Init(&cl)
171-
if isNewInstance {
172+
173+
// If we're not deleting this and the service object doesn't have our finalizer, add it.
174+
if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) || isNewInstance {
172175
// Register overall status immediately to have an early feedback e.g. in the cli
173176
return ctrl.Result{}, nil
174177
}

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ require (
1212
github.com/openstack-k8s-operators/glance-operator/api v0.0.0-00010101000000-000000000000
1313
github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240313161042-88282483a04f
1414
github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a
15-
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b
15+
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6
1616
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b
1717
github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b
1818
github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240314165949-fec16b14c33b

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@ github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240313161042-8
8484
github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240313161042-88282483a04f/go.mod h1:qKuzDDDMlAmJn4JWPoUeBEzpAia7J17++hhzR0oPv88=
8585
github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a h1:XcUHh0j65hm8/4orLTH6aRTv3Ah4rGP1rA4yu7G0fR0=
8686
github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a/go.mod h1:8C7VPKXAxiwB5Z4Kwn12VL0guW6onIG0Ayxiio5Vyu0=
87-
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b h1:5EzrrjcGziV69MsEgoBwPdsggY56M6jUxGBP9pp+hwo=
88-
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b/go.mod h1:DL+Ts0k+fzgZmx0XxWArIeAmdKuTkPa1I5DThdybfmE=
87+
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6 h1:4Z7LjnjEF82XiusXJTI/4TqgwnJas3cdvg/qEgkrW8Q=
88+
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6/go.mod h1:DL+Ts0k+fzgZmx0XxWArIeAmdKuTkPa1I5DThdybfmE=
8989
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b h1:FEbadtLx4+ktxf79ZJoKZmfMNsQyqqgL5T9NXWc3i/k=
9090
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b/go.mod h1:ghnFgNIzj4amS897wEto+L+jYzDSg2cJ6y32RNfFGhk=
9191
github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b h1:lygG1KiF5d9HpKpGAl5fa8JVlC9j5VFvC4iKvJkJslA=

0 commit comments

Comments
 (0)