Properly upgrading to new minor version

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,

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"
 )

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.

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)

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
+}

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
 }
 

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
+}

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

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:

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)

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", "") {

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 {

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)

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
 }

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 {