diff --git a/CHANGELOG.md b/CHANGELOG.md index b19a89cb7..cb88034f1 100644 --- a/CHANGELOG.md +++ b/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+ diff --git a/pkg/apis/deployment/v1/deployment_status_members.go b/pkg/apis/deployment/v1/deployment_status_members.go index d261a6026..be3c58526 100644 --- a/pkg/apis/deployment/v1/deployment_status_members.go +++ b/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) - } - 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) + 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(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 } diff --git a/pkg/apis/deployment/v1/deployment_status_members_test.go b/pkg/apis/deployment/v1/deployment_status_members_test.go new file mode 100644 index 000000000..ff6f8eb4b --- /dev/null +++ b/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...) +} diff --git a/pkg/deployment/reconcile/plan_builder_rotate_upgrade.go b/pkg/deployment/reconcile/plan_builder_rotate_upgrade.go index 8a3f460dd..eba20009e 100644 --- a/pkg/deployment/reconcile/plan_builder_rotate_upgrade.go +++ b/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))