From 703d5d7f2d75042fa1738f0837b16b629d06f9c5 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Thu, 15 Mar 2018 12:52:43 +0100 Subject: [PATCH 01/17] Adding server rotation --- examples/nodeport-cluster.yaml | 1 + examples/simple-cluster.yaml | 1 + pkg/apis/deployment/v1alpha/member_state.go | 2 + pkg/apis/deployment/v1alpha/plan.go | 16 +- .../v1alpha/zz_generated.deepcopy.go | 12 +- pkg/deployment/action.go | 38 ++++ pkg/deployment/action_add_member.go | 66 +++++++ pkg/deployment/action_cleanout_member.go | 105 +++++++++++ pkg/deployment/action_context.go | 176 ++++++++++++++++++ pkg/deployment/action_remove_member.go | 80 ++++++++ pkg/deployment/action_rotate_member.go | 116 ++++++++++++ pkg/deployment/action_shutdown_member.go | 106 +++++++++++ pkg/deployment/plan_builder.go | 83 ++++++++- pkg/deployment/plan_executor.go | 172 +++-------------- pkg/deployment/pod_inspector.go | 2 +- 15 files changed, 808 insertions(+), 168 deletions(-) create mode 100644 pkg/deployment/action.go create mode 100644 pkg/deployment/action_add_member.go create mode 100644 pkg/deployment/action_cleanout_member.go create mode 100644 pkg/deployment/action_context.go create mode 100644 pkg/deployment/action_remove_member.go create mode 100644 pkg/deployment/action_rotate_member.go create mode 100644 pkg/deployment/action_shutdown_member.go diff --git a/examples/nodeport-cluster.yaml b/examples/nodeport-cluster.yaml index f5765e61e..5de0b4c45 100644 --- a/examples/nodeport-cluster.yaml +++ b/examples/nodeport-cluster.yaml @@ -7,6 +7,7 @@ spec: app: arangodb role: coordinator type: NodePort + publishNotReadyAddresses: true ports: - protocol: TCP port: 8529 diff --git a/examples/simple-cluster.yaml b/examples/simple-cluster.yaml index 88e17703e..2b1b6df91 100644 --- a/examples/simple-cluster.yaml +++ b/examples/simple-cluster.yaml @@ -4,3 +4,4 @@ metadata: name: "example-simple-cluster" spec: mode: cluster + image: arangodb/arangodb:3.3.2 diff --git a/pkg/apis/deployment/v1alpha/member_state.go b/pkg/apis/deployment/v1alpha/member_state.go index 65062e224..825827b32 100644 --- a/pkg/apis/deployment/v1alpha/member_state.go +++ b/pkg/apis/deployment/v1alpha/member_state.go @@ -34,4 +34,6 @@ const ( MemberStateCleanOut MemberState = "CleanOut" // MemberStateShuttingDown indicates that a member is shutting down MemberStateShuttingDown MemberState = "ShuttingDown" + // MemberStateRotating indicates that a member is being rotated + MemberStateRotating MemberState = "Rotating" ) diff --git a/pkg/apis/deployment/v1alpha/plan.go b/pkg/apis/deployment/v1alpha/plan.go index 9609fcfe1..0df198b96 100644 --- a/pkg/apis/deployment/v1alpha/plan.go +++ b/pkg/apis/deployment/v1alpha/plan.go @@ -36,6 +36,8 @@ const ( ActionTypeCleanOutMember ActionType = "CleanOutMember" // ActionTypeShutdownMember causes a member to be shutdown and removed from the cluster. ActionTypeShutdownMember ActionType = "ShutdownMember" + // ActionTypeRotateMember causes a member to be shutdown and have it's pod removed. + ActionTypeRotateMember ActionType = "RotateMember" ) // Action represents a single action to be taken to update a deployment. @@ -46,8 +48,20 @@ type Action struct { MemberID string `json:"memberID,omitempty"` // Group involved in this action Group ServerGroup `json:"group,omitempty"` + // CreationTime is set the when the action is created. + CreationTime metav1.Time `json:"creationTime"` // StartTime is set the when the action has been started, but needs to wait to be finished. - StartTime *metav1.Time `json:"startTime,omitempty"` + StartTime metav1.Time `json:"startTime,omitempty"` +} + +// NewAction instantiates a new Action. +func NewAction(actionType ActionType, group ServerGroup, memberID string) Action { + return Action{ + Type: actionType, + MemberID: memberID, + Group: group, + CreationTime: metav1.Now(), + } } // Plan is a list of actions that will be taken to update a deployment. diff --git a/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go b/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go index c810cead6..c67e72e67 100644 --- a/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go +++ b/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go @@ -25,22 +25,14 @@ package v1alpha import ( - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Action) DeepCopyInto(out *Action) { *out = *in - if in.StartTime != nil { - in, out := &in.StartTime, &out.StartTime - if *in == nil { - *out = nil - } else { - *out = new(v1.Time) - (*in).DeepCopyInto(*out) - } - } + in.CreationTime.DeepCopyInto(&out.CreationTime) + in.StartTime.DeepCopyInto(&out.StartTime) return } diff --git a/pkg/deployment/action.go b/pkg/deployment/action.go new file mode 100644 index 000000000..ea064559b --- /dev/null +++ b/pkg/deployment/action.go @@ -0,0 +1,38 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 Ewout Prangsma +// + +package deployment + +import ( + "context" +) + +// Action executes a single Plan item. +type Action interface { + // Start performs the start of the action. + // Returns true if the action is completely finished, false in case + // the start time needs to be recorded and a ready condition needs to be checked. + Start(ctx context.Context) (bool, error) + // CheckProgress checks the progress of the action. + // Returns true if the action is completely finished, false otherwise. + CheckProgress(ctx context.Context) (bool, error) +} diff --git a/pkg/deployment/action_add_member.go b/pkg/deployment/action_add_member.go new file mode 100644 index 000000000..8e11e4893 --- /dev/null +++ b/pkg/deployment/action_add_member.go @@ -0,0 +1,66 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 Ewout Prangsma +// + +package deployment + +import ( + "context" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" +) + +// NewAddMemberAction creates a new Action that implements the given +// planned AddMember action. +func NewAddMemberAction(log zerolog.Logger, action api.Action, actionCtx ActionContext) Action { + return &actionAddMember{ + log: log, + action: action, + actionCtx: actionCtx, + } +} + +// actionAddMember implements an AddMemberAction. +type actionAddMember struct { + log zerolog.Logger + action api.Action + actionCtx ActionContext +} + +// Start performs the start of the action. +// Returns true if the action is completely finished, false in case +// the start time needs to be recorded and a ready condition needs to be checked. +func (a *actionAddMember) Start(ctx context.Context) (bool, error) { + if err := a.actionCtx.CreateMember(a.action.Group); err != nil { + log.Debug().Err(err).Msg("Failed to create member") + return false, maskAny(err) + } + return true, nil +} + +// CheckProgress checks the progress of the action. +// Returns true if the action is completely finished, false otherwise. +func (a *actionAddMember) CheckProgress(ctx context.Context) (bool, error) { + // Nothing todo + return true, nil +} diff --git a/pkg/deployment/action_cleanout_member.go b/pkg/deployment/action_cleanout_member.go new file mode 100644 index 000000000..24e47db5e --- /dev/null +++ b/pkg/deployment/action_cleanout_member.go @@ -0,0 +1,105 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 Ewout Prangsma +// + +package deployment + +import ( + "context" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/rs/zerolog" +) + +// NewCleanOutMemberAction creates a new Action that implements the given +// planned CleanOutMember action. +func NewCleanOutMemberAction(log zerolog.Logger, action api.Action, actionCtx ActionContext) Action { + return &actionCleanoutMember{ + log: log, + action: action, + actionCtx: actionCtx, + } +} + +// actionCleanoutMember implements an CleanOutMemberAction. +type actionCleanoutMember struct { + log zerolog.Logger + action api.Action + actionCtx ActionContext +} + +// Start performs the start of the action. +// Returns true if the action is completely finished, false in case +// the start time needs to be recorded and a ready condition needs to be checked. +func (a *actionCleanoutMember) Start(ctx context.Context) (bool, error) { + m, ok := a.actionCtx.GetMemberStatusByID(a.action.MemberID) + if !ok { + // We wanted to remove and it is already gone. All ok + return true, nil + } + log := a.log + c, err := a.actionCtx.GetDatabaseClient(ctx) + if err != nil { + log.Debug().Err(err).Msg("Failed to create member client") + return false, maskAny(err) + } + cluster, err := c.Cluster(ctx) + if err != nil { + log.Debug().Err(err).Msg("Failed to access cluster") + return false, maskAny(err) + } + if err := cluster.CleanOutServer(ctx, a.action.MemberID); err != nil { + log.Debug().Err(err).Msg("Failed to cleanout member") + return false, maskAny(err) + } + // Update status + m.State = api.MemberStateCleanOut + if a.actionCtx.UpdateMember(m); err != nil { + return false, maskAny(err) + } + return true, nil +} + +// CheckProgress checks the progress of the action. +// Returns true if the action is completely finished, false otherwise. +func (a *actionCleanoutMember) CheckProgress(ctx context.Context) (bool, error) { + log := a.log + c, err := a.actionCtx.GetDatabaseClient(ctx) + if err != nil { + log.Debug().Err(err).Msg("Failed to create member client") + return false, maskAny(err) + } + cluster, err := c.Cluster(ctx) + if err != nil { + log.Debug().Err(err).Msg("Failed to access cluster") + return false, maskAny(err) + } + cleanedOut, err := cluster.IsCleanedOut(ctx, a.action.MemberID) + if err != nil { + return false, maskAny(err) + } + if !cleanedOut { + // We're not done yet + return false, nil + } + // Cleanout completed + return true, nil +} diff --git a/pkg/deployment/action_context.go b/pkg/deployment/action_context.go new file mode 100644 index 000000000..d7d0f87b9 --- /dev/null +++ b/pkg/deployment/action_context.go @@ -0,0 +1,176 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 Ewout Prangsma +// + +package deployment + +import ( + "context" + "fmt" + + driver "github.com/arangodb/go-driver" + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// ActionContext provides methods to the Action implementations +// to control their context. +type ActionContext interface { + // GetDatabaseClient returns a cached client for the entire database (cluster coordinators or single server), + // creating one if needed. + GetDatabaseClient(ctx context.Context) (driver.Client, error) + // GetServerClient returns a cached client for a specific server. + GetServerClient(ctx context.Context, group api.ServerGroup, id string) (driver.Client, error) + // GetMemberStatusByID returns the current member status + // for the member with given id. + // Returns member status, true when found, or false + // when no such member is found. + GetMemberStatusByID(id string) (api.MemberStatus, bool) + // CreateMember adds a new member to the given group. + CreateMember(group api.ServerGroup) error + // UpdateMember updates the deployment status wrt the given member. + UpdateMember(member api.MemberStatus) error + // RemoveMemberByID removes a member with given id. + RemoveMemberByID(id string) error + // DeletePod deletes a pod with given name in the namespace + // of the deployment. If the pod does not exist, the error is ignored. + DeletePod(podName string) error + // DeletePvc deletes a persistent volume claim with given name in the namespace + // of the deployment. If the pvc does not exist, the error is ignored. + DeletePvc(pvcName string) error +} + +// NewActionContext creates a new ActionContext implementation. +func NewActionContext(log zerolog.Logger, deployment *Deployment) ActionContext { + return &actionContext{ + log: log, + deployment: deployment, + } +} + +// actionContext implements ActionContext +type actionContext struct { + log zerolog.Logger + deployment *Deployment +} + +// GetDatabaseClient returns a cached client for the entire database (cluster coordinators or single server), +// creating one if needed. +func (ac *actionContext) GetDatabaseClient(ctx context.Context) (driver.Client, error) { + c, err := ac.deployment.clientCache.GetDatabase(ctx) + if err != nil { + return nil, maskAny(err) + } + return c, nil +} + +// GetServerClient returns a cached client for a specific server. +func (ac *actionContext) GetServerClient(ctx context.Context, group api.ServerGroup, id string) (driver.Client, error) { + c, err := ac.deployment.clientCache.Get(ctx, group, id) + if err != nil { + return nil, maskAny(err) + } + return c, nil +} + +// GetMemberStatusByID returns the current member status +// for the member with given id. +// Returns member status, true when found, or false +// when no such member is found. +func (ac *actionContext) GetMemberStatusByID(id string) (api.MemberStatus, bool) { + m, _, ok := ac.deployment.status.Members.ElementByID(id) + return m, ok +} + +// CreateMember adds a new member to the given group. +func (ac *actionContext) CreateMember(group api.ServerGroup) error { + d := ac.deployment + if err := d.createMember(group, d.apiObject); err != nil { + ac.log.Debug().Err(err).Str("group", group.AsRole()).Msg("Failed to create member") + return maskAny(err) + } + // Save added member + if err := d.updateCRStatus(); err != nil { + log.Debug().Err(err).Msg("Updating CR status failed") + return maskAny(err) + } + return nil +} + +// UpdateMember updates the deployment status wrt the given member. +func (ac *actionContext) UpdateMember(member api.MemberStatus) error { + d := ac.deployment + _, group, found := ac.deployment.status.Members.ElementByID(member.ID) + if !found { + return maskAny(fmt.Errorf("Member %s not found", member.ID)) + } + d.status.Members.UpdateMemberStatus(member, group) + if err := d.updateCRStatus(); err != nil { + log.Debug().Err(err).Msg("Updating CR status failed") + return maskAny(err) + } + return nil +} + +// RemoveMemberByID removes a member with given id. +func (ac *actionContext) RemoveMemberByID(id string) error { + d := ac.deployment + _, group, found := d.status.Members.ElementByID(id) + if !found { + return nil + } + if err := d.status.Members.RemoveByID(id, group); err != nil { + log.Debug().Err(err).Str("group", group.AsRole()).Msg("Failed to remove member") + return maskAny(err) + } + // Save removed member + if err := d.updateCRStatus(); err != nil { + return maskAny(err) + } + return nil +} + +// DeletePod deletes a pod with given name in the namespace +// of the deployment. If the pod does not exist, the error is ignored. +func (ac *actionContext) DeletePod(podName string) error { + d := ac.deployment + ns := d.apiObject.GetNamespace() + if err := d.deps.KubeCli.Core().Pods(ns).Delete(podName, &metav1.DeleteOptions{}); err != nil && !k8sutil.IsNotFound(err) { + log.Debug().Err(err).Str("pod", podName).Msg("Failed to remove pod") + return maskAny(err) + } + return nil +} + +// DeletePvc deletes a persistent volume claim with given name in the namespace +// of the deployment. If the pvc does not exist, the error is ignored. +func (ac *actionContext) DeletePvc(pvcName string) error { + d := ac.deployment + ns := d.apiObject.GetNamespace() + if err := d.deps.KubeCli.Core().PersistentVolumeClaims(ns).Delete(pvcName, &metav1.DeleteOptions{}); err != nil && !k8sutil.IsNotFound(err) { + log.Debug().Err(err).Str("pvc", pvcName).Msg("Failed to remove pvc") + return maskAny(err) + } + return nil +} diff --git a/pkg/deployment/action_remove_member.go b/pkg/deployment/action_remove_member.go new file mode 100644 index 000000000..127f6fda5 --- /dev/null +++ b/pkg/deployment/action_remove_member.go @@ -0,0 +1,80 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 Ewout Prangsma +// + +package deployment + +import ( + "context" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/rs/zerolog" +) + +// NewRemoveMemberAction creates a new Action that implements the given +// planned RemoveMember action. +func NewRemoveMemberAction(log zerolog.Logger, action api.Action, actionCtx ActionContext) Action { + return &actionRemoveMember{ + log: log, + action: action, + actionCtx: actionCtx, + } +} + +// actionRemoveMember implements an RemoveMemberAction. +type actionRemoveMember struct { + log zerolog.Logger + action api.Action + actionCtx ActionContext +} + +// Start performs the start of the action. +// Returns true if the action is completely finished, false in case +// the start time needs to be recorded and a ready condition needs to be checked. +func (a *actionRemoveMember) Start(ctx context.Context) (bool, error) { + m, ok := a.actionCtx.GetMemberStatusByID(a.action.MemberID) + if !ok { + // We wanted to remove and it is already gone. All ok + return true, nil + } + // Remove the pod (if any) + if err := a.actionCtx.DeletePod(m.PodName); err != nil { + return false, maskAny(err) + } + // Remove the pvc (if any) + if m.PersistentVolumeClaimName != "" { + if err := a.actionCtx.DeletePvc(m.PersistentVolumeClaimName); err != nil { + return false, maskAny(err) + } + } + // Remove member + if err := a.actionCtx.RemoveMemberByID(a.action.MemberID); err != nil { + return false, maskAny(err) + } + return true, nil +} + +// CheckProgress checks the progress of the action. +// Returns true if the action is completely finished, false otherwise. +func (a *actionRemoveMember) CheckProgress(ctx context.Context) (bool, error) { + // Nothing todo + return true, nil +} diff --git a/pkg/deployment/action_rotate_member.go b/pkg/deployment/action_rotate_member.go new file mode 100644 index 000000000..91b40bb86 --- /dev/null +++ b/pkg/deployment/action_rotate_member.go @@ -0,0 +1,116 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 Ewout Prangsma +// + +package deployment + +import ( + "context" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/rs/zerolog" +) + +// NewRotateMemberAction creates a new Action that implements the given +// planned RotateMember action. +func NewRotateMemberAction(log zerolog.Logger, action api.Action, actionCtx ActionContext) Action { + return &actionRotateMember{ + log: log, + action: action, + actionCtx: actionCtx, + } +} + +// actionRotateMember implements an RotateMember. +type actionRotateMember struct { + log zerolog.Logger + action api.Action + actionCtx ActionContext +} + +// Start performs the start of the action. +// Returns true if the action is completely finished, false in case +// the start time needs to be recorded and a ready condition needs to be checked. +func (a *actionRotateMember) Start(ctx context.Context) (bool, error) { + log := a.log + group := a.action.Group + m, ok := a.actionCtx.GetMemberStatusByID(a.action.MemberID) + if !ok { + log.Error().Msg("No such member") + return true, nil + } + if group.IsArangod() { + // Invoke shutdown endpoint + c, err := a.actionCtx.GetServerClient(ctx, group, a.action.MemberID) + if err != nil { + log.Debug().Err(err).Msg("Failed to create member client") + return false, maskAny(err) + } + removeFromCluster := false + log.Debug().Bool("removeFromCluster", removeFromCluster).Msg("Shutting down member") + if err := c.Shutdown(ctx, removeFromCluster); err != nil { + // Shutdown failed. Let's check if we're already done + if ready, err := a.CheckProgress(ctx); err == nil && ready { + // We're done + return true, nil + } + log.Debug().Err(err).Msg("Failed to shutdown member") + return false, maskAny(err) + } + } else if group.IsArangosync() { + // Terminate pod + if err := a.actionCtx.DeletePod(m.PodName); err != nil { + return false, maskAny(err) + } + } + // Update status + m.State = api.MemberStateRotating + if err := a.actionCtx.UpdateMember(m); err != nil { + return false, maskAny(err) + } + return true, nil +} + +// CheckProgress checks the progress of the action. +// Returns true if the action is completely finished, false otherwise. +func (a *actionRotateMember) CheckProgress(ctx context.Context) (bool, error) { + // Check that pod is removed + log := a.log + m, found := a.actionCtx.GetMemberStatusByID(a.action.MemberID) + if !found { + log.Error().Msg("No such member") + return true, nil + } + if !m.Conditions.IsTrue(api.ConditionTypeTerminated) { + // Pod is not yet terminated + return false, nil + } + // Pod is terminated, we can now remove it + if err := a.actionCtx.DeletePod(m.PodName); err != nil { + return false, maskAny(err) + } + // Pod is now gone, update the member status + m.State = api.MemberStateNone + if err := a.actionCtx.UpdateMember(m); err != nil { + return false, maskAny(err) + } + return true, nil +} diff --git a/pkg/deployment/action_shutdown_member.go b/pkg/deployment/action_shutdown_member.go new file mode 100644 index 000000000..6612a89ab --- /dev/null +++ b/pkg/deployment/action_shutdown_member.go @@ -0,0 +1,106 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 Ewout Prangsma +// + +package deployment + +import ( + "context" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/rs/zerolog" +) + +// NewShutdownMemberAction creates a new Action that implements the given +// planned ShutdownMember action. +func NewShutdownMemberAction(log zerolog.Logger, action api.Action, actionCtx ActionContext) Action { + return &actionShutdownMember{ + log: log, + action: action, + actionCtx: actionCtx, + } +} + +// actionShutdownMember implements an ShutdownMemberAction. +type actionShutdownMember struct { + log zerolog.Logger + action api.Action + actionCtx ActionContext +} + +// Start performs the start of the action. +// Returns true if the action is completely finished, false in case +// the start time needs to be recorded and a ready condition needs to be checked. +func (a *actionShutdownMember) Start(ctx context.Context) (bool, error) { + log := a.log + group := a.action.Group + m, ok := a.actionCtx.GetMemberStatusByID(a.action.MemberID) + if !ok { + log.Error().Msg("No such member") + return true, nil + } + if group.IsArangod() { + // Invoke shutdown endpoint + c, err := a.actionCtx.GetServerClient(ctx, group, a.action.MemberID) + if err != nil { + log.Debug().Err(err).Msg("Failed to create member client") + return false, maskAny(err) + } + removeFromCluster := true + log.Debug().Bool("removeFromCluster", removeFromCluster).Msg("Shutting down member") + if err := c.Shutdown(ctx, removeFromCluster); err != nil { + // Shutdown failed. Let's check if we're already done + if ready, err := a.CheckProgress(ctx); err == nil && ready { + // We're done + return true, nil + } + log.Debug().Err(err).Msg("Failed to shutdown member") + return false, maskAny(err) + } + } else if group.IsArangosync() { + // Terminate pod + if err := a.actionCtx.DeletePod(m.PodName); err != nil { + return false, maskAny(err) + } + } + // Update status + m.State = api.MemberStateShuttingDown + if err := a.actionCtx.UpdateMember(m); err != nil { + return false, maskAny(err) + } + return true, nil +} + +// CheckProgress checks the progress of the action. +// Returns true if the action is completely finished, false otherwise. +func (a *actionShutdownMember) CheckProgress(ctx context.Context) (bool, error) { + m, found := a.actionCtx.GetMemberStatusByID(a.action.MemberID) + if !found { + // Member not long exists + return true, nil + } + if m.Conditions.IsTrue(api.ConditionTypeTerminated) { + // Shutdown completed + return true, nil + } + // Member still not shutdown, retry soon + return false, nil +} diff --git a/pkg/deployment/plan_builder.go b/pkg/deployment/plan_builder.go index de9cfcbb4..0fe0aff19 100644 --- a/pkg/deployment/plan_builder.go +++ b/pkg/deployment/plan_builder.go @@ -24,15 +24,31 @@ package deployment import ( api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" "github.com/rs/zerolog" + "github.com/rs/zerolog/log" + "k8s.io/api/core/v1" ) // createPlan considers the current specification & status of the deployment creates a plan to // get the status in line with the specification. // If a plan already exists, nothing is done. func (d *Deployment) createPlan() error { + // Get all current pods + pods, err := d.deps.KubeCli.CoreV1().Pods(d.apiObject.GetNamespace()).List(k8sutil.DeploymentListOpt(d.apiObject.GetName())) + if err != nil { + log.Debug().Err(err).Msg("Failed to list pods") + return maskAny(err) + } + myPods := make([]v1.Pod, 0, len(pods.Items)) + for _, p := range pods.Items { + if d.isOwnerOf(&p) { + myPods = append(myPods, p) + } + } + // Create plan - newPlan, changed := createPlan(d.deps.Log, d.status.Plan, d.apiObject.Spec, d.status) + newPlan, changed := createPlan(d.deps.Log, d.status.Plan, d.apiObject.Spec, d.status, myPods) // If not change, we're done if !changed { @@ -54,7 +70,7 @@ func (d *Deployment) createPlan() error { // createPlan considers the given specification & status and creates a plan to get the status in line with the specification. // If a plan already exists, the given plan is returned with false. // Otherwise the new plan is returned with a boolean true. -func createPlan(log zerolog.Logger, currentPlan api.Plan, spec api.DeploymentSpec, status api.DeploymentStatus) (api.Plan, bool) { +func createPlan(log zerolog.Logger, currentPlan api.Plan, spec api.DeploymentSpec, status api.DeploymentStatus, pods []v1.Pod) (api.Plan, bool) { if len(currentPlan) > 0 { // Plan already exists, complete that first return currentPlan, false @@ -78,10 +94,56 @@ func createPlan(log zerolog.Logger, currentPlan api.Plan, spec api.DeploymentSpe plan = append(plan, createScalePlan(log, status.Members.SyncWorkers, api.ServerGroupSyncWorkers, spec.SyncWorkers.Count)...) } + // Check for the need to rotate one or more members + getPod := func(podName string) *v1.Pod { + for _, p := range pods { + if p.GetName() == podName { + return &p + } + } + return nil + } + status.Members.ForeachServerGroup(func(group api.ServerGroup, members *api.MemberStatusList) error { + for _, m := range *members { + if len(plan) > 0 { + // Only 1 change at a time + continue + } + if podName := m.PodName; podName != "" { + if p := getPod(podName); p != nil { + // Got pod, compare it with what it should be + if podNeedsRotation(*p, spec) { + plan = append(plan, createRotateMemberPlan(log, m, group)...) + } + } + } + } + return nil + }) + // Return plan return plan, true } +// podNeedsRotation returns true when the specification of the +// given pod differs from what it should be according to the +// given deployment spec. +func podNeedsRotation(p v1.Pod, spec api.DeploymentSpec) bool { + // Check number of containers + if len(p.Spec.Containers) != 1 { + return true + } + // Check image + c := p.Spec.Containers[0] + if c.Image != spec.Image || c.ImagePullPolicy != spec.ImagePullPolicy { + return true + } + // Check arguments + // TODO + + return false +} + // createScalePlan creates a scaling plan for a single server group func createScalePlan(log zerolog.Logger, members api.MemberStatusList, group api.ServerGroup, count int) api.Plan { var plan api.Plan @@ -89,7 +151,7 @@ func createScalePlan(log zerolog.Logger, members api.MemberStatusList, group api // Scale up toAdd := count - len(members) for i := 0; i < toAdd; i++ { - plan = append(plan, api.Action{Type: api.ActionTypeAddMember, Group: group}) + plan = append(plan, api.NewAction(api.ActionTypeAddMember, group, "")) } log.Debug(). Int("delta", toAdd). @@ -100,12 +162,12 @@ func createScalePlan(log zerolog.Logger, members api.MemberStatusList, group api if m, err := members.SelectMemberToRemove(); err == nil { if group == api.ServerGroupDBServers { plan = append(plan, - api.Action{Type: api.ActionTypeCleanOutMember, Group: group, MemberID: m.ID}, + api.NewAction(api.ActionTypeCleanOutMember, group, m.ID), ) } plan = append(plan, - api.Action{Type: api.ActionTypeShutdownMember, Group: group, MemberID: m.ID}, - api.Action{Type: api.ActionTypeRemoveMember, Group: group, MemberID: m.ID}, + api.NewAction(api.ActionTypeShutdownMember, group, m.ID), + api.NewAction(api.ActionTypeRemoveMember, group, m.ID), ) log.Debug(). Str("role", group.AsRole()). @@ -114,3 +176,12 @@ func createScalePlan(log zerolog.Logger, members api.MemberStatusList, group api } return plan } + +// createRotateMemberPlan creates a plan to rotate (stop-recreate-start) an existing +// member. +func createRotateMemberPlan(log zerolog.Logger, member api.MemberStatus, group api.ServerGroup) api.Plan { + plan := api.Plan{ + api.NewAction(api.ActionTypeRotateMember, group, member.ID), + } + return plan +} diff --git a/pkg/deployment/plan_executor.go b/pkg/deployment/plan_executor.go index 884bbe123..d850a1af3 100644 --- a/pkg/deployment/plan_executor.go +++ b/pkg/deployment/plan_executor.go @@ -29,7 +29,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" - "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" ) // executePlan tries to execute the plan as far as possible. @@ -45,13 +44,14 @@ func (d *Deployment) executePlan(ctx context.Context) (bool, error) { } // Take first action - action := d.status.Plan[0] - if action.StartTime.IsZero() { + planAction := d.status.Plan[0] + log := log.With().Str("action-type", string(planAction.Type)).Logger() + action := d.createAction(ctx, planAction) + if planAction.StartTime.IsZero() { // Not started yet - ready, err := d.startAction(ctx, action) + ready, err := action.Start(ctx) if err != nil { log.Debug().Err(err). - Str("action-type", string(action.Type)). Msg("Failed to start action") return false, maskAny(err) } @@ -60,13 +60,14 @@ func (d *Deployment) executePlan(ctx context.Context) (bool, error) { d.status.Plan = d.status.Plan[1:] } else { // Mark start time - now := metav1.Now() - d.status.Plan[0].StartTime = &now + d.status.Plan[0].StartTime = metav1.Now() } // Save plan update if err := d.updateCRStatus(); err != nil { + log.Debug().Err(err).Msg("Failed to update CR status") return false, maskAny(err) } + log.Debug().Bool("ready", ready).Msg("Start completed") if !ready { // We need to check back soon return true, nil @@ -74,10 +75,9 @@ func (d *Deployment) executePlan(ctx context.Context) (bool, error) { // Continue with next action } else { // First action of plan has been started, check its progress - ready, err := d.checkActionProgress(ctx, action) + ready, err := action.CheckProgress(ctx) if err != nil { log.Debug().Err(err). - Str("action-type", string(action.Type)). Msg("Failed to check action progress") return false, maskAny(err) } @@ -99,152 +99,24 @@ func (d *Deployment) executePlan(ctx context.Context) (bool, error) { // startAction performs the start of the given action // Returns true if the action is completely finished, false in case // the start time needs to be recorded and a ready condition needs to be checked. -func (d *Deployment) startAction(ctx context.Context, action api.Action) (bool, error) { - log := d.deps.Log - ns := d.apiObject.GetNamespace() - +func (d *Deployment) createAction(ctx context.Context, action api.Action) Action { + log := d.deps.Log.With(). + Str("group", action.Group.AsRole()). + Str("id", action.MemberID). + Logger() + actionCtx := NewActionContext(log, d) switch action.Type { case api.ActionTypeAddMember: - if err := d.createMember(action.Group, d.apiObject); err != nil { - log.Debug().Err(err).Str("group", action.Group.AsRole()).Msg("Failed to create member") - return false, maskAny(err) - } - // Save added member - if err := d.updateCRStatus(); err != nil { - return false, maskAny(err) - } - return true, nil + return NewAddMemberAction(log, action, actionCtx) case api.ActionTypeRemoveMember: - m, _, ok := d.status.Members.ElementByID(action.MemberID) - if !ok { - // We wanted to remove and it is already gone. All ok - return true, nil - } - // Remove the pod (if any) - if err := d.deps.KubeCli.Core().Pods(ns).Delete(m.PodName, &metav1.DeleteOptions{}); err != nil && !k8sutil.IsNotFound(err) { - log.Debug().Err(err).Str("pod", m.PodName).Msg("Failed to remove pod") - return false, maskAny(err) - } - // Remove the pvc (if any) - if m.PersistentVolumeClaimName != "" { - if err := d.deps.KubeCli.Core().PersistentVolumeClaims(ns).Delete(m.PersistentVolumeClaimName, &metav1.DeleteOptions{}); err != nil && !k8sutil.IsNotFound(err) { - log.Debug().Err(err).Str("pod", m.PodName).Msg("Failed to remove pvc") - return false, maskAny(err) - } - } - // Remove member - if err := d.status.Members.RemoveByID(action.MemberID, action.Group); err != nil { - log.Debug().Err(err).Str("group", action.Group.AsRole()).Msg("Failed to remove member") - return false, maskAny(err) - } - // Save removed member - if err := d.updateCRStatus(); err != nil { - return false, maskAny(err) - } - return true, nil + return NewRemoveMemberAction(log, action, actionCtx) case api.ActionTypeCleanOutMember: - m, ok := d.status.Members.DBServers.ElementByID(action.MemberID) - if !ok { - log.Error().Str("group", action.Group.AsRole()).Str("id", action.MemberID).Msg("No such member") - return true, nil - } - c, err := d.clientCache.GetDatabase(ctx) - if err != nil { - log.Debug().Err(err).Str("group", action.Group.AsRole()).Msg("Failed to create member client") - return false, maskAny(err) - } - cluster, err := c.Cluster(ctx) - if err != nil { - log.Debug().Err(err).Str("group", action.Group.AsRole()).Msg("Failed to access cluster") - return false, maskAny(err) - } - if err := cluster.CleanOutServer(ctx, action.MemberID); err != nil { - log.Debug().Err(err).Str("group", action.Group.AsRole()).Msg("Failed to cleanout member") - return false, maskAny(err) - } - // Update status - m.State = api.MemberStateCleanOut - if err := d.updateCRStatus(); err != nil { - return false, maskAny(err) - } - return true, nil + return NewCleanOutMemberAction(log, action, actionCtx) case api.ActionTypeShutdownMember: - m, _, ok := d.status.Members.ElementByID(action.MemberID) - if !ok { - log.Error().Str("group", action.Group.AsRole()).Str("id", action.MemberID).Msg("No such member") - return true, nil - } - if action.Group.IsArangod() { - // Invoke shutdown endpoint - c, err := d.clientCache.Get(ctx, action.Group, action.MemberID) - if err != nil { - log.Debug().Err(err).Str("group", action.Group.AsRole()).Msg("Failed to create member client") - return false, maskAny(err) - } - if err := c.Shutdown(ctx, true); err != nil { - log.Debug().Err(err).Str("group", action.Group.AsRole()).Msg("Failed to shutdown member") - return false, maskAny(err) - } - } else if action.Group.IsArangosync() { - // Terminate pod - if err := d.deps.KubeCli.Core().Pods(ns).Delete(m.PodName, &metav1.DeleteOptions{}); err != nil && !k8sutil.IsNotFound(err) { - log.Debug().Err(err).Str("pod", m.PodName).Msg("Failed to remove pod") - return false, maskAny(err) - } - } - // Update status - m.State = api.MemberStateShuttingDown - if err := d.updateCRStatus(); err != nil { - return false, maskAny(err) - } - return true, nil + return NewShutdownMemberAction(log, action, actionCtx) + case api.ActionTypeRotateMember: + return NewRotateMemberAction(log, action, actionCtx) default: - return false, maskAny(fmt.Errorf("Unknown action type")) - } -} - -// checkActionProgress checks the progress of the given action. -// Returns true if the action is completely finished, false otherwise. -func (d *Deployment) checkActionProgress(ctx context.Context, action api.Action) (bool, error) { - switch action.Type { - case api.ActionTypeAddMember: - // Nothing todo - return true, nil - case api.ActionTypeRemoveMember: - // Nothing todo - return true, nil - case api.ActionTypeCleanOutMember: - c, err := d.clientCache.GetDatabase(ctx) - if err != nil { - return false, maskAny(err) - } - cluster, err := c.Cluster(ctx) - if err != nil { - return false, maskAny(err) - } - cleanedOut, err := cluster.IsCleanedOut(ctx, action.MemberID) - if err != nil { - return false, maskAny(err) - } - if !cleanedOut { - // We're not done yet - return false, nil - } - // Cleanout completed - return true, nil - case api.ActionTypeShutdownMember: - m, _, ok := d.status.Members.ElementByID(action.MemberID) - if !ok { - // Member not long exists - return true, nil - } - if m.Conditions.IsTrue(api.ConditionTypeTerminated) { - // Shutdown completed - return true, nil - } - // Member still not shutdown, retry soon - return false, nil - default: - return false, maskAny(fmt.Errorf("Unknown action type")) + panic(fmt.Sprintf("Unknown action type '%s'", action.Type)) } } diff --git a/pkg/deployment/pod_inspector.go b/pkg/deployment/pod_inspector.go index 20d03845e..bc591fe13 100644 --- a/pkg/deployment/pod_inspector.go +++ b/pkg/deployment/pod_inspector.go @@ -113,7 +113,7 @@ func (d *Deployment) inspectPods() error { switch m.State { case api.MemberStateNone: // Do nothing - case api.MemberStateShuttingDown: + case api.MemberStateShuttingDown, api.MemberStateRotating: // Shutdown was intended, so not need to do anything here. // Just mark terminated if m.Conditions.Update(api.ConditionTypeTerminated, true, "Pod Terminated", "") { From 610360963257d8ab12c979797ceb46e9fdd9a704 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Thu, 15 Mar 2018 13:33:49 +0100 Subject: [PATCH 02/17] Added wait-for-member-up action --- pkg/apis/deployment/v1alpha/plan.go | 2 + pkg/deployment/action_context.go | 7 ++ pkg/deployment/action_wait_for_member_up.go | 127 ++++++++++++++++++++ pkg/deployment/deployment.go | 11 +- pkg/deployment/plan_builder.go | 9 ++ pkg/deployment/plan_executor.go | 4 +- 6 files changed, 155 insertions(+), 5 deletions(-) create mode 100644 pkg/deployment/action_wait_for_member_up.go diff --git a/pkg/apis/deployment/v1alpha/plan.go b/pkg/apis/deployment/v1alpha/plan.go index 0df198b96..64f6e3ebe 100644 --- a/pkg/apis/deployment/v1alpha/plan.go +++ b/pkg/apis/deployment/v1alpha/plan.go @@ -38,6 +38,8 @@ const ( ActionTypeShutdownMember ActionType = "ShutdownMember" // ActionTypeRotateMember causes a member to be shutdown and have it's pod removed. ActionTypeRotateMember ActionType = "RotateMember" + // ActionTypeWaitForMemberUp causes the plan to wait until the member is considered "up". + ActionTypeWaitForMemberUp ActionType = "WaitForMemberUp" ) // Action represents a single action to be taken to update a deployment. diff --git a/pkg/deployment/action_context.go b/pkg/deployment/action_context.go index d7d0f87b9..3b075d905 100644 --- a/pkg/deployment/action_context.go +++ b/pkg/deployment/action_context.go @@ -37,6 +37,8 @@ import ( // ActionContext provides methods to the Action implementations // to control their context. type ActionContext interface { + // Gets the specified mode of deployment + GetMode() api.DeploymentMode // GetDatabaseClient returns a cached client for the entire database (cluster coordinators or single server), // creating one if needed. GetDatabaseClient(ctx context.Context) (driver.Client, error) @@ -75,6 +77,11 @@ type actionContext struct { deployment *Deployment } +// Gets the specified mode of deployment +func (ac *actionContext) GetMode() api.DeploymentMode { + return ac.deployment.apiObject.Spec.Mode +} + // GetDatabaseClient returns a cached client for the entire database (cluster coordinators or single server), // creating one if needed. func (ac *actionContext) GetDatabaseClient(ctx context.Context) (driver.Client, error) { diff --git a/pkg/deployment/action_wait_for_member_up.go b/pkg/deployment/action_wait_for_member_up.go new file mode 100644 index 000000000..9ea9f5499 --- /dev/null +++ b/pkg/deployment/action_wait_for_member_up.go @@ -0,0 +1,127 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 Ewout Prangsma +// + +package deployment + +import ( + "context" + + driver "github.com/arangodb/go-driver" + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/rs/zerolog" +) + +// NewWaitForMemberUpAction creates a new Action that implements the given +// planned WaitForMemberUp action. +func NewWaitForMemberUpAction(log zerolog.Logger, action api.Action, actionCtx ActionContext) Action { + return &actionWaitForMemberUp{ + log: log, + action: action, + actionCtx: actionCtx, + } +} + +// actionWaitForMemberUp implements an WaitForMemberUp. +type actionWaitForMemberUp struct { + log zerolog.Logger + action api.Action + actionCtx ActionContext +} + +// Start performs the start of the action. +// Returns true if the action is completely finished, false in case +// the start time needs to be recorded and a ready condition needs to be checked. +func (a *actionWaitForMemberUp) Start(ctx context.Context) (bool, error) { + ready, err := a.CheckProgress(ctx) + if err != nil { + return false, maskAny(err) + } + return ready, nil +} + +// CheckProgress checks the progress of the action. +// Returns true if the action is completely finished, false otherwise. +func (a *actionWaitForMemberUp) CheckProgress(ctx context.Context) (bool, error) { + if a.action.Group.IsArangosync() { + return a.checkProgressArangoSync(ctx) + } + switch a.actionCtx.GetMode() { + case api.DeploymentModeSingle: + return a.checkProgressSingle(ctx) + default: + return a.checkProgressCluster(ctx) + } +} + +// checkProgressSingle checks the progress of the action in the case +// of a single server. +func (a *actionWaitForMemberUp) checkProgressSingle(ctx context.Context) (bool, error) { + log := a.log + c, err := a.actionCtx.GetDatabaseClient(ctx) + if err != nil { + log.Debug().Err(err).Msg("Failed to create database client") + return false, maskAny(err) + } + if _, err := c.Version(ctx); err != nil { + log.Debug().Err(err).Msg("Failed to get version") + return false, maskAny(err) + } + return true, nil +} + +// checkProgressCluster checks the progress of the action in the case +// of a cluster deployment. +func (a *actionWaitForMemberUp) checkProgressCluster(ctx context.Context) (bool, error) { + log := a.log + c, err := a.actionCtx.GetDatabaseClient(ctx) + if err != nil { + log.Debug().Err(err).Msg("Failed to create database client") + return false, maskAny(err) + } + cluster, err := c.Cluster(ctx) + if err != nil { + log.Debug().Err(err).Msg("Failed to access cluster") + return false, maskAny(err) + } + h, err := cluster.Health(ctx) + if err != nil { + log.Debug().Err(err).Msg("Failed to get cluster health") + return false, maskAny(err) + } + sh, found := h.Health[driver.ServerID(a.action.MemberID)] + if !found { + log.Debug().Msg("Member not yet found in cluster health") + return false, nil + } + if sh.Status != driver.ServerStatusGood { + log.Debug().Str("status", string(sh.Status)).Msg("Member set status not yet good") + return false, nil + } + return true, nil +} + +// checkProgressArangoSync checks the progress of the action in the case +// of a sync master / worker. +func (a *actionWaitForMemberUp) checkProgressArangoSync(ctx context.Context) (bool, error) { + // TODO + return true, nil +} diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go index c2856b119..13a032813 100644 --- a/pkg/deployment/deployment.go +++ b/pkg/deployment/deployment.go @@ -321,10 +321,13 @@ func (d *Deployment) createEvent(evt *v1.Event) { } // Update the status of the API object from the internal status -func (d *Deployment) updateCRStatus() error { - if reflect.DeepEqual(d.apiObject.Status, d.status) { - // Nothing has changed - return nil +func (d *Deployment) updateCRStatus(force ...bool) error { + // TODO Remove force.... + if len(force) == 0 || !force[0] { + if reflect.DeepEqual(d.apiObject.Status, d.status) { + // Nothing has changed + return nil + } } // Send update to API server diff --git a/pkg/deployment/plan_builder.go b/pkg/deployment/plan_builder.go index 0fe0aff19..b67f61b2d 100644 --- a/pkg/deployment/plan_builder.go +++ b/pkg/deployment/plan_builder.go @@ -109,6 +109,10 @@ func createPlan(log zerolog.Logger, currentPlan api.Plan, spec api.DeploymentSpe // Only 1 change at a time continue } + if m.State != api.MemberStateCreated { + // Only rotate when state is created + continue + } if podName := m.PodName; podName != "" { if p := getPod(podName); p != nil { // Got pod, compare it with what it should be @@ -180,8 +184,13 @@ func createScalePlan(log zerolog.Logger, members api.MemberStatusList, group api // createRotateMemberPlan creates a plan to rotate (stop-recreate-start) an existing // member. func createRotateMemberPlan(log zerolog.Logger, member api.MemberStatus, group api.ServerGroup) api.Plan { + log.Debug(). + Str("id", member.ID). + Str("role", group.AsRole()). + Msg("Creating rotation plan") plan := api.Plan{ api.NewAction(api.ActionTypeRotateMember, group, member.ID), + api.NewAction(api.ActionTypeWaitForMemberUp, group, member.ID), } return plan } diff --git a/pkg/deployment/plan_executor.go b/pkg/deployment/plan_executor.go index d850a1af3..9799250c4 100644 --- a/pkg/deployment/plan_executor.go +++ b/pkg/deployment/plan_executor.go @@ -63,7 +63,7 @@ func (d *Deployment) executePlan(ctx context.Context) (bool, error) { d.status.Plan[0].StartTime = metav1.Now() } // Save plan update - if err := d.updateCRStatus(); err != nil { + if err := d.updateCRStatus(true); err != nil { log.Debug().Err(err).Msg("Failed to update CR status") return false, maskAny(err) } @@ -116,6 +116,8 @@ func (d *Deployment) createAction(ctx context.Context, action api.Action) Action return NewShutdownMemberAction(log, action, actionCtx) case api.ActionTypeRotateMember: return NewRotateMemberAction(log, action, actionCtx) + case api.ActionTypeWaitForMemberUp: + return NewWaitForMemberUpAction(log, action, actionCtx) default: panic(fmt.Sprintf("Unknown action type '%s'", action.Type)) } From 7187bd72bfc3a83c0105eb088c4cf7de6ee08942 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Thu, 15 Mar 2018 15:01:39 +0100 Subject: [PATCH 03/17] Various improvements --- pkg/deployment/action_cleanout_member.go | 2 +- pkg/deployment/action_rotate_member.go | 2 +- pkg/deployment/action_shutdown_member.go | 2 +- pkg/deployment/action_wait_for_member_up.go | 21 +++++++++++++++- pkg/deployment/members.go | 18 ++++++++++++- pkg/util/k8sutil/names.go | 14 ++++++++++- pkg/util/k8sutil/pods.go | 28 +++++++++++++++++++-- pkg/util/k8sutil/pvc.go | 2 +- 8 files changed, 80 insertions(+), 9 deletions(-) diff --git a/pkg/deployment/action_cleanout_member.go b/pkg/deployment/action_cleanout_member.go index 24e47db5e..d386fcc88 100644 --- a/pkg/deployment/action_cleanout_member.go +++ b/pkg/deployment/action_cleanout_member.go @@ -75,7 +75,7 @@ func (a *actionCleanoutMember) Start(ctx context.Context) (bool, error) { if a.actionCtx.UpdateMember(m); err != nil { return false, maskAny(err) } - return true, nil + return false, nil } // CheckProgress checks the progress of the action. diff --git a/pkg/deployment/action_rotate_member.go b/pkg/deployment/action_rotate_member.go index 91b40bb86..df9ba3fce 100644 --- a/pkg/deployment/action_rotate_member.go +++ b/pkg/deployment/action_rotate_member.go @@ -86,7 +86,7 @@ func (a *actionRotateMember) Start(ctx context.Context) (bool, error) { if err := a.actionCtx.UpdateMember(m); err != nil { return false, maskAny(err) } - return true, nil + return false, nil } // CheckProgress checks the progress of the action. diff --git a/pkg/deployment/action_shutdown_member.go b/pkg/deployment/action_shutdown_member.go index 6612a89ab..586bef644 100644 --- a/pkg/deployment/action_shutdown_member.go +++ b/pkg/deployment/action_shutdown_member.go @@ -86,7 +86,7 @@ func (a *actionShutdownMember) Start(ctx context.Context) (bool, error) { if err := a.actionCtx.UpdateMember(m); err != nil { return false, maskAny(err) } - return true, nil + return false, nil } // CheckProgress checks the progress of the action. diff --git a/pkg/deployment/action_wait_for_member_up.go b/pkg/deployment/action_wait_for_member_up.go index 9ea9f5499..1b2ed9a40 100644 --- a/pkg/deployment/action_wait_for_member_up.go +++ b/pkg/deployment/action_wait_for_member_up.go @@ -68,6 +68,9 @@ func (a *actionWaitForMemberUp) CheckProgress(ctx context.Context) (bool, error) case api.DeploymentModeSingle: return a.checkProgressSingle(ctx) default: + if a.action.Group == api.ServerGroupAgents { + return a.checkProgressAgent(ctx) + } return a.checkProgressCluster(ctx) } } @@ -88,8 +91,24 @@ func (a *actionWaitForMemberUp) checkProgressSingle(ctx context.Context) (bool, return true, nil } +// checkProgressAgent checks the progress of the action in the case +// of an agent. +func (a *actionWaitForMemberUp) checkProgressAgent(ctx context.Context) (bool, error) { + log := a.log + c, err := a.actionCtx.GetDatabaseClient(ctx) + if err != nil { + log.Debug().Err(err).Msg("Failed to create database client") + return false, maskAny(err) + } + if _, err := c.Version(ctx); err != nil { + log.Debug().Err(err).Msg("Failed to get version") + return false, maskAny(err) + } + return true, nil +} + // checkProgressCluster checks the progress of the action in the case -// of a cluster deployment. +// of a cluster deployment (coordinator/dbserver). func (a *actionWaitForMemberUp) checkProgressCluster(ctx context.Context) (bool, error) { log := a.log c, err := a.actionCtx.GetDatabaseClient(ctx) diff --git a/pkg/deployment/members.go b/pkg/deployment/members.go index 6876e8797..537bf2f79 100644 --- a/pkg/deployment/members.go +++ b/pkg/deployment/members.go @@ -65,8 +65,9 @@ func (d *Deployment) createInitialMembers(apiObject *api.ArangoDeployment) error func (d *Deployment) createMember(group api.ServerGroup, apiObject *api.ArangoDeployment) error { log := d.deps.Log var id string + idPrefix := getArangodIDPrefix(group) for { - id = strings.ToLower(uniuri.NewLen(8)) // K8s accepts only lowercase, so we use it here as well + id = idPrefix + strings.ToLower(uniuri.NewLen(8)) // K8s accepts only lowercase, so we use it here as well if !d.status.Members.ContainsID(id) { break } @@ -142,3 +143,18 @@ func (d *Deployment) createMember(group api.ServerGroup, apiObject *api.ArangoDe return nil } + +// getArangodIDPrefix returns the prefix required ID's of arangod servers +// in the given group. +func getArangodIDPrefix(group api.ServerGroup) string { + switch group { + case api.ServerGroupCoordinators: + return "CRDN-" + case api.ServerGroupDBServers: + return "PRMR-" + case api.ServerGroupAgents: + return "AGNT-" + default: + return "" + } +} diff --git a/pkg/util/k8sutil/names.go b/pkg/util/k8sutil/names.go index 6474a2988..d75836352 100644 --- a/pkg/util/k8sutil/names.go +++ b/pkg/util/k8sutil/names.go @@ -25,10 +25,12 @@ package k8sutil import ( "fmt" "regexp" + "strings" ) var ( - resourceNameRE = regexp.MustCompile(`^([0-9\-\.a-z])+$`) + resourceNameRE = regexp.MustCompile(`^([0-9\-\.a-z])+$`) + arangodPrefixes = []string{"CRDN-", "PRMR-", "AGNT-"} ) // ValidateOptionalResourceName validates a kubernetes resource name. @@ -55,3 +57,13 @@ func ValidateResourceName(name string) error { } return maskAny(fmt.Errorf("Name '%s' is not a valid resource name", name)) } + +// stripArangodPrefix removes well know arangod ID prefixes from the given id. +func stripArangodPrefix(id string) string { + for _, prefix := range arangodPrefixes { + if strings.HasPrefix(id, prefix) { + return id[len(prefix):] + } + } + return id +} diff --git a/pkg/util/k8sutil/pods.go b/pkg/util/k8sutil/pods.go index e3a33ec89..096a4214e 100644 --- a/pkg/util/k8sutil/pods.go +++ b/pkg/util/k8sutil/pods.go @@ -23,12 +23,16 @@ package k8sutil import ( + "fmt" + "path/filepath" + "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" ) const ( + alpineImage = "alpine" arangodVolumeName = "arangod-data" tlsKeyfileVolumeName = "tls-keyfile" rocksdbEncryptionVolumeName = "rocksdb-encryption" @@ -97,7 +101,7 @@ func getPodCondition(status *v1.PodStatus, condType v1.PodConditionType) *v1.Pod // CreatePodName returns the name of the pod for a member with // a given id in a deployment with a given name. func CreatePodName(deploymentName, role, id string) string { - return deploymentName + "-" + role + "-" + id + return deploymentName + "-" + role + "-" + stripArangodPrefix(id) } // CreateTLSKeyfileSecretName returns the name of the Secret that holds the TLS keyfile for a member with @@ -133,6 +137,23 @@ func rocksdbEncryptionVolumeMounts() []v1.VolumeMount { } } +// arangodInitContainer creates a container configured to +// initalize a UUID file. +func arangodInitContainer(name, id string) v1.Container { + uuidFile := filepath.Join(ArangodVolumeMountDir, "UUID") + c := v1.Container{ + Command: []string{ + "/bin/sh", + "-c", + fmt.Sprintf("test -f %s || echo '%s' > %s", uuidFile, id, uuidFile), + }, + Name: name, + Image: alpineImage, + VolumeMounts: arangodVolumeMounts(), + } + return c +} + // arangodContainer creates a container configured to run `arangod`. func arangodContainer(name string, image string, imagePullPolicy v1.PullPolicy, args []string, env map[string]EnvValue, livenessProbe *HTTPProbeConfig, readinessProbe *HTTPProbeConfig) v1.Container { c := v1.Container{ @@ -216,7 +237,7 @@ func CreateArangodPod(kubecli kubernetes.Interface, developmentMode bool, deploy p := newPod(deployment.GetName(), deployment.GetNamespace(), role, id) // Add arangod container - c := arangodContainer(p.GetName(), image, imagePullPolicy, args, env, livenessProbe, readinessProbe) + c := arangodContainer("server", image, imagePullPolicy, args, env, livenessProbe, readinessProbe) if tlsKeyfileSecretName != "" { c.VolumeMounts = append(c.VolumeMounts, tlsKeyfileVolumeMounts()...) } @@ -225,6 +246,9 @@ func CreateArangodPod(kubecli kubernetes.Interface, developmentMode bool, deploy } p.Spec.Containers = append(p.Spec.Containers, c) + // Add UUID init container + p.Spec.InitContainers = append(p.Spec.InitContainers, arangodInitContainer("uuid", id)) + // Add volume if pvcName != "" { // Create PVC diff --git a/pkg/util/k8sutil/pvc.go b/pkg/util/k8sutil/pvc.go index d5f038186..cfed48981 100644 --- a/pkg/util/k8sutil/pvc.go +++ b/pkg/util/k8sutil/pvc.go @@ -31,7 +31,7 @@ import ( // CreatePersistentVolumeClaimName returns the name of the persistent volume claim for a member with // a given id in a deployment with a given name. func CreatePersistentVolumeClaimName(deploymentName, role, id string) string { - return deploymentName + "-" + role + "-" + id + return deploymentName + "-" + role + "-" + stripArangodPrefix(id) } // CreatePersistentVolumeClaim creates a persistent volume claim with given name and configuration. From 2fc4c92253cfd755a0b5ba9b1e00e9b17bd983fe Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Thu, 15 Mar 2018 16:33:28 +0100 Subject: [PATCH 04/17] Various fixes --- pkg/apis/deployment/v1alpha/plan.go | 10 +- .../v1alpha/zz_generated.deepcopy.go | 11 ++- pkg/deployment/action_rotate_member.go | 2 + pkg/deployment/action_shutdown_member.go | 7 ++ pkg/deployment/deployment.go | 44 +-------- pkg/deployment/deployment_inspector.go | 91 +++++++++++++++++++ pkg/deployment/plan_executor.go | 41 +++++---- pkg/deployment/pod_creator.go | 2 + 8 files changed, 148 insertions(+), 60 deletions(-) create mode 100644 pkg/deployment/deployment_inspector.go diff --git a/pkg/apis/deployment/v1alpha/plan.go b/pkg/apis/deployment/v1alpha/plan.go index 64f6e3ebe..f6bb32198 100644 --- a/pkg/apis/deployment/v1alpha/plan.go +++ b/pkg/apis/deployment/v1alpha/plan.go @@ -22,7 +22,10 @@ package v1alpha -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + "github.com/dchest/uniuri" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // ActionType is a strongly typed name for a plan action item type ActionType string @@ -44,6 +47,8 @@ const ( // Action represents a single action to be taken to update a deployment. type Action struct { + // ID of this action (unique for every action) + ID string `json:"id"` // Type of action. Type ActionType `json:"type"` // ID reference of the member involved in this action (if any) @@ -53,12 +58,13 @@ type Action struct { // CreationTime is set the when the action is created. CreationTime metav1.Time `json:"creationTime"` // StartTime is set the when the action has been started, but needs to wait to be finished. - StartTime metav1.Time `json:"startTime,omitempty"` + StartTime *metav1.Time `json:"startTime,omitempty"` } // NewAction instantiates a new Action. func NewAction(actionType ActionType, group ServerGroup, memberID string) Action { return Action{ + ID: uniuri.New(), Type: actionType, MemberID: memberID, Group: group, diff --git a/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go b/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go index c67e72e67..cecba1f18 100644 --- a/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go +++ b/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go @@ -25,6 +25,7 @@ package v1alpha import ( + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -32,7 +33,15 @@ import ( func (in *Action) DeepCopyInto(out *Action) { *out = *in in.CreationTime.DeepCopyInto(&out.CreationTime) - in.StartTime.DeepCopyInto(&out.StartTime) + if in.StartTime != nil { + in, out := &in.StartTime, &out.StartTime + if *in == nil { + *out = nil + } else { + *out = new(v1.Time) + (*in).DeepCopyInto(*out) + } + } return } diff --git a/pkg/deployment/action_rotate_member.go b/pkg/deployment/action_rotate_member.go index df9ba3fce..a1ac28637 100644 --- a/pkg/deployment/action_rotate_member.go +++ b/pkg/deployment/action_rotate_member.go @@ -66,6 +66,8 @@ func (a *actionRotateMember) Start(ctx context.Context) (bool, error) { } removeFromCluster := false log.Debug().Bool("removeFromCluster", removeFromCluster).Msg("Shutting down member") + ctx, cancel := context.WithTimeout(ctx, shutdownTimeout) + defer cancel() if err := c.Shutdown(ctx, removeFromCluster); err != nil { // Shutdown failed. Let's check if we're already done if ready, err := a.CheckProgress(ctx); err == nil && ready { diff --git a/pkg/deployment/action_shutdown_member.go b/pkg/deployment/action_shutdown_member.go index 586bef644..57dad6f60 100644 --- a/pkg/deployment/action_shutdown_member.go +++ b/pkg/deployment/action_shutdown_member.go @@ -24,11 +24,16 @@ package deployment import ( "context" + "time" api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" "github.com/rs/zerolog" ) +const ( + shutdownTimeout = time.Second * 15 +) + // NewShutdownMemberAction creates a new Action that implements the given // planned ShutdownMember action. func NewShutdownMemberAction(log zerolog.Logger, action api.Action, actionCtx ActionContext) Action { @@ -66,6 +71,8 @@ func (a *actionShutdownMember) Start(ctx context.Context) (bool, error) { } removeFromCluster := true log.Debug().Bool("removeFromCluster", removeFromCluster).Msg("Shutting down member") + ctx, cancel := context.WithTimeout(ctx, shutdownTimeout) + defer cancel() if err := c.Shutdown(ctx, removeFromCluster); err != nil { // Shutdown failed. Let's check if we're already done if ready, err := a.CheckProgress(ctx); err == nil && ready { diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go index 13a032813..328977043 100644 --- a/pkg/deployment/deployment.go +++ b/pkg/deployment/deployment.go @@ -23,7 +23,6 @@ package deployment import ( - "context" "fmt" "reflect" "time" @@ -89,8 +88,9 @@ type Deployment struct { eventsCli corev1.EventInterface - inspectTrigger trigger.Trigger - clientCache *clientCache + inspectTrigger trigger.Trigger + recentInspectionErrors int + clientCache *clientCache } // New creates a new Deployment from the given API object. @@ -191,7 +191,6 @@ func (d *Deployment) run() { } inspectionInterval := maxInspectionInterval - recentInspectionErrors := 0 for { select { case <-d.stopCh: @@ -214,42 +213,7 @@ func (d *Deployment) run() { } case <-d.inspectTrigger.Done(): - hasError := false - ctx := context.Background() - // Inspection of generated resources needed - if err := d.inspectPods(); err != nil { - hasError = true - d.createEvent(k8sutil.NewErrorEvent("Pod inspection failed", err, d.apiObject)) - } - // Create scale/update plan - if err := d.createPlan(); err != nil { - hasError = true - d.createEvent(k8sutil.NewErrorEvent("Plan creation failed", err, d.apiObject)) - } - // Execute current step of scale/update plan - if retrySoon, err := d.executePlan(ctx); err != nil { - hasError = true - d.createEvent(k8sutil.NewErrorEvent("Plan execution failed", err, d.apiObject)) - } else if retrySoon { - inspectionInterval = minInspectionInterval - } - // Ensure all resources are created - if err := d.ensurePVCs(d.apiObject); err != nil { - hasError = true - d.createEvent(k8sutil.NewErrorEvent("PVC creation failed", err, d.apiObject)) - } - if err := d.ensurePods(d.apiObject); err != nil { - hasError = true - d.createEvent(k8sutil.NewErrorEvent("Pod creation failed", err, d.apiObject)) - } - if hasError { - if recentInspectionErrors == 0 { - inspectionInterval = minInspectionInterval - recentInspectionErrors++ - } - } else { - recentInspectionErrors = 0 - } + inspectionInterval = d.inspectDeployment(inspectionInterval) case <-time.After(inspectionInterval): // Trigger inspection diff --git a/pkg/deployment/deployment_inspector.go b/pkg/deployment/deployment_inspector.go new file mode 100644 index 000000000..b0baa299d --- /dev/null +++ b/pkg/deployment/deployment_inspector.go @@ -0,0 +1,91 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 Ewout Prangsma +// + +package deployment + +import ( + "context" + "time" + + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" +) + +// inspectDeployment inspects the entire deployment, creates +// a plan to update if needed and inspects underlying resources. +// This function should be called when: +// - the deployment has changed +// - any of the underlying resources has changed +// - once in a while +// Returns the delay until this function should be called again. +func (d *Deployment) inspectDeployment(lastInterval time.Duration) time.Duration { + // log := d.deps.Log + + nextInterval := lastInterval + hasError := false + ctx := context.Background() + + // Inspection of generated resources needed + if err := d.inspectPods(); err != nil { + hasError = true + d.createEvent(k8sutil.NewErrorEvent("Pod inspection failed", err, d.apiObject)) + } + + // Create scale/update plan + if err := d.createPlan(); err != nil { + hasError = true + d.createEvent(k8sutil.NewErrorEvent("Plan creation failed", err, d.apiObject)) + } + + // Execute current step of scale/update plan + retrySoon, err := d.executePlan(ctx) + if err != nil { + hasError = true + d.createEvent(k8sutil.NewErrorEvent("Plan execution failed", err, d.apiObject)) + } + if retrySoon { + nextInterval = minInspectionInterval + } + + // Ensure all resources are created + if err := d.ensurePVCs(d.apiObject); err != nil { + hasError = true + d.createEvent(k8sutil.NewErrorEvent("PVC creation failed", err, d.apiObject)) + } + if err := d.ensurePods(d.apiObject); err != nil { + hasError = true + d.createEvent(k8sutil.NewErrorEvent("Pod creation failed", err, d.apiObject)) + } + + // Update next interval (on errors) + if hasError { + if d.recentInspectionErrors == 0 { + nextInterval = minInspectionInterval + d.recentInspectionErrors++ + } + } else { + d.recentInspectionErrors = 0 + } + if nextInterval > maxInspectionInterval { + nextInterval = maxInspectionInterval + } + return nextInterval +} diff --git a/pkg/deployment/plan_executor.go b/pkg/deployment/plan_executor.go index 9799250c4..9f0bbd05c 100644 --- a/pkg/deployment/plan_executor.go +++ b/pkg/deployment/plan_executor.go @@ -29,6 +29,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/rs/zerolog" ) // executePlan tries to execute the plan as far as possible. @@ -45,8 +46,14 @@ func (d *Deployment) executePlan(ctx context.Context) (bool, error) { // Take first action planAction := d.status.Plan[0] - log := log.With().Str("action-type", string(planAction.Type)).Logger() - action := d.createAction(ctx, planAction) + log := log.With(). + Int("plan-len", len(d.status.Plan)). + Str("action-id", planAction.ID). + Str("action-type", string(planAction.Type)). + Str("group", planAction.Group.AsRole()). + Str("member-id", planAction.MemberID). + Logger() + action := d.createAction(ctx, log, planAction) if planAction.StartTime.IsZero() { // Not started yet ready, err := action.Start(ctx) @@ -60,14 +67,15 @@ func (d *Deployment) executePlan(ctx context.Context) (bool, error) { d.status.Plan = d.status.Plan[1:] } else { // Mark start time - d.status.Plan[0].StartTime = metav1.Now() + now := metav1.Now() + d.status.Plan[0].StartTime = &now } // Save plan update if err := d.updateCRStatus(true); err != nil { log.Debug().Err(err).Msg("Failed to update CR status") return false, maskAny(err) } - log.Debug().Bool("ready", ready).Msg("Start completed") + log.Debug().Bool("ready", ready).Msg("Action Start completed") if !ready { // We need to check back soon return true, nil @@ -77,20 +85,23 @@ func (d *Deployment) executePlan(ctx context.Context) (bool, error) { // First action of plan has been started, check its progress ready, err := action.CheckProgress(ctx) if err != nil { - log.Debug().Err(err). - Msg("Failed to check action progress") + log.Debug().Err(err).Msg("Failed to check action progress") return false, maskAny(err) } + if ready { + // Remove action from list + d.status.Plan = d.status.Plan[1:] + // Save plan update + if err := d.updateCRStatus(); err != nil { + log.Debug().Err(err).Msg("Failed to update CR status") + return false, maskAny(err) + } + } + log.Debug().Bool("ready", ready).Msg("Action CheckProgress completed") if !ready { // Not ready check, come back soon return true, nil } - // Remove action from list - d.status.Plan = d.status.Plan[1:] - // Save plan update - if err := d.updateCRStatus(); err != nil { - return false, maskAny(err) - } // Continue with next action } } @@ -99,11 +110,7 @@ func (d *Deployment) executePlan(ctx context.Context) (bool, error) { // startAction performs the start of the given action // Returns true if the action is completely finished, false in case // the start time needs to be recorded and a ready condition needs to be checked. -func (d *Deployment) createAction(ctx context.Context, action api.Action) Action { - log := d.deps.Log.With(). - Str("group", action.Group.AsRole()). - Str("id", action.MemberID). - Logger() +func (d *Deployment) createAction(ctx context.Context, log zerolog.Logger, action api.Action) Action { actionCtx := NewActionContext(log, d) switch action.Type { case api.ActionTypeAddMember: diff --git a/pkg/deployment/pod_creator.go b/pkg/deployment/pod_creator.go index 12d9ba270..44ead6f08 100644 --- a/pkg/deployment/pod_creator.go +++ b/pkg/deployment/pod_creator.go @@ -368,6 +368,8 @@ func (d *Deployment) ensurePods(apiObject *api.ArangoDeployment) error { } // Record new member state m.State = api.MemberStateCreated + m.Conditions.Remove(api.ConditionTypeReady) + m.Conditions.Remove(api.ConditionTypeTerminated) if err := status.Update(m); err != nil { return maskAny(err) } From ca8325f539a7ff27fedc8788f38650bd6e3c0b6f Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Thu, 15 Mar 2018 16:58:12 +0100 Subject: [PATCH 05/17] Logging improvements --- examples/simple-cluster.yaml | 2 +- main.go | 2 +- pkg/logging/logger.go | 3 ++- pkg/operator/crd.go | 2 +- pkg/operator/operator.go | 11 +++++++---- pkg/operator/operator_deployment.go | 13 +++++-------- pkg/operator/operator_local_storage.go | 13 +++++-------- 7 files changed, 22 insertions(+), 24 deletions(-) diff --git a/examples/simple-cluster.yaml b/examples/simple-cluster.yaml index 2b1b6df91..e4a1ba0f3 100644 --- a/examples/simple-cluster.yaml +++ b/examples/simple-cluster.yaml @@ -4,4 +4,4 @@ metadata: name: "example-simple-cluster" spec: mode: cluster - image: arangodb/arangodb:3.3.2 + image: arangodb/arangodb:3.3.3 diff --git a/main.go b/main.go index 0ccf7752f..259c3b097 100644 --- a/main.go +++ b/main.go @@ -211,7 +211,7 @@ func newOperatorConfigAndDeps(namespace, name string) (operator.Config, operator CreateCRD: createCRD, } deps := operator.Dependencies{ - Log: logService.MustGetLogger("operator"), + LogService: logService, KubeCli: kubecli, KubeExtCli: kubeExtCli, CRCli: crCli, diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go index 2932f0cd9..663b4aa2e 100644 --- a/pkg/logging/logger.go +++ b/pkg/logging/logger.go @@ -35,7 +35,8 @@ var ( // The defaultLevels list is used during development to increase the // default level for components that we care a little less about. defaultLevels = map[string]string{ - //"something.status": "info", + "operator": "info", + //"something.status": "info", } ) diff --git a/pkg/operator/crd.go b/pkg/operator/crd.go index 9c8dcb3dc..b92a4bdfe 100644 --- a/pkg/operator/crd.go +++ b/pkg/operator/crd.go @@ -45,7 +45,7 @@ func (o *Operator) initResourceIfNeeded() error { // initCRD creates the CustomResourceDefinition and waits for it to be ready. func (o *Operator) initCRD() error { - log := o.Dependencies.Log + log := o.log log.Debug().Msg("Creating ArangoDeployment CRD") if err := crd.CreateCRD(o.KubeExtCli, deplapi.SchemeGroupVersion, deplapi.ArangoDeploymentCRDName, deplapi.ArangoDeploymentResourceKind, deplapi.ArangoDeploymentResourcePlural, deplapi.ArangoDeploymentShortNames...); err != nil { diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index d767f7cb8..ccdb1394a 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -26,7 +26,6 @@ import ( "context" "time" - "github.com/rs/zerolog" apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" kwatch "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes" @@ -35,7 +34,9 @@ import ( lsapi "github.com/arangodb/kube-arangodb/pkg/apis/storage/v1alpha" "github.com/arangodb/kube-arangodb/pkg/deployment" "github.com/arangodb/kube-arangodb/pkg/generated/clientset/versioned" + "github.com/arangodb/kube-arangodb/pkg/logging" "github.com/arangodb/kube-arangodb/pkg/storage" + "github.com/rs/zerolog" ) const ( @@ -52,6 +53,7 @@ type Operator struct { Config Dependencies + log zerolog.Logger deployments map[string]*deployment.Deployment localStorages map[string]*storage.LocalStorage } @@ -64,7 +66,7 @@ type Config struct { } type Dependencies struct { - Log zerolog.Logger + LogService logging.Service KubeCli kubernetes.Interface KubeExtCli apiextensionsclient.Interface CRCli versioned.Interface @@ -75,6 +77,7 @@ func NewOperator(config Config, deps Dependencies) (*Operator, error) { o := &Operator{ Config: config, Dependencies: deps, + log: deps.LogService.MustGetLogger("operator"), deployments: make(map[string]*deployment.Deployment), localStorages: make(map[string]*storage.LocalStorage), } @@ -83,7 +86,7 @@ func NewOperator(config Config, deps Dependencies) (*Operator, error) { // Start the operator func (o *Operator) Start() error { - log := o.Dependencies.Log + log := o.log for { if err := o.initResourceIfNeeded(); err == nil { @@ -103,7 +106,7 @@ func (o *Operator) Start() error { // run the operator. // This registers a listener and waits until the process stops. func (o *Operator) run() { - log := o.Dependencies.Log + log := o.log log.Info().Msgf("Running controller in namespace '%s'", o.Config.Namespace) diff --git a/pkg/operator/operator_deployment.go b/pkg/operator/operator_deployment.go index 26dfed5c5..542d98341 100644 --- a/pkg/operator/operator_deployment.go +++ b/pkg/operator/operator_deployment.go @@ -66,9 +66,8 @@ func (o *Operator) runDeployments() { // onAddArangoDeployment deployment addition callback func (o *Operator) onAddArangoDeployment(obj interface{}) { - log := o.Dependencies.Log apiObject := obj.(*api.ArangoDeployment) - log.Debug(). + o.log.Debug(). Str("name", apiObject.GetObjectMeta().GetName()). Msg("ArangoDeployment added") o.syncArangoDeployment(apiObject) @@ -76,9 +75,8 @@ func (o *Operator) onAddArangoDeployment(obj interface{}) { // onUpdateArangoDeployment deployment update callback func (o *Operator) onUpdateArangoDeployment(oldObj, newObj interface{}) { - log := o.Dependencies.Log apiObject := newObj.(*api.ArangoDeployment) - log.Debug(). + o.log.Debug(). Str("name", apiObject.GetObjectMeta().GetName()). Msg("ArangoDeployment updated") o.syncArangoDeployment(apiObject) @@ -86,7 +84,7 @@ func (o *Operator) onUpdateArangoDeployment(oldObj, newObj interface{}) { // onDeleteArangoDeployment deployment delete callback func (o *Operator) onDeleteArangoDeployment(obj interface{}) { - log := o.Dependencies.Log + log := o.log apiObject, ok := obj.(*api.ArangoDeployment) if !ok { tombstone, ok := obj.(cache.DeletedFinalStateUnknown) @@ -132,7 +130,7 @@ func (o *Operator) syncArangoDeployment(apiObject *api.ArangoDeployment) { //pt.start() err := o.handleDeploymentEvent(ev) if err != nil { - o.Dependencies.Log.Warn().Err(err).Msg("Failed to handle event") + o.log.Warn().Err(err).Msg("Failed to handle event") } //pt.stop() } @@ -200,8 +198,7 @@ func (o *Operator) makeDeploymentConfigAndDeps(apiObject *api.ArangoDeployment) ServiceAccount: o.Config.ServiceAccount, } deps := deployment.Dependencies{ - Log: o.Dependencies.Log.With(). - Str("component", "deployment"). + Log: o.Dependencies.LogService.MustGetLogger("deployment").With(). Str("deployment", apiObject.GetName()). Logger(), KubeCli: o.Dependencies.KubeCli, diff --git a/pkg/operator/operator_local_storage.go b/pkg/operator/operator_local_storage.go index f9e4d7d91..9148d96e7 100644 --- a/pkg/operator/operator_local_storage.go +++ b/pkg/operator/operator_local_storage.go @@ -66,9 +66,8 @@ func (o *Operator) runLocalStorages() { // onAddArangoLocalStorage local storage addition callback func (o *Operator) onAddArangoLocalStorage(obj interface{}) { - log := o.Dependencies.Log apiObject := obj.(*api.ArangoLocalStorage) - log.Debug(). + o.log.Debug(). Str("name", apiObject.GetObjectMeta().GetName()). Msg("ArangoLocalStorage added") o.syncArangoLocalStorage(apiObject) @@ -76,9 +75,8 @@ func (o *Operator) onAddArangoLocalStorage(obj interface{}) { // onUpdateArangoLocalStorage local storage update callback func (o *Operator) onUpdateArangoLocalStorage(oldObj, newObj interface{}) { - log := o.Dependencies.Log apiObject := newObj.(*api.ArangoLocalStorage) - log.Debug(). + o.log.Debug(). Str("name", apiObject.GetObjectMeta().GetName()). Msg("ArangoLocalStorage updated") o.syncArangoLocalStorage(apiObject) @@ -86,7 +84,7 @@ func (o *Operator) onUpdateArangoLocalStorage(oldObj, newObj interface{}) { // onDeleteArangoLocalStorage local storage delete callback func (o *Operator) onDeleteArangoLocalStorage(obj interface{}) { - log := o.Dependencies.Log + log := o.log apiObject, ok := obj.(*api.ArangoLocalStorage) if !ok { tombstone, ok := obj.(cache.DeletedFinalStateUnknown) @@ -132,7 +130,7 @@ func (o *Operator) syncArangoLocalStorage(apiObject *api.ArangoLocalStorage) { //pt.start() err := o.handleLocalStorageEvent(ev) if err != nil { - o.Dependencies.Log.Warn().Err(err).Msg("Failed to handle event") + o.log.Warn().Err(err).Msg("Failed to handle event") } //pt.stop() } @@ -201,8 +199,7 @@ func (o *Operator) makeLocalStorageConfigAndDeps(apiObject *api.ArangoLocalStora ServiceAccount: o.Config.ServiceAccount, } deps := storage.Dependencies{ - Log: o.Dependencies.Log.With(). - Str("component", "storage"). + Log: o.Dependencies.LogService.MustGetLogger("storage").With(). Str("localStorage", apiObject.GetName()). Logger(), KubeCli: o.Dependencies.KubeCli, From 7de4beec4463303968ba9cfd6ba70a0a134d6aa5 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Thu, 15 Mar 2018 20:16:43 +0100 Subject: [PATCH 06/17] Renamed generated pod names --- examples/nodeport-cluster.yaml | 2 +- examples/nodeport-single-server.yaml | 2 +- pkg/apis/deployment/v1alpha/server_group.go | 12 +++---- .../deployment/v1alpha/server_group_test.go | 12 +++---- pkg/deployment/members.go | 12 +++---- pkg/deployment/pod_creator.go | 17 ++++++++-- pkg/util/k8sutil/dns.go | 2 +- pkg/util/k8sutil/pods.go | 31 ++++++++++++------- 8 files changed, 55 insertions(+), 35 deletions(-) diff --git a/examples/nodeport-cluster.yaml b/examples/nodeport-cluster.yaml index 5de0b4c45..680ad42cf 100644 --- a/examples/nodeport-cluster.yaml +++ b/examples/nodeport-cluster.yaml @@ -5,7 +5,7 @@ metadata: spec: selector: app: arangodb - role: coordinator + role: crdn type: NodePort publishNotReadyAddresses: true ports: diff --git a/examples/nodeport-single-server.yaml b/examples/nodeport-single-server.yaml index 60cc3f304..6166d0211 100644 --- a/examples/nodeport-single-server.yaml +++ b/examples/nodeport-single-server.yaml @@ -5,7 +5,7 @@ metadata: spec: selector: app: arangodb - role: single + role: sngl type: NodePort ports: - protocol: TCP diff --git a/pkg/apis/deployment/v1alpha/server_group.go b/pkg/apis/deployment/v1alpha/server_group.go index 2c8d6e63d..288869e89 100644 --- a/pkg/apis/deployment/v1alpha/server_group.go +++ b/pkg/apis/deployment/v1alpha/server_group.go @@ -49,17 +49,17 @@ var ( func (g ServerGroup) AsRole() string { switch g { case ServerGroupSingle: - return "single" + return "sngl" case ServerGroupAgents: - return "agent" + return "agnt" case ServerGroupDBServers: - return "dbserver" + return "prmr" case ServerGroupCoordinators: - return "coordinator" + return "crdn" case ServerGroupSyncMasters: - return "syncmaster" + return "syma" case ServerGroupSyncWorkers: - return "syncworker" + return "sywo" default: return "?" } diff --git a/pkg/apis/deployment/v1alpha/server_group_test.go b/pkg/apis/deployment/v1alpha/server_group_test.go index 835702c7d..1adeeced0 100644 --- a/pkg/apis/deployment/v1alpha/server_group_test.go +++ b/pkg/apis/deployment/v1alpha/server_group_test.go @@ -29,12 +29,12 @@ import ( ) func TestServerGroupAsRole(t *testing.T) { - assert.Equal(t, "single", ServerGroupSingle.AsRole()) - assert.Equal(t, "agent", ServerGroupAgents.AsRole()) - assert.Equal(t, "dbserver", ServerGroupDBServers.AsRole()) - assert.Equal(t, "coordinator", ServerGroupCoordinators.AsRole()) - assert.Equal(t, "syncmaster", ServerGroupSyncMasters.AsRole()) - assert.Equal(t, "syncworker", ServerGroupSyncWorkers.AsRole()) + assert.Equal(t, "sngl", ServerGroupSingle.AsRole()) + assert.Equal(t, "agnt", ServerGroupAgents.AsRole()) + assert.Equal(t, "prmr", ServerGroupDBServers.AsRole()) + assert.Equal(t, "crdn", ServerGroupCoordinators.AsRole()) + assert.Equal(t, "syma", ServerGroupSyncMasters.AsRole()) + assert.Equal(t, "sywo", ServerGroupSyncWorkers.AsRole()) } func TestServerGroupIsArangod(t *testing.T) { diff --git a/pkg/deployment/members.go b/pkg/deployment/members.go index 537bf2f79..d8140bbed 100644 --- a/pkg/deployment/members.go +++ b/pkg/deployment/members.go @@ -83,7 +83,7 @@ func (d *Deployment) createMember(group api.ServerGroup, apiObject *api.ArangoDe ID: id, State: api.MemberStateNone, PersistentVolumeClaimName: k8sutil.CreatePersistentVolumeClaimName(deploymentName, role, id), - PodName: k8sutil.CreatePodName(deploymentName, role, id), + PodName: "", }); err != nil { return maskAny(err) } @@ -93,7 +93,7 @@ func (d *Deployment) createMember(group api.ServerGroup, apiObject *api.ArangoDe ID: id, State: api.MemberStateNone, PersistentVolumeClaimName: k8sutil.CreatePersistentVolumeClaimName(deploymentName, role, id), - PodName: k8sutil.CreatePodName(deploymentName, role, id), + PodName: "", }); err != nil { return maskAny(err) } @@ -103,7 +103,7 @@ func (d *Deployment) createMember(group api.ServerGroup, apiObject *api.ArangoDe ID: id, State: api.MemberStateNone, PersistentVolumeClaimName: k8sutil.CreatePersistentVolumeClaimName(deploymentName, role, id), - PodName: k8sutil.CreatePodName(deploymentName, role, id), + PodName: "", }); err != nil { return maskAny(err) } @@ -113,7 +113,7 @@ func (d *Deployment) createMember(group api.ServerGroup, apiObject *api.ArangoDe ID: id, State: api.MemberStateNone, PersistentVolumeClaimName: "", - PodName: k8sutil.CreatePodName(deploymentName, role, id), + PodName: "", }); err != nil { return maskAny(err) } @@ -123,7 +123,7 @@ func (d *Deployment) createMember(group api.ServerGroup, apiObject *api.ArangoDe ID: id, State: api.MemberStateNone, PersistentVolumeClaimName: "", - PodName: k8sutil.CreatePodName(deploymentName, role, id), + PodName: "", }); err != nil { return maskAny(err) } @@ -133,7 +133,7 @@ func (d *Deployment) createMember(group api.ServerGroup, apiObject *api.ArangoDe ID: id, State: api.MemberStateNone, PersistentVolumeClaimName: "", - PodName: k8sutil.CreatePodName(deploymentName, role, id), + PodName: "", }); err != nil { return maskAny(err) } diff --git a/pkg/deployment/pod_creator.go b/pkg/deployment/pod_creator.go index 44ead6f08..c17f7a89d 100644 --- a/pkg/deployment/pod_creator.go +++ b/pkg/deployment/pod_creator.go @@ -23,6 +23,8 @@ package deployment import ( + "crypto/sha1" + "encoding/json" "fmt" "net" "path/filepath" @@ -310,8 +312,11 @@ func (d *Deployment) ensurePods(apiObject *api.ArangoDeployment) error { if m.State != api.MemberStateNone { continue } - // Create pod + // Update pod name role := group.AsRole() + podSuffix := createPodSuffix(apiObject.Spec) + m.PodName = k8sutil.CreatePodName(apiObject.GetName(), role, m.ID, podSuffix) + // Create pod if group.IsArangod() { args := createArangodArgs(apiObject, apiObject.Spec, group, spec, d.status.Members.Agents, m.ID) env := make(map[string]k8sutil.EnvValue) @@ -348,7 +353,7 @@ func (d *Deployment) ensurePods(apiObject *api.ArangoDeployment) error { SecretKey: constants.SecretKeyJWT, } } - if err := k8sutil.CreateArangodPod(kubecli, apiObject.Spec.IsDevelopment(), apiObject, role, m.ID, m.PersistentVolumeClaimName, apiObject.Spec.Image, apiObject.Spec.ImagePullPolicy, args, env, livenessProbe, readinessProbe, tlsKeyfileSecretName, rocksdbEncryptionSecretName); err != nil { + if err := k8sutil.CreateArangodPod(kubecli, apiObject.Spec.IsDevelopment(), apiObject, role, m.ID, m.PodName, m.PersistentVolumeClaimName, apiObject.Spec.Image, apiObject.Spec.ImagePullPolicy, args, env, livenessProbe, readinessProbe, tlsKeyfileSecretName, rocksdbEncryptionSecretName); err != nil { return maskAny(err) } } else if group.IsArangosync() { @@ -362,7 +367,7 @@ func (d *Deployment) ensurePods(apiObject *api.ArangoDeployment) error { if group == api.ServerGroupSyncWorkers { affinityWithRole = api.ServerGroupDBServers.AsRole() } - if err := k8sutil.CreateArangoSyncPod(kubecli, apiObject.Spec.IsDevelopment(), apiObject, role, m.ID, apiObject.Spec.Sync.Image, apiObject.Spec.Sync.ImagePullPolicy, args, env, livenessProbe, affinityWithRole); err != nil { + if err := k8sutil.CreateArangoSyncPod(kubecli, apiObject.Spec.IsDevelopment(), apiObject, role, m.ID, m.PodName, apiObject.Spec.Sync.Image, apiObject.Spec.Sync.ImagePullPolicy, args, env, livenessProbe, affinityWithRole); err != nil { return maskAny(err) } } @@ -385,3 +390,9 @@ func (d *Deployment) ensurePods(apiObject *api.ArangoDeployment) error { } return nil } + +func createPodSuffix(spec api.DeploymentSpec) string { + raw, _ := json.Marshal(spec) + hash := sha1.Sum(raw) + return fmt.Sprintf("%0x", hash)[:6] +} diff --git a/pkg/util/k8sutil/dns.go b/pkg/util/k8sutil/dns.go index ce7d147a2..2fb4e7171 100644 --- a/pkg/util/k8sutil/dns.go +++ b/pkg/util/k8sutil/dns.go @@ -29,7 +29,7 @@ import ( // CreatePodDNSName returns the DNS of a pod with a given role & id in // a given deployment. func CreatePodDNSName(deployment metav1.Object, role, id string) string { - return CreatePodName(deployment.GetName(), role, id) + "." + + return CreatePodHostName(deployment.GetName(), role, id) + "." + CreateHeadlessServiceName(deployment.GetName()) + "." + deployment.GetNamespace() + ".svc" } diff --git a/pkg/util/k8sutil/pods.go b/pkg/util/k8sutil/pods.go index 096a4214e..9a7c6e28b 100644 --- a/pkg/util/k8sutil/pods.go +++ b/pkg/util/k8sutil/pods.go @@ -100,14 +100,23 @@ func getPodCondition(status *v1.PodStatus, condType v1.PodConditionType) *v1.Pod // CreatePodName returns the name of the pod for a member with // a given id in a deployment with a given name. -func CreatePodName(deploymentName, role, id string) string { +func CreatePodName(deploymentName, role, id, suffix string) string { + if len(suffix) > 0 && suffix[0] != '-' { + suffix = "-" + suffix + } + return CreatePodHostName(deploymentName, role, id) + suffix +} + +// CreatePodHostName returns the hostname of the pod for a member with +// a given id in a deployment with a given name. +func CreatePodHostName(deploymentName, role, id string) string { return deploymentName + "-" + role + "-" + stripArangodPrefix(id) } // CreateTLSKeyfileSecretName returns the name of the Secret that holds the TLS keyfile for a member with // a given id in a deployment with a given name. func CreateTLSKeyfileSecretName(deploymentName, role, id string) string { - return CreatePodName(deploymentName, role, id) + "-tls-keyfile" + return CreatePodName(deploymentName, role, id, "-tls-keyfile") } // arangodVolumeMounts creates a volume mount structure for arangod. @@ -209,17 +218,17 @@ func arangosyncContainer(name string, image string, imagePullPolicy v1.PullPolic } // newPod creates a basic Pod for given settings. -func newPod(deploymentName, ns, role, id string) v1.Pod { - name := CreatePodName(deploymentName, role, id) +func newPod(deploymentName, ns, role, id, podName string) v1.Pod { + hostname := CreatePodHostName(deploymentName, role, id) p := v1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: name, + Name: podName, Labels: LabelsForDeployment(deploymentName, role), }, Spec: v1.PodSpec{ - Hostname: name, + Hostname: hostname, Subdomain: CreateHeadlessServiceName(deploymentName), - RestartPolicy: v1.RestartPolicyOnFailure, + RestartPolicy: v1.RestartPolicyNever, }, } return p @@ -229,12 +238,12 @@ func newPod(deploymentName, ns, role, id string) v1.Pod { // If the pod already exists, nil is returned. // If another error occurs, that error is returned. func CreateArangodPod(kubecli kubernetes.Interface, developmentMode bool, deployment APIObject, - role, id, pvcName, image string, imagePullPolicy v1.PullPolicy, + role, id, podName, pvcName, image string, imagePullPolicy v1.PullPolicy, args []string, env map[string]EnvValue, livenessProbe *HTTPProbeConfig, readinessProbe *HTTPProbeConfig, tlsKeyfileSecretName, rocksdbEncryptionSecretName string) error { // Prepare basic pod - p := newPod(deployment.GetName(), deployment.GetNamespace(), role, id) + p := newPod(deployment.GetName(), deployment.GetNamespace(), role, id, podName) // Add arangod container c := arangodContainer("server", image, imagePullPolicy, args, env, livenessProbe, readinessProbe) @@ -310,10 +319,10 @@ func CreateArangodPod(kubecli kubernetes.Interface, developmentMode bool, deploy // CreateArangoSyncPod creates a Pod that runs `arangosync`. // If the pod already exists, nil is returned. // If another error occurs, that error is returned. -func CreateArangoSyncPod(kubecli kubernetes.Interface, developmentMode bool, deployment APIObject, role, id, image string, imagePullPolicy v1.PullPolicy, +func CreateArangoSyncPod(kubecli kubernetes.Interface, developmentMode bool, deployment APIObject, role, id, podName, image string, imagePullPolicy v1.PullPolicy, args []string, env map[string]EnvValue, livenessProbe *HTTPProbeConfig, affinityWithRole string) error { // Prepare basic pod - p := newPod(deployment.GetName(), deployment.GetNamespace(), role, id) + p := newPod(deployment.GetName(), deployment.GetNamespace(), role, id, podName) // Add arangosync container c := arangosyncContainer(p.GetName(), image, imagePullPolicy, args, env, livenessProbe) From 749428a7a2729377786ad6ac63092be7e68ec5af Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Thu, 15 Mar 2018 20:20:55 +0100 Subject: [PATCH 07/17] Fixed service selector --- pkg/util/k8sutil/services.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/util/k8sutil/services.go b/pkg/util/k8sutil/services.go index e881f3cc0..78f719fe7 100644 --- a/pkg/util/k8sutil/services.go +++ b/pkg/util/k8sutil/services.go @@ -83,9 +83,9 @@ func CreateDatabaseClientService(kubecli kubernetes.Interface, deployment metav1 } var role string if single { - role = "single" + role = "sngl" } else { - role = "coordinator" + role = "crdn" } publishNotReadyAddresses := true sessionAffinity := v1.ServiceAffinityClientIP From 74a2244d6c9f046aec17da5cc3d58a333dba3d67 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Thu, 15 Mar 2018 20:28:31 +0100 Subject: [PATCH 08/17] Respond quicker --- pkg/deployment/plan_executor.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/deployment/plan_executor.go b/pkg/deployment/plan_executor.go index 9f0bbd05c..f6c28651a 100644 --- a/pkg/deployment/plan_executor.go +++ b/pkg/deployment/plan_executor.go @@ -37,11 +37,12 @@ import ( // False otherwise. func (d *Deployment) executePlan(ctx context.Context) (bool, error) { log := d.deps.Log + initialPlanLen := len(d.status.Plan) for { if len(d.status.Plan) == 0 { // No plan exists, nothing to be done - return false, nil + return initialPlanLen > 0, nil } // Take first action From 0f9567bc534c2b910508bdb48b4b4ec056d47254 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Fri, 16 Mar 2018 08:18:19 +0100 Subject: [PATCH 09/17] Fixed unit tests --- pkg/deployment/plan_builder_test.go | 18 +++++------ pkg/deployment/pod_creator_agent_args_test.go | 24 +++++++------- .../pod_creator_coordinator_args_test.go | 32 +++++++++---------- .../pod_creator_dbserver_args_test.go | 32 +++++++++---------- .../pod_creator_single_args_test.go | 8 ++--- 5 files changed, 57 insertions(+), 57 deletions(-) diff --git a/pkg/deployment/plan_builder_test.go b/pkg/deployment/plan_builder_test.go index 161f07b08..042446cf0 100644 --- a/pkg/deployment/plan_builder_test.go +++ b/pkg/deployment/plan_builder_test.go @@ -42,7 +42,7 @@ func TestCreatePlanSingleScale(t *testing.T) { // Test with empty status var status api.DeploymentStatus - newPlan, changed := createPlan(log, nil, spec, status) + newPlan, changed := createPlan(log, nil, spec, status, nil) assert.True(t, changed) assert.Len(t, newPlan, 0) // Single mode does not scale @@ -53,7 +53,7 @@ func TestCreatePlanSingleScale(t *testing.T) { PodName: "something", }, } - newPlan, changed = createPlan(log, nil, spec, status) + newPlan, changed = createPlan(log, nil, spec, status, nil) assert.True(t, changed) assert.Len(t, newPlan, 0) // Single mode does not scale @@ -68,7 +68,7 @@ func TestCreatePlanSingleScale(t *testing.T) { PodName: "something1", }, } - newPlan, changed = createPlan(log, nil, spec, status) + newPlan, changed = createPlan(log, nil, spec, status, nil) assert.True(t, changed) assert.Len(t, newPlan, 0) // Single mode does not scale } @@ -84,7 +84,7 @@ func TestCreatePlanResilientSingleScale(t *testing.T) { // Test with empty status var status api.DeploymentStatus - newPlan, changed := createPlan(log, nil, spec, status) + newPlan, changed := createPlan(log, nil, spec, status, nil) assert.True(t, changed) require.Len(t, newPlan, 2) assert.Equal(t, api.ActionTypeAddMember, newPlan[0].Type) @@ -97,7 +97,7 @@ func TestCreatePlanResilientSingleScale(t *testing.T) { PodName: "something", }, } - newPlan, changed = createPlan(log, nil, spec, status) + newPlan, changed = createPlan(log, nil, spec, status, nil) assert.True(t, changed) require.Len(t, newPlan, 1) assert.Equal(t, api.ActionTypeAddMember, newPlan[0].Type) @@ -122,7 +122,7 @@ func TestCreatePlanResilientSingleScale(t *testing.T) { PodName: "something4", }, } - newPlan, changed = createPlan(log, nil, spec, status) + newPlan, changed = createPlan(log, nil, spec, status, nil) assert.True(t, changed) require.Len(t, newPlan, 2) // Note: Downscaling is only down 1 at a time assert.Equal(t, api.ActionTypeShutdownMember, newPlan[0].Type) @@ -141,7 +141,7 @@ func TestCreatePlanClusterScale(t *testing.T) { // Test with empty status var status api.DeploymentStatus - newPlan, changed := createPlan(log, nil, spec, status) + newPlan, changed := createPlan(log, nil, spec, status, nil) assert.True(t, changed) require.Len(t, newPlan, 6) // Adding 3 dbservers & 3 coordinators (note: agents do not scale now) assert.Equal(t, api.ActionTypeAddMember, newPlan[0].Type) @@ -174,7 +174,7 @@ func TestCreatePlanClusterScale(t *testing.T) { PodName: "coordinator1", }, } - newPlan, changed = createPlan(log, nil, spec, status) + newPlan, changed = createPlan(log, nil, spec, status, nil) assert.True(t, changed) require.Len(t, newPlan, 3) assert.Equal(t, api.ActionTypeAddMember, newPlan[0].Type) @@ -211,7 +211,7 @@ func TestCreatePlanClusterScale(t *testing.T) { } spec.DBServers.Count = 1 spec.Coordinators.Count = 1 - newPlan, changed = createPlan(log, nil, spec, status) + newPlan, changed = createPlan(log, nil, spec, status, nil) assert.True(t, changed) require.Len(t, newPlan, 5) // Note: Downscaling is done 1 at a time assert.Equal(t, api.ActionTypeCleanOutMember, newPlan[0].Type) diff --git a/pkg/deployment/pod_creator_agent_args_test.go b/pkg/deployment/pod_creator_agent_args_test.go index 38f10f12c..a94d00f9c 100644 --- a/pkg/deployment/pod_creator_agent_args_test.go +++ b/pkg/deployment/pod_creator_agent_args_test.go @@ -54,9 +54,9 @@ func TestCreateArangodArgsAgent(t *testing.T) { assert.Equal(t, []string{ "--agency.activate=true", - "--agency.endpoint=tcp://name-agent-a2.name-int.ns.svc:8529", - "--agency.endpoint=tcp://name-agent-a3.name-int.ns.svc:8529", - "--agency.my-address=tcp://name-agent-a1.name-int.ns.svc:8529", + "--agency.endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", + "--agency.endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", + "--agency.my-address=tcp://name-agnt-a1.name-int.ns.svc:8529", "--agency.size=3", "--agency.supervision=true", "--cluster.my-id=a1", @@ -98,9 +98,9 @@ func TestCreateArangodArgsAgent(t *testing.T) { assert.Equal(t, []string{ "--agency.activate=true", - "--agency.endpoint=ssl://name-agent-a2.name-int.ns.svc:8529", - "--agency.endpoint=ssl://name-agent-a3.name-int.ns.svc:8529", - "--agency.my-address=ssl://name-agent-a1.name-int.ns.svc:8529", + "--agency.endpoint=ssl://name-agnt-a2.name-int.ns.svc:8529", + "--agency.endpoint=ssl://name-agnt-a3.name-int.ns.svc:8529", + "--agency.my-address=ssl://name-agnt-a1.name-int.ns.svc:8529", "--agency.size=3", "--agency.supervision=true", "--cluster.my-id=a1", @@ -143,9 +143,9 @@ func TestCreateArangodArgsAgent(t *testing.T) { assert.Equal(t, []string{ "--agency.activate=true", - "--agency.endpoint=tcp://name-agent-a2.name-int.ns.svc:8529", - "--agency.endpoint=tcp://name-agent-a3.name-int.ns.svc:8529", - "--agency.my-address=tcp://name-agent-a1.name-int.ns.svc:8529", + "--agency.endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", + "--agency.endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", + "--agency.my-address=tcp://name-agnt-a1.name-int.ns.svc:8529", "--agency.size=3", "--agency.supervision=true", "--cluster.my-id=a1", @@ -184,9 +184,9 @@ func TestCreateArangodArgsAgent(t *testing.T) { assert.Equal(t, []string{ "--agency.activate=true", - "--agency.endpoint=tcp://name-agent-a2.name-int.ns.svc:8529", - "--agency.endpoint=tcp://name-agent-a3.name-int.ns.svc:8529", - "--agency.my-address=tcp://name-agent-a1.name-int.ns.svc:8529", + "--agency.endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", + "--agency.endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", + "--agency.my-address=tcp://name-agnt-a1.name-int.ns.svc:8529", "--agency.size=3", "--agency.supervision=true", "--cluster.my-id=a1", diff --git a/pkg/deployment/pod_creator_coordinator_args_test.go b/pkg/deployment/pod_creator_coordinator_args_test.go index c66c55eef..00236a379 100644 --- a/pkg/deployment/pod_creator_coordinator_args_test.go +++ b/pkg/deployment/pod_creator_coordinator_args_test.go @@ -53,10 +53,10 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, apiObject.Spec.Coordinators, agents, "id1") assert.Equal(t, []string{ - "--cluster.agency-endpoint=tcp://name-agent-a1.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a2.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a3.name-int.ns.svc:8529", - "--cluster.my-address=tcp://name-coordinator-id1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", + "--cluster.my-address=tcp://name-crdn-id1.name-int.ns.svc:8529", "--cluster.my-id=id1", "--cluster.my-role=COORDINATOR", "--database.directory=/data", @@ -96,10 +96,10 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, apiObject.Spec.Coordinators, agents, "id1") assert.Equal(t, []string{ - "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", - "--cluster.agency-endpoint=ssl://name-agent-a2.name-int.ns.svc:8529", - "--cluster.agency-endpoint=ssl://name-agent-a3.name-int.ns.svc:8529", - "--cluster.my-address=ssl://name-coordinator-id1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=ssl://name-agnt-a1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=ssl://name-agnt-a2.name-int.ns.svc:8529", + "--cluster.agency-endpoint=ssl://name-agnt-a3.name-int.ns.svc:8529", + "--cluster.my-address=ssl://name-crdn-id1.name-int.ns.svc:8529", "--cluster.my-id=id1", "--cluster.my-role=COORDINATOR", "--database.directory=/data", @@ -139,10 +139,10 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, apiObject.Spec.Coordinators, agents, "id1") assert.Equal(t, []string{ - "--cluster.agency-endpoint=tcp://name-agent-a1.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a2.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a3.name-int.ns.svc:8529", - "--cluster.my-address=tcp://name-coordinator-id1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", + "--cluster.my-address=tcp://name-crdn-id1.name-int.ns.svc:8529", "--cluster.my-id=id1", "--cluster.my-role=COORDINATOR", "--database.directory=/data", @@ -180,10 +180,10 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, apiObject.Spec.Coordinators, agents, "id1") assert.Equal(t, []string{ - "--cluster.agency-endpoint=tcp://name-agent-a1.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a2.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a3.name-int.ns.svc:8529", - "--cluster.my-address=tcp://name-coordinator-id1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", + "--cluster.my-address=tcp://name-crdn-id1.name-int.ns.svc:8529", "--cluster.my-id=id1", "--cluster.my-role=COORDINATOR", "--database.directory=/data", diff --git a/pkg/deployment/pod_creator_dbserver_args_test.go b/pkg/deployment/pod_creator_dbserver_args_test.go index da4b49352..d3a577473 100644 --- a/pkg/deployment/pod_creator_dbserver_args_test.go +++ b/pkg/deployment/pod_creator_dbserver_args_test.go @@ -53,10 +53,10 @@ func TestCreateArangodArgsDBServer(t *testing.T) { cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, apiObject.Spec.DBServers, agents, "id1") assert.Equal(t, []string{ - "--cluster.agency-endpoint=tcp://name-agent-a1.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a2.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a3.name-int.ns.svc:8529", - "--cluster.my-address=tcp://name-dbserver-id1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", + "--cluster.my-address=tcp://name-prmr-id1.name-int.ns.svc:8529", "--cluster.my-id=id1", "--cluster.my-role=PRIMARY", "--database.directory=/data", @@ -96,10 +96,10 @@ func TestCreateArangodArgsDBServer(t *testing.T) { cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, apiObject.Spec.DBServers, agents, "id1") assert.Equal(t, []string{ - "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", - "--cluster.agency-endpoint=ssl://name-agent-a2.name-int.ns.svc:8529", - "--cluster.agency-endpoint=ssl://name-agent-a3.name-int.ns.svc:8529", - "--cluster.my-address=ssl://name-dbserver-id1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=ssl://name-agnt-a1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=ssl://name-agnt-a2.name-int.ns.svc:8529", + "--cluster.agency-endpoint=ssl://name-agnt-a3.name-int.ns.svc:8529", + "--cluster.my-address=ssl://name-prmr-id1.name-int.ns.svc:8529", "--cluster.my-id=id1", "--cluster.my-role=PRIMARY", "--database.directory=/data", @@ -139,10 +139,10 @@ func TestCreateArangodArgsDBServer(t *testing.T) { cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, apiObject.Spec.DBServers, agents, "id1") assert.Equal(t, []string{ - "--cluster.agency-endpoint=tcp://name-agent-a1.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a2.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a3.name-int.ns.svc:8529", - "--cluster.my-address=tcp://name-dbserver-id1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", + "--cluster.my-address=tcp://name-prmr-id1.name-int.ns.svc:8529", "--cluster.my-id=id1", "--cluster.my-role=PRIMARY", "--database.directory=/data", @@ -180,10 +180,10 @@ func TestCreateArangodArgsDBServer(t *testing.T) { cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, apiObject.Spec.DBServers, agents, "id1") assert.Equal(t, []string{ - "--cluster.agency-endpoint=tcp://name-agent-a1.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a2.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a3.name-int.ns.svc:8529", - "--cluster.my-address=tcp://name-dbserver-id1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", + "--cluster.my-address=tcp://name-prmr-id1.name-int.ns.svc:8529", "--cluster.my-id=id1", "--cluster.my-role=PRIMARY", "--database.directory=/data", diff --git a/pkg/deployment/pod_creator_single_args_test.go b/pkg/deployment/pod_creator_single_args_test.go index 7ede1717f..0c2e56216 100644 --- a/pkg/deployment/pod_creator_single_args_test.go +++ b/pkg/deployment/pod_creator_single_args_test.go @@ -187,10 +187,10 @@ func TestCreateArangodArgsSingle(t *testing.T) { cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, apiObject.Spec.Single, agents, "id1") assert.Equal(t, []string{ - "--cluster.agency-endpoint=tcp://name-agent-a1.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a2.name-int.ns.svc:8529", - "--cluster.agency-endpoint=tcp://name-agent-a3.name-int.ns.svc:8529", - "--cluster.my-address=tcp://name-single-id1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", + "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", + "--cluster.my-address=tcp://name-sngl-id1.name-int.ns.svc:8529", "--cluster.my-id=id1", "--cluster.my-role=SINGLE", "--database.directory=/data", From f7bc06ff42f3259d7f0dfafbe4854413574a1f05 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Fri, 16 Mar 2018 08:29:02 +0100 Subject: [PATCH 10/17] Removed obsolete `--cluster.my-id` option --- pkg/deployment/pod_creator.go | 10 +--------- pkg/deployment/pod_creator_agent_args_test.go | 8 ++++---- pkg/deployment/pod_creator_coordinator_args_test.go | 4 ---- pkg/deployment/pod_creator_dbserver_args_test.go | 4 ---- pkg/deployment/pod_creator_single_args_test.go | 1 - 5 files changed, 5 insertions(+), 22 deletions(-) diff --git a/pkg/deployment/pod_creator.go b/pkg/deployment/pod_creator.go index c17f7a89d..4ac4e3704 100644 --- a/pkg/deployment/pod_creator.go +++ b/pkg/deployment/pod_creator.go @@ -138,7 +138,7 @@ func createArangodArgs(apiObject metav1.Object, deplSpec api.DeploymentSpec, gro switch group { case api.ServerGroupAgents: options = append(options, - optionPair{"--cluster.my-id", id}, + optionPair{"--agency.disaster-recovery-id", id}, optionPair{"--agency.activate", "true"}, optionPair{"--agency.my-address", myTCPURL}, optionPair{"--agency.size", strconv.Itoa(deplSpec.Agents.Count)}, @@ -154,15 +154,9 @@ func createArangodArgs(apiObject metav1.Object, deplSpec api.DeploymentSpec, gro ) } } - /*if agentRecoveryID != "" { - options = append(options, - optionPair{"--agency.disaster-recovery-id", agentRecoveryID}, - ) - }*/ case api.ServerGroupDBServers: addAgentEndpoints = true options = append(options, - optionPair{"--cluster.my-id", id}, optionPair{"--cluster.my-address", myTCPURL}, optionPair{"--cluster.my-role", "PRIMARY"}, optionPair{"--foxx.queues", "false"}, @@ -171,7 +165,6 @@ func createArangodArgs(apiObject metav1.Object, deplSpec api.DeploymentSpec, gro case api.ServerGroupCoordinators: addAgentEndpoints = true options = append(options, - optionPair{"--cluster.my-id", id}, optionPair{"--cluster.my-address", myTCPURL}, optionPair{"--cluster.my-role", "COORDINATOR"}, optionPair{"--foxx.queues", "true"}, @@ -186,7 +179,6 @@ func createArangodArgs(apiObject metav1.Object, deplSpec api.DeploymentSpec, gro addAgentEndpoints = true options = append(options, optionPair{"--replication.automatic-failover", "true"}, - optionPair{"--cluster.my-id", id}, optionPair{"--cluster.my-address", myTCPURL}, optionPair{"--cluster.my-role", "SINGLE"}, ) diff --git a/pkg/deployment/pod_creator_agent_args_test.go b/pkg/deployment/pod_creator_agent_args_test.go index a94d00f9c..68dcd1a74 100644 --- a/pkg/deployment/pod_creator_agent_args_test.go +++ b/pkg/deployment/pod_creator_agent_args_test.go @@ -54,12 +54,12 @@ func TestCreateArangodArgsAgent(t *testing.T) { assert.Equal(t, []string{ "--agency.activate=true", + "--agency.disaster-recovery-id=a1", "--agency.endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", "--agency.endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", "--agency.my-address=tcp://name-agnt-a1.name-int.ns.svc:8529", "--agency.size=3", "--agency.supervision=true", - "--cluster.my-id=a1", "--database.directory=/data", "--foxx.queues=false", "--log.level=INFO", @@ -98,12 +98,12 @@ func TestCreateArangodArgsAgent(t *testing.T) { assert.Equal(t, []string{ "--agency.activate=true", + "--agency.disaster-recovery-id=a1", "--agency.endpoint=ssl://name-agnt-a2.name-int.ns.svc:8529", "--agency.endpoint=ssl://name-agnt-a3.name-int.ns.svc:8529", "--agency.my-address=ssl://name-agnt-a1.name-int.ns.svc:8529", "--agency.size=3", "--agency.supervision=true", - "--cluster.my-id=a1", "--database.directory=/data", "--foxx.queues=false", "--log.level=INFO", @@ -143,12 +143,12 @@ func TestCreateArangodArgsAgent(t *testing.T) { assert.Equal(t, []string{ "--agency.activate=true", + "--agency.disaster-recovery-id=a1", "--agency.endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", "--agency.endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", "--agency.my-address=tcp://name-agnt-a1.name-int.ns.svc:8529", "--agency.size=3", "--agency.supervision=true", - "--cluster.my-id=a1", "--database.directory=/data", "--foxx.queues=false", "--log.level=INFO", @@ -184,12 +184,12 @@ func TestCreateArangodArgsAgent(t *testing.T) { assert.Equal(t, []string{ "--agency.activate=true", + "--agency.disaster-recovery-id=a1", "--agency.endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", "--agency.endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", "--agency.my-address=tcp://name-agnt-a1.name-int.ns.svc:8529", "--agency.size=3", "--agency.supervision=true", - "--cluster.my-id=a1", "--database.directory=/data", "--foxx.queues=false", "--log.level=INFO", diff --git a/pkg/deployment/pod_creator_coordinator_args_test.go b/pkg/deployment/pod_creator_coordinator_args_test.go index 00236a379..197a792c3 100644 --- a/pkg/deployment/pod_creator_coordinator_args_test.go +++ b/pkg/deployment/pod_creator_coordinator_args_test.go @@ -57,7 +57,6 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", "--cluster.my-address=tcp://name-crdn-id1.name-int.ns.svc:8529", - "--cluster.my-id=id1", "--cluster.my-role=COORDINATOR", "--database.directory=/data", "--foxx.queues=true", @@ -100,7 +99,6 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { "--cluster.agency-endpoint=ssl://name-agnt-a2.name-int.ns.svc:8529", "--cluster.agency-endpoint=ssl://name-agnt-a3.name-int.ns.svc:8529", "--cluster.my-address=ssl://name-crdn-id1.name-int.ns.svc:8529", - "--cluster.my-id=id1", "--cluster.my-role=COORDINATOR", "--database.directory=/data", "--foxx.queues=true", @@ -143,7 +141,6 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", "--cluster.my-address=tcp://name-crdn-id1.name-int.ns.svc:8529", - "--cluster.my-id=id1", "--cluster.my-role=COORDINATOR", "--database.directory=/data", "--foxx.queues=true", @@ -184,7 +181,6 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", "--cluster.my-address=tcp://name-crdn-id1.name-int.ns.svc:8529", - "--cluster.my-id=id1", "--cluster.my-role=COORDINATOR", "--database.directory=/data", "--foxx.queues=true", diff --git a/pkg/deployment/pod_creator_dbserver_args_test.go b/pkg/deployment/pod_creator_dbserver_args_test.go index d3a577473..18126a76d 100644 --- a/pkg/deployment/pod_creator_dbserver_args_test.go +++ b/pkg/deployment/pod_creator_dbserver_args_test.go @@ -57,7 +57,6 @@ func TestCreateArangodArgsDBServer(t *testing.T) { "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", "--cluster.my-address=tcp://name-prmr-id1.name-int.ns.svc:8529", - "--cluster.my-id=id1", "--cluster.my-role=PRIMARY", "--database.directory=/data", "--foxx.queues=false", @@ -100,7 +99,6 @@ func TestCreateArangodArgsDBServer(t *testing.T) { "--cluster.agency-endpoint=ssl://name-agnt-a2.name-int.ns.svc:8529", "--cluster.agency-endpoint=ssl://name-agnt-a3.name-int.ns.svc:8529", "--cluster.my-address=ssl://name-prmr-id1.name-int.ns.svc:8529", - "--cluster.my-id=id1", "--cluster.my-role=PRIMARY", "--database.directory=/data", "--foxx.queues=false", @@ -143,7 +141,6 @@ func TestCreateArangodArgsDBServer(t *testing.T) { "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", "--cluster.my-address=tcp://name-prmr-id1.name-int.ns.svc:8529", - "--cluster.my-id=id1", "--cluster.my-role=PRIMARY", "--database.directory=/data", "--foxx.queues=false", @@ -184,7 +181,6 @@ func TestCreateArangodArgsDBServer(t *testing.T) { "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", "--cluster.my-address=tcp://name-prmr-id1.name-int.ns.svc:8529", - "--cluster.my-id=id1", "--cluster.my-role=PRIMARY", "--database.directory=/data", "--foxx.queues=false", diff --git a/pkg/deployment/pod_creator_single_args_test.go b/pkg/deployment/pod_creator_single_args_test.go index 0c2e56216..ba7d1a8c5 100644 --- a/pkg/deployment/pod_creator_single_args_test.go +++ b/pkg/deployment/pod_creator_single_args_test.go @@ -191,7 +191,6 @@ func TestCreateArangodArgsSingle(t *testing.T) { "--cluster.agency-endpoint=tcp://name-agnt-a2.name-int.ns.svc:8529", "--cluster.agency-endpoint=tcp://name-agnt-a3.name-int.ns.svc:8529", "--cluster.my-address=tcp://name-sngl-id1.name-int.ns.svc:8529", - "--cluster.my-id=id1", "--cluster.my-role=SINGLE", "--database.directory=/data", "--foxx.queues=true", From bc87d31cd39ed2f7461d74b53673f258e698e985 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Fri, 16 Mar 2018 14:35:25 +0100 Subject: [PATCH 11/17] Simplify createArangodArgs args --- .../deployment/v1alpha/deployment_spec.go | 21 +++++++++++++++++++ pkg/deployment/pod_creator.go | 5 +++-- pkg/deployment/pod_creator_agent_args_test.go | 8 +++---- .../pod_creator_coordinator_args_test.go | 8 +++---- .../pod_creator_dbserver_args_test.go | 8 +++---- .../pod_creator_single_args_test.go | 12 +++++------ 6 files changed, 42 insertions(+), 20 deletions(-) diff --git a/pkg/apis/deployment/v1alpha/deployment_spec.go b/pkg/apis/deployment/v1alpha/deployment_spec.go index 99973a314..c3a986c75 100644 --- a/pkg/apis/deployment/v1alpha/deployment_spec.go +++ b/pkg/apis/deployment/v1alpha/deployment_spec.go @@ -73,6 +73,27 @@ func (s DeploymentSpec) IsSecure() bool { return s.TLS.IsSecure() } +// GetServerGroupSpec returns the server group spec (from this +// deployment spec) for the given group. +func (s DeploymentSpec) GetServerGroupSpec(group ServerGroup) ServerGroupSpec { + switch group { + case ServerGroupSingle: + return s.Single + case ServerGroupAgents: + return s.Agents + case ServerGroupDBServers: + return s.DBServers + case ServerGroupCoordinators: + return s.Coordinators + case ServerGroupSyncMasters: + return s.SyncMasters + case ServerGroupSyncWorkers: + return s.SyncWorkers + default: + return ServerGroupSpec{} + } +} + // SetDefaults fills in default values when a field is not specified. func (s *DeploymentSpec) SetDefaults(deploymentName string) { if s.Mode == "" { diff --git a/pkg/deployment/pod_creator.go b/pkg/deployment/pod_creator.go index 4ac4e3704..5c470eedd 100644 --- a/pkg/deployment/pod_creator.go +++ b/pkg/deployment/pod_creator.go @@ -57,8 +57,9 @@ func (o optionPair) CompareTo(other optionPair) int { } // createArangodArgs creates command line arguments for an arangod server in the given group. -func createArangodArgs(apiObject metav1.Object, deplSpec api.DeploymentSpec, group api.ServerGroup, svrSpec api.ServerGroupSpec, agents api.MemberStatusList, id string) []string { +func createArangodArgs(apiObject metav1.Object, deplSpec api.DeploymentSpec, group api.ServerGroup, agents api.MemberStatusList, id string) []string { options := make([]optionPair, 0, 64) + svrSpec := deplSpec.GetServerGroupSpec(group) // Endpoint listenAddr := "[::]" @@ -310,7 +311,7 @@ func (d *Deployment) ensurePods(apiObject *api.ArangoDeployment) error { m.PodName = k8sutil.CreatePodName(apiObject.GetName(), role, m.ID, podSuffix) // Create pod if group.IsArangod() { - args := createArangodArgs(apiObject, apiObject.Spec, group, spec, d.status.Members.Agents, m.ID) + args := createArangodArgs(apiObject, apiObject.Spec, group, d.status.Members.Agents, m.ID) env := make(map[string]k8sutil.EnvValue) livenessProbe, err := d.createLivenessProbe(apiObject, group) if err != nil { diff --git a/pkg/deployment/pod_creator_agent_args_test.go b/pkg/deployment/pod_creator_agent_args_test.go index 68dcd1a74..982a8c691 100644 --- a/pkg/deployment/pod_creator_agent_args_test.go +++ b/pkg/deployment/pod_creator_agent_args_test.go @@ -50,7 +50,7 @@ func TestCreateArangodArgsAgent(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, apiObject.Spec.Agents, agents, "a1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1") assert.Equal(t, []string{ "--agency.activate=true", @@ -94,7 +94,7 @@ func TestCreateArangodArgsAgent(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, apiObject.Spec.Agents, agents, "a1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1") assert.Equal(t, []string{ "--agency.activate=true", @@ -139,7 +139,7 @@ func TestCreateArangodArgsAgent(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, apiObject.Spec.Agents, agents, "a1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1") assert.Equal(t, []string{ "--agency.activate=true", @@ -180,7 +180,7 @@ func TestCreateArangodArgsAgent(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, apiObject.Spec.Agents, agents, "a1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1") assert.Equal(t, []string{ "--agency.activate=true", diff --git a/pkg/deployment/pod_creator_coordinator_args_test.go b/pkg/deployment/pod_creator_coordinator_args_test.go index 197a792c3..4426d3109 100644 --- a/pkg/deployment/pod_creator_coordinator_args_test.go +++ b/pkg/deployment/pod_creator_coordinator_args_test.go @@ -50,7 +50,7 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, apiObject.Spec.Coordinators, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1") assert.Equal(t, []string{ "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", @@ -92,7 +92,7 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, apiObject.Spec.Coordinators, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1") assert.Equal(t, []string{ "--cluster.agency-endpoint=ssl://name-agnt-a1.name-int.ns.svc:8529", @@ -134,7 +134,7 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, apiObject.Spec.Coordinators, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1") assert.Equal(t, []string{ "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", @@ -174,7 +174,7 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, apiObject.Spec.Coordinators, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1") assert.Equal(t, []string{ "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", diff --git a/pkg/deployment/pod_creator_dbserver_args_test.go b/pkg/deployment/pod_creator_dbserver_args_test.go index 18126a76d..7b12bdccc 100644 --- a/pkg/deployment/pod_creator_dbserver_args_test.go +++ b/pkg/deployment/pod_creator_dbserver_args_test.go @@ -50,7 +50,7 @@ func TestCreateArangodArgsDBServer(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, apiObject.Spec.DBServers, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1") assert.Equal(t, []string{ "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", @@ -92,7 +92,7 @@ func TestCreateArangodArgsDBServer(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, apiObject.Spec.DBServers, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1") assert.Equal(t, []string{ "--cluster.agency-endpoint=ssl://name-agnt-a1.name-int.ns.svc:8529", @@ -134,7 +134,7 @@ func TestCreateArangodArgsDBServer(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, apiObject.Spec.DBServers, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1") assert.Equal(t, []string{ "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", @@ -174,7 +174,7 @@ func TestCreateArangodArgsDBServer(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, apiObject.Spec.DBServers, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1") assert.Equal(t, []string{ "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", diff --git a/pkg/deployment/pod_creator_single_args_test.go b/pkg/deployment/pod_creator_single_args_test.go index ba7d1a8c5..1cd78e6e2 100644 --- a/pkg/deployment/pod_creator_single_args_test.go +++ b/pkg/deployment/pod_creator_single_args_test.go @@ -41,7 +41,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { }, } apiObject.Spec.SetDefaults("test") - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, apiObject.Spec.Single, nil, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1") assert.Equal(t, []string{ "--database.directory=/data", @@ -69,7 +69,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { }, } apiObject.Spec.SetDefaults("test") - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, apiObject.Spec.Single, nil, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1") assert.Equal(t, []string{ "--database.directory=/data", @@ -97,7 +97,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { }, } apiObject.Spec.SetDefaults("test") - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, apiObject.Spec.Single, nil, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1") assert.Equal(t, []string{ "--database.directory=/data", @@ -123,7 +123,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { } apiObject.Spec.Authentication.JWTSecretName = "None" apiObject.Spec.SetDefaults("test") - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, apiObject.Spec.Single, nil, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1") assert.Equal(t, []string{ "--database.directory=/data", @@ -148,7 +148,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { } apiObject.Spec.Single.Args = []string{"--foo1", "--foo2"} apiObject.Spec.SetDefaults("test") - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, apiObject.Spec.Single, nil, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1") assert.Equal(t, []string{ "--database.directory=/data", @@ -184,7 +184,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, apiObject.Spec.Single, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, agents, "id1") assert.Equal(t, []string{ "--cluster.agency-endpoint=tcp://name-agnt-a1.name-int.ns.svc:8529", From 10962142a4eb14d593eb4335db24c5390d3478de Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Fri, 16 Mar 2018 15:05:49 +0100 Subject: [PATCH 12/17] Adding action reason to action --- examples/simple-cluster.yaml | 3 +++ pkg/apis/deployment/v1alpha/plan.go | 10 ++++++-- pkg/deployment/plan_builder.go | 39 ++++++++++++++++++++--------- 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/examples/simple-cluster.yaml b/examples/simple-cluster.yaml index e4a1ba0f3..fea5954db 100644 --- a/examples/simple-cluster.yaml +++ b/examples/simple-cluster.yaml @@ -5,3 +5,6 @@ metadata: spec: mode: cluster image: arangodb/arangodb:3.3.3 + coordinators: + args: + - --log.level=true diff --git a/pkg/apis/deployment/v1alpha/plan.go b/pkg/apis/deployment/v1alpha/plan.go index f6bb32198..988257d63 100644 --- a/pkg/apis/deployment/v1alpha/plan.go +++ b/pkg/apis/deployment/v1alpha/plan.go @@ -59,17 +59,23 @@ type Action struct { CreationTime metav1.Time `json:"creationTime"` // StartTime is set the when the action has been started, but needs to wait to be finished. StartTime *metav1.Time `json:"startTime,omitempty"` + // Reason for this action + Reason string `json:"reason,omitempty"` } // NewAction instantiates a new Action. -func NewAction(actionType ActionType, group ServerGroup, memberID string) Action { - return Action{ +func NewAction(actionType ActionType, group ServerGroup, memberID string, reason ...string) Action { + a := Action{ ID: uniuri.New(), Type: actionType, MemberID: memberID, Group: group, CreationTime: metav1.Now(), } + if len(reason) != 0 { + a.Reason = reason[0] + } + return a } // Plan is a list of actions that will be taken to update a deployment. diff --git a/pkg/deployment/plan_builder.go b/pkg/deployment/plan_builder.go index b67f61b2d..accb79d6f 100644 --- a/pkg/deployment/plan_builder.go +++ b/pkg/deployment/plan_builder.go @@ -28,6 +28,7 @@ import ( "github.com/rs/zerolog" "github.com/rs/zerolog/log" "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // createPlan considers the current specification & status of the deployment creates a plan to @@ -48,7 +49,7 @@ func (d *Deployment) createPlan() error { } // Create plan - newPlan, changed := createPlan(d.deps.Log, d.status.Plan, d.apiObject.Spec, d.status, myPods) + newPlan, changed := createPlan(d.deps.Log, d.apiObject, d.status.Plan, d.apiObject.Spec, d.status, myPods) // If not change, we're done if !changed { @@ -70,7 +71,7 @@ func (d *Deployment) createPlan() error { // createPlan considers the given specification & status and creates a plan to get the status in line with the specification. // If a plan already exists, the given plan is returned with false. // Otherwise the new plan is returned with a boolean true. -func createPlan(log zerolog.Logger, currentPlan api.Plan, spec api.DeploymentSpec, status api.DeploymentStatus, pods []v1.Pod) (api.Plan, bool) { +func createPlan(log zerolog.Logger, apiObject metav1.Object, currentPlan api.Plan, spec api.DeploymentSpec, status api.DeploymentStatus, pods []v1.Pod) (api.Plan, bool) { if len(currentPlan) > 0 { // Plan already exists, complete that first return currentPlan, false @@ -116,8 +117,9 @@ func createPlan(log zerolog.Logger, currentPlan api.Plan, spec api.DeploymentSpe if podName := m.PodName; podName != "" { if p := getPod(podName); p != nil { // Got pod, compare it with what it should be - if podNeedsRotation(*p, spec) { - plan = append(plan, createRotateMemberPlan(log, m, group)...) + rotNeeded, reason := podNeedsRotation(*p, apiObject, spec, group, status.Members.Agents, m.ID) + if rotNeeded { + plan = append(plan, createRotateMemberPlan(log, m, group, reason)...) } } } @@ -132,20 +134,33 @@ func createPlan(log zerolog.Logger, currentPlan api.Plan, spec api.DeploymentSpe // podNeedsRotation returns true when the specification of the // given pod differs from what it should be according to the // given deployment spec. -func podNeedsRotation(p v1.Pod, spec api.DeploymentSpec) bool { +// When true is returned, a reason for the rotation is already returned. +func podNeedsRotation(p v1.Pod, apiObject metav1.Object, spec api.DeploymentSpec, + group api.ServerGroup, agents api.MemberStatusList, id string) (bool, string) { // Check number of containers if len(p.Spec.Containers) != 1 { - return true + return true, "Number of containers changed" } // Check image c := p.Spec.Containers[0] - if c.Image != spec.Image || c.ImagePullPolicy != spec.ImagePullPolicy { - return true + if c.Image != spec.Image { + return true, "Image changed" + } + if c.ImagePullPolicy != spec.ImagePullPolicy { + return true, "Image pull policy changed" } // Check arguments - // TODO + /*expectedArgs := createArangodArgs(apiObject, spec, group, agents, id) + if len(expectedArgs) != len(c.Args) { + return true, "Arguments changed" + } + for i, a := range expectedArgs { + if c.Args[i] != a { + return true, "Arguments changed" + } + }*/ - return false + return false, "" } // createScalePlan creates a scaling plan for a single server group @@ -183,13 +198,13 @@ func createScalePlan(log zerolog.Logger, members api.MemberStatusList, group api // createRotateMemberPlan creates a plan to rotate (stop-recreate-start) an existing // member. -func createRotateMemberPlan(log zerolog.Logger, member api.MemberStatus, group api.ServerGroup) api.Plan { +func createRotateMemberPlan(log zerolog.Logger, member api.MemberStatus, group api.ServerGroup, reason string) api.Plan { log.Debug(). Str("id", member.ID). Str("role", group.AsRole()). Msg("Creating rotation plan") plan := api.Plan{ - api.NewAction(api.ActionTypeRotateMember, group, member.ID), + api.NewAction(api.ActionTypeRotateMember, group, member.ID, reason), api.NewAction(api.ActionTypeWaitForMemberUp, group, member.ID), } return plan From 8d4fb8ea118c05dc3d8a9316e25372d079296c9a Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Tue, 20 Mar 2018 13:41:01 +0100 Subject: [PATCH 13/17] Using clear roles again --- examples/nodeport-cluster.yaml | 2 +- examples/nodeport-single-server.yaml | 2 +- pkg/apis/deployment/v1alpha/server_group.go | 20 ++++++++++++++++++ .../deployment/v1alpha/server_group_test.go | 21 +++++++++++++------ pkg/deployment/pod_creator.go | 3 ++- pkg/util/k8sutil/services.go | 4 ++-- 6 files changed, 41 insertions(+), 11 deletions(-) diff --git a/examples/nodeport-cluster.yaml b/examples/nodeport-cluster.yaml index 680ad42cf..5de0b4c45 100644 --- a/examples/nodeport-cluster.yaml +++ b/examples/nodeport-cluster.yaml @@ -5,7 +5,7 @@ metadata: spec: selector: app: arangodb - role: crdn + role: coordinator type: NodePort publishNotReadyAddresses: true ports: diff --git a/examples/nodeport-single-server.yaml b/examples/nodeport-single-server.yaml index 6166d0211..60cc3f304 100644 --- a/examples/nodeport-single-server.yaml +++ b/examples/nodeport-single-server.yaml @@ -5,7 +5,7 @@ metadata: spec: selector: app: arangodb - role: sngl + role: single type: NodePort ports: - protocol: TCP diff --git a/pkg/apis/deployment/v1alpha/server_group.go b/pkg/apis/deployment/v1alpha/server_group.go index 288869e89..8d8725693 100644 --- a/pkg/apis/deployment/v1alpha/server_group.go +++ b/pkg/apis/deployment/v1alpha/server_group.go @@ -47,6 +47,26 @@ var ( // AsRole returns the "role" value for the given group. func (g ServerGroup) AsRole() string { + switch g { + case ServerGroupSingle: + return "single" + case ServerGroupAgents: + return "agent" + case ServerGroupDBServers: + return "dbserver" + case ServerGroupCoordinators: + return "coordinator" + case ServerGroupSyncMasters: + return "syncmaster" + case ServerGroupSyncWorkers: + return "syncworker" + default: + return "?" + } +} + +// AsRoleAbbreviated returns the abbreviation of the "role" value for the given group. +func (g ServerGroup) AsRoleAbbreviated() string { switch g { case ServerGroupSingle: return "sngl" diff --git a/pkg/apis/deployment/v1alpha/server_group_test.go b/pkg/apis/deployment/v1alpha/server_group_test.go index 1adeeced0..f3464d6ce 100644 --- a/pkg/apis/deployment/v1alpha/server_group_test.go +++ b/pkg/apis/deployment/v1alpha/server_group_test.go @@ -29,12 +29,21 @@ import ( ) func TestServerGroupAsRole(t *testing.T) { - assert.Equal(t, "sngl", ServerGroupSingle.AsRole()) - assert.Equal(t, "agnt", ServerGroupAgents.AsRole()) - assert.Equal(t, "prmr", ServerGroupDBServers.AsRole()) - assert.Equal(t, "crdn", ServerGroupCoordinators.AsRole()) - assert.Equal(t, "syma", ServerGroupSyncMasters.AsRole()) - assert.Equal(t, "sywo", ServerGroupSyncWorkers.AsRole()) + assert.Equal(t, "single", ServerGroupSingle.AsRole()) + assert.Equal(t, "agent", ServerGroupAgents.AsRole()) + assert.Equal(t, "dbserver", ServerGroupDBServers.AsRole()) + assert.Equal(t, "coordinator", ServerGroupCoordinators.AsRole()) + assert.Equal(t, "syncmaster", ServerGroupSyncMasters.AsRole()) + assert.Equal(t, "syncworker", ServerGroupSyncWorkers.AsRole()) +} + +func TestServerGroupAsRoleAbbreviated(t *testing.T) { + assert.Equal(t, "sngl", ServerGroupSingle.AsRoleAbbreviated()) + assert.Equal(t, "agnt", ServerGroupAgents.AsRoleAbbreviated()) + assert.Equal(t, "prmr", ServerGroupDBServers.AsRoleAbbreviated()) + assert.Equal(t, "crdn", ServerGroupCoordinators.AsRoleAbbreviated()) + assert.Equal(t, "syma", ServerGroupSyncMasters.AsRoleAbbreviated()) + assert.Equal(t, "sywo", ServerGroupSyncWorkers.AsRoleAbbreviated()) } func TestServerGroupIsArangod(t *testing.T) { diff --git a/pkg/deployment/pod_creator.go b/pkg/deployment/pod_creator.go index 72038ce50..ca6b8494a 100644 --- a/pkg/deployment/pod_creator.go +++ b/pkg/deployment/pod_creator.go @@ -307,8 +307,9 @@ func (d *Deployment) ensurePods(apiObject *api.ArangoDeployment) error { } // Update pod name role := group.AsRole() + roleAbbr := group.AsRoleAbbreviated() podSuffix := createPodSuffix(apiObject.Spec) - m.PodName = k8sutil.CreatePodName(apiObject.GetName(), role, m.ID, podSuffix) + m.PodName = k8sutil.CreatePodName(apiObject.GetName(), roleAbbr, m.ID, podSuffix) // Create pod if group.IsArangod() { // Find image ID diff --git a/pkg/util/k8sutil/services.go b/pkg/util/k8sutil/services.go index 78f719fe7..e881f3cc0 100644 --- a/pkg/util/k8sutil/services.go +++ b/pkg/util/k8sutil/services.go @@ -83,9 +83,9 @@ func CreateDatabaseClientService(kubecli kubernetes.Interface, deployment metav1 } var role string if single { - role = "sngl" + role = "single" } else { - role = "crdn" + role = "coordinator" } publishNotReadyAddresses := true sessionAffinity := v1.ServiceAffinityClientIP From affa7235e53417e2cca1aa926e289e73bf2e7efb Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Tue, 20 Mar 2018 15:46:28 +0100 Subject: [PATCH 14/17] Proper upgrade detection and --database.auto-upgrade flag --- examples/simple-cluster.yaml | 4 +- pkg/apis/deployment/v1alpha/conditions.go | 2 + pkg/apis/deployment/v1alpha/plan.go | 2 + pkg/deployment/action_rotate_member.go | 8 ++- pkg/deployment/plan_builder.go | 76 ++++++++++++++++++++--- pkg/deployment/pod_creator.go | 15 ++++- 6 files changed, 93 insertions(+), 14 deletions(-) diff --git a/examples/simple-cluster.yaml b/examples/simple-cluster.yaml index fea5954db..56ae336a6 100644 --- a/examples/simple-cluster.yaml +++ b/examples/simple-cluster.yaml @@ -4,7 +4,9 @@ metadata: name: "example-simple-cluster" spec: mode: cluster - image: arangodb/arangodb:3.3.3 + image: arangodb/arangodb:3.2.9 + tls: + altNames: ["kube-01", "kube-02", "kube-03"] coordinators: args: - --log.level=true diff --git a/pkg/apis/deployment/v1alpha/conditions.go b/pkg/apis/deployment/v1alpha/conditions.go index 8930de65e..9ecf645e3 100644 --- a/pkg/apis/deployment/v1alpha/conditions.go +++ b/pkg/apis/deployment/v1alpha/conditions.go @@ -35,6 +35,8 @@ const ( ConditionTypeReady ConditionType = "Ready" // ConditionTypeTerminated indicates that the member has terminated and will not restart. ConditionTypeTerminated ConditionType = "Terminated" + // ConditionTypeAutoUpgrade indicates that the member has to be started with `--database.auto-upgrade` once. + ConditionTypeAutoUpgrade ConditionType = "AutoUpgrade" ) // Condition represents one current condition of a deployment or deployment member. diff --git a/pkg/apis/deployment/v1alpha/plan.go b/pkg/apis/deployment/v1alpha/plan.go index 988257d63..401b125c5 100644 --- a/pkg/apis/deployment/v1alpha/plan.go +++ b/pkg/apis/deployment/v1alpha/plan.go @@ -61,6 +61,8 @@ type Action struct { StartTime *metav1.Time `json:"startTime,omitempty"` // Reason for this action Reason string `json:"reason,omitempty"` + // AutoUpgrade indicates the need for an `--database.auto-upgrade` of the member. + AutoUpgrade bool `json:"auto-upgrade,omitempty"` } // NewAction instantiates a new Action. diff --git a/pkg/deployment/action_rotate_member.go b/pkg/deployment/action_rotate_member.go index a1ac28637..b14d6a070 100644 --- a/pkg/deployment/action_rotate_member.go +++ b/pkg/deployment/action_rotate_member.go @@ -55,7 +55,13 @@ func (a *actionRotateMember) Start(ctx context.Context) (bool, error) { m, ok := a.actionCtx.GetMemberStatusByID(a.action.MemberID) if !ok { log.Error().Msg("No such member") - return true, nil + } + if a.action.AutoUpgrade { + // Set AutoUpgrade condition + m.Conditions.Update(api.ConditionTypeAutoUpgrade, true, "Rotate with AutoUpgrade", "AutoUpgrade on first restart") + if err := a.actionCtx.UpdateMember(m); err != nil { + return false, maskAny(err) + } } if group.IsArangod() { // Invoke shutdown endpoint diff --git a/pkg/deployment/plan_builder.go b/pkg/deployment/plan_builder.go index accb79d6f..5dcfbdf68 100644 --- a/pkg/deployment/plan_builder.go +++ b/pkg/deployment/plan_builder.go @@ -31,6 +31,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// upgradeDecision is the result of an upgrade check. +type upgradeDecision struct { + UpgradeNeeded bool // If set, the image version has changed + UpgradeAllowed bool // If set, it is an allowed version change + AutoUpgradeNeeded bool // If set, the database must be started with `--database.auto-upgrade` once +} + // createPlan considers the current specification & status of the deployment creates a plan to // get the status in line with the specification. // If a plan already exists, nothing is done. @@ -71,7 +78,9 @@ func (d *Deployment) createPlan() error { // createPlan considers the given specification & status and creates a plan to get the status in line with the specification. // If a plan already exists, the given plan is returned with false. // Otherwise the new plan is returned with a boolean true. -func createPlan(log zerolog.Logger, apiObject metav1.Object, currentPlan api.Plan, spec api.DeploymentSpec, status api.DeploymentStatus, pods []v1.Pod) (api.Plan, bool) { +func createPlan(log zerolog.Logger, apiObject metav1.Object, + currentPlan api.Plan, spec api.DeploymentSpec, + status api.DeploymentStatus, pods []v1.Pod) (api.Plan, bool) { if len(currentPlan) > 0 { // Plan already exists, complete that first return currentPlan, false @@ -117,9 +126,14 @@ func createPlan(log zerolog.Logger, apiObject metav1.Object, currentPlan api.Pla if podName := m.PodName; podName != "" { if p := getPod(podName); p != nil { // Got pod, compare it with what it should be - rotNeeded, reason := podNeedsRotation(*p, apiObject, spec, group, status.Members.Agents, m.ID) - if rotNeeded { - plan = append(plan, createRotateMemberPlan(log, m, group, reason)...) + decision := podNeedsUpgrading(*p, spec, status.Images) + if decision.UpgradeNeeded && decision.UpgradeAllowed { + plan = append(plan, createRotateMemberPlan(log, m, group, "Version upgrade", decision.AutoUpgradeNeeded)...) + } else { + rotNeeded, reason := podNeedsRotation(*p, apiObject, spec, group, status.Members.Agents, m.ID) + if rotNeeded { + plan = append(plan, createRotateMemberPlan(log, m, group, reason, false)...) + } } } } @@ -131,6 +145,48 @@ func createPlan(log zerolog.Logger, apiObject metav1.Object, currentPlan api.Pla return plan, true } +// podNeedsUpgrading decides if an upgrade of the pod is needed (to comply with +// the given spec) and if that is allowed. +func podNeedsUpgrading(p v1.Pod, spec api.DeploymentSpec, images api.ImageInfoList) upgradeDecision { + if len(p.Spec.Containers) == 1 { + c := p.Spec.Containers[0] + specImageInfo, found := images.GetByImage(spec.Image) + if !found { + return upgradeDecision{UpgradeNeeded: false} + } + podImageInfo, found := images.GetByImageID(c.Image) + if !found { + return upgradeDecision{UpgradeNeeded: false} + } + if specImageInfo.ImageID == podImageInfo.ImageID { + // No change + return upgradeDecision{UpgradeNeeded: false} + } + // Image changed, check if change is allowed + specVersion := specImageInfo.ArangoDBVersion + podVersion := podImageInfo.ArangoDBVersion + if specVersion.Major() != podVersion.Major() { + // E.g. 3.x -> 4.x, we cannot allow automatically + return upgradeDecision{UpgradeNeeded: true, UpgradeAllowed: false} + } + if specVersion.Minor() != podVersion.Minor() { + // Is allowed, with `--database.auto-upgrade` + return upgradeDecision{ + UpgradeNeeded: true, + UpgradeAllowed: true, + AutoUpgradeNeeded: true, + } + } + // Patch version change, rotate only + return upgradeDecision{ + UpgradeNeeded: true, + UpgradeAllowed: true, + AutoUpgradeNeeded: false, + } + } + return upgradeDecision{UpgradeNeeded: false} +} + // podNeedsRotation returns true when the specification of the // given pod differs from what it should be according to the // given deployment spec. @@ -141,11 +197,8 @@ func podNeedsRotation(p v1.Pod, apiObject metav1.Object, spec api.DeploymentSpec if len(p.Spec.Containers) != 1 { return true, "Number of containers changed" } - // Check image + // Check image pull policy c := p.Spec.Containers[0] - if c.Image != spec.Image { - return true, "Image changed" - } if c.ImagePullPolicy != spec.ImagePullPolicy { return true, "Image pull policy changed" } @@ -198,13 +251,16 @@ func createScalePlan(log zerolog.Logger, members api.MemberStatusList, group api // createRotateMemberPlan creates a plan to rotate (stop-recreate-start) an existing // member. -func createRotateMemberPlan(log zerolog.Logger, member api.MemberStatus, group api.ServerGroup, reason string) api.Plan { +func createRotateMemberPlan(log zerolog.Logger, member api.MemberStatus, + group api.ServerGroup, reason string, autoUpgrade bool) api.Plan { log.Debug(). Str("id", member.ID). Str("role", group.AsRole()). Msg("Creating rotation plan") + rotateAction := api.NewAction(api.ActionTypeRotateMember, group, member.ID, reason) + rotateAction.AutoUpgrade = autoUpgrade plan := api.Plan{ - api.NewAction(api.ActionTypeRotateMember, group, member.ID, reason), + rotateAction, api.NewAction(api.ActionTypeWaitForMemberUp, group, member.ID), } return plan diff --git a/pkg/deployment/pod_creator.go b/pkg/deployment/pod_creator.go index ca6b8494a..960eb9e02 100644 --- a/pkg/deployment/pod_creator.go +++ b/pkg/deployment/pod_creator.go @@ -57,7 +57,8 @@ func (o optionPair) CompareTo(other optionPair) int { } // createArangodArgs creates command line arguments for an arangod server in the given group. -func createArangodArgs(apiObject metav1.Object, deplSpec api.DeploymentSpec, group api.ServerGroup, agents api.MemberStatusList, id string) []string { +func createArangodArgs(apiObject metav1.Object, deplSpec api.DeploymentSpec, group api.ServerGroup, + agents api.MemberStatusList, id string, autoUpgrade bool) []string { options := make([]optionPair, 0, 64) svrSpec := deplSpec.GetServerGroupSpec(group) @@ -126,6 +127,14 @@ func createArangodArgs(apiObject metav1.Object, deplSpec api.DeploymentSpec, gro optionPair{"--database.directory", k8sutil.ArangodVolumeMountDir}, optionPair{"--log.output", "+"}, ) + + // Auto upgrade? + if autoUpgrade { + options = append(options, + optionPair{"--database.auto-upgrade", "true"}, + ) + } + /* if config.ServerThreads != 0 { options = append(options, optionPair{"--server.threads", strconv.Itoa(config.ServerThreads)}) @@ -319,7 +328,8 @@ func (d *Deployment) ensurePods(apiObject *api.ArangoDeployment) error { return nil } // Prepare arguments - args := createArangodArgs(apiObject, apiObject.Spec, group, d.status.Members.Agents, m.ID) + autoUpgrade := m.Conditions.IsTrue(api.ConditionTypeAutoUpgrade) + args := createArangodArgs(apiObject, apiObject.Spec, group, d.status.Members.Agents, m.ID, autoUpgrade) env := make(map[string]k8sutil.EnvValue) livenessProbe, err := d.createLivenessProbe(apiObject, group) if err != nil { @@ -383,6 +393,7 @@ func (d *Deployment) ensurePods(apiObject *api.ArangoDeployment) error { m.State = api.MemberStateCreated m.Conditions.Remove(api.ConditionTypeReady) m.Conditions.Remove(api.ConditionTypeTerminated) + m.Conditions.Remove(api.ConditionTypeAutoUpgrade) if err := status.Update(m); err != nil { return maskAny(err) } From 0635fd41b119d036884a9258c63d4192fc3e40b0 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Tue, 20 Mar 2018 17:14:28 +0100 Subject: [PATCH 15/17] Working on agency is happy check --- examples/simple-cluster.yaml | 2 +- pkg/deployment/action_context.go | 26 +++- pkg/deployment/action_wait_for_member_up.go | 77 ++++++++++- pkg/util/arangod/agency.go | 146 ++++++++++++++++++++ pkg/util/arangod/endpoint.go | 42 ++++++ pkg/util/arangod/error.go | 27 ++++ 6 files changed, 311 insertions(+), 9 deletions(-) create mode 100644 pkg/util/arangod/agency.go create mode 100644 pkg/util/arangod/endpoint.go diff --git a/examples/simple-cluster.yaml b/examples/simple-cluster.yaml index 56ae336a6..52fb63621 100644 --- a/examples/simple-cluster.yaml +++ b/examples/simple-cluster.yaml @@ -4,7 +4,7 @@ metadata: name: "example-simple-cluster" spec: mode: cluster - image: arangodb/arangodb:3.2.9 + image: arangodb/arangodb:3.3.4 tls: altNames: ["kube-01", "kube-02", "kube-03"] coordinators: diff --git a/pkg/deployment/action_context.go b/pkg/deployment/action_context.go index 3b075d905..1ce049edd 100644 --- a/pkg/deployment/action_context.go +++ b/pkg/deployment/action_context.go @@ -27,11 +27,13 @@ import ( "fmt" driver "github.com/arangodb/go-driver" - api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" - "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" "github.com/rs/zerolog" "github.com/rs/zerolog/log" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/arangodb/kube-arangodb/pkg/util/arangod" + "github.com/arangodb/kube-arangodb/pkg/util/k8sutil" ) // ActionContext provides methods to the Action implementations @@ -44,6 +46,8 @@ type ActionContext interface { GetDatabaseClient(ctx context.Context) (driver.Client, error) // GetServerClient returns a cached client for a specific server. GetServerClient(ctx context.Context, group api.ServerGroup, id string) (driver.Client, error) + // GetAgencyClients returns a client connection for every agency member. + GetAgencyClients(ctx context.Context) ([]arangod.Agency, error) // GetMemberStatusByID returns the current member status // for the member with given id. // Returns member status, true when found, or false @@ -101,6 +105,24 @@ func (ac *actionContext) GetServerClient(ctx context.Context, group api.ServerGr return c, nil } +// GetAgencyClients returns a client connection for every agency member. +func (ac *actionContext) GetAgencyClients(ctx context.Context) ([]arangod.Agency, error) { + agencyMembers := ac.deployment.status.Members.Agents + result := make([]arangod.Agency, 0, len(agencyMembers)) + for _, m := range agencyMembers { + client, err := ac.GetServerClient(ctx, api.ServerGroupAgents, m.ID) + if err != nil { + return nil, maskAny(err) + } + aClient, err := arangod.NewAgencyClient(client) + if err != nil { + return nil, maskAny(err) + } + result = append(result, aClient) + } + return result, nil +} + // GetMemberStatusByID returns the current member status // for the member with given id. // Returns member status, true when found, or false diff --git a/pkg/deployment/action_wait_for_member_up.go b/pkg/deployment/action_wait_for_member_up.go index 1b2ed9a40..be90fedae 100644 --- a/pkg/deployment/action_wait_for_member_up.go +++ b/pkg/deployment/action_wait_for_member_up.go @@ -24,10 +24,14 @@ package deployment import ( "context" + "sync" + "time" driver "github.com/arangodb/go-driver" - api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" "github.com/rs/zerolog" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/arangodb/kube-arangodb/pkg/util/arangod" ) // NewWaitForMemberUpAction creates a new Action that implements the given @@ -40,6 +44,10 @@ func NewWaitForMemberUpAction(log zerolog.Logger, action api.Action, actionCtx A } } +const ( + maxAgentResponseTime = time.Second * 10 +) + // actionWaitForMemberUp implements an WaitForMemberUp. type actionWaitForMemberUp struct { log zerolog.Logger @@ -91,19 +99,76 @@ func (a *actionWaitForMemberUp) checkProgressSingle(ctx context.Context) (bool, return true, nil } +type agentStatus struct { + IsLeader bool + LeaderEndpoint string + IsResponding bool +} + // checkProgressAgent checks the progress of the action in the case // of an agent. func (a *actionWaitForMemberUp) checkProgressAgent(ctx context.Context) (bool, error) { log := a.log - c, err := a.actionCtx.GetDatabaseClient(ctx) + clients, err := a.actionCtx.GetAgencyClients(ctx) if err != nil { - log.Debug().Err(err).Msg("Failed to create database client") + log.Debug().Err(err).Msg("Failed to create agency clients") return false, maskAny(err) } - if _, err := c.Version(ctx); err != nil { - log.Debug().Err(err).Msg("Failed to get version") - return false, maskAny(err) + + wg := sync.WaitGroup{} + invalidKey := []string{"does-not-exists-149e97e8-4b81-5664-a8a8-9ba93881d64c"} + statuses := make([]agentStatus, len(clients)) + for i, c := range clients { + wg.Add(1) + go func(i int, c arangod.Agency) { + defer wg.Done() + var trash interface{} + lctx, cancel := context.WithTimeout(ctx, maxAgentResponseTime) + defer cancel() + if err := c.ReadKey(lctx, invalidKey, &trash); err == nil || arangod.IsKeyNotFound(err) { + // We got a valid read from the leader + statuses[i].IsLeader = true + statuses[i].LeaderEndpoint = c.Endpoint() + statuses[i].IsResponding = true + } else { + if location, ok := arangod.IsNotLeader(err); ok { + // Valid response from a follower + statuses[i].IsLeader = false + statuses[i].LeaderEndpoint = location + statuses[i].IsResponding = true + } else { + // Unexpected / invalid response + statuses[i].IsResponding = false + } + } + }(i, c) } + wg.Wait() + + // Check the results + noLeaders := 0 + for i, status := range statuses { + if !status.IsResponding { + log.Debug().Msg("Not all agents are responding") + return false, nil + } + if status.IsLeader { + noLeaders++ + } + if i > 0 { + // Compare leader endpoint with previous + prev := statuses[i-1].LeaderEndpoint + if !arangod.IsSameEndpoint(prev, status.LeaderEndpoint) { + log.Debug().Msg("Not all agents report the same leader endpoint") + return false, nil + } + } + } + if noLeaders != 1 { + log.Debug().Int("leaders", noLeaders).Msg("Unexpected number of agency leaders") + return false, nil + } + return true, nil } diff --git a/pkg/util/arangod/agency.go b/pkg/util/arangod/agency.go new file mode 100644 index 000000000..db956417f --- /dev/null +++ b/pkg/util/arangod/agency.go @@ -0,0 +1,146 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 Ewout Prangsma +// + +package arangod + +import ( + "context" + "encoding/json" + "fmt" + "strings" + + driver "github.com/arangodb/go-driver" + "github.com/pkg/errors" +) + +// Agency provides API implemented by the ArangoDB agency. +type Agency interface { + // ReadKey reads the value of a given key in the agency. + ReadKey(ctx context.Context, key []string, value interface{}) error + // Endpoint returns the endpoint of this agent connection + Endpoint() string +} + +// NewAgencyClient creates a new Agency connection from the given client +// connection. +// The number of endpoints of the client must be exactly 1. +func NewAgencyClient(c driver.Client) (Agency, error) { + if len(c.Connection().Endpoints()) > 1 { + return nil, maskAny(fmt.Errorf("Got multiple endpoints")) + } + return &agency{ + conn: c.Connection(), + }, nil +} + +type agency struct { + conn driver.Connection +} + +// ReadKey reads the value of a given key in the agency. +func (a *agency) ReadKey(ctx context.Context, key []string, value interface{}) error { + conn := a.conn + req, err := conn.NewRequest("POST", "_api/agency/read") + if err != nil { + return maskAny(err) + } + fullKey := createFullKey(key) + input := [][]string{{fullKey}} + req, err = req.SetBody(input) + if err != nil { + return maskAny(err) + } + //var raw []byte + //ctx = driver.WithRawResponse(ctx, &raw) + resp, err := conn.Do(ctx, req) + if err != nil { + return maskAny(err) + } + if resp.StatusCode() == 307 { + // Not leader + location := resp.Header("Location") + return NotLeaderError{Leader: location} + } + if err := resp.CheckStatus(200, 201, 202); err != nil { + return maskAny(err) + } + //fmt.Printf("Agent response: %s\n", string(raw)) + elems, err := resp.ParseArrayBody() + if err != nil { + return maskAny(err) + } + if len(elems) != 1 { + return maskAny(fmt.Errorf("Expected 1 element, got %d", len(elems))) + } + // If empty key parse directly + if len(key) == 0 { + if err := elems[0].ParseBody("", &value); err != nil { + return maskAny(err) + } + } else { + // Now remove all wrapping objects for each key element + var rawObject map[string]interface{} + if err := elems[0].ParseBody("", &rawObject); err != nil { + return maskAny(err) + } + var rawMsg interface{} + for keyIndex := 0; keyIndex < len(key); keyIndex++ { + if keyIndex > 0 { + var ok bool + rawObject, ok = rawMsg.(map[string]interface{}) + if !ok { + return maskAny(fmt.Errorf("Data is not an object at key %s", key[:keyIndex+1])) + } + } + var found bool + rawMsg, found = rawObject[key[keyIndex]] + if !found { + return errors.Wrapf(KeyNotFoundError, "Missing data at key %s", key[:keyIndex+1]) + } + } + // Encode to json ... + encoded, err := json.Marshal(rawMsg) + if err != nil { + return maskAny(err) + } + // and decode back into result + if err := json.Unmarshal(encoded, &value); err != nil { + return maskAny(err) + } + } + + // fmt.Printf("result as JSON: %s\n", rawResult) + return nil +} + +// Endpoint returns the endpoint of this agent connection +func (a *agency) Endpoint() string { + ep := a.conn.Endpoints() + if len(ep) == 0 { + return "" + } + return ep[0] +} + +func createFullKey(key []string) string { + return "/" + strings.Join(key, "/") +} diff --git a/pkg/util/arangod/endpoint.go b/pkg/util/arangod/endpoint.go new file mode 100644 index 000000000..d5f2d0641 --- /dev/null +++ b/pkg/util/arangod/endpoint.go @@ -0,0 +1,42 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 Ewout Prangsma +// + +package arangod + +import "net/url" + +// IsSameEndpoint returns true when the 2 given endpoints +// refer to the same server. +func IsSameEndpoint(a, b string) bool { + if a == b { + return true + } + ua, err := url.Parse(a) + if err != nil { + return false + } + ub, err := url.Parse(b) + if err != nil { + return false + } + return ua.Hostname() == ub.Hostname() +} diff --git a/pkg/util/arangod/error.go b/pkg/util/arangod/error.go index 7ef7f5e00..b87e57ac4 100644 --- a/pkg/util/arangod/error.go +++ b/pkg/util/arangod/error.go @@ -25,5 +25,32 @@ package arangod import "github.com/pkg/errors" var ( + KeyNotFoundError = errors.New("Key not found") + maskAny = errors.WithStack ) + +// IsKeyNotFound returns true if the given error is (or is caused by) a KeyNotFoundError. +func IsKeyNotFound(err error) bool { + return errors.Cause(err) == KeyNotFoundError +} + +// NotLeaderError indicates the response of an agent when it is +// not the leader of the agency. +type NotLeaderError struct { + Leader string // Endpoint of the current leader +} + +// Error implements error. +func (e NotLeaderError) Error() string { + return "not the leader" +} + +// IsNotLeader returns true if the given error is (or is caused by) a NotLeaderError. +func IsNotLeader(err error) (string, bool) { + nlErr, ok := err.(NotLeaderError) + if ok { + return nlErr.Leader, true + } + return "", false +} From 687aa650a387be474d89ab8e35a40fedfd1e17e5 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Thu, 22 Mar 2018 11:27:50 +0100 Subject: [PATCH 16/17] Properly upgrading to new minor version --- .../arangodb/go-driver/http/connection.go | 9 +- pkg/apis/deployment/v1alpha/member_state.go | 2 + pkg/apis/deployment/v1alpha/plan.go | 4 +- pkg/deployment/action_rotate_member.go | 7 - pkg/deployment/action_upgrade_member.go | 127 ++++++++++++++++++ pkg/deployment/action_wait_for_member_up.go | 6 + pkg/deployment/images.go | 10 +- pkg/deployment/plan_builder.go | 25 +++- pkg/deployment/plan_executor.go | 2 + pkg/deployment/pod_creator.go | 6 +- pkg/deployment/pod_inspector.go | 6 +- pkg/util/arangod/agency.go | 1 + pkg/util/arangod/client.go | 5 +- pkg/util/k8sutil/util.go | 26 +++- tools/manifests/manifest_builder.go | 7 +- 15 files changed, 213 insertions(+), 30 deletions(-) create mode 100644 pkg/deployment/action_upgrade_member.go diff --git a/deps/github.com/arangodb/go-driver/http/connection.go b/deps/github.com/arangodb/go-driver/http/connection.go index bb6f70a47..92ee9f4b1 100644 --- a/deps/github.com/arangodb/go-driver/http/connection.go +++ b/deps/github.com/arangodb/go-driver/http/connection.go @@ -67,6 +67,9 @@ type ConnectionConfig struct { // directly after use, resulting in a large number of connections in `TIME_WAIT` state. // When this value is not set, the driver will set it to 64 automatically. Transport http.RoundTripper + // DontFollowRedirect; if set, redirect will not be followed, response from the initial request will be returned without an error + // DontFollowRedirect takes precendance over FailOnRedirect. + DontFollowRedirect bool // FailOnRedirect; if set, redirect will not be followed, instead the status code is returned as error FailOnRedirect bool // Cluster configuration settings @@ -137,7 +140,11 @@ func newHTTPConnection(endpoint string, config ConnectionConfig) (driver.Connect httpClient := &http.Client{ Transport: config.Transport, } - if config.FailOnRedirect { + if config.DontFollowRedirect { + httpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse // Do not wrap, standard library will not understand + } + } else if config.FailOnRedirect { httpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { return driver.ArangoError{ HasError: true, diff --git a/pkg/apis/deployment/v1alpha/member_state.go b/pkg/apis/deployment/v1alpha/member_state.go index 825827b32..4fe637268 100644 --- a/pkg/apis/deployment/v1alpha/member_state.go +++ b/pkg/apis/deployment/v1alpha/member_state.go @@ -36,4 +36,6 @@ const ( MemberStateShuttingDown MemberState = "ShuttingDown" // MemberStateRotating indicates that a member is being rotated MemberStateRotating MemberState = "Rotating" + // MemberStateUpgrading indicates that a member is in the process of upgrading its database data format + MemberStateUpgrading MemberState = "Upgrading" ) diff --git a/pkg/apis/deployment/v1alpha/plan.go b/pkg/apis/deployment/v1alpha/plan.go index 401b125c5..27793d45d 100644 --- a/pkg/apis/deployment/v1alpha/plan.go +++ b/pkg/apis/deployment/v1alpha/plan.go @@ -41,6 +41,8 @@ const ( ActionTypeShutdownMember ActionType = "ShutdownMember" // ActionTypeRotateMember causes a member to be shutdown and have it's pod removed. ActionTypeRotateMember ActionType = "RotateMember" + // ActionTypeUpgradeMember causes a member to be shutdown and have it's pod removed, restarted with AutoUpgrade option, waited until termination and the restarted again. + ActionTypeUpgradeMember ActionType = "UpgradeMember" // ActionTypeWaitForMemberUp causes the plan to wait until the member is considered "up". ActionTypeWaitForMemberUp ActionType = "WaitForMemberUp" ) @@ -61,8 +63,6 @@ type Action struct { StartTime *metav1.Time `json:"startTime,omitempty"` // Reason for this action Reason string `json:"reason,omitempty"` - // AutoUpgrade indicates the need for an `--database.auto-upgrade` of the member. - AutoUpgrade bool `json:"auto-upgrade,omitempty"` } // NewAction instantiates a new Action. diff --git a/pkg/deployment/action_rotate_member.go b/pkg/deployment/action_rotate_member.go index b14d6a070..4fa495023 100644 --- a/pkg/deployment/action_rotate_member.go +++ b/pkg/deployment/action_rotate_member.go @@ -56,13 +56,6 @@ func (a *actionRotateMember) Start(ctx context.Context) (bool, error) { if !ok { log.Error().Msg("No such member") } - if a.action.AutoUpgrade { - // Set AutoUpgrade condition - m.Conditions.Update(api.ConditionTypeAutoUpgrade, true, "Rotate with AutoUpgrade", "AutoUpgrade on first restart") - if err := a.actionCtx.UpdateMember(m); err != nil { - return false, maskAny(err) - } - } if group.IsArangod() { // Invoke shutdown endpoint c, err := a.actionCtx.GetServerClient(ctx, group, a.action.MemberID) diff --git a/pkg/deployment/action_upgrade_member.go b/pkg/deployment/action_upgrade_member.go new file mode 100644 index 000000000..ed4f65ac9 --- /dev/null +++ b/pkg/deployment/action_upgrade_member.go @@ -0,0 +1,127 @@ +// +// DISCLAIMER +// +// Copyright 2018 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 Ewout Prangsma +// + +package deployment + +import ( + "context" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha" + "github.com/rs/zerolog" +) + +// NewUpgradeMemberAction creates a new Action that implements the given +// planned UpgradeMember action. +func NewUpgradeMemberAction(log zerolog.Logger, action api.Action, actionCtx ActionContext) Action { + return &actionUpgradeMember{ + log: log, + action: action, + actionCtx: actionCtx, + } +} + +// actionUpgradeMember implements an UpgradeMember. +type actionUpgradeMember struct { + log zerolog.Logger + action api.Action + actionCtx ActionContext +} + +// Start performs the start of the action. +// Returns true if the action is completely finished, false in case +// the start time needs to be recorded and a ready condition needs to be checked. +func (a *actionUpgradeMember) Start(ctx context.Context) (bool, error) { + log := a.log + group := a.action.Group + m, ok := a.actionCtx.GetMemberStatusByID(a.action.MemberID) + if !ok { + log.Error().Msg("No such member") + } + // Set AutoUpgrade condition + m.Conditions.Update(api.ConditionTypeAutoUpgrade, true, "Upgrading", "AutoUpgrade on first restart") + if err := a.actionCtx.UpdateMember(m); err != nil { + return false, maskAny(err) + } + if group.IsArangod() { + // Invoke shutdown endpoint + c, err := a.actionCtx.GetServerClient(ctx, group, a.action.MemberID) + if err != nil { + log.Debug().Err(err).Msg("Failed to create member client") + return false, maskAny(err) + } + removeFromCluster := false + log.Debug().Bool("removeFromCluster", removeFromCluster).Msg("Shutting down member") + ctx, cancel := context.WithTimeout(ctx, shutdownTimeout) + defer cancel() + if err := c.Shutdown(ctx, removeFromCluster); err != nil { + // Shutdown failed. Let's check if we're already done + if ready, err := a.CheckProgress(ctx); err == nil && ready { + // We're done + return true, nil + } + log.Debug().Err(err).Msg("Failed to shutdown member") + return false, maskAny(err) + } + } else if group.IsArangosync() { + // Terminate pod + if err := a.actionCtx.DeletePod(m.PodName); err != nil { + return false, maskAny(err) + } + } + // Update status + m.State = api.MemberStateRotating // We keep the rotation state here, since only when a new pod is created, it will get the Upgrading state. + if err := a.actionCtx.UpdateMember(m); err != nil { + return false, maskAny(err) + } + return false, nil +} + +// CheckProgress checks the progress of the action. +// Returns true if the action is completely finished, false otherwise. +func (a *actionUpgradeMember) CheckProgress(ctx context.Context) (bool, error) { + // Check that pod is removed + log := a.log + m, found := a.actionCtx.GetMemberStatusByID(a.action.MemberID) + if !found { + log.Error().Msg("No such member") + return true, nil + } + isUpgrading := m.State == api.MemberStateUpgrading + log = log.With(). + Str("pod-name", m.PodName). + Bool("is-upgrading", isUpgrading).Logger() + if !m.Conditions.IsTrue(api.ConditionTypeTerminated) { + // Pod is not yet terminated + return false, nil + } + // Pod is terminated, we can now remove it + log.Debug().Msg("Deleting pod") + if err := a.actionCtx.DeletePod(m.PodName); err != nil { + return false, maskAny(err) + } + // Pod is now gone, update the member status + m.State = api.MemberStateNone + if err := a.actionCtx.UpdateMember(m); err != nil { + return false, maskAny(err) + } + return isUpgrading, nil +} diff --git a/pkg/deployment/action_wait_for_member_up.go b/pkg/deployment/action_wait_for_member_up.go index be90fedae..d5dd75aa4 100644 --- a/pkg/deployment/action_wait_for_member_up.go +++ b/pkg/deployment/action_wait_for_member_up.go @@ -138,6 +138,7 @@ func (a *actionWaitForMemberUp) checkProgressAgent(ctx context.Context) (bool, e statuses[i].IsResponding = true } else { // Unexpected / invalid response + log.Debug().Err(err).Str("endpoint", c.Endpoint()).Msg("Agent is not responding") statuses[i].IsResponding = false } } @@ -169,6 +170,11 @@ func (a *actionWaitForMemberUp) checkProgressAgent(ctx context.Context) (bool, e return false, nil } + log.Debug(). + Int("leaders", noLeaders). + Int("followers", len(statuses)-noLeaders). + Msg("Agency is happy") + return true, nil } diff --git a/pkg/deployment/images.go b/pkg/deployment/images.go index 502b3d27f..dede0d0d4 100644 --- a/pkg/deployment/images.go +++ b/pkg/deployment/images.go @@ -29,6 +29,7 @@ import ( "strings" "github.com/rs/zerolog" + "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" @@ -39,6 +40,7 @@ import ( const ( dockerPullableImageIDPrefix = "docker-pullable://" + imageIDAndVersionRole = "id" // Role use by identification pods ) type imagesBuilder struct { @@ -97,7 +99,7 @@ func (ib *imagesBuilder) Run(ctx context.Context) (bool, error) { // When no pod exists, it is created, otherwise the ID is fetched & version detected. // Returns: retrySoon, error func (ib *imagesBuilder) fetchArangoDBImageIDAndVersion(ctx context.Context, image string) (bool, error) { - role := "id" + role := imageIDAndVersionRole id := fmt.Sprintf("%0x", sha1.Sum([]byte(image)))[:6] podName := k8sutil.CreatePodName(ib.APIObject.GetName(), role, id, "") ns := ib.APIObject.GetNamespace() @@ -173,3 +175,9 @@ func (ib *imagesBuilder) fetchArangoDBImageIDAndVersion(ctx context.Context, ima // Come back soon to inspect the pod return true, nil } + +// isArangoDBImageIDAndVersionPod returns true if the given pod is used for fetching image ID and ArangoDB version of an image +func isArangoDBImageIDAndVersionPod(p v1.Pod) bool { + role, found := p.GetLabels()[k8sutil.LabelKeyRole] + return found && role == imageIDAndVersionRole +} diff --git a/pkg/deployment/plan_builder.go b/pkg/deployment/plan_builder.go index 5dcfbdf68..5c1bc9ac1 100644 --- a/pkg/deployment/plan_builder.go +++ b/pkg/deployment/plan_builder.go @@ -128,11 +128,11 @@ func createPlan(log zerolog.Logger, apiObject metav1.Object, // Got pod, compare it with what it should be decision := podNeedsUpgrading(*p, spec, status.Images) if decision.UpgradeNeeded && decision.UpgradeAllowed { - plan = append(plan, createRotateMemberPlan(log, m, group, "Version upgrade", decision.AutoUpgradeNeeded)...) + plan = append(plan, createUpgradeMemberPlan(log, m, group, "Version upgrade")...) } else { rotNeeded, reason := podNeedsRotation(*p, apiObject, spec, group, status.Members.Agents, m.ID) if rotNeeded { - plan = append(plan, createRotateMemberPlan(log, m, group, reason, false)...) + plan = append(plan, createRotateMemberPlan(log, m, group, reason)...) } } } @@ -252,15 +252,28 @@ func createScalePlan(log zerolog.Logger, members api.MemberStatusList, group api // createRotateMemberPlan creates a plan to rotate (stop-recreate-start) an existing // member. func createRotateMemberPlan(log zerolog.Logger, member api.MemberStatus, - group api.ServerGroup, reason string, autoUpgrade bool) api.Plan { + group api.ServerGroup, reason string) api.Plan { log.Debug(). Str("id", member.ID). Str("role", group.AsRole()). Msg("Creating rotation plan") - rotateAction := api.NewAction(api.ActionTypeRotateMember, group, member.ID, reason) - rotateAction.AutoUpgrade = autoUpgrade plan := api.Plan{ - rotateAction, + api.NewAction(api.ActionTypeRotateMember, group, member.ID, reason), + api.NewAction(api.ActionTypeWaitForMemberUp, group, member.ID), + } + return plan +} + +// createUpgradeMemberPlan creates a plan to upgrade (stop-recreateWithAutoUpgrade-stop-start) an existing +// member. +func createUpgradeMemberPlan(log zerolog.Logger, member api.MemberStatus, + group api.ServerGroup, reason string) api.Plan { + log.Debug(). + Str("id", member.ID). + Str("role", group.AsRole()). + Msg("Creating upgrade plan") + plan := api.Plan{ + api.NewAction(api.ActionTypeUpgradeMember, group, member.ID, reason), api.NewAction(api.ActionTypeWaitForMemberUp, group, member.ID), } return plan diff --git a/pkg/deployment/plan_executor.go b/pkg/deployment/plan_executor.go index f6c28651a..137e03082 100644 --- a/pkg/deployment/plan_executor.go +++ b/pkg/deployment/plan_executor.go @@ -124,6 +124,8 @@ func (d *Deployment) createAction(ctx context.Context, log zerolog.Logger, actio return NewShutdownMemberAction(log, action, actionCtx) case api.ActionTypeRotateMember: return NewRotateMemberAction(log, action, actionCtx) + case api.ActionTypeUpgradeMember: + return NewUpgradeMemberAction(log, action, actionCtx) case api.ActionTypeWaitForMemberUp: return NewWaitForMemberUpAction(log, action, actionCtx) default: diff --git a/pkg/deployment/pod_creator.go b/pkg/deployment/pod_creator.go index 960eb9e02..3f0e7811b 100644 --- a/pkg/deployment/pod_creator.go +++ b/pkg/deployment/pod_creator.go @@ -319,6 +319,7 @@ func (d *Deployment) ensurePods(apiObject *api.ArangoDeployment) error { roleAbbr := group.AsRoleAbbreviated() podSuffix := createPodSuffix(apiObject.Spec) m.PodName = k8sutil.CreatePodName(apiObject.GetName(), roleAbbr, m.ID, podSuffix) + newState := api.MemberStateCreated // Create pod if group.IsArangod() { // Find image ID @@ -329,6 +330,9 @@ func (d *Deployment) ensurePods(apiObject *api.ArangoDeployment) error { } // Prepare arguments autoUpgrade := m.Conditions.IsTrue(api.ConditionTypeAutoUpgrade) + if autoUpgrade { + newState = api.MemberStateUpgrading + } args := createArangodArgs(apiObject, apiObject.Spec, group, d.status.Members.Agents, m.ID, autoUpgrade) env := make(map[string]k8sutil.EnvValue) livenessProbe, err := d.createLivenessProbe(apiObject, group) @@ -390,7 +394,7 @@ func (d *Deployment) ensurePods(apiObject *api.ArangoDeployment) error { } } // Record new member state - m.State = api.MemberStateCreated + m.State = newState m.Conditions.Remove(api.ConditionTypeReady) m.Conditions.Remove(api.ConditionTypeTerminated) m.Conditions.Remove(api.ConditionTypeAutoUpgrade) diff --git a/pkg/deployment/pod_inspector.go b/pkg/deployment/pod_inspector.go index bc591fe13..ac0456c89 100644 --- a/pkg/deployment/pod_inspector.go +++ b/pkg/deployment/pod_inspector.go @@ -53,6 +53,10 @@ func (d *Deployment) inspectPods() error { log.Debug().Str("pod", p.GetName()).Msg("pod not owned by this deployment") continue } + if isArangoDBImageIDAndVersionPod(p) { + // Image ID pods are not relevant to inspect here + continue + } // Pod belongs to this deployment, update metric inspectedPodCounter.Inc() @@ -113,7 +117,7 @@ func (d *Deployment) inspectPods() error { switch m.State { case api.MemberStateNone: // Do nothing - case api.MemberStateShuttingDown, api.MemberStateRotating: + case api.MemberStateShuttingDown, api.MemberStateRotating, api.MemberStateUpgrading: // Shutdown was intended, so not need to do anything here. // Just mark terminated if m.Conditions.Update(api.ConditionTypeTerminated, true, "Pod Terminated", "") { diff --git a/pkg/util/arangod/agency.go b/pkg/util/arangod/agency.go index db956417f..0e1f71148 100644 --- a/pkg/util/arangod/agency.go +++ b/pkg/util/arangod/agency.go @@ -73,6 +73,7 @@ func (a *agency) ReadKey(ctx context.Context, key []string, value interface{}) e //ctx = driver.WithRawResponse(ctx, &raw) resp, err := conn.Do(ctx, req) if err != nil { + fmt.Printf("conn.Do failed, err=%v, resp=%#v\n", err, resp) return maskAny(err) } if resp.StatusCode() == 307 { diff --git a/pkg/util/arangod/client.go b/pkg/util/arangod/client.go index ce9cd7a53..706165cb6 100644 --- a/pkg/util/arangod/client.go +++ b/pkg/util/arangod/client.go @@ -129,8 +129,9 @@ func createArangodClientForDNSName(ctx context.Context, cli corev1.CoreV1Interfa transport = sharedHTTPSTransport } connConfig := http.ConnectionConfig{ - Endpoints: []string{scheme + "://" + net.JoinHostPort(dnsName, strconv.Itoa(k8sutil.ArangoPort))}, - Transport: transport, + Endpoints: []string{scheme + "://" + net.JoinHostPort(dnsName, strconv.Itoa(k8sutil.ArangoPort))}, + Transport: transport, + DontFollowRedirect: true, } // TODO deal with TLS with proper CA checking conn, err := http.NewConnection(connConfig) diff --git a/pkg/util/k8sutil/util.go b/pkg/util/k8sutil/util.go index c8753f9b2..ac1273fab 100644 --- a/pkg/util/k8sutil/util.go +++ b/pkg/util/k8sutil/util.go @@ -27,6 +27,20 @@ import ( "k8s.io/apimachinery/pkg/labels" ) +const ( + // LabelKeyArangoDeployment is the key of the label used to store the ArangoDeployment name in + LabelKeyArangoDeployment = "arango_deployment" + // LabelKeyArangoLocalStorage is the key of the label used to store the ArangoLocalStorage name in + LabelKeyArangoLocalStorage = "arango_local_storage" + // LabelKeyApp is the key of the label used to store the application name in (fixed to AppName) + LabelKeyApp = "app" + // LabelKeyRole is the key of the label used to store the role of the resource in + LabelKeyRole = "role" + + // AppName is the fixed value for the "app" label + AppName = "arangodb" +) + // addOwnerRefToObject adds given owner reference to given object func addOwnerRefToObject(obj metav1.Object, ownerRef *metav1.OwnerReference) { if ownerRef != nil { @@ -37,11 +51,11 @@ func addOwnerRefToObject(obj metav1.Object, ownerRef *metav1.OwnerReference) { // LabelsForDeployment returns a map of labels, given to all resources for given deployment name func LabelsForDeployment(deploymentName, role string) map[string]string { l := map[string]string{ - "arango_deployment": deploymentName, - "app": "arangodb", + LabelKeyArangoDeployment: deploymentName, + LabelKeyApp: AppName, } if role != "" { - l["role"] = role + l[LabelKeyRole] = role } return l } @@ -49,11 +63,11 @@ func LabelsForDeployment(deploymentName, role string) map[string]string { // LabelsForLocalStorage returns a map of labels, given to all resources for given local storage name func LabelsForLocalStorage(localStorageName, role string) map[string]string { l := map[string]string{ - "arango_local_storage": localStorageName, - "app": "arangodb", + LabelKeyArangoLocalStorage: localStorageName, + LabelKeyApp: AppName, } if role != "" { - l["role"] = role + l[LabelKeyRole] = role } return l } diff --git a/tools/manifests/manifest_builder.go b/tools/manifests/manifest_builder.go index 9ab7766d5..043e7dd2e 100644 --- a/tools/manifests/manifest_builder.go +++ b/tools/manifests/manifest_builder.go @@ -177,11 +177,12 @@ func main() { } // Save output - outputPath, err := filepath.Abs(filepath.Join("manifests", "arango-"+group+options.OutputSuffix+".yaml")) + outputDir, err := filepath.Abs("manifests") if err != nil { - log.Fatalf("Failed to get absolute output path: %v\n", err) + log.Fatalf("Failed to get absolute output dir: %v\n", err) } - if err := os.MkdirAll(filepath.Base(outputPath), 0755); err != nil { + outputPath := filepath.Join(outputDir, "arango-"+group+options.OutputSuffix+".yaml") + if err := os.MkdirAll(outputDir, 0755); err != nil { log.Fatalf("Failed to create output directory: %v\n", err) } if err := ioutil.WriteFile(outputPath, output.Bytes(), 0644); err != nil { From ac86d005a55eb4355cb6afda96e161beeff7f489 Mon Sep 17 00:00:00 2001 From: Ewout Prangsma Date: Thu, 22 Mar 2018 11:43:13 +0100 Subject: [PATCH 17/17] Fixed unit tests --- pkg/deployment/pod_creator_agent_args_test.go | 52 +++++++++++++++++-- .../pod_creator_coordinator_args_test.go | 50 ++++++++++++++++-- .../pod_creator_dbserver_args_test.go | 50 ++++++++++++++++-- .../pod_creator_single_args_test.go | 40 +++++++++++--- 4 files changed, 174 insertions(+), 18 deletions(-) diff --git a/pkg/deployment/pod_creator_agent_args_test.go b/pkg/deployment/pod_creator_agent_args_test.go index f4e44fd11..9303e59a2 100644 --- a/pkg/deployment/pod_creator_agent_args_test.go +++ b/pkg/deployment/pod_creator_agent_args_test.go @@ -50,7 +50,7 @@ func TestCreateArangodArgsAgent(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1", false) assert.Equal(t, []string{ "--agency.activate=true", @@ -76,6 +76,50 @@ func TestCreateArangodArgsAgent(t *testing.T) { ) } + // Default+AutoUpgrade deployment + { + apiObject := &api.ArangoDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + Namespace: "ns", + }, + Spec: api.DeploymentSpec{ + Mode: api.DeploymentModeCluster, + }, + } + apiObject.Spec.SetDefaults("test") + agents := api.MemberStatusList{ + api.MemberStatus{ID: "a1"}, + api.MemberStatus{ID: "a2"}, + api.MemberStatus{ID: "a3"}, + } + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1", true) + assert.Equal(t, + []string{ + "--agency.activate=true", + "--agency.disaster-recovery-id=a1", + "--agency.endpoint=ssl://name-agent-a2.name-int.ns.svc:8529", + "--agency.endpoint=ssl://name-agent-a3.name-int.ns.svc:8529", + "--agency.my-address=ssl://name-agent-a1.name-int.ns.svc:8529", + "--agency.size=3", + "--agency.supervision=true", + "--database.auto-upgrade=true", + "--database.directory=/data", + "--foxx.queues=false", + "--log.level=INFO", + "--log.output=+", + "--server.authentication=true", + "--server.endpoint=ssl://[::]:8529", + "--server.jwt-secret=$(ARANGOD_JWT_SECRET)", + "--server.statistics=false", + "--server.storage-engine=rocksdb", + "--ssl.ecdh-curve=", + "--ssl.keyfile=/secrets/tls/tls.keyfile", + }, + cmdline, + ) + } + // Default+TLS disabled deployment { apiObject := &api.ArangoDeployment{ @@ -96,7 +140,7 @@ func TestCreateArangodArgsAgent(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1", false) assert.Equal(t, []string{ "--agency.activate=true", @@ -139,7 +183,7 @@ func TestCreateArangodArgsAgent(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1", false) assert.Equal(t, []string{ "--agency.activate=true", @@ -182,7 +226,7 @@ func TestCreateArangodArgsAgent(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupAgents, agents, "a1", false) assert.Equal(t, []string{ "--agency.activate=true", diff --git a/pkg/deployment/pod_creator_coordinator_args_test.go b/pkg/deployment/pod_creator_coordinator_args_test.go index 0ef0ab0e6..79a71a6d6 100644 --- a/pkg/deployment/pod_creator_coordinator_args_test.go +++ b/pkg/deployment/pod_creator_coordinator_args_test.go @@ -50,7 +50,7 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1", false) assert.Equal(t, []string{ "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", @@ -74,6 +74,48 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { ) } + // Default+AutoUpgrade deployment + { + apiObject := &api.ArangoDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + Namespace: "ns", + }, + Spec: api.DeploymentSpec{ + Mode: api.DeploymentModeCluster, + }, + } + apiObject.Spec.SetDefaults("test") + agents := api.MemberStatusList{ + api.MemberStatus{ID: "a1"}, + api.MemberStatus{ID: "a2"}, + api.MemberStatus{ID: "a3"}, + } + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1", true) + assert.Equal(t, + []string{ + "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=ssl://name-agent-a2.name-int.ns.svc:8529", + "--cluster.agency-endpoint=ssl://name-agent-a3.name-int.ns.svc:8529", + "--cluster.my-address=ssl://name-coordinator-id1.name-int.ns.svc:8529", + "--cluster.my-role=COORDINATOR", + "--database.auto-upgrade=true", + "--database.directory=/data", + "--foxx.queues=true", + "--log.level=INFO", + "--log.output=+", + "--server.authentication=true", + "--server.endpoint=ssl://[::]:8529", + "--server.jwt-secret=$(ARANGOD_JWT_SECRET)", + "--server.statistics=true", + "--server.storage-engine=rocksdb", + "--ssl.ecdh-curve=", + "--ssl.keyfile=/secrets/tls/tls.keyfile", + }, + cmdline, + ) + } + // Default+TLS disabled deployment { apiObject := &api.ArangoDeployment{ @@ -94,7 +136,7 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1", false) assert.Equal(t, []string{ "--cluster.agency-endpoint=tcp://name-agent-a1.name-int.ns.svc:8529", @@ -134,7 +176,7 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1", false) assert.Equal(t, []string{ "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", @@ -176,7 +218,7 @@ func TestCreateArangodArgsCoordinator(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupCoordinators, agents, "id1", false) assert.Equal(t, []string{ "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", diff --git a/pkg/deployment/pod_creator_dbserver_args_test.go b/pkg/deployment/pod_creator_dbserver_args_test.go index aafec0b1a..e91bfe8d7 100644 --- a/pkg/deployment/pod_creator_dbserver_args_test.go +++ b/pkg/deployment/pod_creator_dbserver_args_test.go @@ -50,7 +50,7 @@ func TestCreateArangodArgsDBServer(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1", false) assert.Equal(t, []string{ "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", @@ -74,6 +74,48 @@ func TestCreateArangodArgsDBServer(t *testing.T) { ) } + // Default+AutoUpgrade deployment + { + apiObject := &api.ArangoDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + Namespace: "ns", + }, + Spec: api.DeploymentSpec{ + Mode: api.DeploymentModeCluster, + }, + } + apiObject.Spec.SetDefaults("test") + agents := api.MemberStatusList{ + api.MemberStatus{ID: "a1"}, + api.MemberStatus{ID: "a2"}, + api.MemberStatus{ID: "a3"}, + } + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1", true) + assert.Equal(t, + []string{ + "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", + "--cluster.agency-endpoint=ssl://name-agent-a2.name-int.ns.svc:8529", + "--cluster.agency-endpoint=ssl://name-agent-a3.name-int.ns.svc:8529", + "--cluster.my-address=ssl://name-dbserver-id1.name-int.ns.svc:8529", + "--cluster.my-role=PRIMARY", + "--database.auto-upgrade=true", + "--database.directory=/data", + "--foxx.queues=false", + "--log.level=INFO", + "--log.output=+", + "--server.authentication=true", + "--server.endpoint=ssl://[::]:8529", + "--server.jwt-secret=$(ARANGOD_JWT_SECRET)", + "--server.statistics=true", + "--server.storage-engine=rocksdb", + "--ssl.ecdh-curve=", + "--ssl.keyfile=/secrets/tls/tls.keyfile", + }, + cmdline, + ) + } + // Default+TLS disabled deployment { apiObject := &api.ArangoDeployment{ @@ -94,7 +136,7 @@ func TestCreateArangodArgsDBServer(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1", false) assert.Equal(t, []string{ "--cluster.agency-endpoint=tcp://name-agent-a1.name-int.ns.svc:8529", @@ -134,7 +176,7 @@ func TestCreateArangodArgsDBServer(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1", false) assert.Equal(t, []string{ "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", @@ -176,7 +218,7 @@ func TestCreateArangodArgsDBServer(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupDBServers, agents, "id1", false) assert.Equal(t, []string{ "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529", diff --git a/pkg/deployment/pod_creator_single_args_test.go b/pkg/deployment/pod_creator_single_args_test.go index de1e89a16..cec80cfb2 100644 --- a/pkg/deployment/pod_creator_single_args_test.go +++ b/pkg/deployment/pod_creator_single_args_test.go @@ -41,7 +41,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { }, } apiObject.Spec.SetDefaults("test") - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1", false) assert.Equal(t, []string{ "--database.directory=/data", @@ -60,6 +60,34 @@ func TestCreateArangodArgsSingle(t *testing.T) { ) } + // Default+AutoUpgrade deployment + { + apiObject := &api.ArangoDeployment{ + Spec: api.DeploymentSpec{ + Mode: api.DeploymentModeSingle, + }, + } + apiObject.Spec.SetDefaults("test") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1", true) + assert.Equal(t, + []string{ + "--database.auto-upgrade=true", + "--database.directory=/data", + "--foxx.queues=true", + "--log.level=INFO", + "--log.output=+", + "--server.authentication=true", + "--server.endpoint=ssl://[::]:8529", + "--server.jwt-secret=$(ARANGOD_JWT_SECRET)", + "--server.statistics=true", + "--server.storage-engine=rocksdb", + "--ssl.ecdh-curve=", + "--ssl.keyfile=/secrets/tls/tls.keyfile", + }, + cmdline, + ) + } + // Default+TLS disabled deployment { apiObject := &api.ArangoDeployment{ @@ -71,7 +99,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { }, } apiObject.Spec.SetDefaults("test") - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1", false) assert.Equal(t, []string{ "--database.directory=/data", @@ -97,7 +125,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { }, } apiObject.Spec.SetDefaults("test") - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1", false) assert.Equal(t, []string{ "--database.directory=/data", @@ -125,7 +153,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { } apiObject.Spec.Authentication.JWTSecretName = "None" apiObject.Spec.SetDefaults("test") - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1", false) assert.Equal(t, []string{ "--database.directory=/data", @@ -152,7 +180,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { } apiObject.Spec.Single.Args = []string{"--foo1", "--foo2"} apiObject.Spec.SetDefaults("test") - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, nil, "id1", false) assert.Equal(t, []string{ "--database.directory=/data", @@ -190,7 +218,7 @@ func TestCreateArangodArgsSingle(t *testing.T) { api.MemberStatus{ID: "a2"}, api.MemberStatus{ID: "a3"}, } - cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, agents, "id1") + cmdline := createArangodArgs(apiObject, apiObject.Spec, api.ServerGroupSingle, agents, "id1", false) assert.Equal(t, []string{ "--cluster.agency-endpoint=ssl://name-agent-a1.name-int.ns.svc:8529",