diff --git a/pkg/apis/kudo/v1beta1/instance_types.go b/pkg/apis/kudo/v1beta1/instance_types.go index 5ac97dac9..4dc207e9c 100644 --- a/pkg/apis/kudo/v1beta1/instance_types.go +++ b/pkg/apis/kudo/v1beta1/instance_types.go @@ -118,9 +118,9 @@ func (s *StepStatus) SetWithMessage(status ExecutionStatus, message string) { // Step returns the StepStatus with the given name, or nil if no such step status exists func (s *PhaseStatus) Step(name string) *StepStatus { - for _, stepStatus := range s.Steps { + for i, stepStatus := range s.Steps { if stepStatus.Name == name { - return &stepStatus + return &s.Steps[i] } } return nil @@ -138,9 +138,9 @@ func (s *PhaseStatus) SetWithMessage(status ExecutionStatus, message string) { // Step returns the PhaseStatus with the given name, or nil if no such phase status exists func (s *PlanStatus) Phase(name string) *PhaseStatus { - for _, phaseStatus := range s.Phases { + for i, phaseStatus := range s.Phases { if phaseStatus.Name == name { - return &phaseStatus + return &s.Phases[i] } } return nil diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index d39c79721..71a0906fb 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -540,26 +540,37 @@ func ensurePlanStatusInitialized(i *kudoapi.Instance, ov *kudoapi.OperatorVersio Phases: make([]kudoapi.PhaseStatus, 0), } } + // We fully rebuild the phase status here to make sure that newly + // added phases have a status in the correct order + newPhaseStatus := make([]kudoapi.PhaseStatus, 0) for _, phase := range plan.Phases { phaseStatus := planStatus.Phase(phase.Name) if phaseStatus == nil { - planStatus.Phases = append(planStatus.Phases, kudoapi.PhaseStatus{ + phaseStatus = &kudoapi.PhaseStatus{ Name: phase.Name, Status: kudoapi.ExecutionNeverRun, Steps: make([]kudoapi.StepStatus, 0), - }) - phaseStatus = &planStatus.Phases[len(planStatus.Phases)-1] + } } + + // Same here, full rebuild of the slice, so newly added steps + // have a matching status + newStepStatus := make([]kudoapi.StepStatus, 0) for _, step := range phase.Steps { stepStatus := phaseStatus.Step(step.Name) if stepStatus == nil { - phaseStatus.Steps = append(phaseStatus.Steps, kudoapi.StepStatus{ + stepStatus = &kudoapi.StepStatus{ Name: step.Name, Status: kudoapi.ExecutionNeverRun, - }) + } } + newStepStatus = append(newStepStatus, *stepStatus) } + phaseStatus.Steps = newStepStatus + newPhaseStatus = append(newPhaseStatus, *phaseStatus) } + planStatus.Phases = newPhaseStatus + // We might have updated the plan status, so set it just to make sure i.Status.PlanStatus[planName] = planStatus } diff --git a/pkg/controller/instance/instance_controller_test.go b/pkg/controller/instance/instance_controller_test.go index c694e5f07..f376d58a2 100644 --- a/pkg/controller/instance/instance_controller_test.go +++ b/pkg/controller/instance/instance_controller_test.go @@ -577,7 +577,7 @@ func Test_ensurePlanStatusInitialized(t *testing.T) { }, { name: "a new phase is correctly initialized", - i: instance, + i: instance.DeepCopy(), ov: func() *kudoapi.OperatorVersion { modifiedOv := ov.DeepCopy() pln := modifiedOv.Spec.Plans["deploy"] @@ -626,6 +626,48 @@ func Test_ensurePlanStatusInitialized(t *testing.T) { }, }, }, + { + name: "a new step is correctly initialized", + i: instance.DeepCopy(), + ov: func() *kudoapi.OperatorVersion { + modifiedOv := ov.DeepCopy() + pln := modifiedOv.Spec.Plans["deploy"] + pln.Phases[0].Steps = append([]kudoapi.Step{ + { + Name: "additionalstep", + Tasks: []string{}, + }, + }, pln.Phases[0].Steps...) + modifiedOv.Spec.Plans["deploy"] = pln + return modifiedOv + }(), + want: kudoapi.InstanceStatus{ + PlanStatus: map[string]kudoapi.PlanStatus{ + "deploy": { + Name: "deploy", + Status: kudoapi.ExecutionNeverRun, + UID: "", + Phases: []kudoapi.PhaseStatus{ + { + Name: "phase", + Status: kudoapi.ExecutionNeverRun, + Steps: []kudoapi.StepStatus{ + { + Name: "additionalstep", + Status: kudoapi.ExecutionNeverRun, + }, + { + Name: "step", + Status: kudoapi.ExecutionNeverRun, + }, + }, + }, + }, + }, + "backup": backupStatus, + }, + }, + }, } for _, tt := range tests { diff --git a/test/e2e/upgrade/first-operator-v2/operator.yaml b/test/e2e/upgrade/first-operator-v2/operator.yaml index eda855422..fcd9b054f 100644 --- a/test/e2e/upgrade/first-operator-v2/operator.yaml +++ b/test/e2e/upgrade/first-operator-v2/operator.yaml @@ -18,6 +18,11 @@ tasks: spec: resources: - configmap.yaml + - name: newstep + kind: Apply + spec: + resources: + - configmap.yaml plans: deploy: strategy: serial @@ -31,6 +36,9 @@ plans: - name: main strategy: parallel steps: + - name: prependstep + tasks: + - newstep - name: everything tasks: - app