Skip to content

Commit

Permalink
Keep instance helpers deterministic
Browse files Browse the repository at this point in the history
Summary:
this is a minor followup to #1365, which keeps the `UpdateInstanceStatus` method deterministic, making it testable.

Signed-off-by: Aleksey Dukhovniy <alex.dukhovniy@googlemail.com>
  • Loading branch information
zen-dog committed Mar 2, 2020
1 parent 2dd0f03 commit 825c3ca
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 7 deletions.
9 changes: 4 additions & 5 deletions pkg/apis/kudo/v1beta1/instance_types_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package v1beta1

import (
"fmt"
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/uuid"
Expand Down Expand Up @@ -56,10 +55,10 @@ func (i *Instance) NoPlanEverExecuted() bool {
}

// UpdateInstanceStatus updates `Status.PlanStatus` and `Status.AggregatedStatus` property based on the given plan
func (i *Instance) UpdateInstanceStatus(planStatus *PlanStatus) {
func (i *Instance) UpdateInstanceStatus(planStatus *PlanStatus, updatedTimestamp *metav1.Time) {
for k, v := range i.Status.PlanStatus {
if v.Name == planStatus.Name {
planStatus.LastUpdatedTimestamp = &metav1.Time{Time: time.Now()}
planStatus.LastUpdatedTimestamp = updatedTimestamp
i.Status.PlanStatus[k] = *planStatus
i.Status.AggregatedStatus.Status = planStatus.Status
if planStatus.Status.IsTerminal() {
Expand All @@ -71,7 +70,7 @@ func (i *Instance) UpdateInstanceStatus(planStatus *PlanStatus) {

// ResetPlanStatus method resets a PlanStatus for a passed plan name and instance. Plan/phase/step statuses
// are set to ExecutionPending meaning that the controller will restart plan execution.
func (i *Instance) ResetPlanStatus(plan string) error {
func (i *Instance) ResetPlanStatus(plan string, updatedTimestamp *metav1.Time) error {
planStatus := i.PlanStatus(plan)
if planStatus == nil {
return fmt.Errorf("failed to find planStatus for the plan '%s'", plan)
Expand All @@ -90,7 +89,7 @@ func (i *Instance) ResetPlanStatus(plan string) error {
}

// update instance aggregated status
i.UpdateInstanceStatus(planStatus)
i.UpdateInstanceStatus(planStatus, updatedTimestamp)
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/kudo/v1beta1/instance_types_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func TestInstance_ResetPlanStatus(t *testing.T) {

oldUID := instance.Status.PlanStatus["deploy"].UID

err := instance.ResetPlanStatus("deploy")
err := instance.ResetPlanStatus("deploy", &metav1.Time{Time: time.Now()})
assert.NoError(t, err)

// we test that UID has changed. afterwards, we replace it with the old one and compare new
Expand Down
4 changes: 3 additions & 1 deletion pkg/controller/instance/instance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import (
"fmt"
"log"
"reflect"
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/client-go/discovery"
"k8s.io/client-go/rest"
Expand Down Expand Up @@ -241,7 +243,7 @@ func (r *Reconciler) Reconcile(request ctrl.Request) (ctrl.Result, error) {

// ---------- 5. Update status of instance after the execution proceeded ----------
if newStatus != nil {
instance.UpdateInstanceStatus(newStatus)
instance.UpdateInstanceStatus(newStatus, &metav1.Time{Time: time.Now()})
}
if err != nil {
err = r.handleError(err, instance, oldInstance)
Expand Down

0 comments on commit 825c3ca

Please sign in to comment.