Add external storage support for etcd

gottwald · gottwald · commit 922d077642ce · 2020-12-18T16:33:17.000+01:00
This allows DOMachines to be provisioned with attached block storage
volumes. The current implemenation does not support resizing disks. This
will be added as a follow-up.
diff --git a/api/v1alpha3/domachine_types.go b/api/v1alpha3/domachine_types.go
@@ -38,6 +38,8 @@ type DOMachineSpec struct {
 	Size string `json:"size"`
 	// Droplet image can be image id or slug. See https://developers.digitalocean.com/documentation/v2/#list-all-images
 	Image intstr.IntOrString `json:"image"`
+	// DataDisks specifies the parameters that are used to add one or more data disks to the machine
+	DataDisks []DataDisk `json:"dataDisks,omitempty"`
 	// SSHKeys is the ssh key id or fingerprint to attach in DigitalOcean droplet.
 	// It must be available on DigitalOcean account. See https://developers.digitalocean.com/documentation/v2/#list-all-keys
 	SSHKeys []intstr.IntOrString `json:"sshKeys"`
diff --git a/api/v1alpha3/types.go b/api/v1alpha3/types.go
@@ -16,7 +16,10 @@ limitations under the License.
 
 package v1alpha3
 
-import "strings"
+import (
+	"fmt"
+	"strings"
+)
 
 // APIEndpoint represents a reachable Kubernetes API endpoint.
 type APIEndpoint struct {
@@ -72,6 +75,28 @@ type DOMachineTemplateResource struct {
 	Spec DOMachineSpec `json:"spec"`
 }
 
+// DataDiskName is the volume name used for a data disk of a droplet.
+// It's in the form of <dropletName>_<dataDiskNameSuffix>.
+func DataDiskName(m *DOMachine, suffix string) string {
+	return DOSafeName(fmt.Sprintf("%s-%s", m.Name, suffix))
+}
+
+// DataDisk specifies the parameters that are used to add a data disk to the machine.
+type DataDisk struct {
+	// NameSuffix is the suffix to be appended to the machine name to generate the disk name.
+	// Each disk name will be in format <dropletName>_<nameSuffix>.
+	NameSuffix string `json:"nameSuffix"`
+	// DiskSizeGB is the size in GB to assign to the data disk.
+	DiskSizeGB int64 `json:"diskSizeGB"`
+	// FilesystemType to be used on the volume. When provided the volume will
+	// be automatically formatted.
+	FilesystemType string `json:"filesystemType,omitempty"`
+	// FilesystemLabel is the label that is applied to the created filesystem.
+	// Character limits apply: 16 for ext4; 12 for xfs.
+	// May only be used in conjunction with filesystemType.
+	FilesystemLabel string `json:"filesystemLabel,omitempty"`
+}
+
 // DONetwork encapsulates DigitalOcean networking configuration.
 type DONetwork struct {
 	// Configures an API Server loadbalancers
diff --git a/api/v1alpha3/zz_generated.deepcopy.go b/api/v1alpha3/zz_generated.deepcopy.go
diff --git a/cloud/scope/clients.go b/cloud/scope/clients.go
@@ -22,6 +22,7 @@ import (
 type DOClients struct {
 	Actions       godo.ActionsService
 	Droplets      godo.DropletsService
+	Storage       godo.StorageService
 	Images        godo.ImagesService
 	Keys          godo.KeysService
 	LoadBalancers godo.LoadBalancersService
diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go
@@ -66,6 +66,10 @@ func NewClusterScope(params ClusterScopeParams) (*ClusterScope, error) {
 		params.DOClients.Droplets = session.Droplets
 	}
 
+	if params.DOClients.Storage == nil {
+		params.DOClients.Storage = session.Storage
+	}
+
 	if params.DOClients.Images == nil {
 		params.DOClients.Images = session.Images
 	}
diff --git a/cloud/services/computes/droplets.go b/cloud/services/computes/droplets.go
@@ -17,6 +17,7 @@ limitations under the License.
 package computes
 
 import (
+	"fmt"
 	"net/http"
 	"strconv"
 
@@ -82,6 +83,19 @@ func (s *Service) CreateDroplet(scope *scope.MachineScope) (*godo.Droplet, error
 		})
 	}
 
+	volumes := []godo.DropletCreateVolume{}
+	for _, disk := range scope.DOMachine.Spec.DataDisks {
+		volName := infrav1.DataDiskName(scope.DOMachine, disk.NameSuffix)
+		vol, err := s.GetVolumeByName(volName)
+		if err != nil {
+			return nil, fmt.Errorf("could not get volume to attach to droplet: %w", err)
+		}
+		if vol == nil {
+			return nil, fmt.Errorf("volume %q does not exist", volName)
+		}
+		volumes = append(volumes, godo.DropletCreateVolume{ID: vol.ID})
+	}
+
 	request := &godo.DropletCreateRequest{
 		Name:    instanceName,
 		Region:  s.scope.Region(),
@@ -92,6 +106,7 @@ func (s *Service) CreateDroplet(scope *scope.MachineScope) (*godo.Droplet, error
 		},
 		UserData:          bootstrapData,
 		PrivateNetworking: true,
+		Volumes:           volumes,
 	}
 
 	request.Tags = infrav1.BuildTags(infrav1.BuildTagParams{
diff --git a/cloud/services/computes/service.go b/cloud/services/computes/service.go
@@ -22,6 +22,12 @@ import (
 	"sigs.k8s.io/cluster-api-provider-digitalocean/cloud/scope"
 )
 
+// TemporaryError is an error that is raised for temporary DO API errors that
+// could be retried in upper levels of the callstack.
+type TemporaryError struct {
+	error
+}
+
 // Service holds a collection of interfaces.
 type Service struct {
 	scope *scope.ClusterScope
diff --git a/cloud/services/computes/volumes.go b/cloud/services/computes/volumes.go
@@ -0,0 +1,56 @@
+package computes
+
+import (
+	"fmt"
+
+	"github.com/digitalocean/godo"
+	"github.com/pkg/errors"
+
+	infrav1 "sigs.k8s.io/cluster-api-provider-digitalocean/api/v1alpha3"
+)
+
+// GetVolumeByName takes a volume name and returns a Volume if found.
+func (s *Service) GetVolumeByName(name string) (*godo.Volume, error) {
+	vols, _, err := s.scope.Storage.ListVolumes(s.ctx, &godo.ListVolumeParams{
+		Name:   name,
+		Region: s.scope.Region(),
+	})
+	if err != nil {
+		return nil, TemporaryError{fmt.Errorf("failed to list volumes: %w", err)}
+	}
+	if len(vols) == 0 {
+		return nil, nil
+	}
+	if len(vols) > 1 {
+		return nil, errors.New("volume names are not unique per region")
+	}
+	return &vols[0], nil
+}
+
+// CreateVolume creates a block storage volume.
+func (s *Service) CreateVolume(disk infrav1.DataDisk, volName string) (*godo.Volume, error) {
+	r := &godo.VolumeCreateRequest{
+		Region:          s.scope.Region(),
+		Name:            volName,
+		SizeGigaBytes:   disk.DiskSizeGB,
+		FilesystemType:  disk.FilesystemType,
+		FilesystemLabel: disk.FilesystemLabel,
+	}
+	v, _, err := s.scope.Storage.CreateVolume(s.ctx, r)
+	return v, errors.Wrap(err, "failed to create new volume")
+}
+
+// DeleteVolume deletes a block storage volume.
+func (s *Service) DeleteVolume(id string) error {
+	s.scope.V(2).Info("Attempting to delete block storage volume", "volume-id", id)
+
+	if resp, err := s.scope.Storage.DeleteVolume(s.ctx, id); err != nil {
+		if resp.StatusCode >= 500 {
+			err = TemporaryError{err}
+		}
+		return fmt.Errorf("failed to delete instance with id %q: %w", id, err)
+	}
+
+	s.scope.V(2).Info("Deleted block storage volume", "volume-id", id)
+	return nil
+}
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_domachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_domachines.yaml
@@ -184,6 +184,37 @@ spec:
                 items:
                   type: string
                 type: array
+              dataDisks:
+                description: DataDisks specifies the parameters that are used to add
+                  one or more data disks to the machine
+                items:
+                  description: DataDisk specifies the parameters that are used to
+                    add a data disk to the machine.
+                  properties:
+                    diskSizeGB:
+                      description: DiskSizeGB is the size in GB to assign to the data
+                        disk.
+                      format: int64
+                      type: integer
+                    filesystemLabel:
+                      description: 'FilesystemLabel is the label that is applied to
+                        the created filesystem. Character limits apply: 16 for ext4;
+                        12 for xfs. May only be used in conjunction with filesystemType.'
+                      type: string
+                    filesystemType:
+                      description: FilesystemType to be used on the volume. When provided
+                        the volume will be automatically formatted.
+                      type: string
+                    nameSuffix:
+                      description: NameSuffix is the suffix to be appended to the
+                        machine name to generate the disk name. Each disk name will
+                        be in format <dropletName>_<nameSuffix>.
+                      type: string
+                  required:
+                  - diskSizeGB
+                  - nameSuffix
+                  type: object
+                type: array
               image:
                 anyOf:
                 - type: integer
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_domachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_domachinetemplates.yaml
@@ -126,6 +126,38 @@ spec:
                         items:
                           type: string
                         type: array
+                      dataDisks:
+                        description: DataDisks specifies the parameters that are used
+                          to add one or more data disks to the machine
+                        items:
+                          description: DataDisk specifies the parameters that are
+                            used to add a data disk to the machine.
+                          properties:
+                            diskSizeGB:
+                              description: DiskSizeGB is the size in GB to assign
+                                to the data disk.
+                              format: int64
+                              type: integer
+                            filesystemLabel:
+                              description: 'FilesystemLabel is the label that is applied
+                                to the created filesystem. Character limits apply:
+                                16 for ext4; 12 for xfs. May only be used in conjunction
+                                with filesystemType.'
+                              type: string
+                            filesystemType:
+                              description: FilesystemType to be used on the volume.
+                                When provided the volume will be automatically formatted.
+                              type: string
+                            nameSuffix:
+                              description: NameSuffix is the suffix to be appended
+                                to the machine name to generate the disk name. Each
+                                disk name will be in format <dropletName>_<nameSuffix>.
+                              type: string
+                          required:
+                          - diskSizeGB
+                          - nameSuffix
+                          type: object
+                        type: array
                       image:
                         anyOf:
                         - type: integer
diff --git a/controllers/domachine_controller.go b/controllers/domachine_controller.go
@@ -18,6 +18,7 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 	"strconv"
 	"time"
 
@@ -194,6 +195,29 @@ func (r *DOMachineReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr
 	return r.reconcile(ctx, machineScope, clusterScope)
 }
 
+func (r *DOMachineReconciler) reconcileVolumes(ctx context.Context, mscope *scope.MachineScope, cscope *scope.ClusterScope) (reconcile.Result, error) {
+	mscope.Info("Reconciling DOMachine Volumes")
+	computesvc := computes.NewService(ctx, cscope)
+	for _, disk := range mscope.DOMachine.Spec.DataDisks {
+		volName := infrav1.DataDiskName(mscope.DOMachine, disk.NameSuffix)
+		vol, err := computesvc.GetVolumeByName(volName)
+		if err != nil {
+			if errors.As(err, &computes.TemporaryError{}) {
+				return reconcile.Result{Requeue: true}, err
+			}
+			return reconcile.Result{}, err
+		}
+		if vol == nil {
+			vol, err = computesvc.CreateVolume(disk, volName)
+			if err != nil {
+				return reconcile.Result{}, err
+			}
+		}
+		// TODO(gottwald): reconcile disk resizes here (at least grow)
+	}
+	return reconcile.Result{}, nil
+}
+
 func (r *DOMachineReconciler) reconcile(ctx context.Context, machineScope *scope.MachineScope, clusterScope *scope.ClusterScope) (reconcile.Result, error) {
 	machineScope.Info("Reconciling DOMachine")
 	domachine := machineScope.DOMachine
@@ -217,6 +241,11 @@ func (r *DOMachineReconciler) reconcile(ctx context.Context, machineScope *scope
 		return reconcile.Result{}, nil
 	}
 
+	// Make sure the droplet volumes are reconciled
+	if result, err := r.reconcileVolumes(ctx, machineScope, clusterScope); err != nil {
+		return result, fmt.Errorf("failed to reconcile volumes: %w", err)
+	}
+
 	computesvc := computes.NewService(ctx, clusterScope)
 	droplet, err := computesvc.GetDroplet(machineScope.GetInstanceID())
 	if err != nil {
@@ -259,6 +288,32 @@ func (r *DOMachineReconciler) reconcile(ctx context.Context, machineScope *scope
 		return reconcile.Result{}, nil
 	}
 }
+func (r *DOMachineReconciler) reconcileDeleteVolumes(ctx context.Context, mscope *scope.MachineScope, cscope *scope.ClusterScope) (reconcile.Result, error) {
+	mscope.Info("Reconciling delete DOMachine Volumes")
+	computesvc := computes.NewService(ctx, cscope)
+	domachine := mscope.DOMachine
+	for _, disk := range mscope.DOMachine.Spec.DataDisks {
+		volName := infrav1.DataDiskName(domachine, disk.NameSuffix)
+		vol, err := computesvc.GetVolumeByName(volName)
+		if err != nil {
+			if errors.As(err, &computes.TemporaryError{}) {
+				return reconcile.Result{Requeue: true}, err
+			}
+			return reconcile.Result{}, err
+		}
+		if vol == nil {
+			continue
+		}
+		if err = computesvc.DeleteVolume(vol.ID); err != nil {
+			if errors.As(err, &computes.TemporaryError{}) {
+				return reconcile.Result{Requeue: true}, err
+			}
+			return reconcile.Result{}, err
+		}
+		r.Recorder.Eventf(domachine, corev1.EventTypeNormal, "VolumeDeleted", "Deleted the storage volume - %s", vol.Name)
+	}
+	return reconcile.Result{}, nil
+}
 
 func (r *DOMachineReconciler) reconcileDelete(ctx context.Context, machineScope *scope.MachineScope, clusterScope *scope.ClusterScope) (reconcile.Result, error) {
 	machineScope.Info("Reconciling delete DOMachine")
@@ -270,18 +325,18 @@ func (r *DOMachineReconciler) reconcileDelete(ctx context.Context, machineScope
 		return reconcile.Result{}, err
 	}
 
-	if droplet == nil {
+	if droplet != nil {
+		if err := computesvc.DeleteDroplet(machineScope.GetInstanceID()); err != nil {
+			return reconcile.Result{}, err
+		}
+	} else {
 		clusterScope.V(2).Info("Unable to locate droplet instance")
 		r.Recorder.Eventf(domachine, corev1.EventTypeWarning, "NoInstanceFound", "Skip deleting")
-		controllerutil.RemoveFinalizer(domachine, infrav1.MachineFinalizer)
-		return reconcile.Result{}, nil
 	}
-
-	if err := computesvc.DeleteDroplet(machineScope.GetInstanceID()); err != nil {
-		return reconcile.Result{}, err
+	if result, err := r.reconcileDeleteVolumes(ctx, machineScope, clusterScope); err != nil {
+		return result, fmt.Errorf("failed to reconcile delete volumes: %w", err)
 	}
-
-	r.Recorder.Eventf(domachine, corev1.EventTypeNormal, "InstanceDeleted", "Deleted a instance - %s", droplet.Name)
+	r.Recorder.Eventf(domachine, corev1.EventTypeNormal, "InstanceDeleted", "Deleted a instance - %s", machineScope.Name())
 	controllerutil.RemoveFinalizer(domachine, infrav1.MachineFinalizer)
 	return reconcile.Result{}, nil
 }
diff --git a/templates/cluster-template.yaml b/templates/cluster-template.yaml

Original file line number	Diff line number	Diff line change
`@@ -66,6 +66,10 @@ func NewClusterScope(params ClusterScopeParams) (*ClusterScope, error) {`
`66`	`66`	`params.DOClients.Droplets = session.Droplets`
`67`	`67`	`}`
`68`	`68`
	`69`	`+ if params.DOClients.Storage == nil {`
	`70`	`+ params.DOClients.Storage = session.Storage`
	`71`	`+ }`
	`72`	`+`
`69`	`73`	`if params.DOClients.Images == nil {`
`70`	`74`	`params.DOClients.Images = session.Images`
`71`	`75`	`}`