[Bugfix] Add grecefulness to kill action (#912)

Author: ajanikow

CHANGELOG.md

@@ -12,6 +12,7 @@
 - (Feature) Allow to disable external port (sidecar managed connection)
 - (Bugfix) Fix 3.6 -> 3.7 Upgrade procedure
 - (Bugfix) Add missing finalizer
+- (Bugfix) Add graceful to kill command
 
 ## [1.2.7](https://github.com/arangodb/kube-arangodb/tree/1.2.7) (2022-01-17)
 - Add Plan BackOff functionality

pkg/deployment/actions/wrapper.go

@@ -0,0 +1,70 @@
+//
+// DISCLAIMER
+//
+// Copyright 2016-2022 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
+//
+
+package actions
+
+import api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
+
+func NewAction(actionType api.ActionType, group api.ServerGroup, member api.MemberStatus, reason ...string) api.Action {
+	return actionWrap(api.NewAction(actionType, group, member.ID, reason...), &member, actionWrapMemberUID)
+}
+
+func NewClusterAction(actionType api.ActionType, reason ...string) api.Action {
+	m := api.MemberStatus{}
+	return NewAction(actionType, api.ServerGroupUnknown, m, reason...)
+}
+
+func NewActionBuilderWrap(group api.ServerGroup, member api.MemberStatus) api.ActionBuilder {
+	return &actionBuilderWrap{
+		group:  group,
+		member: member,
+	}
+}
+
+type actionBuilderWrap struct {
+	group  api.ServerGroup
+	member api.MemberStatus
+}
+
+func (a actionBuilderWrap) NewAction(actionType api.ActionType, reason ...string) api.Action {
+	return NewAction(actionType, a.group, a.member, reason...)
+}
+
+func actionWrap(a api.Action, member *api.MemberStatus, wrap ...actionWrapper) api.Action {
+	for _, w := range wrap {
+		a = w(a, member)
+	}
+
+	return a
+}
+
+func actionWrapMemberUID(a api.Action, member *api.MemberStatus) api.Action {
+	switch a.Type {
+	case api.ActionTypeShutdownMember, api.ActionTypeKillMemberPod, api.ActionTypeRotateStartMember, api.ActionTypeUpgradeMember:
+		if q := member.PodUID; q != "" {
+			return a.AddParam(api.ParamPodUID, string(q))
+		}
+		return a
+	default:
+		return a
+	}
+}
+
+type actionWrapper func(a api.Action, member *api.MemberStatus) api.Action

pkg/deployment/reconcile/action.go

@@ -91,6 +91,30 @@ type ActionStartFailureGracePeriod interface {
 	StartFailureGracePeriod() time.Duration
 }
 
+func wrapActionStartFailureGracePeriod(action Action, failureGracePeriod time.Duration) Action {
+	return &actionStartFailureGracePeriod{
+		Action:             action,
+		failureGracePeriod: failureGracePeriod,
+	}
+}
+
+func withActionStartFailureGracePeriod(in actionFactory, failureGracePeriod time.Duration) actionFactory {
+	return func(log zerolog.Logger, action api.Action, actionCtx ActionContext) Action {
+		return wrapActionStartFailureGracePeriod(in(log, action, actionCtx), failureGracePeriod)
+	}
+}
+
+var _ ActionStartFailureGracePeriod = &actionStartFailureGracePeriod{}
+
+type actionStartFailureGracePeriod struct {
+	Action
+	failureGracePeriod time.Duration
+}
+
+func (a actionStartFailureGracePeriod) StartFailureGracePeriod() time.Duration {
+	return a.failureGracePeriod
+}
+
 func getStartFailureGracePeriod(a Action) time.Duration {
 	if c, ok := a.(ActionStartFailureGracePeriod); !ok {
 		return 0
@@ -118,27 +142,27 @@ func getActionPlanAppender(a Action, plan api.Plan) (api.Plan, bool) {
 type actionFactory func(log zerolog.Logger, action api.Action, actionCtx ActionContext) Action
 
 var (
-	actions     = map[api.ActionType]actionFactory{}
-	actionsLock sync.Mutex
+	definedActions     = map[api.ActionType]actionFactory{}
+	definedActionsLock sync.Mutex
 )
 
 func registerAction(t api.ActionType, f actionFactory) {
-	actionsLock.Lock()
-	defer actionsLock.Unlock()
+	definedActionsLock.Lock()
+	defer definedActionsLock.Unlock()
 
-	_, ok := actions[t]
+	_, ok := definedActions[t]
 	if ok {
 		panic(fmt.Sprintf("Action already defined %s", t))
 	}
 
-	actions[t] = f
+	definedActions[t] = f
 }
 
 func getActionFactory(t api.ActionType) (actionFactory, bool) {
-	actionsLock.Lock()
-	defer actionsLock.Unlock()
+	definedActionsLock.Lock()
+	defer definedActionsLock.Unlock()
 
-	f, ok := actions[t]
+	f, ok := definedActions[t]
 	return f, ok
 }
 

pkg/deployment/reconcile/action_add_member.go

@@ -29,6 +29,7 @@ import (
 	"github.com/arangodb/kube-arangodb/pkg/util/errors"
 
 	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
+	"github.com/arangodb/kube-arangodb/pkg/deployment/actions"
 	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
 )
@@ -81,11 +82,11 @@ func (a *actionAddMember) ActionPlanAppender(current api.Plan) (api.Plan, bool)
 	var app api.Plan
 
 	if _, ok := a.action.Params[api.ActionTypeWaitForMemberUp.String()]; ok {
-		app = append(app, api.NewAction(api.ActionTypeWaitForMemberUp, a.action.Group, a.newMemberID, "Wait for member in sync after creation"))
+		app = append(app, actions.NewAction(api.ActionTypeWaitForMemberUp, a.action.Group, withPredefinedMember(a.newMemberID), "Wait for member in sync after creation"))
 	}
 
 	if _, ok := a.action.Params[api.ActionTypeWaitForMemberInSync.String()]; ok {
-		app = append(app, api.NewAction(api.ActionTypeWaitForMemberInSync, a.action.Group, a.newMemberID, "Wait for member in sync after creation"))
+		app = append(app, actions.NewAction(api.ActionTypeWaitForMemberInSync, a.action.Group, withPredefinedMember(a.newMemberID), "Wait for member in sync after creation"))
 	}
 
 	if len(app) > 0 {

pkg/deployment/reconcile/action_rotate_member.go

@@ -34,7 +34,7 @@ import (
 )
 
 func init() {
-	registerAction(api.ActionTypeRotateMember, newRotateMemberAction)
+	registerAction(api.ActionTypeRotateMember, withActionStartFailureGracePeriod(newRotateMemberAction, time.Minute))
 }
 
 // newRotateMemberAction creates a new Action that implements the given
@@ -109,7 +109,3 @@ func (a *actionRotateMember) CheckProgress(ctx context.Context) (bool, bool, err
 	}
 	return true, false, nil
 }
-
-func (a *actionRotateMember) StartFailureGracePeriod() time.Duration {
-	return 30 * time.Second
-}

pkg/deployment/reconcile/action_rotate_start_member.go

@@ -26,13 +26,15 @@ import (
 	"github.com/rs/zerolog"
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	"time"
+
 	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
 	"github.com/arangodb/kube-arangodb/pkg/util/errors"
 	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
 )
 
 func init() {
-	registerAction(api.ActionTypeRotateStartMember, newRotateStartMemberAction)
+	registerAction(api.ActionTypeRotateStartMember, withActionStartFailureGracePeriod(newRotateStartMemberAction, time.Minute))
 }
 
 // newRotateStartMemberAction creates a new Action that implements the given

pkg/deployment/reconcile/action_shutdown_member.go

@@ -25,12 +25,14 @@ import (
 
 	"github.com/arangodb/kube-arangodb/pkg/util/errors"
 
+	"time"
+
 	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
 	"github.com/rs/zerolog"
 )
 
 func init() {
-	registerAction(api.ActionTypeShutdownMember, newShutdownMemberAction)
+	registerAction(api.ActionTypeShutdownMember, withActionStartFailureGracePeriod(newShutdownMemberAction, time.Minute))
 }
 
 // newShutdownMemberAction creates a new Action that implements the given

pkg/deployment/reconcile/action_test.go

@@ -0,0 +1,56 @@
+//
+// DISCLAIMER
+//
+// Copyright 2016-2022 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
+//
+
+package reconcile
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+type gracefulAction struct {
+	actionEmpty
+
+	graceful time.Duration
+}
+
+func (g gracefulAction) StartFailureGracePeriod() time.Duration {
+	return g.graceful
+}
+
+var _ ActionStartFailureGracePeriod = gracefulAction{}
+
+func Test_GracefulTimeouts(t *testing.T) {
+	t.Run("Default", func(t *testing.T) {
+		require.EqualValues(t, 0, getStartFailureGracePeriod(actionEmpty{}))
+	})
+	t.Run("Set", func(t *testing.T) {
+		require.EqualValues(t, time.Second, getStartFailureGracePeriod(gracefulAction{
+			graceful: time.Second,
+		}))
+	})
+	t.Run("Override", func(t *testing.T) {
+		require.EqualValues(t, time.Minute, getStartFailureGracePeriod(wrapActionStartFailureGracePeriod(gracefulAction{
+			graceful: time.Second,
+		}, time.Minute)))
+	})
+}

pkg/deployment/reconcile/helper_wrap.go

@@ -22,6 +22,7 @@ package reconcile
 
 import (
 	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
+	"github.com/arangodb/kube-arangodb/pkg/deployment/actions"
 	"github.com/arangodb/kube-arangodb/pkg/deployment/features"
 )
 
@@ -30,38 +31,38 @@ func withMaintenance(plan ...api.Action) api.Plan {
 		return plan
 	}
 
-	return withMaintenanceStart(plan...).After(api.NewAction(api.ActionTypeDisableMaintenance, api.ServerGroupUnknown, "", "Disable maintenance after actions"))
+	return withMaintenanceStart(plan...).After(actions.NewClusterAction(api.ActionTypeDisableMaintenance, "Disable maintenance after actions"))
 }
 func withMaintenanceStart(plan ...api.Action) api.Plan {
 	if !features.Maintenance().Enabled() {
 		return plan
 	}
 
 	return api.AsPlan(plan).Before(
-		api.NewAction(api.ActionTypeEnableMaintenance, api.ServerGroupUnknown, "", "Enable maintenance before actions"),
-		api.NewAction(api.ActionTypeSetMaintenanceCondition, api.ServerGroupUnknown, "", "Enable maintenance before actions"))
+		actions.NewClusterAction(api.ActionTypeEnableMaintenance, "Enable maintenance before actions"),
+		actions.NewClusterAction(api.ActionTypeSetMaintenanceCondition, "Enable maintenance before actions"))
 }
 
 func withResignLeadership(group api.ServerGroup, member api.MemberStatus, reason string, plan ...api.Action) api.Plan {
 	if member.Image == nil {
 		return plan
 	}
 
-	return api.AsPlan(plan).Before(api.NewAction(api.ActionTypeResignLeadership, group, member.ID, reason))
+	return api.AsPlan(plan).Before(actions.NewAction(api.ActionTypeResignLeadership, group, member, reason))
 }
 
 func cleanOutMember(group api.ServerGroup, m api.MemberStatus) api.Plan {
 	var plan api.Plan
 
 	if group == api.ServerGroupDBServers {
 		plan = append(plan,
-			api.NewAction(api.ActionTypeCleanOutMember, group, m.ID),
+			actions.NewAction(api.ActionTypeCleanOutMember, group, m),
 		)
 	}
 	plan = append(plan,
-		api.NewAction(api.ActionTypeKillMemberPod, group, m.ID),
-		api.NewAction(api.ActionTypeShutdownMember, group, m.ID),
-		api.NewAction(api.ActionTypeRemoveMember, group, m.ID),
+		actions.NewAction(api.ActionTypeKillMemberPod, group, m),
+		actions.NewAction(api.ActionTypeShutdownMember, group, m),
+		actions.NewAction(api.ActionTypeRemoveMember, group, m),
 	)
 
 	return plan

pkg/deployment/reconcile/plan_builder_bootstrap.go

@@ -24,6 +24,7 @@ import (
 	"context"
 
 	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
+	"github.com/arangodb/kube-arangodb/pkg/deployment/actions"
 	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
 	inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector"
 	"github.com/rs/zerolog"
@@ -54,8 +55,8 @@ func createBootstrapPlan(ctx context.Context,
 			}
 		}
 
-		return api.Plan{api.NewAction(api.ActionTypeBootstrapSetPassword, api.ServerGroupUnknown, "", "Updating password").AddParam("user", user)}
+		return api.Plan{actions.NewClusterAction(api.ActionTypeBootstrapSetPassword, "Updating password").AddParam("user", user)}
 	}
 
-	return api.Plan{api.NewAction(api.ActionTypeBootstrapUpdate, api.ServerGroupUnknown, "", "Finalizing bootstrap")}
+	return api.Plan{actions.NewClusterAction(api.ActionTypeBootstrapUpdate, "Finalizing bootstrap")}
 }