[Feature] Change Rotation by Annotation order (#608)

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## [master](https://github.com/arangodb/kube-arangodb/tree/master) (N/A)
 - Add Labels and Annotations to ServiceMonitor
 - Allow to expose Exporter in HTTP with secured Deployments
+- Change rotation by annotation order (coordinator before dbserver)
 
 ## [1.0.4](https://github.com/arangodb/kube-arangodb/tree/1.0.4) (2020-07-28)
 - Add Encryption Key rotation feature for ArangoDB EE 3.7+

pkg/apis/deployment/v1/deployment_status_members.go

@@ -84,23 +84,45 @@ func (ds DeploymentStatusMembers) ElementByID(id string) (MemberStatus, ServerGr
 // If the callback returns an error, this error is returned and the callback is
 // not called for the remaining groups.
 func (ds DeploymentStatusMembers) ForeachServerGroup(cb func(group ServerGroup, list MemberStatusList) error) error {
-	if err := cb(ServerGroupSingle, ds.Single); err != nil {
-		return maskAny(err)
+	return ds.ForeachServerInGroups(cb, AllServerGroups...)
+}
+
+func (ds DeploymentStatusMembers) ForeachServerInGroups(cb func(group ServerGroup, list MemberStatusList) error, groups ...ServerGroup) error {
+	for _, group := range groups {
+		if err := ds.ForServerGroup(cb, group); err != nil {
+			return err
+		}
 	}
-	if err := cb(ServerGroupAgents, ds.Agents); err != nil {
-		return maskAny(err)
-	}
-	if err := cb(ServerGroupDBServers, ds.DBServers); err != nil {
-		return maskAny(err)
-	}
-	if err := cb(ServerGroupCoordinators, ds.Coordinators); err != nil {
-		return maskAny(err)
-	}
-	if err := cb(ServerGroupSyncMasters, ds.SyncMasters); err != nil {
-		return maskAny(err)
-	}
-	if err := cb(ServerGroupSyncWorkers, ds.SyncWorkers); err != nil {
-		return maskAny(err)
+
+	return nil
+}
+
+func (ds DeploymentStatusMembers) ForServerGroup(cb func(group ServerGroup, list MemberStatusList) error, group ServerGroup) error {
+	switch group {
+	case ServerGroupSingle:
+		if err := cb(ServerGroupSingle, ds.Single); err != nil {
+			return maskAny(err)
+		}
+	case ServerGroupAgents:
+		if err := cb(ServerGroupAgents, ds.Agents); err != nil {
+			return maskAny(err)
+		}
+	case ServerGroupDBServers:
+		if err := cb(ServerGroupDBServers, ds.DBServers); err != nil {
+			return maskAny(err)
+		}
+	case ServerGroupCoordinators:
+		if err := cb(ServerGroupCoordinators, ds.Coordinators); err != nil {
+			return maskAny(err)
+		}
+	case ServerGroupSyncMasters:
+		if err := cb(ServerGroupSyncMasters, ds.SyncMasters); err != nil {
+			return maskAny(err)
+		}
+	case ServerGroupSyncWorkers:
+		if err := cb(ServerGroupSyncWorkers, ds.SyncWorkers); err != nil {
+			return maskAny(err)
+		}
 	}
 	return nil
 }

pkg/apis/deployment/v1/deployment_status_members_test.go

@@ -0,0 +1,86 @@
+//
+// DISCLAIMER
+//
+// Copyright 2020 ArangoDB GmbH, Cologne, Germany
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+// Author Adam Janikowski
+//
+
+package v1
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func newMemberList() DeploymentStatusMembers {
+	return DeploymentStatusMembers{
+		Single:       MemberStatusList{{ID: ServerGroupSingle.AsRole()}},
+		Agents:       MemberStatusList{{ID: ServerGroupAgents.AsRole()}},
+		DBServers:    MemberStatusList{{ID: ServerGroupDBServers.AsRole()}},
+		Coordinators: MemberStatusList{{ID: ServerGroupCoordinators.AsRole()}},
+		SyncMasters:  MemberStatusList{{ID: ServerGroupSyncMasters.AsRole()}},
+		SyncWorkers:  MemberStatusList{{ID: ServerGroupSyncWorkers.AsRole()}},
+	}
+}
+
+func Test_StatusMemberList_EnsureDefaultExecutionOrder(t *testing.T) {
+	statusMembers := newMemberList()
+
+	order := AllServerGroups
+
+	orderIndex := 0
+
+	statusMembers.ForeachServerGroup(func(group ServerGroup, list MemberStatusList) error {
+		require.True(t, orderIndex < len(order))
+
+		require.Equal(t, order[orderIndex], group)
+
+		require.Len(t, list, 1)
+
+		require.Equal(t, order[orderIndex].AsRole(), list[0].ID)
+
+		orderIndex += 1
+
+		return nil
+	})
+}
+
+func Test_StatusMemberList_CustomExecutionOrder(t *testing.T) {
+	statusMembers := newMemberList()
+
+	order := []ServerGroup{
+		ServerGroupDBServers,
+	}
+
+	orderIndex := 0
+
+	statusMembers.ForeachServerInGroups(func(group ServerGroup, list MemberStatusList) error {
+		require.True(t, orderIndex < len(order))
+
+		require.Equal(t, order[orderIndex], group)
+
+		require.Len(t, list, 1)
+
+		require.Equal(t, order[orderIndex].AsRole(), list[0].ID)
+
+		orderIndex += 1
+
+		return nil
+	}, order...)
+}

pkg/deployment/reconcile/plan_builder_rotate_upgrade.go

@@ -37,8 +37,19 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// createRotateOrUpgradePlan goes over all pods to check if an upgrade or rotate is needed.
+var (
+	// rotationByAnnotationOrder - Change order of execution - Coordinators and Agents should be executed before DBServer to save time
+	rotationByAnnotationOrder = []api.ServerGroup{
+		api.ServerGroupSingle,
+		api.ServerGroupAgents,
+		api.ServerGroupCoordinators,
+		api.ServerGroupDBServers,
+		api.ServerGroupSyncMasters,
+		api.ServerGroupSyncWorkers,
+	}
+)
 
+// createRotateOrUpgradePlan goes over all pods to check if an upgrade or rotate is needed.
 func createRotateOrUpgradePlan(ctx context.Context,
 	log zerolog.Logger, apiObject k8sutil.APIObject,
 	spec api.DeploymentSpec, status api.DeploymentStatus,
@@ -109,6 +120,26 @@ func createRotateOrUpgradePlanInternal(log zerolog.Logger, apiObject k8sutil.API
 				// Only rotate/upgrade 1 pod at a time
 				continue
 			}
+		}
+		return nil
+	})
+
+	status.Members.ForeachServerInGroups(func(group api.ServerGroup, members api.MemberStatusList) error {
+		for _, m := range members {
+			if m.Phase != api.MemberPhaseCreated || m.PodName == "" {
+				// Only rotate when phase is created
+				continue
+			}
+
+			if !newPlan.IsEmpty() {
+				// Only rotate/upgrade 1 pod at a time
+				continue
+			}
+
+			pod, found := cachedStatus.Pod(m.PodName)
+			if !found {
+				continue
+			}
 
 			if pod.Annotations != nil {
 				if _, ok := pod.Annotations[deployment.ArangoDeploymentPodRotateAnnotation]; ok {
@@ -116,8 +147,9 @@ func createRotateOrUpgradePlanInternal(log zerolog.Logger, apiObject k8sutil.API
 				}
 			}
 		}
+
 		return nil
-	})
+	}, rotationByAnnotationOrder...)
 
 	if upgradeNotAllowed {
 		context.CreateEvent(k8sutil.NewUpgradeNotAllowedEvent(apiObject, fromVersion, toVersion, fromLicense, toLicense))