From ed75d84148fc4e9675fad3ea0f2f1631eee2efa6 Mon Sep 17 00:00:00 2001 From: Ludwig Date: Fri, 12 Jul 2024 10:49:03 +0200 Subject: [PATCH] add implementation for reconcile functions --- .golangci.yml | 6 + api/v1alpha1/ionoscloudloadbalancer_types.go | 8 +- .../ionoscloudloadbalancer_controller.go | 107 +++++++++++++++--- internal/loadbalancing/provisioner.go | 8 +- .../loadbalancing/provisioner_external.go | 8 ++ internal/loadbalancing/provisioner_ha.go | 12 +- internal/loadbalancing/provisioner_nlb.go | 12 +- scope/loadbalancer.go | 24 ++-- 8 files changed, 152 insertions(+), 33 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 23c722d0..4b16e044 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -136,6 +136,12 @@ issues: # changes in PRs and avoid nitpicking. exclude-use-default: false exclude-rules: + # TODO(lubedacht): remove this exclusion, once the provisioners are implemented + # I couldn't find a place where to put nolint to disable this linter inline. + - linters: + - dupl + path: "provisioner_(.+).go" + text: "(\\d+)-(\\d+) lines are duplicate of .*" - linters: - containedctx path: '(.+)_test\.go' diff --git a/api/v1alpha1/ionoscloudloadbalancer_types.go b/api/v1alpha1/ionoscloudloadbalancer_types.go index 3a375879..d75101d7 100644 --- a/api/v1alpha1/ionoscloudloadbalancer_types.go +++ b/api/v1alpha1/ionoscloudloadbalancer_types.go @@ -26,8 +26,12 @@ const ( // associated with the IonosCloudLoadBalancer before removing it from the API server. LoadBalancerFinalizer = "ionoscloudloadbalancer.infrastructure.cluster.x-k8s.io" - // IonosCloudLoadBalancerReady is the condition for the IonosCloudLoadBalancer, which indicates that the load balancer is ready. - IonosCloudLoadBalancerReady clusterv1.ConditionType = "LoadBalancerReady" + // LoadBalancerReadyCondition is the condition for the IonosCloudLoadBalancer, which indicates that the load balancer is ready. + LoadBalancerReadyCondition clusterv1.ConditionType = "LoadBalancerReady" + + // InvalidEndpointConfigurationReason indicates that the endpoints for IonosCloudCluster and IonosCloudLoadBalancer + // have not been properly configured. + InvalidEndpointConfigurationReason = "InvalidEndpointConfiguration" ) // LoadBalancerType represents the type of load balancer to create. diff --git a/internal/controller/ionoscloudloadbalancer_controller.go b/internal/controller/ionoscloudloadbalancer_controller.go index 1adfa152..f2a5788d 100644 --- a/internal/controller/ionoscloudloadbalancer_controller.go +++ b/internal/controller/ionoscloudloadbalancer_controller.go @@ -19,17 +19,21 @@ package controller import ( "context" "errors" + "fmt" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/klog/v2" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/annotations" + "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/predicates" ctrl "sigs.k8s.io/controller-runtime" "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/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -71,7 +75,10 @@ func (r *IonosCloudLoadBalancerReconciler) Reconcile( ctx context.Context, ionosCloudLoadBalancer *infrav1.IonosCloudLoadBalancer, ) (_ ctrl.Result, retErr error) { - logger := log.FromContext(ctx, "ionoscloudloadbalancer", klog.KObj(ionosCloudLoadBalancer)) + logger := log.FromContext(ctx, + "ionoscloudloadbalancer", klog.KObj(ionosCloudLoadBalancer), + "type", ionosCloudLoadBalancer.Spec.Type, + ) ctx = log.IntoContext(ctx, logger) logger.V(4).Info("Reconciling IonosCloudLoadBalancer") @@ -125,7 +132,12 @@ func (r *IonosCloudLoadBalancerReconciler) Reconcile( retErr = errors.Join(retErr, err) }() - prov, err := loadbalancing.NewProvisioner(nil, ionosCloudLoadBalancer.Spec.Type) + cloudService, err := createServiceFromCluster(ctx, r.Client, loadBalancerScope.ClusterScope.IonosCluster, logger) + if err != nil { + return ctrl.Result{}, err + } + + prov, err := loadbalancing.NewProvisioner(cloudService, ionosCloudLoadBalancer.Spec.Type) if err != nil { return ctrl.Result{}, err } @@ -155,29 +167,78 @@ func (r *IonosCloudLoadBalancerReconciler) getIonosCluster( return &ionosCluster, nil } -func (*IonosCloudLoadBalancerReconciler) reconcileNormal( +func (r *IonosCloudLoadBalancerReconciler) reconcileNormal( ctx context.Context, - _ *scope.LoadBalancer, - _ loadbalancing.Provisioner, + loadBalancerScope *scope.LoadBalancer, + prov loadbalancing.Provisioner, ) (ctrl.Result, error) { logger := log.FromContext(ctx) - logger.V(4).Info("Reconciling IonosCloudLoadBalancer") - // TODO(lubedacht): Implement reconcileNormal - // - Check if the Endpoint was already set in the IonosCloudCluster - Do nothing if it is already set - // - Create Webhook for HA to apply kube-vip configuration to cloud init - // - Update IonosCloudCluster with Provisioner Endpoint - // - Set IonosCloudLoadBalancer status Ready + controllerutil.AddFinalizer(loadBalancerScope.LoadBalancer, infrav1.LoadBalancerFinalizer) + + if err := r.validateEndpoints(loadBalancerScope); err != nil { + logger.Error(err, "terminal error while validating endpoints. Reconciliation will not continue.") + conditions.MarkFalse( + loadBalancerScope.LoadBalancer, + infrav1.LoadBalancerReadyCondition, + infrav1.InvalidEndpointConfigurationReason, + clusterv1.ConditionSeverityError, "") + return ctrl.Result{}, nil + } + + reconcileSequence := []serviceReconcileStep[scope.LoadBalancer]{ + // NOTE(lubedacht): Prepare should do things like reserving IP addresses and making sure the load balancer + // spec contains a valid endpoint and port. + {name: "PrepareEnvironment", fn: prov.PrepareEnvironment}, + // NOTE(lubedacht): Provision should do the actual provisioning logic for the load balancer if possible + {name: "ProvisionLoadBalancer", fn: prov.ProvisionLoadBalancer}, + // NOTE(lubedacht): PostProvision can do things like setting up the endpoint for the infra cluster. + {name: "PostProvision", fn: prov.PostProvision}, + } + + for _, step := range reconcileSequence { + if requeue, err := step.fn(ctx, loadBalancerScope); err != nil || requeue { + if err != nil { + err = fmt.Errorf("error in step %s: %w", step.name, err) + } + + return ctrl.Result{RequeueAfter: defaultReconcileDuration}, err + } + } + conditions.MarkTrue(loadBalancerScope.LoadBalancer, infrav1.LoadBalancerReadyCondition) + loadBalancerScope.LoadBalancer.Status.Ready = true + + logger.V(4).Info("Successfully reconciled IonosCloudLoadBalancer") return ctrl.Result{}, nil } func (*IonosCloudLoadBalancerReconciler) reconcileDelete( - _ context.Context, - _ *scope.LoadBalancer, - _ loadbalancing.Provisioner, + ctx context.Context, + loadBalancerScope *scope.LoadBalancer, + prov loadbalancing.Provisioner, ) (ctrl.Result, error) { + logger := log.FromContext(ctx) + logger.V(4).Info("Deleting IonosCloudLoadBalancer") + reconcileSequence := []serviceReconcileStep[scope.LoadBalancer]{ + {name: "PrepareCleanup", fn: prov.PrepareCleanup}, + {name: "DestroyLoadBalancer", fn: prov.DestroyLoadBalancer}, + {name: "CleanupResources", fn: prov.CleanupResources}, + } + + for _, step := range reconcileSequence { + if requeue, err := step.fn(ctx, loadBalancerScope); err != nil || requeue { + if err != nil { + err = fmt.Errorf("error in step %s: %w", step.name, err) + } + + return ctrl.Result{RequeueAfter: defaultReconcileDuration}, err + } + } + + controllerutil.RemoveFinalizer(loadBalancerScope.LoadBalancer, infrav1.LoadBalancerFinalizer) + logger.V(4).Info("Successfully deleted IonosCloudLoadBalancer") return ctrl.Result{}, nil } @@ -192,3 +253,21 @@ func (r *IonosCloudLoadBalancerReconciler) SetupWithManager(ctx context.Context, WithEventFilter(predicates.ResourceNotPaused(ctrl.LoggerFrom(ctx))). Complete(reconcile.AsReconciler[*infrav1.IonosCloudLoadBalancer](r.Client, r)) } + +func (*IonosCloudLoadBalancerReconciler) validateEndpoints(loadBalancerScope *scope.LoadBalancer) error { + s := loadBalancerScope + + if s.InfraClusterEndpoint().IsValid() && s.Endpoint().IsZero() { + return errors.New("infra cluster already has an endpoint set, but the load balancer does not") + } + + if s.InfraClusterEndpoint().IsValid() && s.Endpoint().IsValid() { + if s.InfraClusterEndpoint() == s.Endpoint() { + return nil + } + + return errors.New("infra cluster and load balancer endpoints do not match") + } + + return nil +} diff --git a/internal/loadbalancing/provisioner.go b/internal/loadbalancing/provisioner.go index 5c311c02..4d7ec3f1 100644 --- a/internal/loadbalancing/provisioner.go +++ b/internal/loadbalancing/provisioner.go @@ -23,7 +23,7 @@ import ( "fmt" infrav1 "github.com/ionos-cloud/cluster-api-provider-ionoscloud/api/v1alpha1" - "github.com/ionos-cloud/cluster-api-provider-ionoscloud/internal/ionoscloud" + "github.com/ionos-cloud/cluster-api-provider-ionoscloud/internal/service/cloud" "github.com/ionos-cloud/cluster-api-provider-ionoscloud/scope" ) @@ -33,7 +33,11 @@ type Provisioner interface { PrepareEnvironment(ctx context.Context, loadBalancerScope *scope.LoadBalancer) (requeue bool, err error) // ProvisionLoadBalancer is responsible for creating the load balancer. ProvisionLoadBalancer(ctx context.Context, loadBalancerScope *scope.LoadBalancer) (requeue bool, err error) + // PostProvision is responsible for setting the post conditions for the load balancer after it has been created. + PostProvision(ctx context.Context, loadBalancerScope *scope.LoadBalancer) (requeue bool, err error) + // PrepareCleanup is responsible for setting the preconditions for the load balancer to be deleted. + PrepareCleanup(ctx context.Context, loadBalancerScope *scope.LoadBalancer) (requeue bool, err error) // DestroyLoadBalancer is responsible for deleting the load balancer. DestroyLoadBalancer(ctx context.Context, loadBalancerScope *scope.LoadBalancer) (requeue bool, err error) // CleanupResources is responsible for cleaning up any resources associated with the load balancer. @@ -41,7 +45,7 @@ type Provisioner interface { } // NewProvisioner creates a new load balancer provisioner, based on the load balancer type. -func NewProvisioner(_ ionoscloud.Client, lbType infrav1.LoadBalancerType) (Provisioner, error) { +func NewProvisioner(_ *cloud.Service, lbType infrav1.LoadBalancerType) (Provisioner, error) { switch lbType { case infrav1.LoadBalancerTypeHA: return &haProvisioner{}, nil diff --git a/internal/loadbalancing/provisioner_external.go b/internal/loadbalancing/provisioner_external.go index 8c3727d7..5abce641 100644 --- a/internal/loadbalancing/provisioner_external.go +++ b/internal/loadbalancing/provisioner_external.go @@ -34,6 +34,14 @@ func (*externalProvisioner) ProvisionLoadBalancer(_ context.Context, _ *scope.Lo return false, nil } +func (*externalProvisioner) PostProvision(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { + return false, nil +} + +func (*externalProvisioner) PrepareCleanup(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { + return false, nil +} + func (*externalProvisioner) DestroyLoadBalancer(_ context.Context, _ *scope.LoadBalancer) (bool, error) { return false, nil } diff --git a/internal/loadbalancing/provisioner_ha.go b/internal/loadbalancing/provisioner_ha.go index ad224da4..633f2441 100644 --- a/internal/loadbalancing/provisioner_ha.go +++ b/internal/loadbalancing/provisioner_ha.go @@ -25,21 +25,25 @@ import ( type haProvisioner struct{} func (*haProvisioner) PrepareEnvironment(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { - // TODO implement me panic("implement me") } func (*haProvisioner) ProvisionLoadBalancer(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { - // TODO implement me + panic("implement me") +} + +func (*haProvisioner) PostProvision(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { + panic("implement me") +} + +func (*haProvisioner) PrepareCleanup(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { panic("implement me") } func (*haProvisioner) DestroyLoadBalancer(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { - // TODO implement me panic("implement me") } func (*haProvisioner) CleanupResources(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { - // TODO implement me panic("implement me") } diff --git a/internal/loadbalancing/provisioner_nlb.go b/internal/loadbalancing/provisioner_nlb.go index 3efc7b64..7a716793 100644 --- a/internal/loadbalancing/provisioner_nlb.go +++ b/internal/loadbalancing/provisioner_nlb.go @@ -25,21 +25,25 @@ import ( type nlbProvisioner struct{} func (*nlbProvisioner) PrepareEnvironment(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { - // TODO implement me panic("implement me") } func (*nlbProvisioner) ProvisionLoadBalancer(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { - // TODO implement me + panic("implement me") +} + +func (*nlbProvisioner) PostProvision(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { + panic("implement me") +} + +func (*nlbProvisioner) PrepareCleanup(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { panic("implement me") } func (*nlbProvisioner) DestroyLoadBalancer(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { - // TODO implement me panic("implement me") } func (*nlbProvisioner) CleanupResources(_ context.Context, _ *scope.LoadBalancer) (requeue bool, err error) { - // TODO implement me panic("implement me") } diff --git a/scope/loadbalancer.go b/scope/loadbalancer.go index cca63c0c..55768c8b 100644 --- a/scope/loadbalancer.go +++ b/scope/loadbalancer.go @@ -81,12 +81,22 @@ func NewLoadBalancer(params LoadBalancerParams) (*LoadBalancer, error) { }, nil } +// Endpoint returns the load balancer endpoint. +func (l *LoadBalancer) Endpoint() clusterv1.APIEndpoint { + return l.LoadBalancer.Spec.LoadBalancerEndpoint +} + +// InfraClusterEndpoint returns the endpoint from the infra cluster.. +func (l *LoadBalancer) InfraClusterEndpoint() clusterv1.APIEndpoint { + return l.ClusterScope.IonosCluster.Spec.ControlPlaneEndpoint +} + // PatchObject will apply all changes from the IonosCloudLoadBalancer. // It will also make sure to patch the status subresource. -func (m *LoadBalancer) PatchObject() error { - conditions.SetSummary(m.LoadBalancer, +func (l *LoadBalancer) PatchObject() error { + conditions.SetSummary(l.LoadBalancer, conditions.WithConditions( - infrav1.IonosCloudLoadBalancerReady)) + infrav1.LoadBalancerReadyCondition)) timeoutCtx, cancel := context.WithTimeout(context.Background(), time.Second*10) defer cancel() @@ -94,9 +104,9 @@ func (m *LoadBalancer) PatchObject() error { // We don't accept and forward a context here. This is on purpose: Even if a reconciliation is // aborted, we want to make sure that the final patch is applied. Reusing the context from the reconciliation // would cause the patch to be aborted as well. - return m.patchHelper.Patch( + return l.patchHelper.Patch( timeoutCtx, - m.LoadBalancer, + l.LoadBalancer, patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{ clusterv1.ReadyCondition, }}) @@ -105,7 +115,7 @@ func (m *LoadBalancer) PatchObject() error { // Finalize will make sure to apply a patch to the current IonosCloudLoadBalancer. // It also implements a retry mechanism to increase the chance of success // in case the patch operation was not successful. -func (m *LoadBalancer) Finalize() error { +func (l *LoadBalancer) Finalize() error { // NOTE(lubedacht) retry is only a way to reduce the failure chance, // but in general, the reconciliation logic must be resilient // to handle an outdated resource from that API server. @@ -113,5 +123,5 @@ func (m *LoadBalancer) Finalize() error { return retry.OnError( retry.DefaultBackoff, shouldRetry, - m.PatchObject) + l.PatchObject) }