Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add EMLB deletion #771

Merged
merged 10 commits into from
Jun 18, 2024
3 changes: 3 additions & 0 deletions api/v1beta1/packetcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import (
)

const (
// ClusterFinalizer allows DockerClusterReconciler to clean up resources associated with DockerCluster before
// removing it from the apiserver.
ClusterFinalizer = "packetcluster.infrastructure.cluster.x-k8s.io"
// NetworkInfrastructureReadyCondition reports of current status of cluster infrastructure.
NetworkInfrastructureReadyCondition clusterv1.ConditionType = "NetworkInfrastructureReady"
)
Expand Down
31 changes: 28 additions & 3 deletions controllers/packetcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package controllers
import (
"context"
"errors"
"fmt"

apierrors "k8s.io/apimachinery/pkg/api/errors"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
Expand All @@ -31,6 +32,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/handler"

infrav1 "sigs.k8s.io/cluster-api-provider-packet/api/v1beta1"
Expand Down Expand Up @@ -102,7 +104,14 @@ func (r *PacketClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques

// Handle deleted clusters
if !cluster.DeletionTimestamp.IsZero() {
return r.reconcileDelete(ctx, clusterScope)
return ctrl.Result{}, r.reconcileDelete(ctx, clusterScope)
}

// Add finalizer first if not set to avoid the race condition between init and delete.
// Note: Finalizers in general can only be added when the deletionTimestamp is not set.
if !controllerutil.ContainsFinalizer(packetcluster, infrav1.ClusterFinalizer) {
controllerutil.AddFinalizer(packetcluster, infrav1.ClusterFinalizer)
return ctrl.Result{}, nil
}

err = r.reconcileNormal(ctx, clusterScope)
Expand Down Expand Up @@ -177,12 +186,28 @@ func (r *PacketClusterReconciler) reconcileNormal(ctx context.Context, clusterSc
return nil
}

func (r *PacketClusterReconciler) reconcileDelete(_ context.Context, _ *scope.ClusterScope) (ctrl.Result, error) {
func (r *PacketClusterReconciler) reconcileDelete(ctx context.Context, clusterScope *scope.ClusterScope) error {
log := ctrl.LoggerFrom(ctx).WithValues("cluster", clusterScope.Cluster.Name)
log.Info("Reconciling PacketCluster Deletion")

packetCluster := clusterScope.PacketCluster

if packetCluster.Spec.VIPManager == emlb.EMLBVIPID {
// Create new EMLB object
lb := emlb.NewEMLB(r.PacketClient.GetConfig().DefaultHeader["X-Auth-Token"], packetCluster.Spec.ProjectID, packetCluster.Spec.Metro)

if err := lb.DeleteLoadBalancer(ctx, clusterScope); err != nil {
return fmt.Errorf("failed to delete load balancer: %w", err)
}
}
// Initially I created this handler to remove an elastic IP when a cluster
// gets delete, but it does not sound like a good idea. It is better to
// leave to the users the ability to decide if they want to keep and resign
// the IP or if they do not need it anymore
return ctrl.Result{}, nil

// Cluster is deleted so remove the finalizer.
controllerutil.RemoveFinalizer(packetCluster, infrav1.ClusterFinalizer)
return nil
}

func (r *PacketClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
Expand Down
24 changes: 17 additions & 7 deletions controllers/packetmachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,13 @@ func (r *PacketMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques
}
}()

// Add finalizer first if not set to avoid the race condition between init and delete.
displague marked this conversation as resolved.
Show resolved Hide resolved
// Note: Finalizers in general can only be added when the deletionTimestamp is not set.
if packetmachine.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(packetmachine, infrav1.MachineFinalizer) {
controllerutil.AddFinalizer(packetmachine, infrav1.MachineFinalizer)
return ctrl.Result{}, nil
}

// Handle deleted machines
if !packetmachine.ObjectMeta.DeletionTimestamp.IsZero() {
err = r.reconcileDelete(ctx, machineScope)
Expand Down Expand Up @@ -262,12 +269,6 @@ func (r *PacketMachineReconciler) reconcile(ctx context.Context, machineScope *s
return ctrl.Result{}, nil
}

// If the PacketMachine doesn't have our finalizer, add it.
controllerutil.AddFinalizer(packetmachine, infrav1.MachineFinalizer)
if err := machineScope.PatchObject(ctx); err != nil {
log.Error(err, "unable to patch object")
}

if !machineScope.Cluster.Status.InfrastructureReady {
log.Info("Cluster infrastructure is not ready yet")
conditions.MarkFalse(machineScope.PacketMachine, infrav1.DeviceReadyCondition, infrav1.WaitingForClusterInfrastructureReason, clusterv1.ConditionSeverityInfo, "")
Expand Down Expand Up @@ -536,12 +537,21 @@ func (r *PacketMachineReconciler) reconcileDelete(ctx context.Context, machineSc
device = dev
}

// We should never get there but this is a safetly check
// We should never get there but this is a safety check
if device == nil {
controllerutil.RemoveFinalizer(packetmachine, infrav1.MachineFinalizer)
return fmt.Errorf("%w: %s", errMissingDevice, packetmachine.Name)
}

if machineScope.PacketCluster.Spec.VIPManager == emlb.EMLBVIPID {
// Create new EMLB object
lb := emlb.NewEMLB(r.PacketClient.GetConfig().DefaultHeader["X-Auth-Token"], machineScope.PacketCluster.Spec.ProjectID, packetmachine.Spec.Metro)

if err := lb.DeleteLoadBalancerOrigin(ctx, machineScope); err != nil {
return fmt.Errorf("failed to delete load balancer origin: %w", err)
}
}

apiRequest := r.PacketClient.DevicesApi.DeleteDevice(ctx, device.GetId()).ForceDelete(force)
if _, err := apiRequest.Execute(); err != nil { //nolint:bodyclose // see https://github.com/timakin/bodyclose/issues/42
return fmt.Errorf("failed to delete the machine: %w", err)
Expand Down
137 changes: 136 additions & 1 deletion internal/emlb/emlb.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func NewEMLB(metalAPIKey, projectID, metro string) *EMLB {
return manager
}

// ReconcileLoadBalancer creates a new Equinix Metal Load Balancer.
// ReconcileLoadBalancer creates a new Equinix Metal Load Balancer and associates it with the given ClusterScope.
func (e *EMLB) ReconcileLoadBalancer(ctx context.Context, clusterScope *scope.ClusterScope) error {
log := ctrl.LoggerFrom(ctx)

Expand Down Expand Up @@ -233,6 +233,113 @@ func (e *EMLB) ReconcileVIPOrigin(ctx context.Context, machineScope *scope.Machi
return nil
}

// DeleteLoadBalancer deletes the Equinix Metal Load Balancer associated with a given ClusterScope.
func (e *EMLB) DeleteLoadBalancer(ctx context.Context, clusterScope *scope.ClusterScope) error {
log := ctrl.LoggerFrom(ctx)

packetCluster := clusterScope.PacketCluster
clusterName := packetCluster.Name

// Make sure the cluster already has an EMLB ID in its packetCluster annotations, otherwise abort.
lbID, exists := packetCluster.Annotations[loadBalancerIDAnnotation]
if !exists || (lbID == "") {
log.Info("no Equinix Metal Load Balancer found in cluster's annotations, skipping EMLB delete")
return nil
}

// See if the EMLB has a Port ID in its packetCluster annotations.
lbPortNumber, exists := packetCluster.Annotations[loadBalancerPortNumberAnnotation]
if !exists || (lbPortNumber == "") {
log.Info("no Equinix Metal Load Balancer Port Number found in cluster's annotations, skipping EMLB delete")
return nil
}

log.Info("Deleting EMLB", "Cluster Metro", e.metro, "Cluster Name", clusterName, "Project ID", e.projectID, "Load Balancer ID", lbID)

// Fetch the Load Balancer object.
// Skip if 404, otherwise error
displague marked this conversation as resolved.
Show resolved Hide resolved
lb, err := e.getLoadBalancer(ctx, lbID)
if err != nil {
log.Error(err, "failed to load the loadbalancer object, cannot proceed with deletion")
return err
}

// Get an int version of the listener port number.
portNumber, err := strconv.ParseInt(lbPortNumber, 10, 32)
if err != nil {
log.Error(err, "failed to convert the loadbalancer port number to an int, cannot proceed with deletion")
return err
}

// Get the entire listener port object.
// Skip if 404, otherwise error
lbPort, err := e.getLoadBalancerPort(ctx, lbID, int32(portNumber))
if err != nil {
log.Error(err, "failed to load the loadbalancer port object, cannot proceed with deletion")
return err
}

resp, err := e.deleteListenerPort(ctx, lbPort.GetId())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, the LB ports are automatically deleted when the LB is deleted. The only time we need to explicitly delete the port is if we're keeping the LB and need to change which ports are open.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed. Going to remove this.

if err != nil {
log.Error(err, "LB Port Delete Failed", "EMLB ID", lb.GetId(), "Port ID", lbPort.GetId(), "Response Body", resp.Body)
return err
}

resp, err = e.deleteLoadBalancer(ctx, lb.GetId())
if err != nil {
log.Error(err, "LB Delete Failed", "EMLB ID", lb.GetId(), "Response Body", resp.Body)
return err
cprivitere marked this conversation as resolved.
Show resolved Hide resolved
}

return nil
cprivitere marked this conversation as resolved.
Show resolved Hide resolved
}

// DeleteLoadBalancerOrigin deletes the Equinix Metal Load Balancer associated with a given ClusterScope.
func (e *EMLB) DeleteLoadBalancerOrigin(ctx context.Context, machineScope *scope.MachineScope) error {
log := ctrl.LoggerFrom(ctx)

clusterName := machineScope.Cluster.Name

// Make sure the machine has an EMLB Pool ID in its packetMachine annotations, otherwise abort.
lbPoolID, exists := machineScope.PacketMachine.Annotations[loadBalancerPoolIDAnnotation]
if !exists || (lbPoolID == "") {
return fmt.Errorf("no Equinix Metal Load Balancer Pool found in machine's annotations")
}

// Make sure the machine has an EMLB Origin ID in its packetMachine annotations, otherwise abort.
lbOriginID, exists := machineScope.PacketMachine.Annotations[loadBalancerOriginIDAnnotation]
if !exists || (lbOriginID == "") {
return fmt.Errorf("no Equinix Metal Load Balancer Origin found in machine's annotations")
}

log.Info("Deleting EMLB Origin from Pool", "Cluster Metro", e.metro, "Cluster Name", clusterName, "Project ID", e.projectID, "Pool ID", lbPoolID, "Origin ID", lbOriginID)

// Fetch the Load Balancer Pool object.
lbPool, err := e.getLoadBalancerPool(ctx, lbPoolID)
if err != nil {
log.Error(err, "failed to load the loadbalancer pool object, cannot proceed with deletion")
return err
}

resp, err := e.deleteOrigin(ctx, lbOriginID)
if err != nil {
log.Error(err, "LB Origin Delete Failed", "Pool ID", lbPool.GetId(), "Origin ID", lbOriginID, "Response Body", resp.Body)
return err
}

// TODO: Validate pool is empty

log.Info("Deleting EMLB Pool", "Cluster Metro", e.metro, "Cluster Name", clusterName, "Project ID", e.projectID, "Pool ID", lbPoolID)

resp, err = e.deletePool(ctx, lbPool.GetId())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with where this fits in to the overall cluster lifecycle. Is there any risk here that cluster-api could empty and delete the pool because we're making some kind of cluster change (recreating control plane nodes or something). Even if there is that risk, does it really matter, or would cluster-api just create a new pool when a new control plane node came up?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is called on node deletion. We have a pool per node so it's no problem, currently.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, OK. If there's a pool per node then I think this is similar to the LB deletion, and deleting the pool will automatically delete the origins inside the pool.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm this works here too and will delete the origin deleting parts. My original idea was to delete all the components explicitly so that if things start failing, as little will be deleted as possible, but given the one to one nature, I guess that doesn't make much sense.

if err != nil {
log.Error(err, "LB Pool Delete Failed", "Pool ID", lbPool.GetId(), "Response Body", resp.Body)
return err
cprivitere marked this conversation as resolved.
Show resolved Hide resolved
}

return nil
cprivitere marked this conversation as resolved.
Show resolved Hide resolved
}

// getLoadBalancer Returns a Load Balancer object given an id.
func (e *EMLB) getLoadBalancer(ctx context.Context, id string) (*lbaas.LoadBalancer, error) {
ctx = context.WithValue(ctx, lbaas.ContextOAuth2, e.tokenExchanger)
Expand All @@ -249,6 +356,14 @@ func (e *EMLB) getLoadBalancerPort(ctx context.Context, id string, portNumber in
return LoadBalancerPort, err
}

// getLoadBalancerPool Returns a Load Balancer Pool object given an id.
func (e *EMLB) getLoadBalancerPool(ctx context.Context, id string) (*lbaas.LoadBalancerPool, error) {
ctx = context.WithValue(ctx, lbaas.ContextOAuth2, e.tokenExchanger)

LoadBalancerPool, _, err := e.client.PoolsApi.GetLoadBalancerPool(ctx, id).Execute()
return LoadBalancerPool, err
}

// EnsureLoadBalancerOrigin takes the devices list of IP addresses in a Load Balancer Origin Pool and ensures an origin
// for the first IPv4 address in the list exists.
func (e *EMLB) ensureLoadBalancerOrigin(ctx context.Context, originID, poolID, lbName string, deviceAddr []corev1.NodeAddress) (*lbaas.LoadBalancerPoolOrigin, error) {
Expand Down Expand Up @@ -401,6 +516,26 @@ func (e *EMLB) createOrigin(ctx context.Context, poolID, originName string, targ
return e.client.PoolsApi.CreateLoadBalancerPoolOrigin(ctx, poolID).LoadBalancerPoolOriginCreate(createOriginRequest).Execute()
}

func (e *EMLB) deleteLoadBalancer(ctx context.Context, lbID string) (*http.Response, error) {
ctx = context.WithValue(ctx, lbaas.ContextOAuth2, e.tokenExchanger)
return e.client.LoadBalancersApi.DeleteLoadBalancer(ctx, lbID).Execute()
}

func (e *EMLB) deleteListenerPort(ctx context.Context, portID string) (*http.Response, error) {
ctx = context.WithValue(ctx, lbaas.ContextOAuth2, e.tokenExchanger)
return e.client.PortsApi.DeleteLoadBalancerPort(ctx, portID).Execute()
}

func (e *EMLB) deletePool(ctx context.Context, poolID string) (*http.Response, error) {
ctx = context.WithValue(ctx, lbaas.ContextOAuth2, e.tokenExchanger)
return e.client.PoolsApi.DeleteLoadBalancerPool(ctx, poolID).Execute()
}

func (e *EMLB) deleteOrigin(ctx context.Context, originID string) (*http.Response, error) {
ctx = context.WithValue(ctx, lbaas.ContextOAuth2, e.tokenExchanger)
return e.client.OriginsApi.DeleteLoadBalancerOrigin(ctx, originID).Execute()
}

func (e *EMLB) updateListenerPort(ctx context.Context, poolID, lbPortID string) (*lbaas.LoadBalancerPort, error) {
ctx = context.WithValue(ctx, lbaas.ContextOAuth2, e.tokenExchanger)

Expand Down