Add failure domain support to CAPD

Author: fabriziopandini

test/infrastructure/docker/api/v1beta1/dockercluster_types.go

@@ -37,7 +37,7 @@ type DockerClusterSpec struct {
 	// +optional
 	ControlPlaneEndpoint APIEndpoint `json:"controlPlaneEndpoint"`
 
-	// FailureDomains are not usulaly defined on the spec.
+	// FailureDomains are usually not defined in the spec.
 	// The docker provider is special since failure domains don't mean anything in a local docker environment.
 	// Instead, the docker cluster controller will simply copy these into the Status and allow the Cluster API
 	// controllers to do what they will with the defined failure domains.

test/infrastructure/docker/config/crd/bases/infrastructure.cluster.x-k8s.io_dockerclusters.yaml

@@ -371,7 +371,7 @@ spec:
                         is suitable for use by control plane machines.
                       type: boolean
                   type: object
-                description: FailureDomains are not usulaly defined on the spec. The
+                description: FailureDomains are usually not defined in the spec. The
                   docker provider is special since failure domains don't mean anything
                   in a local docker environment. Instead, the docker cluster controller
                   will simply copy these into the Status and allow the Cluster API

test/infrastructure/docker/config/crd/bases/infrastructure.cluster.x-k8s.io_dockerclustertemplates.yaml

@@ -203,7 +203,7 @@ spec:
                                 domain is suitable for use by control plane machines.
                               type: boolean
                           type: object
-                        description: FailureDomains are not usulaly defined on the
+                        description: FailureDomains are usually not defined in the
                           spec. The docker provider is special since failure domains
                           don't mean anything in a local docker environment. Instead,
                           the docker cluster controller will simply copy these into

test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go

@@ -20,6 +20,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/pkg/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -185,7 +186,8 @@ func (r *DockerMachinePoolReconciler) reconcileNormal(ctx context.Context, clust
 	}
 
 	// Reconcile machines and updates Status.Instances
-	if res, err := pool.ReconcileMachines(ctx); err != nil || !res.IsZero() {
+	res, err := pool.ReconcileMachines(ctx)
+	if err != nil {
 		return res, err
 	}
 
@@ -207,7 +209,12 @@ func (r *DockerMachinePoolReconciler) reconcileNormal(ctx context.Context, clust
 	}
 
 	dockerMachinePool.Status.Ready = len(dockerMachinePool.Spec.ProviderIDList) == int(*machinePool.Spec.Replicas)
-	return ctrl.Result{}, nil
+
+	// if some machine is still provisioning, force reconcile in few seconds to check again infrastructure.
+	if !dockerMachinePool.Status.Ready && res.IsZero() {
+		return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
+	}
+	return res, nil
 }
 
 func getDockerMachinePoolProviderID(clusterName, dockerMachinePoolName string) string {

test/infrastructure/docker/exp/internal/docker/nodepool.go

@@ -21,6 +21,8 @@ import (
 	"context"
 	"encoding/base64"
 	"fmt"
+	"math/rand"
+	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -43,7 +45,7 @@ const (
 )
 
 // NodePool is a wrapper around a collection of like machines which are owned by a DockerMachinePool. A node pool
-// provides a friendly way of managing (adding, deleting, reimaging) a set of docker machines. The node pool will also
+// provides a friendly way of managing (adding, deleting, updating) a set of docker machines. The node pool will also
 // sync the docker machine pool status Instances field with the state of the docker machines.
 type NodePool struct {
 	client            client.Client
@@ -86,7 +88,7 @@ func (np *NodePool) ReconcileMachines(ctx context.Context) (ctrl.Result, error)
 	for _, machine := range np.machines {
 		totalNumberOfMachines++
 		if totalNumberOfMachines > desiredReplicas || !np.isMachineMatchingInfrastructureSpec(machine) {
-			externalMachine, err := docker.NewMachine(ctx, np.cluster, machine.Name(), np.dockerMachinePool.Spec.Template.CustomImage, np.labelFilters)
+			externalMachine, err := docker.NewMachine(ctx, np.cluster, machine.Name(), np.labelFilters)
 			if err != nil {
 				return ctrl.Result{}, errors.Wrapf(err, "failed to create helper for managing the externalMachine named %s", machine.Name())
 			}
@@ -151,7 +153,7 @@ func (np *NodePool) ReconcileMachines(ctx context.Context) (ctrl.Result, error)
 // Delete will delete all of the machines in the node pool.
 func (np *NodePool) Delete(ctx context.Context) error {
 	for _, machine := range np.machines {
-		externalMachine, err := docker.NewMachine(ctx, np.cluster, machine.Name(), np.dockerMachinePool.Spec.Template.CustomImage, np.labelFilters)
+		externalMachine, err := docker.NewMachine(ctx, np.cluster, machine.Name(), np.labelFilters)
 		if err != nil {
 			return errors.Wrapf(err, "failed to create helper for managing the externalMachine named %s", machine.Name())
 		}
@@ -165,7 +167,15 @@ func (np *NodePool) Delete(ctx context.Context) error {
 }
 
 func (np *NodePool) isMachineMatchingInfrastructureSpec(machine *docker.Machine) bool {
-	return machine.ImageVersion() == container.SemverToOCIImageTag(*np.machinePool.Spec.Template.Spec.Version)
+	return imageVersion(machine) == container.SemverToOCIImageTag(*np.machinePool.Spec.Template.Spec.Version)
+}
+
+// ImageVersion returns the version of the image used or nil if not specified
+// NOTE: Image version might be different from the Kubernetes version, because some characters
+// allowed by semver (e.g. +) can't be used for image tags, so they are replaced with "_".
+func imageVersion(m *docker.Machine) string {
+	containerImage := m.ContainerImage()
+	return containerImage[strings.LastIndex(containerImage, ":")+1:]
 }
 
 // machinesMatchingInfrastructureSpec returns all of the docker.Machines which match the machine pool / docker machine pool spec.
@@ -183,12 +193,28 @@ func (np *NodePool) machinesMatchingInfrastructureSpec() []*docker.Machine {
 // addMachine will add a new machine to the node pool and update the docker machine pool status.
 func (np *NodePool) addMachine(ctx context.Context) error {
 	instanceName := fmt.Sprintf("worker-%s", util.RandomString(6))
-	externalMachine, err := docker.NewMachine(ctx, np.cluster, instanceName, np.dockerMachinePool.Spec.Template.CustomImage, np.labelFilters)
+	externalMachine, err := docker.NewMachine(ctx, np.cluster, instanceName, np.labelFilters)
 	if err != nil {
 		return errors.Wrapf(err, "failed to create helper for managing the externalMachine named %s", instanceName)
 	}
 
-	if err := externalMachine.Create(ctx, constants.WorkerNodeRoleValue, np.machinePool.Spec.Template.Spec.Version, np.dockerMachinePool.Spec.Template.ExtraMounts); err != nil {
+	// NOTE: FailureDomains don't mean much in CAPD since it's all local, but we are setting a label on
+	// each container, so we can check placement.
+	labels := map[string]string{}
+	for k, v := range np.labelFilters {
+		labels[k] = v
+	}
+
+	if len(np.machinePool.Spec.FailureDomains) > 0 {
+		// For MachinePools placement is expected to be managed by the underlying infrastructure primitive, but
+		// given that there is no such an thing in CAPD, we are picking a random failure domain.
+		randomIndex := rand.Intn(len(np.machinePool.Spec.FailureDomains)) //nolint:gosec
+		for k, v := range docker.FailureDomainLabel(&np.machinePool.Spec.FailureDomains[randomIndex]) {
+			labels[k] = v
+		}
+	}
+
+	if err := externalMachine.Create(ctx, np.dockerMachinePool.Spec.Template.CustomImage, constants.WorkerNodeRoleValue, np.machinePool.Spec.Template.Spec.Version, labels, np.dockerMachinePool.Spec.Template.ExtraMounts); err != nil {
 		return errors.Wrapf(err, "failed to create docker machine with instance name %s", instanceName)
 	}
 	return nil
@@ -244,7 +270,7 @@ func (np *NodePool) reconcileMachine(ctx context.Context, machine *docker.Machin
 		}
 	}()
 
-	externalMachine, err := docker.NewMachine(ctx, np.cluster, machine.Name(), np.dockerMachinePool.Spec.Template.CustomImage, np.labelFilters)
+	externalMachine, err := docker.NewMachine(ctx, np.cluster, machine.Name(), np.labelFilters)
 	if err != nil {
 		return ctrl.Result{}, errors.Wrapf(err, "failed to create helper for managing the externalMachine named %s", machine.Name())
 	}

test/infrastructure/docker/internal/controllers/dockermachine_controller.go

@@ -143,7 +143,7 @@ func (r *DockerMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	}
 
 	// Create a helper for managing the docker container hosting the machine.
-	externalMachine, err := docker.NewMachine(ctx, cluster, machine.Name, dockerMachine.Spec.CustomImage, nil)
+	externalMachine, err := docker.NewMachine(ctx, cluster, machine.Name, nil)
 	if err != nil {
 		return ctrl.Result{}, errors.Wrapf(err, "failed to create helper for managing the externalMachine")
 	}
@@ -231,7 +231,9 @@ func (r *DockerMachineReconciler) reconcileNormal(ctx context.Context, cluster *
 
 	// Create the machine if not existing yet
 	if !externalMachine.Exists() {
-		if err := externalMachine.Create(ctx, role, machine.Spec.Version, dockerMachine.Spec.ExtraMounts); err != nil {
+		// NOTE: FailureDomains don't mean much in CAPD since it's all local, but we are setting a label on
+		// each container, so we can check placement.
+		if err := externalMachine.Create(ctx, dockerMachine.Spec.CustomImage, role, machine.Spec.Version, docker.FailureDomainLabel(machine.Spec.FailureDomain), dockerMachine.Spec.ExtraMounts); err != nil {
 			return ctrl.Result{}, errors.Wrap(err, "failed to create worker DockerMachine")
 		}
 	}

test/infrastructure/docker/internal/docker/kind_manager.go

@@ -79,6 +79,7 @@ func (m *Manager) CreateControlPlaneNode(ctx context.Context, name, image, clust
 		Role:         constants.ControlPlaneNodeRoleValue,
 		PortMappings: portMappingsWithAPIServer,
 		Mounts:       mounts,
+		Labels:       labels,
 		IPFamily:     ipFamily,
 	}
 	node, err := createNode(ctx, createOpts)

test/infrastructure/docker/internal/docker/machine.go

@@ -58,16 +58,14 @@ type nodeCreator interface {
 type Machine struct {
 	cluster   string
 	machine   string
-	image     string
 	ipFamily  clusterv1.ClusterIPFamily
-	labels    map[string]string
 	container *types.Node
 
 	nodeCreator nodeCreator
 }
 
 // NewMachine returns a new Machine service for the given Cluster/DockerCluster pair.
-func NewMachine(ctx context.Context, cluster *clusterv1.Cluster, machine, image string, labels map[string]string) (*Machine, error) {
+func NewMachine(ctx context.Context, cluster *clusterv1.Cluster, machine string, filterLabels map[string]string) (*Machine, error) {
 	if cluster == nil {
 		return nil, errors.New("cluster is required when creating a docker.Machine")
 	}
@@ -81,7 +79,7 @@ func NewMachine(ctx context.Context, cluster *clusterv1.Cluster, machine, image
 	filters := container.FilterBuilder{}
 	filters.AddKeyNameValue(filterLabel, clusterLabelKey, cluster.Name)
 	filters.AddKeyValue(filterName, fmt.Sprintf("^%s$", machineContainerName(cluster.Name, machine)))
-	for key, val := range labels {
+	for key, val := range filterLabels {
 		filters.AddKeyNameValue(filterLabel, key, val)
 	}
 
@@ -98,10 +96,8 @@ func NewMachine(ctx context.Context, cluster *clusterv1.Cluster, machine, image
 	return &Machine{
 		cluster:     cluster.Name,
 		machine:     machine,
-		image:       image,
 		ipFamily:    ipFamily,
 		container:   newContainer,
-		labels:      labels,
 		nodeCreator: &Manager{},
 	}, nil
 }
@@ -136,9 +132,7 @@ func ListMachinesByCluster(ctx context.Context, cluster *clusterv1.Cluster, labe
 		machines[i] = &Machine{
 			cluster:     cluster.Name,
 			machine:     machineFromContainerName(cluster.Name, containerNode.Name),
-			image:       containerNode.Image,
 			ipFamily:    ipFamily,
-			labels:      labels,
 			container:   containerNode,
 			nodeCreator: &Manager{},
 		}
@@ -155,17 +149,6 @@ func (m *Machine) IsControlPlane() bool {
 	return m.container.ClusterRole == constants.ControlPlaneNodeRoleValue
 }
 
-// ImageVersion returns the version of the image used or nil if not specified
-// NOTE: Image version might be different from the Kubernetes version, because some characters
-// allowed by semver (e.g. +) can't be used for image tags, so they are replaced with "_".
-func (m *Machine) ImageVersion() string {
-	if m.image == "" {
-		return defaultImageTag
-	}
-
-	return m.image[strings.LastIndex(m.image, ":")+1 : len(m.image)]
-}
-
 // Exists returns true if the container for this machine exists.
 func (m *Machine) Exists() bool {
 	return m.container != nil
@@ -200,17 +183,26 @@ func (m *Machine) Address(ctx context.Context) (string, error) {
 	return ipv4, nil
 }
 
+// ContainerImage return the image of the container for this machine
+// or empty string if the container does not exist yet.
+func (m *Machine) ContainerImage() string {
+	if m.container == nil {
+		return ""
+	}
+	return m.container.Image
+}
+
 // Create creates a docker container hosting a Kubernetes node.
-func (m *Machine) Create(ctx context.Context, role string, version *string, mounts []infrav1.Mount) error {
+func (m *Machine) Create(ctx context.Context, image string, role string, version *string, labels map[string]string, mounts []infrav1.Mount) error {
 	log := ctrl.LoggerFrom(ctx)
 
 	// Create if not exists.
 	if m.container == nil {
 		var err error
 
 		machineImage := m.machineImage(version)
-		if m.image != "" {
-			machineImage = m.image
+		if image != "" {
+			machineImage = image
 		}
 
 		switch role {
@@ -225,7 +217,7 @@ func (m *Machine) Create(ctx context.Context, role string, version *string, moun
 				0,
 				kindMounts(mounts),
 				nil,
-				m.labels,
+				labels,
 				m.ipFamily,
 			)
 			if err != nil {
@@ -240,7 +232,7 @@ func (m *Machine) Create(ctx context.Context, role string, version *string, moun
 				m.cluster,
 				kindMounts(mounts),
 				nil,
-				m.labels,
+				labels,
 				m.ipFamily,
 			)
 			if err != nil {

test/infrastructure/docker/internal/docker/util.go

@@ -27,10 +27,22 @@ import (
 	"sigs.k8s.io/cluster-api/test/infrastructure/docker/internal/docker/types"
 )
 
-const clusterLabelKey = "io.x-k8s.kind.cluster"
-const nodeRoleLabelKey = "io.x-k8s.kind.role"
-const filterLabel = "label"
-const filterName = "name"
+const (
+	clusterLabelKey  = "io.x-k8s.kind.cluster"
+	nodeRoleLabelKey = "io.x-k8s.kind.role"
+	filterLabel      = "label"
+	filterName       = "name"
+
+	failureDomainLabelKey = "io.x-k8s.cluster.failureDomain"
+)
+
+// FailureDomainLabel returns a map with the docker label for the given failure domain.
+func FailureDomainLabel(failureDomain *string) map[string]string {
+	if failureDomain != nil && *failureDomain != "" {
+		return map[string]string{failureDomainLabelKey: *failureDomain}
+	}
+	return nil
+}
 
 func machineContainerName(cluster, machine string) string {
 	if strings.HasPrefix(machine, cluster) {
@@ -100,6 +112,7 @@ func list(ctx context.Context, visit func(context.Context, string, *types.Node),
 		cluster := clusterLabelKey
 		image := cntr.Image
 		status := cntr.Status
+
 		visit(ctx, cluster, types.NewNode(name, image, "undetermined").WithStatus(status))
 	}