Merge pull request #183 from arangodb/feature/change-storage-class

Allow changing server group storage class.

pkg/apis/deployment/v1alpha/plan.go

@@ -51,6 +51,12 @@ const (
 	ActionTypeRenewTLSCACertificate ActionType = "RenewTLSCACertificate"
 )
 
+const (
+	// MemberIDPreviousAction is used for Action.MemberID when the MemberID
+	// should be derived from the previous action.
+	MemberIDPreviousAction = "@previous"
+)
+
 // Action represents a single action to be taken to update a deployment.
 type Action struct {
 	// ID of this action (unique for every action)

pkg/apis/deployment/v1alpha/server_group_spec.go

@@ -191,9 +191,5 @@ func (s ServerGroupSpec) ResetImmutableFields(group ServerGroup, fieldPrefix str
 			resetFields = append(resetFields, fieldPrefix+".count")
 		}
 	}
-	if s.GetStorageClassName() != target.GetStorageClassName() {
-		target.StorageClassName = util.NewStringOrNil(s.StorageClassName)
-		resetFields = append(resetFields, fieldPrefix+".storageClassName")
-	}
 	return resetFields
 }

pkg/deployment/context_impl.go

@@ -30,6 +30,7 @@ import (
 	"github.com/arangodb/arangosync/tasks"
 	driver "github.com/arangodb/go-driver"
 	"github.com/arangodb/go-driver/agency"
+	"github.com/rs/zerolog/log"
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -191,23 +192,23 @@ func (d *Deployment) GetSyncServerClient(ctx context.Context, group api.ServerGr
 
 // CreateMember adds a new member to the given group.
 // If ID is non-empty, it will be used, otherwise a new ID is created.
-func (d *Deployment) CreateMember(group api.ServerGroup, id string) error {
+func (d *Deployment) CreateMember(group api.ServerGroup, id string) (string, error) {
 	log := d.deps.Log
 	status, lastVersion := d.GetStatus()
 	id, err := createMember(log, &status, group, id, d.apiObject)
 	if err != nil {
 		log.Debug().Err(err).Str("group", group.AsRole()).Msg("Failed to create member")
-		return maskAny(err)
+		return "", maskAny(err)
 	}
 	// Save added member
 	if err := d.UpdateStatus(status, lastVersion); err != nil {
 		log.Debug().Err(err).Msg("Updating CR status failed")
-		return maskAny(err)
+		return "", maskAny(err)
 	}
 	// Create event about it
 	d.CreateEvent(k8sutil.NewMemberAddEvent(id, group.AsRole(), d.apiObject))
 
-	return nil
+	return id, nil
 }
 
 // DeletePod deletes a pod with given name in the namespace
@@ -304,6 +305,16 @@ func (d *Deployment) GetOwnedPVCs() ([]v1.PersistentVolumeClaim, error) {
 	return myPVCs, nil
 }
 
+// GetPvc gets a PVC by the given name, in the samespace of the deployment.
+func (d *Deployment) GetPvc(pvcName string) (*v1.PersistentVolumeClaim, error) {
+	pvc, err := d.deps.KubeCli.CoreV1().PersistentVolumeClaims(d.apiObject.GetNamespace()).Get(pvcName, metav1.GetOptions{})
+	if err != nil {
+		log.Debug().Err(err).Str("pvc-name", pvcName).Msg("Failed to get PVC")
+		return nil, maskAny(err)
+	}
+	return pvc, nil
+}
+
 // GetTLSKeyfile returns the keyfile encoded TLS certificate+key for
 // the given member.
 func (d *Deployment) GetTLSKeyfile(group api.ServerGroup, member api.MemberStatus) (string, error) {

pkg/deployment/reconcile/action.go

@@ -38,4 +38,6 @@ type Action interface {
 	CheckProgress(ctx context.Context) (bool, bool, error)
 	// Timeout returns the amount of time after which this action will timeout.
 	Timeout() time.Duration
+	// Return the MemberID used / created in this action
+	MemberID() string
 }

pkg/deployment/reconcile/action_add_member.go

@@ -43,19 +43,22 @@ func NewAddMemberAction(log zerolog.Logger, action api.Action, actionCtx ActionC
 
 // actionAddMember implements an AddMemberAction.
 type actionAddMember struct {
-	log       zerolog.Logger
-	action    api.Action
-	actionCtx ActionContext
+	log         zerolog.Logger
+	action      api.Action
+	actionCtx   ActionContext
+	newMemberID string
 }
 
 // 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, a.action.MemberID); err != nil {
+	newID, err := a.actionCtx.CreateMember(a.action.Group, a.action.MemberID)
+	if err != nil {
 		log.Debug().Err(err).Msg("Failed to create member")
 		return false, maskAny(err)
 	}
+	a.newMemberID = newID
 	return true, nil
 }
 
@@ -70,3 +73,8 @@ func (a *actionAddMember) CheckProgress(ctx context.Context) (bool, bool, error)
 func (a *actionAddMember) Timeout() time.Duration {
 	return addMemberTimeout
 }
+
+// Return the MemberID used / created in this action
+func (a *actionAddMember) MemberID() string {
+	return a.newMemberID
+}

pkg/deployment/reconcile/action_cleanout_member.go

@@ -155,3 +155,8 @@ func (a *actionCleanoutMember) CheckProgress(ctx context.Context) (bool, bool, e
 func (a *actionCleanoutMember) Timeout() time.Duration {
 	return cleanoutMemberTimeout
 }
+
+// Return the MemberID used / created in this action
+func (a *actionCleanoutMember) MemberID() string {
+	return a.action.MemberID
+}

pkg/deployment/reconcile/action_context.go

@@ -59,7 +59,7 @@ type ActionContext interface {
 	GetMemberStatusByID(id string) (api.MemberStatus, bool)
 	// CreateMember adds a new member to the given group.
 	// If ID is non-empty, it will be used, otherwise a new ID is created.
-	CreateMember(group api.ServerGroup, id string) error
+	CreateMember(group api.ServerGroup, id string) (string, error)
 	// UpdateMember updates the deployment status wrt the given member.
 	UpdateMember(member api.MemberStatus) error
 	// RemoveMemberByID removes a member with given id.
@@ -157,11 +157,12 @@ func (ac *actionContext) GetMemberStatusByID(id string) (api.MemberStatus, bool)
 
 // CreateMember adds a new member to the given group.
 // If ID is non-empty, it will be used, otherwise a new ID is created.
-func (ac *actionContext) CreateMember(group api.ServerGroup, id string) error {
-	if err := ac.context.CreateMember(group, id); err != nil {
-		return maskAny(err)
+func (ac *actionContext) CreateMember(group api.ServerGroup, id string) (string, error) {
+	result, err := ac.context.CreateMember(group, id)
+	if err != nil {
+		return "", maskAny(err)
 	}
-	return nil
+	return result, nil
 }
 
 // UpdateMember updates the deployment status wrt the given member.

pkg/deployment/reconcile/action_remove_member.go

@@ -105,3 +105,8 @@ func (a *actionRemoveMember) CheckProgress(ctx context.Context) (bool, bool, err
 func (a *actionRemoveMember) Timeout() time.Duration {
 	return removeMemberTimeout
 }
+
+// Return the MemberID used / created in this action
+func (a *actionRemoveMember) MemberID() string {
+	return a.action.MemberID
+}

pkg/deployment/reconcile/action_renew_tls_ca_certificate.go

@@ -69,3 +69,8 @@ func (a *renewTLSCACertificateAction) CheckProgress(ctx context.Context) (bool,
 func (a *renewTLSCACertificateAction) Timeout() time.Duration {
 	return renewTLSCACertificateTimeout
 }
+
+// Return the MemberID used / created in this action
+func (a *renewTLSCACertificateAction) MemberID() string {
+	return a.action.MemberID
+}

pkg/deployment/reconcile/action_renew_tls_certificate.go

@@ -75,3 +75,8 @@ func (a *renewTLSCertificateAction) CheckProgress(ctx context.Context) (bool, bo
 func (a *renewTLSCertificateAction) Timeout() time.Duration {
 	return renewTLSCertificateTimeout
 }
+
+// Return the MemberID used / created in this action
+func (a *renewTLSCertificateAction) MemberID() string {
+	return a.action.MemberID
+}

pkg/deployment/reconcile/action_rotate_member.go

@@ -127,3 +127,8 @@ func (a *actionRotateMember) CheckProgress(ctx context.Context) (bool, bool, err
 func (a *actionRotateMember) Timeout() time.Duration {
 	return rotateMemberTimeout
 }
+
+// Return the MemberID used / created in this action
+func (a *actionRotateMember) MemberID() string {
+	return a.action.MemberID
+}

pkg/deployment/reconcile/action_shutdown_member.go

@@ -116,3 +116,8 @@ func (a *actionShutdownMember) CheckProgress(ctx context.Context) (bool, bool, e
 func (a *actionShutdownMember) Timeout() time.Duration {
 	return shutdownMemberTimeout
 }
+
+// Return the MemberID used / created in this action
+func (a *actionShutdownMember) MemberID() string {
+	return a.action.MemberID
+}

pkg/deployment/reconcile/action_upgrade_member.go

@@ -133,3 +133,8 @@ func (a *actionUpgradeMember) CheckProgress(ctx context.Context) (bool, bool, er
 func (a *actionUpgradeMember) Timeout() time.Duration {
 	return upgradeMemberTimeout
 }
+
+// Return the MemberID used / created in this action
+func (a *actionUpgradeMember) MemberID() string {
+	return a.action.MemberID
+}

pkg/deployment/reconcile/action_wait_for_member_up.go

@@ -74,7 +74,7 @@ func (a *actionWaitForMemberUp) CheckProgress(ctx context.Context) (bool, bool,
 		if a.action.Group == api.ServerGroupAgents {
 			return a.checkProgressAgent(ctx)
 		}
-		return a.checkProgressSingle(ctx)
+		return a.checkProgressSingleInActiveFailover(ctx)
 	default:
 		if a.action.Group == api.ServerGroupAgents {
 			return a.checkProgressAgent(ctx)
@@ -99,6 +99,26 @@ func (a *actionWaitForMemberUp) checkProgressSingle(ctx context.Context) (bool,
 	return true, false, nil
 }
 
+// checkProgressSingleInActiveFailover checks the progress of the action in the case
+// of a single server as part of an active failover deployment.
+func (a *actionWaitForMemberUp) checkProgressSingleInActiveFailover(ctx context.Context) (bool, 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, false, maskAny(err)
+	}
+	if _, err := c.Version(ctx); err != nil {
+		log.Debug().Err(err).Msg("Failed to get version")
+		return false, false, maskAny(err)
+	}
+	if _, err := c.Databases(ctx); err != nil {
+		log.Debug().Err(err).Msg("Failed to get databases")
+		return false, false, maskAny(err)
+	}
+	return true, false, nil
+}
+
 // checkProgressAgent checks the progress of the action in the case
 // of an agent.
 func (a *actionWaitForMemberUp) checkProgressAgent(ctx context.Context) (bool, bool, error) {
@@ -170,3 +190,8 @@ func (a *actionWaitForMemberUp) checkProgressArangoSync(ctx context.Context) (bo
 func (a *actionWaitForMemberUp) Timeout() time.Duration {
 	return waitForMemberUpTimeout
 }
+
+// Return the MemberID used / created in this action
+func (a *actionWaitForMemberUp) MemberID() string {
+	return a.action.MemberID
+}

pkg/deployment/reconcile/context.go

@@ -62,7 +62,8 @@ type Context interface {
 	CreateEvent(evt *v1.Event)
 	// CreateMember adds a new member to the given group.
 	// If ID is non-empty, it will be used, otherwise a new ID is created.
-	CreateMember(group api.ServerGroup, id string) error
+	// Returns ID, error
+	CreateMember(group api.ServerGroup, id string) (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
@@ -74,6 +75,8 @@ type Context interface {
 	RemovePodFinalizers(podName string) error
 	// GetOwnedPods returns a list of all pods owned by the deployment.
 	GetOwnedPods() ([]v1.Pod, error)
+	// GetPvc gets a PVC by the given name, in the samespace of the deployment.
+	GetPvc(pvcName string) (*v1.PersistentVolumeClaim, error)
 	// GetTLSKeyfile returns the keyfile encoded TLS certificate+key for
 	// the given member.
 	GetTLSKeyfile(group api.ServerGroup, member api.MemberStatus) (string, error)

pkg/deployment/reconcile/plan_builder.go

@@ -54,7 +54,7 @@ func (d *Reconciler) CreatePlan() error {
 	apiObject := d.context.GetAPIObject()
 	spec := d.context.GetSpec()
 	status, lastVersion := d.context.GetStatus()
-	newPlan, changed := createPlan(d.log, apiObject, status.Plan, spec, status, pods, d.context.GetTLSKeyfile, d.context.GetTLSCA)
+	newPlan, changed := createPlan(d.log, apiObject, status.Plan, spec, status, pods, d.context.GetTLSKeyfile, d.context.GetTLSCA, d.context.GetPvc, d.context.CreateEvent)
 
 	// If not change, we're done
 	if !changed {
@@ -76,11 +76,13 @@ func (d *Reconciler) 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,
+func createPlan(log zerolog.Logger, apiObject k8sutil.APIObject,
 	currentPlan api.Plan, spec api.DeploymentSpec,
 	status api.DeploymentStatus, pods []v1.Pod,
 	getTLSKeyfile func(group api.ServerGroup, member api.MemberStatus) (string, error),
-	getTLSCA func(string) (string, string, bool, error)) (api.Plan, bool) {
+	getTLSCA func(string) (string, string, bool, error),
+	getPVC func(pvcName string) (*v1.PersistentVolumeClaim, error),
+	createEvent func(evt *v1.Event)) (api.Plan, bool) {
 	if len(currentPlan) > 0 {
 		// Plan already exists, complete that first
 		return currentPlan, false
@@ -175,14 +177,19 @@ func createPlan(log zerolog.Logger, apiObject metav1.Object,
 		})
 	}
 
-	// Check for the need to rotate TLS CA certificate and all members
+	// Check for the need to rotate TLS certificate of a members
 	if len(plan) == 0 {
-		plan = createRotateTLSCAPlan(log, spec, status, getTLSCA)
+		plan = createRotateTLSServerCertificatePlan(log, spec, status, getTLSKeyfile)
 	}
 
-	// Check for the need to rotate TLS certificate of a members
+	// Check for changes storage classes or requirements
 	if len(plan) == 0 {
-		plan = createRotateTLSServerCertificatePlan(log, spec, status, getTLSKeyfile)
+		plan = createRotateServerStoragePlan(log, apiObject, spec, status, getPVC, createEvent)
+	}
+
+	// Check for the need to rotate TLS CA certificate and all members
+	if len(plan) == 0 {
+		plan = createRotateTLSCAPlan(log, spec, status, getTLSCA)
 	}
 
 	// Return plan