Repository: m3db/m3db-operator
Pull request #284 (2021-03-16 17:36:56)

Backwards compatibility when using the original update annoation with an OnDelete update strategy
Thanks for contributing to the M3DB Operator! We'd love to accept your contribution, but we require that most issues
outside of trivial changes such as typo corrections have an issue associated with the pull request. So please [open an
issue](https://github.com/m3db/m3db-operator/issues/new) first!


codecov[bot]: # [Codecov](https://codecov.io/gh/m3db/m3db-operator/pull/284?src=pr&el=h1) Report
> Merging [#284](https://codecov.io/gh/m3db/m3db-operator/pull/284?src=pr&el=desc) (30899e0) into [master](https://codecov.io/gh/m3db/m3db-operator/commit/89725893048e924d8732daae8645e8a83187b9f6?el=desc) (8972589) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.


[![Impacted file tree graph](https://codecov.io/gh/m3db/m3db-operator/pull/284/graphs/tree.svg?width=650&height=150&src=pr&token=jtHDyWhsto)](https://codecov.io/gh/m3db/m3db-operator/pull/284?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
+ Coverage   76.09%   76.11%   +0.01%     
==========================================
  Files          32       32              
  Lines        2548     2550       +2     
==========================================
+ Hits         1939     1941       +2     
  Misses        430      430              
  Partials      179      179              
```


------

[Continue to review full report at Codecov](https://codecov.io/gh/m3db/m3db-operator/pull/284?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/m3db/m3db-operator/pull/284?src=pr&el=footer). Last update [8972589...30899e0](https://codecov.io/gh/m3db/m3db-operator/pull/284?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
index 600bed8..f75988f 100644
--- a/pkg/controller/controller.go
+++ b/pkg/controller/controller.go
@@ -583,11 +583,11 @@ func (c *M3DBController) handleClusterUpdate(cluster *myspec.M3DBCluster) error
 		}
 
 		// Using an OnDelete strategy, we have to update nodes if:
-		//   - a statefulset update just happened
+		//   - a statefulset update has happened OR
 		//   - we're already in the middle of a rollout
-		//     * because nodes are rolled out in chunks this can happen in many iterations
-		// Therefore, always check to see if pods need to be updated and return from this loop
-		// if the statefulset or pods were updated. If a rollout is finished or there has not
+		//     * because nodes are rolled out in chunks, this can happen over many iterations
+		// Therefore, check to see if pods need to be updated and return from this loop
+		// if pods were updated. If a rollout is finished or there has not
 		// been a change, this call is a no-op.
 		if onDeleteUpdateStrategy {
 			nodesUpdated, err := c.updateStatefulSetPods(cluster, actual)
@@ -1316,6 +1316,12 @@ func copyAnnotations(expected, actual *appsv1.StatefulSet) {
 	// the cluster spec.
 	for k, v := range actual.Annotations {
 		if k == annotations.Update {
+			// NB(nate): add this check for backwards compatibility. Existing components
+			// may be using the old update annotation. We want to ensure rollouts still work as
+			// expected.
+			if expected.Spec.UpdateStrategy.Type == appsv1.OnDeleteStatefulSetStrategyType {
+				expected.Annotations[annotations.ParallelUpdateInProgress] = "1"
+			}
 			continue
 		}
 


Test patch:
diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go
index a51529e..1305d26 100644
--- a/pkg/controller/controller_test.go
+++ b/pkg/controller/controller_test.go
@@ -1011,6 +1011,31 @@ func TestHandleUpdateClusterOnDeleteStrategy(t *testing.T) {
 			},
 			updateVal: 1,
 		},
+		{
+			name: "update with the old update annotation",
+			sets: []*metav1.ObjectMeta{
+				newMeta("cluster1-rep0", nil, map[string]string{
+					annotations.Update: annotations.EnabledVal,
+				}),
+				newMeta("cluster1-rep1", nil, map[string]string{
+					annotations.Update: annotations.EnabledVal,
+				}),
+				newMeta("cluster1-rep2", nil, map[string]string{
+					annotations.Update: annotations.EnabledVal,
+				}),
+			},
+			pods:                  generatePods(clusterName, replicas, 2, currentRevision),
+			expUpdateStatefulSets: []string{"cluster1-rep0", "cluster1-rep1", "cluster1-rep2"},
+			expPodUpdateGroups: [][]string{
+				{"cluster1-rep0-0"},
+				{"cluster1-rep0-1"},
+				{"cluster1-rep1-0"},
+				{"cluster1-rep1-1"},
+				{"cluster1-rep2-0"},
+				{"cluster1-rep2-1"},
+			},
+			updateVal: 1,
+		},
 		{
 			name:                  "no update annotation, do nothing",
 			sets:                  generateSets(clusterName, replicas, "3", false),