Skip to content

Commit

Permalink
Restore condition transition time at the right place
Browse files Browse the repository at this point in the history
As we are resetting the condition list in each reconcile we have the
logic to restore the last transition timestamp of the unchanged
conditions in at the end of the reconcile. However in the nova
controller the restore call was after the instance is was persisted and
therefore it was ineffective. This could lead to extra reconciles in
openstack-operator even if the nova-operator needed to update nothing in
the underlying resources.
  • Loading branch information
gibizer authored and openshift-merge-bot[bot] committed Jun 13, 2024
1 parent c90c70c commit c7395c1
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
2 changes: 1 addition & 1 deletion controllers/nova_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ func (r *NovaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (resul
instance.Status.Conditions.Set(
instance.Status.Conditions.Mirror(condition.ReadyCondition))
}
err := h.PatchInstance(ctx, instance)
condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
err := h.PatchInstance(ctx, instance)
if err != nil {
_err = err
return
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.20

require (
github.com/go-logr/logr v1.4.2
github.com/google/go-cmp v0.6.0
github.com/google/uuid v1.6.0
github.com/gophercloud/gophercloud v1.12.0
github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.4.0
Expand Down Expand Up @@ -42,7 +43,6 @@ require (
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/google/gnostic-models v0.6.8 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/pprof v0.0.0-20240424215950-a892ee059fd6 // indirect
github.com/imdario/mergo v0.3.16 // indirect
Expand Down
24 changes: 24 additions & 0 deletions test/functional/nova_reconfiguration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ package functional_test

import (
"fmt"
"time"

"github.com/google/go-cmp/cmp"
. "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports
. "github.com/onsi/gomega" //revive:disable:dot-imports

Expand Down Expand Up @@ -766,4 +768,26 @@ var _ = Describe("Nova reconfiguration", func() {
g.Expect(GetEnvVarValue(jobEnv, "PURGE_AGE", "")).To(Equal("99"))
}, timeout, interval).Should(Succeed())
})
It("does not change status if label is added", func() {
// ensure that any newly generated timestamp i.e. in the condition list
// will result in a different string representation
time.Sleep(time.Second)

var oldStatus *novav1.NovaStatus
Eventually(func(g Gomega) {
nova := GetNova(novaNames.NovaName)
oldStatus = nova.Status.DeepCopy()

nova.Labels = map[string]string{
"foo": "bar",
}
g.Expect(k8sClient.Update(ctx, nova)).To(Succeed())
}, timeout, interval).Should(Succeed())

Consistently(func(g Gomega) {
newStatus := &GetNova(novaNames.NovaName).Status
diff := cmp.Diff(oldStatus, newStatus)
g.Expect(diff).To(BeEmpty())
}, timeout, interval).Should(Succeed())
})
})

0 comments on commit c7395c1

Please sign in to comment.