Add another scenario to e2e test (#1756)

* Add another scenario to e2e test
* Initialise Status in the same order as the plan
* Fix getters to return correct reference

Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>

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

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
 	}

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 {

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