From 78e332a55fbefd803485237e82de6d1afa894f40 Mon Sep 17 00:00:00 2001 From: Evan Johnson Date: Wed, 24 Jan 2024 17:50:09 -0500 Subject: [PATCH 01/11] add initial LinodeCluster controller logic --- api/v1alpha1/linodecluster_types.go | 17 +- api/v1alpha1/zz_generated.deepcopy.go | 2 +- cloud/services/loadbalancers.go | 141 ++++++++++++ cmd/main.go | 15 +- ...cture.cluster.x-k8s.io_linodeclusters.yaml | 17 +- config/crd/kustomization.yaml | 5 + controller/linodecluster_controller.go | 213 ++++++++++++++++-- 7 files changed, 379 insertions(+), 31 deletions(-) create mode 100644 cloud/services/loadbalancers.go diff --git a/api/v1alpha1/linodecluster_types.go b/api/v1alpha1/linodecluster_types.go index 7b9595e4b..aa70c9dc8 100644 --- a/api/v1alpha1/linodecluster_types.go +++ b/api/v1alpha1/linodecluster_types.go @@ -47,13 +47,13 @@ type LinodeClusterStatus struct { // reconciling the Machine and will contain a succinct value suitable // for machine interpretation. // +optional - FailureReason *errors.MachineStatusError `json:"failureReason"` + FailureReason *errors.ClusterStatusError `json:"failureReason,omitempty"` // FailureMessage will be set in the event that there is a terminal problem // reconciling the Machine and will contain a more verbose string suitable // for logging and human consumption. // +optional - FailureMessage *string `json:"failureMessage"` + FailureMessage *string `json:"failureMessage,omitempty"` // Conditions defines current service state of the LinodeCluster. // +optional @@ -85,9 +85,18 @@ func (lm *LinodeCluster) SetConditions(conditions clusterv1.Conditions) { // NetworkSpec encapsulates Linode networking resources. type NetworkSpec struct { - // NodebalancerID is the id of apiserver Nodebalancer. + // LoadBalancerType is the type of load balancer to use, defaults to NodeBalancer if not otherwise set + // +kubebuilder:validation:Enum=NodeBalancer // +optional - NodebalancerID int `json:"nodebalancerID,omitempty"` + LoadBalancerType string `json:"loadBalancerType,omitempty"` + // LoadBalancerPort used by the api server. It must be valid ports range (1-65535). If omitted, default value is 6443. + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=65535 + // +optional + LoadBalancerPort int `json:"loadBalancerPort,omitempty"` + // NodeBalancerID is the id of api server NodeBalancer. + // +optional + NodeBalancerID int `json:"nodeBalancerID,omitempty"` } // +kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 29963df12..2f7eda4c1 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -153,7 +153,7 @@ func (in *LinodeClusterStatus) DeepCopyInto(out *LinodeClusterStatus) { *out = *in if in.FailureReason != nil { in, out := &in.FailureReason, &out.FailureReason - *out = new(errors.MachineStatusError) + *out = new(errors.ClusterStatusError) **out = **in } if in.FailureMessage != nil { diff --git a/cloud/services/loadbalancers.go b/cloud/services/loadbalancers.go new file mode 100644 index 000000000..88a0fcdf2 --- /dev/null +++ b/cloud/services/loadbalancers.go @@ -0,0 +1,141 @@ +package services + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "slices" + "strconv" + + "github.com/go-logr/logr" + "github.com/linode/cluster-api-provider-linode/cloud/scope" + "github.com/linode/cluster-api-provider-linode/util" + "github.com/linode/linodego" +) + +var ( + defaultLBPort = 6443 +) + +// CreateNodeBalancer creates a new NodeBalancer if one doesn't exist +func CreateNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancer, error) { + var linodeNBs []linodego.NodeBalancer + var linodeNB *linodego.NodeBalancer + NBLabel := fmt.Sprintf("%s-api-server", clusterScope.LinodeCluster.Name) + clusterUID := string(clusterScope.LinodeCluster.UID) + tags := []string{string(clusterScope.LinodeCluster.UID)} + filter := map[string]string{ + "label": NBLabel, + } + + rawFilter, err := json.Marshal(filter) + if err != nil { + return nil, err + } + logger.Info("Creating NodeBalancer") + if linodeNBs, err = clusterScope.LinodeClient.ListNodeBalancers(ctx, linodego.NewListOptions(1, string(rawFilter))); err != nil { + logger.Info("Failed to list NodeBalancers", "error", err.Error()) + + return nil, err + } + + switch len(linodeNBs) { + case 1: + logger.Info(fmt.Sprintf("NodeBalancer %s already exists", *linodeNBs[0].Label)) + if !slices.Contains(linodeNBs[0].Tags, clusterUID) { + err = errors.New("NodeBalancer conflict") + logger.Error(err, fmt.Sprintf("NodeBalancer %s is not associated with cluster UID %s. Owner cluster is %s", *linodeNBs[0].Label, clusterUID, linodeNBs[0].Tags[0])) + + return nil, err + } + linodeNB = &linodeNBs[0] + case 0: + logger.Info(fmt.Sprintf("Creating NodeBalancer %s-api-server", clusterScope.LinodeCluster.Name)) + createConfig := linodego.NodeBalancerCreateOptions{ + Label: util.Pointer(fmt.Sprintf("%s-api-server", clusterScope.LinodeCluster.Name)), + Region: clusterScope.LinodeCluster.Spec.Region, + ClientConnThrottle: nil, + Tags: tags, + } + + if linodeNB, err = clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig); err != nil { + logger.Info("Failed to create Linode NodeBalancer", "error", err.Error()) + + // Already exists is not an error + apiErr := linodego.Error{} + if errors.As(err, &apiErr) && apiErr.Code != http.StatusFound { + return nil, err + } + + err = nil + + if linodeNB != nil { + logger.Info("Linode NodeBalancer already exists", "existing", linodeNB.Label) + } + } + + default: + err = errors.New("multiple NodeBalancers") + + logger.Error(err, "Panic! Multiple NodeBalancers found. This might be a concurrency issue in the controller!!!", "filters", string(rawFilter)) + + return nil, err + } + + if linodeNB == nil { + err = errors.New("missing NodeBalancer") + + logger.Error(err, "Panic! Failed to create NodeBalancer") + + return nil, err + } + + return linodeNB, nil +} + +// CreateNodeBalancerConfig creates NodeBalancer config if it does not exist +func CreateNodeBalancerConfig(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancerConfig, error) { + var linodeNBConfigs []linodego.NodeBalancerConfig + var linodeNBConfig *linodego.NodeBalancerConfig + var err error + + if linodeNBConfigs, err = clusterScope.LinodeClient.ListNodeBalancerConfigs(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, linodego.NewListOptions(1, "")); err != nil { + logger.Info("Failed to list NodeBalancer Configs", "error", err.Error()) + + return nil, err + } + lbPort := defaultLBPort + if clusterScope.LinodeCluster.Spec.Network.LoadBalancerPort != 0 { + lbPort = clusterScope.LinodeCluster.Spec.Network.LoadBalancerPort + } + switch len(linodeNBConfigs) { + case 1: + logger.Info("NodeBalancer ", strconv.Itoa(linodeNBConfigs[0].ID), " already exists") + linodeNBConfig = &linodeNBConfigs[0] + + case 0: + createConfig := linodego.NodeBalancerConfigCreateOptions{ + Port: lbPort, + Protocol: linodego.ProtocolTCP, + Algorithm: linodego.AlgorithmRoundRobin, + Check: linodego.CheckConnection, + } + + if linodeNBConfig, err = clusterScope.LinodeClient.CreateNodeBalancerConfig(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, createConfig); err != nil { + logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error()) + + return nil, err + } + + default: + err = errors.New("multiple NodeBalancer Configs") + + logger.Error(err, "Panic! Multiple NodeBalancer Configs found. This might be a concurrency issue in the controller!!!") + + return nil, err + } + + return linodeNBConfig, nil +} diff --git a/cmd/main.go b/cmd/main.go index a5357b8e1..720857027 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -39,7 +39,7 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" infrastructurev1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" - //+kubebuilder:scaffold:imports + // +kubebuilder:scaffold:imports ) var ( @@ -51,7 +51,7 @@ func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(capi.AddToScheme(scheme)) utilruntime.Must(infrastructurev1alpha1.AddToScheme(scheme)) - //+kubebuilder:scaffold:scheme + // +kubebuilder:scaffold:scheme } func main() { @@ -62,10 +62,12 @@ func main() { } var machineWatchFilter string + var clusterWatchFilter string var metricsAddr string var enableLeaderElection bool var probeAddr string flag.StringVar(&machineWatchFilter, "machine-watch-filter", "", "The machines to watch by label.") + flag.StringVar(&clusterWatchFilter, "cluster-watch-filter", "", "The clusters to watch by label.") flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, @@ -103,8 +105,11 @@ func main() { } if err = (&controller2.LinodeClusterReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor("LinodeClusterReconciler"), + WatchFilterValue: clusterWatchFilter, + LinodeApiKey: linodeToken, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "LinodeCluster") os.Exit(1) @@ -119,7 +124,7 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "LinodeMachine") os.Exit(1) } - //+kubebuilder:scaffold:builder + // +kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up health check") diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml index 706c0ceb3..ba92feda4 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml @@ -68,8 +68,21 @@ spec: description: NetworkSpec encapsulates all things related to Linode network. properties: - nodebalancerID: - description: NodebalancerID is the id of apiserver Nodebalancer. + loadBalancerPort: + description: LoadBalancerPort used by the api server. It must + be valid ports range (1-65535). If omitted, default value is + 6443. + maximum: 65535 + minimum: 1 + type: integer + loadBalancerType: + description: LoadBalancerType is the type of load balancer to + use, defaults to NodeBalancer if not otherwise set + enum: + - NodeBalancer + type: string + nodeBalancerID: + description: NodeBalancerID is the id of api server NodeBalancer. type: integer type: object region: diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 78afaaaa9..878da1a56 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -1,3 +1,8 @@ +# common labels for CRD resources as required by +# https://cluster-api.sigs.k8s.io/developer/providers/contracts.html#api-version-labels +commonLabels: + cluster.x-k8s.io/v1beta1: v1alpha1 + # This kustomization.yaml is not intended to be run by itself, # since it depends on service name and namespace that are out of this kustomize package. # It should be run by config/default diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 490ad8081..3825cb003 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -18,45 +18,220 @@ package controller import ( "context" + "errors" + "fmt" + "net/http" + "time" + + "github.com/go-logr/logr" + "github.com/linode/cluster-api-provider-linode/cloud/scope" + "github.com/linode/cluster-api-provider-linode/cloud/services" + "github.com/linode/cluster-api-provider-linode/util" + "github.com/linode/cluster-api-provider-linode/util/reconciler" + "github.com/linode/linodego" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + cerrs "sigs.k8s.io/cluster-api/errors" + kutil "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" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/source" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" - infrastructurev1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" + infrav1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" ) // LinodeClusterReconciler reconciles a LinodeCluster object type LinodeClusterReconciler struct { client.Client - Scheme *runtime.Scheme + Recorder record.EventRecorder + LinodeApiKey string + WatchFilterValue string + Scheme *runtime.Scheme + ReconcileTimeout time.Duration } -//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters/finalizers,verbs=update +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters/finalizers,verbs=update // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. -// TODO(user): Modify the Reconcile function to compare the state specified by -// the LinodeCluster object against the actual cluster state, and then -// perform operations to make the cluster state reflect the state specified by -// the user. -// -// For more details, check Reconcile and its Result here: -// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.16.0/pkg/reconcile + func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - _ = log.FromContext(ctx) + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultedLoopTimeout(r.ReconcileTimeout)) + defer cancel() + + logger := ctrl.LoggerFrom(ctx).WithName("LinodeClusterReconciler").WithValues("name", req.NamespacedName.String()) + LinodeCluster := &infrav1.LinodeCluster{} + if err := r.Client.Get(ctx, req.NamespacedName, LinodeCluster); err != nil { + logger.Info("Failed to fetch Linode cluster", "error", err.Error()) + + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + cluster, err := kutil.GetOwnerCluster(ctx, r.Client, LinodeCluster.ObjectMeta) + if err != nil { + logger.Info("Failed to get owner cluster", "error", err.Error()) + + return ctrl.Result{}, client.IgnoreNotFound(err) + } else if cluster == nil { + logger.Info("Machine Controller has not yet set OwnerRef, skipping reconciliation") - // TODO(user): your logic here + return ctrl.Result{}, nil + } + if annotations.IsPaused(cluster, LinodeCluster) { + logger.Info("LinodeCluster of linked Cluster is marked as paused. Won't reconcile") - return ctrl.Result{}, nil + return ctrl.Result{}, nil + } + // Create the cluster scope. + clusterScope, err := scope.NewClusterScope( + r.LinodeApiKey, + scope.ClusterScopeParams{ + Client: r.Client, + Cluster: cluster, + LinodeCluster: LinodeCluster, + }) + if err != nil { + logger.Info("Failed to create cluster scope", "error", err.Error()) + + return ctrl.Result{}, fmt.Errorf("failed to create cluster scope: %w", err) + } + + return r.reconcile(ctx, clusterScope, logger) +} + +func (r *LinodeClusterReconciler) reconcile( + ctx context.Context, + clusterScope *scope.ClusterScope, + logger logr.Logger, +) (res ctrl.Result, err error) { + res = ctrl.Result{} + + clusterScope.LinodeCluster.Status.Ready = false + clusterScope.LinodeCluster.Status.FailureReason = nil + clusterScope.LinodeCluster.Status.FailureMessage = util.Pointer("") + + failureReason := cerrs.ClusterStatusError("UnknownError") + + defer func() { + if err != nil { + setFailureReason(clusterScope, failureReason, err, r) + } + + if patchErr := clusterScope.PatchHelper.Patch(ctx, clusterScope.LinodeCluster); patchErr != nil && client.IgnoreNotFound(patchErr) != nil { + logger.Error(patchErr, "failed to patch LinodeCluster") + + err = errors.Join(err, patchErr) + } + }() + + // Delete + if !clusterScope.LinodeCluster.ObjectMeta.DeletionTimestamp.IsZero() { + failureReason = cerrs.DeleteClusterError + + err = r.reconcileDelete(ctx, logger, clusterScope) + + return + } + + controllerutil.AddFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) + // Create + if clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" { + failureReason = cerrs.CreateClusterError + + _, err = r.reconcileCreate(ctx, clusterScope, logger) + } + if err == nil { + clusterScope.LinodeCluster.Status.Ready = true + conditions.MarkTrue(clusterScope.LinodeCluster, clusterv1.ReadyCondition) + + r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Load balancer is ready") + } + + return +} + +func setFailureReason(clusterScope *scope.ClusterScope, failureReason cerrs.ClusterStatusError, err error, lcr *LinodeClusterReconciler) { + clusterScope.LinodeCluster.Status.FailureReason = util.Pointer(failureReason) + clusterScope.LinodeCluster.Status.FailureMessage = util.Pointer(err.Error()) + + conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, string(failureReason), clusterv1.ConditionSeverityError, "%s", err.Error()) + + lcr.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, string(failureReason), err.Error()) +} + +func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancer, error) { + linodeNB, err := services.CreateNodeBalancer(ctx, clusterScope, logger) + if err != nil { + return nil, err + } + + clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = linodeNB.ID + + linodeNBConfig, err := services.CreateNodeBalancerConfig(ctx, clusterScope, logger) + if err != nil { + return nil, err + } + + clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{ + Host: *linodeNB.IPv4, + Port: int32(linodeNBConfig.Port), + } + + return linodeNB, nil +} + +func (*LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { + logger.Info("deleting cluster") + + if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID != 0 { + if err := clusterScope.LinodeClient.DeleteNodeBalancer(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID); err != nil { + logger.Info("Failed to delete Linode NodeBalancer", "error", err.Error()) + + // Not found is not an error + apiErr := linodego.Error{} + if errors.As(err, &apiErr) && apiErr.Code != http.StatusNotFound { + conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted") + clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = 0 + controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) + + return err + } + } + } else { + logger.Info("NodeBalancer ID is missing, nothing to do") + } + + conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted") + + clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = 0 + controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) + + return nil } // SetupWithManager sets up the controller with the Manager. func (r *LinodeClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). - For(&infrastructurev1alpha1.LinodeCluster{}). - Complete(r) + controller, err := ctrl.NewControllerManagedBy(mgr). + For(&infrav1.LinodeCluster{}). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetLogger(), r.WatchFilterValue)). + Build(r) + if err != nil { + return fmt.Errorf("failed to build controller: %w", err) + } + + return controller.Watch( + source.Kind(mgr.GetCache(), &clusterv1.Cluster{}), + handler.EnqueueRequestsFromMapFunc(kutil.ClusterToInfrastructureMapFunc(context.TODO(), infrav1.GroupVersion.WithKind("LinodeCluster"), mgr.GetClient(), &infrav1.LinodeCluster{})), + predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger()), + ) } From 7851a7ab5257d09b5cc6297d544224d3c9ec253d Mon Sep 17 00:00:00 2001 From: Evan Johnson Date: Fri, 26 Jan 2024 09:51:31 -0500 Subject: [PATCH 02/11] simplify case logic in loadbalancers.go --- cloud/services/loadbalancers.go | 92 ++++++++++++--------------------- 1 file changed, 33 insertions(+), 59 deletions(-) diff --git a/cloud/services/loadbalancers.go b/cloud/services/loadbalancers.go index 88a0fcdf2..ed4c3ed2d 100644 --- a/cloud/services/loadbalancers.go +++ b/cloud/services/loadbalancers.go @@ -34,15 +34,12 @@ func CreateNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, l if err != nil { return nil, err } - logger.Info("Creating NodeBalancer") if linodeNBs, err = clusterScope.LinodeClient.ListNodeBalancers(ctx, linodego.NewListOptions(1, string(rawFilter))); err != nil { logger.Info("Failed to list NodeBalancers", "error", err.Error()) return nil, err } - - switch len(linodeNBs) { - case 1: + if len(linodeNBs) == 1 { logger.Info(fmt.Sprintf("NodeBalancer %s already exists", *linodeNBs[0].Label)) if !slices.Contains(linodeNBs[0].Tags, clusterUID) { err = errors.New("NodeBalancer conflict") @@ -50,46 +47,32 @@ func CreateNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, l return nil, err } - linodeNB = &linodeNBs[0] - case 0: - logger.Info(fmt.Sprintf("Creating NodeBalancer %s-api-server", clusterScope.LinodeCluster.Name)) - createConfig := linodego.NodeBalancerCreateOptions{ - Label: util.Pointer(fmt.Sprintf("%s-api-server", clusterScope.LinodeCluster.Name)), - Region: clusterScope.LinodeCluster.Spec.Region, - ClientConnThrottle: nil, - Tags: tags, - } - if linodeNB, err = clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig); err != nil { - logger.Info("Failed to create Linode NodeBalancer", "error", err.Error()) + return &linodeNBs[0], nil + } - // Already exists is not an error - apiErr := linodego.Error{} - if errors.As(err, &apiErr) && apiErr.Code != http.StatusFound { - return nil, err - } + logger.Info(fmt.Sprintf("Creating NodeBalancer %s-api-server", clusterScope.LinodeCluster.Name)) + createConfig := linodego.NodeBalancerCreateOptions{ + Label: util.Pointer(fmt.Sprintf("%s-api-server", clusterScope.LinodeCluster.Name)), + Region: clusterScope.LinodeCluster.Spec.Region, + ClientConnThrottle: nil, + Tags: tags, + } - err = nil + if linodeNB, err = clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig); err != nil { + logger.Info("Failed to create Linode NodeBalancer", "error", err.Error()) - if linodeNB != nil { - logger.Info("Linode NodeBalancer already exists", "existing", linodeNB.Label) - } + // Already exists is not an error + apiErr := linodego.Error{} + if errors.As(err, &apiErr) && apiErr.Code != http.StatusFound { + return nil, err } - default: - err = errors.New("multiple NodeBalancers") - - logger.Error(err, "Panic! Multiple NodeBalancers found. This might be a concurrency issue in the controller!!!", "filters", string(rawFilter)) + err = nil - return nil, err - } - - if linodeNB == nil { - err = errors.New("missing NodeBalancer") - - logger.Error(err, "Panic! Failed to create NodeBalancer") - - return nil, err + if linodeNB != nil { + logger.Info("Linode NodeBalancer already exists", "existing", linodeNB.Label) + } } return linodeNB, nil @@ -106,33 +89,24 @@ func CreateNodeBalancerConfig(ctx context.Context, clusterScope *scope.ClusterSc return nil, err } + if len(linodeNBConfigs) == 1 { + logger.Info("NodeBalancer ", strconv.Itoa(linodeNBConfigs[0].ID), " already exists") + + return &linodeNBConfigs[0], err + } lbPort := defaultLBPort if clusterScope.LinodeCluster.Spec.Network.LoadBalancerPort != 0 { lbPort = clusterScope.LinodeCluster.Spec.Network.LoadBalancerPort } - switch len(linodeNBConfigs) { - case 1: - logger.Info("NodeBalancer ", strconv.Itoa(linodeNBConfigs[0].ID), " already exists") - linodeNBConfig = &linodeNBConfigs[0] - - case 0: - createConfig := linodego.NodeBalancerConfigCreateOptions{ - Port: lbPort, - Protocol: linodego.ProtocolTCP, - Algorithm: linodego.AlgorithmRoundRobin, - Check: linodego.CheckConnection, - } - - if linodeNBConfig, err = clusterScope.LinodeClient.CreateNodeBalancerConfig(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, createConfig); err != nil { - logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error()) - - return nil, err - } - - default: - err = errors.New("multiple NodeBalancer Configs") + createConfig := linodego.NodeBalancerConfigCreateOptions{ + Port: lbPort, + Protocol: linodego.ProtocolTCP, + Algorithm: linodego.AlgorithmRoundRobin, + Check: linodego.CheckConnection, + } - logger.Error(err, "Panic! Multiple NodeBalancer Configs found. This might be a concurrency issue in the controller!!!") + if linodeNBConfig, err = clusterScope.LinodeClient.CreateNodeBalancerConfig(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, createConfig); err != nil { + logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error()) return nil, err } From 31459c5f8c66354f3ad054f893f44cd79a940330 Mon Sep 17 00:00:00 2001 From: Evan Johnson Date: Wed, 24 Jan 2024 17:50:09 -0500 Subject: [PATCH 03/11] add initial LinodeCluster controller logic --- api/v1alpha1/linodecluster_types.go | 17 +- api/v1alpha1/zz_generated.deepcopy.go | 2 +- cloud/services/loadbalancers.go | 141 ++++++++++++ cmd/main.go | 15 +- ...cture.cluster.x-k8s.io_linodeclusters.yaml | 17 +- config/crd/kustomization.yaml | 5 + controller/linodecluster_controller.go | 213 ++++++++++++++++-- 7 files changed, 379 insertions(+), 31 deletions(-) create mode 100644 cloud/services/loadbalancers.go diff --git a/api/v1alpha1/linodecluster_types.go b/api/v1alpha1/linodecluster_types.go index 7b9595e4b..aa70c9dc8 100644 --- a/api/v1alpha1/linodecluster_types.go +++ b/api/v1alpha1/linodecluster_types.go @@ -47,13 +47,13 @@ type LinodeClusterStatus struct { // reconciling the Machine and will contain a succinct value suitable // for machine interpretation. // +optional - FailureReason *errors.MachineStatusError `json:"failureReason"` + FailureReason *errors.ClusterStatusError `json:"failureReason,omitempty"` // FailureMessage will be set in the event that there is a terminal problem // reconciling the Machine and will contain a more verbose string suitable // for logging and human consumption. // +optional - FailureMessage *string `json:"failureMessage"` + FailureMessage *string `json:"failureMessage,omitempty"` // Conditions defines current service state of the LinodeCluster. // +optional @@ -85,9 +85,18 @@ func (lm *LinodeCluster) SetConditions(conditions clusterv1.Conditions) { // NetworkSpec encapsulates Linode networking resources. type NetworkSpec struct { - // NodebalancerID is the id of apiserver Nodebalancer. + // LoadBalancerType is the type of load balancer to use, defaults to NodeBalancer if not otherwise set + // +kubebuilder:validation:Enum=NodeBalancer // +optional - NodebalancerID int `json:"nodebalancerID,omitempty"` + LoadBalancerType string `json:"loadBalancerType,omitempty"` + // LoadBalancerPort used by the api server. It must be valid ports range (1-65535). If omitted, default value is 6443. + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=65535 + // +optional + LoadBalancerPort int `json:"loadBalancerPort,omitempty"` + // NodeBalancerID is the id of api server NodeBalancer. + // +optional + NodeBalancerID int `json:"nodeBalancerID,omitempty"` } // +kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 117c52bfb..8d69fc5d7 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -153,7 +153,7 @@ func (in *LinodeClusterStatus) DeepCopyInto(out *LinodeClusterStatus) { *out = *in if in.FailureReason != nil { in, out := &in.FailureReason, &out.FailureReason - *out = new(errors.MachineStatusError) + *out = new(errors.ClusterStatusError) **out = **in } if in.FailureMessage != nil { diff --git a/cloud/services/loadbalancers.go b/cloud/services/loadbalancers.go new file mode 100644 index 000000000..88a0fcdf2 --- /dev/null +++ b/cloud/services/loadbalancers.go @@ -0,0 +1,141 @@ +package services + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "slices" + "strconv" + + "github.com/go-logr/logr" + "github.com/linode/cluster-api-provider-linode/cloud/scope" + "github.com/linode/cluster-api-provider-linode/util" + "github.com/linode/linodego" +) + +var ( + defaultLBPort = 6443 +) + +// CreateNodeBalancer creates a new NodeBalancer if one doesn't exist +func CreateNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancer, error) { + var linodeNBs []linodego.NodeBalancer + var linodeNB *linodego.NodeBalancer + NBLabel := fmt.Sprintf("%s-api-server", clusterScope.LinodeCluster.Name) + clusterUID := string(clusterScope.LinodeCluster.UID) + tags := []string{string(clusterScope.LinodeCluster.UID)} + filter := map[string]string{ + "label": NBLabel, + } + + rawFilter, err := json.Marshal(filter) + if err != nil { + return nil, err + } + logger.Info("Creating NodeBalancer") + if linodeNBs, err = clusterScope.LinodeClient.ListNodeBalancers(ctx, linodego.NewListOptions(1, string(rawFilter))); err != nil { + logger.Info("Failed to list NodeBalancers", "error", err.Error()) + + return nil, err + } + + switch len(linodeNBs) { + case 1: + logger.Info(fmt.Sprintf("NodeBalancer %s already exists", *linodeNBs[0].Label)) + if !slices.Contains(linodeNBs[0].Tags, clusterUID) { + err = errors.New("NodeBalancer conflict") + logger.Error(err, fmt.Sprintf("NodeBalancer %s is not associated with cluster UID %s. Owner cluster is %s", *linodeNBs[0].Label, clusterUID, linodeNBs[0].Tags[0])) + + return nil, err + } + linodeNB = &linodeNBs[0] + case 0: + logger.Info(fmt.Sprintf("Creating NodeBalancer %s-api-server", clusterScope.LinodeCluster.Name)) + createConfig := linodego.NodeBalancerCreateOptions{ + Label: util.Pointer(fmt.Sprintf("%s-api-server", clusterScope.LinodeCluster.Name)), + Region: clusterScope.LinodeCluster.Spec.Region, + ClientConnThrottle: nil, + Tags: tags, + } + + if linodeNB, err = clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig); err != nil { + logger.Info("Failed to create Linode NodeBalancer", "error", err.Error()) + + // Already exists is not an error + apiErr := linodego.Error{} + if errors.As(err, &apiErr) && apiErr.Code != http.StatusFound { + return nil, err + } + + err = nil + + if linodeNB != nil { + logger.Info("Linode NodeBalancer already exists", "existing", linodeNB.Label) + } + } + + default: + err = errors.New("multiple NodeBalancers") + + logger.Error(err, "Panic! Multiple NodeBalancers found. This might be a concurrency issue in the controller!!!", "filters", string(rawFilter)) + + return nil, err + } + + if linodeNB == nil { + err = errors.New("missing NodeBalancer") + + logger.Error(err, "Panic! Failed to create NodeBalancer") + + return nil, err + } + + return linodeNB, nil +} + +// CreateNodeBalancerConfig creates NodeBalancer config if it does not exist +func CreateNodeBalancerConfig(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancerConfig, error) { + var linodeNBConfigs []linodego.NodeBalancerConfig + var linodeNBConfig *linodego.NodeBalancerConfig + var err error + + if linodeNBConfigs, err = clusterScope.LinodeClient.ListNodeBalancerConfigs(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, linodego.NewListOptions(1, "")); err != nil { + logger.Info("Failed to list NodeBalancer Configs", "error", err.Error()) + + return nil, err + } + lbPort := defaultLBPort + if clusterScope.LinodeCluster.Spec.Network.LoadBalancerPort != 0 { + lbPort = clusterScope.LinodeCluster.Spec.Network.LoadBalancerPort + } + switch len(linodeNBConfigs) { + case 1: + logger.Info("NodeBalancer ", strconv.Itoa(linodeNBConfigs[0].ID), " already exists") + linodeNBConfig = &linodeNBConfigs[0] + + case 0: + createConfig := linodego.NodeBalancerConfigCreateOptions{ + Port: lbPort, + Protocol: linodego.ProtocolTCP, + Algorithm: linodego.AlgorithmRoundRobin, + Check: linodego.CheckConnection, + } + + if linodeNBConfig, err = clusterScope.LinodeClient.CreateNodeBalancerConfig(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, createConfig); err != nil { + logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error()) + + return nil, err + } + + default: + err = errors.New("multiple NodeBalancer Configs") + + logger.Error(err, "Panic! Multiple NodeBalancer Configs found. This might be a concurrency issue in the controller!!!") + + return nil, err + } + + return linodeNBConfig, nil +} diff --git a/cmd/main.go b/cmd/main.go index a5357b8e1..720857027 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -39,7 +39,7 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" infrastructurev1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" - //+kubebuilder:scaffold:imports + // +kubebuilder:scaffold:imports ) var ( @@ -51,7 +51,7 @@ func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(capi.AddToScheme(scheme)) utilruntime.Must(infrastructurev1alpha1.AddToScheme(scheme)) - //+kubebuilder:scaffold:scheme + // +kubebuilder:scaffold:scheme } func main() { @@ -62,10 +62,12 @@ func main() { } var machineWatchFilter string + var clusterWatchFilter string var metricsAddr string var enableLeaderElection bool var probeAddr string flag.StringVar(&machineWatchFilter, "machine-watch-filter", "", "The machines to watch by label.") + flag.StringVar(&clusterWatchFilter, "cluster-watch-filter", "", "The clusters to watch by label.") flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, @@ -103,8 +105,11 @@ func main() { } if err = (&controller2.LinodeClusterReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor("LinodeClusterReconciler"), + WatchFilterValue: clusterWatchFilter, + LinodeApiKey: linodeToken, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "LinodeCluster") os.Exit(1) @@ -119,7 +124,7 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "LinodeMachine") os.Exit(1) } - //+kubebuilder:scaffold:builder + // +kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up health check") diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml index 706c0ceb3..ba92feda4 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml @@ -68,8 +68,21 @@ spec: description: NetworkSpec encapsulates all things related to Linode network. properties: - nodebalancerID: - description: NodebalancerID is the id of apiserver Nodebalancer. + loadBalancerPort: + description: LoadBalancerPort used by the api server. It must + be valid ports range (1-65535). If omitted, default value is + 6443. + maximum: 65535 + minimum: 1 + type: integer + loadBalancerType: + description: LoadBalancerType is the type of load balancer to + use, defaults to NodeBalancer if not otherwise set + enum: + - NodeBalancer + type: string + nodeBalancerID: + description: NodeBalancerID is the id of api server NodeBalancer. type: integer type: object region: diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 9fa6c1346..f088cd229 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -1,3 +1,8 @@ +# common labels for CRD resources as required by +# https://cluster-api.sigs.k8s.io/developer/providers/contracts.html#api-version-labels +commonLabels: + cluster.x-k8s.io/v1beta1: v1alpha1 + # This kustomization.yaml is not intended to be run by itself, # since it depends on service name and namespace that are out of this kustomize package. # It should be run by config/default diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 490ad8081..3825cb003 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -18,45 +18,220 @@ package controller import ( "context" + "errors" + "fmt" + "net/http" + "time" + + "github.com/go-logr/logr" + "github.com/linode/cluster-api-provider-linode/cloud/scope" + "github.com/linode/cluster-api-provider-linode/cloud/services" + "github.com/linode/cluster-api-provider-linode/util" + "github.com/linode/cluster-api-provider-linode/util/reconciler" + "github.com/linode/linodego" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + cerrs "sigs.k8s.io/cluster-api/errors" + kutil "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" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/source" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" - infrastructurev1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" + infrav1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" ) // LinodeClusterReconciler reconciles a LinodeCluster object type LinodeClusterReconciler struct { client.Client - Scheme *runtime.Scheme + Recorder record.EventRecorder + LinodeApiKey string + WatchFilterValue string + Scheme *runtime.Scheme + ReconcileTimeout time.Duration } -//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters/finalizers,verbs=update +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=linodeclusters/finalizers,verbs=update // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. -// TODO(user): Modify the Reconcile function to compare the state specified by -// the LinodeCluster object against the actual cluster state, and then -// perform operations to make the cluster state reflect the state specified by -// the user. -// -// For more details, check Reconcile and its Result here: -// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.16.0/pkg/reconcile + func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - _ = log.FromContext(ctx) + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultedLoopTimeout(r.ReconcileTimeout)) + defer cancel() + + logger := ctrl.LoggerFrom(ctx).WithName("LinodeClusterReconciler").WithValues("name", req.NamespacedName.String()) + LinodeCluster := &infrav1.LinodeCluster{} + if err := r.Client.Get(ctx, req.NamespacedName, LinodeCluster); err != nil { + logger.Info("Failed to fetch Linode cluster", "error", err.Error()) + + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + cluster, err := kutil.GetOwnerCluster(ctx, r.Client, LinodeCluster.ObjectMeta) + if err != nil { + logger.Info("Failed to get owner cluster", "error", err.Error()) + + return ctrl.Result{}, client.IgnoreNotFound(err) + } else if cluster == nil { + logger.Info("Machine Controller has not yet set OwnerRef, skipping reconciliation") - // TODO(user): your logic here + return ctrl.Result{}, nil + } + if annotations.IsPaused(cluster, LinodeCluster) { + logger.Info("LinodeCluster of linked Cluster is marked as paused. Won't reconcile") - return ctrl.Result{}, nil + return ctrl.Result{}, nil + } + // Create the cluster scope. + clusterScope, err := scope.NewClusterScope( + r.LinodeApiKey, + scope.ClusterScopeParams{ + Client: r.Client, + Cluster: cluster, + LinodeCluster: LinodeCluster, + }) + if err != nil { + logger.Info("Failed to create cluster scope", "error", err.Error()) + + return ctrl.Result{}, fmt.Errorf("failed to create cluster scope: %w", err) + } + + return r.reconcile(ctx, clusterScope, logger) +} + +func (r *LinodeClusterReconciler) reconcile( + ctx context.Context, + clusterScope *scope.ClusterScope, + logger logr.Logger, +) (res ctrl.Result, err error) { + res = ctrl.Result{} + + clusterScope.LinodeCluster.Status.Ready = false + clusterScope.LinodeCluster.Status.FailureReason = nil + clusterScope.LinodeCluster.Status.FailureMessage = util.Pointer("") + + failureReason := cerrs.ClusterStatusError("UnknownError") + + defer func() { + if err != nil { + setFailureReason(clusterScope, failureReason, err, r) + } + + if patchErr := clusterScope.PatchHelper.Patch(ctx, clusterScope.LinodeCluster); patchErr != nil && client.IgnoreNotFound(patchErr) != nil { + logger.Error(patchErr, "failed to patch LinodeCluster") + + err = errors.Join(err, patchErr) + } + }() + + // Delete + if !clusterScope.LinodeCluster.ObjectMeta.DeletionTimestamp.IsZero() { + failureReason = cerrs.DeleteClusterError + + err = r.reconcileDelete(ctx, logger, clusterScope) + + return + } + + controllerutil.AddFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) + // Create + if clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" { + failureReason = cerrs.CreateClusterError + + _, err = r.reconcileCreate(ctx, clusterScope, logger) + } + if err == nil { + clusterScope.LinodeCluster.Status.Ready = true + conditions.MarkTrue(clusterScope.LinodeCluster, clusterv1.ReadyCondition) + + r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Load balancer is ready") + } + + return +} + +func setFailureReason(clusterScope *scope.ClusterScope, failureReason cerrs.ClusterStatusError, err error, lcr *LinodeClusterReconciler) { + clusterScope.LinodeCluster.Status.FailureReason = util.Pointer(failureReason) + clusterScope.LinodeCluster.Status.FailureMessage = util.Pointer(err.Error()) + + conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, string(failureReason), clusterv1.ConditionSeverityError, "%s", err.Error()) + + lcr.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, string(failureReason), err.Error()) +} + +func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancer, error) { + linodeNB, err := services.CreateNodeBalancer(ctx, clusterScope, logger) + if err != nil { + return nil, err + } + + clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = linodeNB.ID + + linodeNBConfig, err := services.CreateNodeBalancerConfig(ctx, clusterScope, logger) + if err != nil { + return nil, err + } + + clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{ + Host: *linodeNB.IPv4, + Port: int32(linodeNBConfig.Port), + } + + return linodeNB, nil +} + +func (*LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { + logger.Info("deleting cluster") + + if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID != 0 { + if err := clusterScope.LinodeClient.DeleteNodeBalancer(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID); err != nil { + logger.Info("Failed to delete Linode NodeBalancer", "error", err.Error()) + + // Not found is not an error + apiErr := linodego.Error{} + if errors.As(err, &apiErr) && apiErr.Code != http.StatusNotFound { + conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted") + clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = 0 + controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) + + return err + } + } + } else { + logger.Info("NodeBalancer ID is missing, nothing to do") + } + + conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted") + + clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = 0 + controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) + + return nil } // SetupWithManager sets up the controller with the Manager. func (r *LinodeClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). - For(&infrastructurev1alpha1.LinodeCluster{}). - Complete(r) + controller, err := ctrl.NewControllerManagedBy(mgr). + For(&infrav1.LinodeCluster{}). + WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetLogger(), r.WatchFilterValue)). + Build(r) + if err != nil { + return fmt.Errorf("failed to build controller: %w", err) + } + + return controller.Watch( + source.Kind(mgr.GetCache(), &clusterv1.Cluster{}), + handler.EnqueueRequestsFromMapFunc(kutil.ClusterToInfrastructureMapFunc(context.TODO(), infrav1.GroupVersion.WithKind("LinodeCluster"), mgr.GetClient(), &infrav1.LinodeCluster{})), + predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger()), + ) } From fb894cec6915f5f3522dee3dbc442b6f0ee6ede3 Mon Sep 17 00:00:00 2001 From: Evan Johnson Date: Fri, 26 Jan 2024 09:51:31 -0500 Subject: [PATCH 04/11] simplify case logic in loadbalancers.go --- cloud/services/loadbalancers.go | 92 ++++++++++++--------------------- 1 file changed, 33 insertions(+), 59 deletions(-) diff --git a/cloud/services/loadbalancers.go b/cloud/services/loadbalancers.go index 88a0fcdf2..ed4c3ed2d 100644 --- a/cloud/services/loadbalancers.go +++ b/cloud/services/loadbalancers.go @@ -34,15 +34,12 @@ func CreateNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, l if err != nil { return nil, err } - logger.Info("Creating NodeBalancer") if linodeNBs, err = clusterScope.LinodeClient.ListNodeBalancers(ctx, linodego.NewListOptions(1, string(rawFilter))); err != nil { logger.Info("Failed to list NodeBalancers", "error", err.Error()) return nil, err } - - switch len(linodeNBs) { - case 1: + if len(linodeNBs) == 1 { logger.Info(fmt.Sprintf("NodeBalancer %s already exists", *linodeNBs[0].Label)) if !slices.Contains(linodeNBs[0].Tags, clusterUID) { err = errors.New("NodeBalancer conflict") @@ -50,46 +47,32 @@ func CreateNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, l return nil, err } - linodeNB = &linodeNBs[0] - case 0: - logger.Info(fmt.Sprintf("Creating NodeBalancer %s-api-server", clusterScope.LinodeCluster.Name)) - createConfig := linodego.NodeBalancerCreateOptions{ - Label: util.Pointer(fmt.Sprintf("%s-api-server", clusterScope.LinodeCluster.Name)), - Region: clusterScope.LinodeCluster.Spec.Region, - ClientConnThrottle: nil, - Tags: tags, - } - if linodeNB, err = clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig); err != nil { - logger.Info("Failed to create Linode NodeBalancer", "error", err.Error()) + return &linodeNBs[0], nil + } - // Already exists is not an error - apiErr := linodego.Error{} - if errors.As(err, &apiErr) && apiErr.Code != http.StatusFound { - return nil, err - } + logger.Info(fmt.Sprintf("Creating NodeBalancer %s-api-server", clusterScope.LinodeCluster.Name)) + createConfig := linodego.NodeBalancerCreateOptions{ + Label: util.Pointer(fmt.Sprintf("%s-api-server", clusterScope.LinodeCluster.Name)), + Region: clusterScope.LinodeCluster.Spec.Region, + ClientConnThrottle: nil, + Tags: tags, + } - err = nil + if linodeNB, err = clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig); err != nil { + logger.Info("Failed to create Linode NodeBalancer", "error", err.Error()) - if linodeNB != nil { - logger.Info("Linode NodeBalancer already exists", "existing", linodeNB.Label) - } + // Already exists is not an error + apiErr := linodego.Error{} + if errors.As(err, &apiErr) && apiErr.Code != http.StatusFound { + return nil, err } - default: - err = errors.New("multiple NodeBalancers") - - logger.Error(err, "Panic! Multiple NodeBalancers found. This might be a concurrency issue in the controller!!!", "filters", string(rawFilter)) + err = nil - return nil, err - } - - if linodeNB == nil { - err = errors.New("missing NodeBalancer") - - logger.Error(err, "Panic! Failed to create NodeBalancer") - - return nil, err + if linodeNB != nil { + logger.Info("Linode NodeBalancer already exists", "existing", linodeNB.Label) + } } return linodeNB, nil @@ -106,33 +89,24 @@ func CreateNodeBalancerConfig(ctx context.Context, clusterScope *scope.ClusterSc return nil, err } + if len(linodeNBConfigs) == 1 { + logger.Info("NodeBalancer ", strconv.Itoa(linodeNBConfigs[0].ID), " already exists") + + return &linodeNBConfigs[0], err + } lbPort := defaultLBPort if clusterScope.LinodeCluster.Spec.Network.LoadBalancerPort != 0 { lbPort = clusterScope.LinodeCluster.Spec.Network.LoadBalancerPort } - switch len(linodeNBConfigs) { - case 1: - logger.Info("NodeBalancer ", strconv.Itoa(linodeNBConfigs[0].ID), " already exists") - linodeNBConfig = &linodeNBConfigs[0] - - case 0: - createConfig := linodego.NodeBalancerConfigCreateOptions{ - Port: lbPort, - Protocol: linodego.ProtocolTCP, - Algorithm: linodego.AlgorithmRoundRobin, - Check: linodego.CheckConnection, - } - - if linodeNBConfig, err = clusterScope.LinodeClient.CreateNodeBalancerConfig(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, createConfig); err != nil { - logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error()) - - return nil, err - } - - default: - err = errors.New("multiple NodeBalancer Configs") + createConfig := linodego.NodeBalancerConfigCreateOptions{ + Port: lbPort, + Protocol: linodego.ProtocolTCP, + Algorithm: linodego.AlgorithmRoundRobin, + Check: linodego.CheckConnection, + } - logger.Error(err, "Panic! Multiple NodeBalancer Configs found. This might be a concurrency issue in the controller!!!") + if linodeNBConfig, err = clusterScope.LinodeClient.CreateNodeBalancerConfig(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, createConfig); err != nil { + logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error()) return nil, err } From 4d4ecfbba1da8e6ecbbf490432bb2e0eab0a3634 Mon Sep 17 00:00:00 2001 From: Evan Johnson Date: Fri, 26 Jan 2024 18:27:12 -0500 Subject: [PATCH 05/11] clean up reconcile some more --- cloud/scope/cluster.go | 11 ++++ controller/linodecluster_controller.go | 81 ++++++++++++-------------- 2 files changed, 48 insertions(+), 44 deletions(-) diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 99fbaa08b..e7d9c5511 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -17,6 +17,7 @@ limitations under the License. package scope import ( + "context" "errors" "fmt" "net/http" @@ -92,3 +93,13 @@ type ClusterScope struct { Cluster *clusterv1.Cluster LinodeCluster *infrav1.LinodeCluster } + +// PatchObject persists the cluster configuration and status. +func (s *ClusterScope) PatchObject() error { + return s.PatchHelper.Patch(context.TODO(), s.LinodeCluster) +} + +// Close closes the current scope persisting the cluster configuration and status. +func (s *ClusterScope) Close() error { + return s.PatchObject() +} diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 3825cb003..9c40a73b5 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -113,49 +113,38 @@ func (r *LinodeClusterReconciler) reconcile( ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger, -) (res ctrl.Result, err error) { +) (res ctrl.Result, reterr error) { res = ctrl.Result{} clusterScope.LinodeCluster.Status.Ready = false clusterScope.LinodeCluster.Status.FailureReason = nil clusterScope.LinodeCluster.Status.FailureMessage = util.Pointer("") - failureReason := cerrs.ClusterStatusError("UnknownError") - + // Always close the scope when exiting this function so we can persist any GCPMachine changes. defer func() { - if err != nil { - setFailureReason(clusterScope, failureReason, err, r) - } - - if patchErr := clusterScope.PatchHelper.Patch(ctx, clusterScope.LinodeCluster); patchErr != nil && client.IgnoreNotFound(patchErr) != nil { - logger.Error(patchErr, "failed to patch LinodeCluster") - - err = errors.Join(err, patchErr) + if err := clusterScope.Close(); err != nil && reterr == nil { + logger.Error(err, "failed to patch LinodeCluster") + reterr = err } }() - // Delete - if !clusterScope.LinodeCluster.ObjectMeta.DeletionTimestamp.IsZero() { - failureReason = cerrs.DeleteClusterError - - err = r.reconcileDelete(ctx, logger, clusterScope) - - return + // Handle deleted clusters + if !clusterScope.LinodeCluster.DeletionTimestamp.IsZero() { + return ctrl.Result{}, r.reconcileDelete(ctx, logger, clusterScope) } controllerutil.AddFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) // Create if clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" { - failureReason = cerrs.CreateClusterError - - _, err = r.reconcileCreate(ctx, clusterScope, logger) + if err := r.reconcileCreate(ctx, clusterScope, logger); err != nil { + return res, err + } } - if err == nil { - clusterScope.LinodeCluster.Status.Ready = true - conditions.MarkTrue(clusterScope.LinodeCluster, clusterv1.ReadyCondition) - r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Load balancer is ready") - } + clusterScope.LinodeCluster.Status.Ready = true + conditions.MarkTrue(clusterScope.LinodeCluster, clusterv1.ReadyCondition) + + r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Load balancer is ready") return } @@ -169,17 +158,21 @@ func setFailureReason(clusterScope *scope.ClusterScope, failureReason cerrs.Clus lcr.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, string(failureReason), err.Error()) } -func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancer, error) { +func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) error { linodeNB, err := services.CreateNodeBalancer(ctx, clusterScope, logger) if err != nil { - return nil, err + setFailureReason(clusterScope, cerrs.CreateClusterError, err, r) + + return err } clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = linodeNB.ID linodeNBConfig, err := services.CreateNodeBalancerConfig(ctx, clusterScope, logger) if err != nil { - return nil, err + setFailureReason(clusterScope, cerrs.CreateClusterError, err, r) + + return err } clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{ @@ -187,28 +180,28 @@ func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, clusterSc Port: int32(linodeNBConfig.Port), } - return linodeNB, nil + return nil } -func (*LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { +func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { logger.Info("deleting cluster") + if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == 0 { + logger.Info("NodeBalancer ID is missing, nothing to do") + controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) + + return nil + } - if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID != 0 { - if err := clusterScope.LinodeClient.DeleteNodeBalancer(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID); err != nil { - logger.Info("Failed to delete Linode NodeBalancer", "error", err.Error()) + if err := clusterScope.LinodeClient.DeleteNodeBalancer(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID); err != nil { + logger.Info("Failed to delete Linode NodeBalancer", "error", err.Error()) - // Not found is not an error - apiErr := linodego.Error{} - if errors.As(err, &apiErr) && apiErr.Code != http.StatusNotFound { - conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted") - clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = 0 - controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) + // Not found is not an error + apiErr := linodego.Error{} + if errors.As(err, &apiErr) && apiErr.Code != http.StatusNotFound { + setFailureReason(clusterScope, cerrs.DeleteClusterError, err, r) - return err - } + return err } - } else { - logger.Info("NodeBalancer ID is missing, nothing to do") } conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted") From 4d01c303fbbb10fcca473fdffefb679b55ae57ed Mon Sep 17 00:00:00 2001 From: Evan Johnson Date: Fri, 26 Jan 2024 18:27:12 -0500 Subject: [PATCH 06/11] clean up reconcile some more --- cloud/scope/cluster.go | 11 ++++ controller/linodecluster_controller.go | 83 +++++++++++++------------- 2 files changed, 52 insertions(+), 42 deletions(-) diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 99fbaa08b..e7d9c5511 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -17,6 +17,7 @@ limitations under the License. package scope import ( + "context" "errors" "fmt" "net/http" @@ -92,3 +93,13 @@ type ClusterScope struct { Cluster *clusterv1.Cluster LinodeCluster *infrav1.LinodeCluster } + +// PatchObject persists the cluster configuration and status. +func (s *ClusterScope) PatchObject() error { + return s.PatchHelper.Patch(context.TODO(), s.LinodeCluster) +} + +// Close closes the current scope persisting the cluster configuration and status. +func (s *ClusterScope) Close() error { + return s.PatchObject() +} diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 3825cb003..572628e0e 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -20,6 +20,8 @@ import ( "context" "errors" "fmt" + apierrors "k8s.io/apimachinery/pkg/api/errors" + utilerror "k8s.io/apimachinery/pkg/util/errors" "net/http" "time" @@ -113,49 +115,42 @@ func (r *LinodeClusterReconciler) reconcile( ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger, -) (res ctrl.Result, err error) { +) (res ctrl.Result, reterr error) { res = ctrl.Result{} clusterScope.LinodeCluster.Status.Ready = false clusterScope.LinodeCluster.Status.FailureReason = nil clusterScope.LinodeCluster.Status.FailureMessage = util.Pointer("") - failureReason := cerrs.ClusterStatusError("UnknownError") - + // Always close the scope when exiting this function so we can persist any GCPMachine changes. defer func() { - if err != nil { - setFailureReason(clusterScope, failureReason, err, r) - } - if patchErr := clusterScope.PatchHelper.Patch(ctx, clusterScope.LinodeCluster); patchErr != nil && client.IgnoreNotFound(patchErr) != nil { - logger.Error(patchErr, "failed to patch LinodeCluster") + if err := clusterScope.Close(); err != nil && reterr == nil { + if err = utilerror.FilterOut(err, apierrors.IsNotFound); err != nil { + logger.Error(err, "failed to patch LinodeCluster") + reterr = err + } - err = errors.Join(err, patchErr) } }() - // Delete - if !clusterScope.LinodeCluster.ObjectMeta.DeletionTimestamp.IsZero() { - failureReason = cerrs.DeleteClusterError - - err = r.reconcileDelete(ctx, logger, clusterScope) - - return + // Handle deleted clusters + if !clusterScope.LinodeCluster.DeletionTimestamp.IsZero() { + return ctrl.Result{}, r.reconcileDelete(ctx, logger, clusterScope) } controllerutil.AddFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) // Create if clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" { - failureReason = cerrs.CreateClusterError - - _, err = r.reconcileCreate(ctx, clusterScope, logger) + if err := r.reconcileCreate(ctx, clusterScope, logger); err != nil { + return res, err + } } - if err == nil { - clusterScope.LinodeCluster.Status.Ready = true - conditions.MarkTrue(clusterScope.LinodeCluster, clusterv1.ReadyCondition) - r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Load balancer is ready") - } + clusterScope.LinodeCluster.Status.Ready = true + conditions.MarkTrue(clusterScope.LinodeCluster, clusterv1.ReadyCondition) + + r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Load balancer is ready") return } @@ -169,17 +164,21 @@ func setFailureReason(clusterScope *scope.ClusterScope, failureReason cerrs.Clus lcr.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, string(failureReason), err.Error()) } -func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancer, error) { +func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) error { linodeNB, err := services.CreateNodeBalancer(ctx, clusterScope, logger) if err != nil { - return nil, err + setFailureReason(clusterScope, cerrs.CreateClusterError, err, r) + + return err } clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = linodeNB.ID linodeNBConfig, err := services.CreateNodeBalancerConfig(ctx, clusterScope, logger) if err != nil { - return nil, err + setFailureReason(clusterScope, cerrs.CreateClusterError, err, r) + + return err } clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{ @@ -187,28 +186,28 @@ func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, clusterSc Port: int32(linodeNBConfig.Port), } - return linodeNB, nil + return nil } -func (*LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { +func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { logger.Info("deleting cluster") + if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == 0 { + logger.Info("NodeBalancer ID is missing, nothing to do") + controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) - if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID != 0 { - if err := clusterScope.LinodeClient.DeleteNodeBalancer(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID); err != nil { - logger.Info("Failed to delete Linode NodeBalancer", "error", err.Error()) + return nil + } - // Not found is not an error - apiErr := linodego.Error{} - if errors.As(err, &apiErr) && apiErr.Code != http.StatusNotFound { - conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted") - clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = 0 - controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) + if err := clusterScope.LinodeClient.DeleteNodeBalancer(ctx, clusterScope.LinodeCluster.Spec.Network.NodeBalancerID); err != nil { + logger.Info("Failed to delete Linode NodeBalancer", "error", err.Error()) - return err - } + // Not found is not an error + apiErr := linodego.Error{} + if errors.As(err, &apiErr) && apiErr.Code != http.StatusNotFound { + setFailureReason(clusterScope, cerrs.DeleteClusterError, err, r) + + return err } - } else { - logger.Info("NodeBalancer ID is missing, nothing to do") } conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted") From de063bd94dc3faef00c3e3adbc7d95e7bddd24ad Mon Sep 17 00:00:00 2001 From: Evan Johnson Date: Fri, 26 Jan 2024 19:46:57 -0500 Subject: [PATCH 07/11] fix cluster delete error --- controller/linodecluster_controller.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 9c40a73b5..9a28e97ca 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -23,6 +23,9 @@ import ( "net/http" "time" + apierrors "k8s.io/apimachinery/pkg/api/errors" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "github.com/go-logr/logr" "github.com/linode/cluster-api-provider-linode/cloud/scope" "github.com/linode/cluster-api-provider-linode/cloud/services" @@ -122,7 +125,7 @@ func (r *LinodeClusterReconciler) reconcile( // Always close the scope when exiting this function so we can persist any GCPMachine changes. defer func() { - if err := clusterScope.Close(); err != nil && reterr == nil { + if err := clusterScope.Close(); utilerrors.FilterOut(err, apierrors.IsNotFound) != nil && reterr == nil { logger.Error(err, "failed to patch LinodeCluster") reterr = err } From d586277919066550967d4734faaafe86ff8f7b71 Mon Sep 17 00:00:00 2001 From: Evan Johnson Date: Mon, 29 Jan 2024 10:10:18 -0500 Subject: [PATCH 08/11] fix import naming to be consistent --- api/v1alpha1/linodecluster_types.go | 4 ++-- controller/linodecluster_controller.go | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/api/v1alpha1/linodecluster_types.go b/api/v1alpha1/linodecluster_types.go index aa70c9dc8..8f0633084 100644 --- a/api/v1alpha1/linodecluster_types.go +++ b/api/v1alpha1/linodecluster_types.go @@ -44,13 +44,13 @@ type LinodeClusterStatus struct { Ready bool `json:"ready"` // FailureReason will be set in the event that there is a terminal problem - // reconciling the Machine and will contain a succinct value suitable + // reconciling the LinodeCluster and will contain a succinct value suitable // for machine interpretation. // +optional FailureReason *errors.ClusterStatusError `json:"failureReason,omitempty"` // FailureMessage will be set in the event that there is a terminal problem - // reconciling the Machine and will contain a more verbose string suitable + // reconciling the LinodeCluster and will contain a more verbose string suitable // for logging and human consumption. // +optional FailureMessage *string `json:"failureMessage,omitempty"` diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 9a28e97ca..ccd28a9c5 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -48,7 +48,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - infrav1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" + infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" ) // LinodeClusterReconciler reconciles a LinodeCluster object @@ -73,7 +73,7 @@ func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques defer cancel() logger := ctrl.LoggerFrom(ctx).WithName("LinodeClusterReconciler").WithValues("name", req.NamespacedName.String()) - LinodeCluster := &infrav1.LinodeCluster{} + LinodeCluster := &infrav1alpha1.LinodeCluster{} if err := r.Client.Get(ctx, req.NamespacedName, LinodeCluster); err != nil { logger.Info("Failed to fetch Linode cluster", "error", err.Error()) @@ -136,7 +136,7 @@ func (r *LinodeClusterReconciler) reconcile( return ctrl.Result{}, r.reconcileDelete(ctx, logger, clusterScope) } - controllerutil.AddFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) + controllerutil.AddFinalizer(clusterScope.LinodeCluster, infrav1alpha1.GroupVersion.String()) // Create if clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" { if err := r.reconcileCreate(ctx, clusterScope, logger); err != nil { @@ -190,7 +190,7 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo logger.Info("deleting cluster") if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == 0 { logger.Info("NodeBalancer ID is missing, nothing to do") - controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) + controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1alpha1.GroupVersion.String()) return nil } @@ -210,7 +210,7 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted") clusterScope.LinodeCluster.Spec.Network.NodeBalancerID = 0 - controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1.GroupVersion.String()) + controllerutil.RemoveFinalizer(clusterScope.LinodeCluster, infrav1alpha1.GroupVersion.String()) return nil } @@ -218,7 +218,7 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo // SetupWithManager sets up the controller with the Manager. func (r *LinodeClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { controller, err := ctrl.NewControllerManagedBy(mgr). - For(&infrav1.LinodeCluster{}). + For(&infrav1alpha1.LinodeCluster{}). WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetLogger(), r.WatchFilterValue)). Build(r) if err != nil { @@ -227,7 +227,7 @@ func (r *LinodeClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { return controller.Watch( source.Kind(mgr.GetCache(), &clusterv1.Cluster{}), - handler.EnqueueRequestsFromMapFunc(kutil.ClusterToInfrastructureMapFunc(context.TODO(), infrav1.GroupVersion.WithKind("LinodeCluster"), mgr.GetClient(), &infrav1.LinodeCluster{})), + handler.EnqueueRequestsFromMapFunc(kutil.ClusterToInfrastructureMapFunc(context.TODO(), infrav1alpha1.GroupVersion.WithKind("LinodeCluster"), mgr.GetClient(), &infrav1alpha1.LinodeCluster{})), predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger()), ) } From 1efa73eda60fa9730738c5647ede9a7a4371263a Mon Sep 17 00:00:00 2001 From: Evan Johnson Date: Mon, 29 Jan 2024 18:18:24 -0500 Subject: [PATCH 09/11] address code review feedback --- cloud/services/loadbalancers.go | 9 +++------ controller/linodecluster_controller.go | 26 ++++++++++++++++---------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/cloud/services/loadbalancers.go b/cloud/services/loadbalancers.go index ed4c3ed2d..c48a2ab34 100644 --- a/cloud/services/loadbalancers.go +++ b/cloud/services/loadbalancers.go @@ -53,10 +53,9 @@ func CreateNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, l logger.Info(fmt.Sprintf("Creating NodeBalancer %s-api-server", clusterScope.LinodeCluster.Name)) createConfig := linodego.NodeBalancerCreateOptions{ - Label: util.Pointer(fmt.Sprintf("%s-api-server", clusterScope.LinodeCluster.Name)), - Region: clusterScope.LinodeCluster.Spec.Region, - ClientConnThrottle: nil, - Tags: tags, + Label: util.Pointer(fmt.Sprintf("%s-api-server", clusterScope.LinodeCluster.Name)), + Region: clusterScope.LinodeCluster.Spec.Region, + Tags: tags, } if linodeNB, err = clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig); err != nil { @@ -68,8 +67,6 @@ func CreateNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, l return nil, err } - err = nil - if linodeNB != nil { logger.Info("Linode NodeBalancer already exists", "existing", linodeNB.Label) } diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index ccd28a9c5..79a67fc15 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -73,14 +73,14 @@ func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques defer cancel() logger := ctrl.LoggerFrom(ctx).WithName("LinodeClusterReconciler").WithValues("name", req.NamespacedName.String()) - LinodeCluster := &infrav1alpha1.LinodeCluster{} - if err := r.Client.Get(ctx, req.NamespacedName, LinodeCluster); err != nil { + linodeCluster := &infrav1alpha1.LinodeCluster{} + if err := r.Client.Get(ctx, req.NamespacedName, linodeCluster); err != nil { logger.Info("Failed to fetch Linode cluster", "error", err.Error()) return ctrl.Result{}, client.IgnoreNotFound(err) } - cluster, err := kutil.GetOwnerCluster(ctx, r.Client, LinodeCluster.ObjectMeta) + cluster, err := kutil.GetOwnerCluster(ctx, r.Client, linodeCluster.ObjectMeta) if err != nil { logger.Info("Failed to get owner cluster", "error", err.Error()) @@ -90,7 +90,7 @@ func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, nil } - if annotations.IsPaused(cluster, LinodeCluster) { + if annotations.IsPaused(cluster, linodeCluster) { logger.Info("LinodeCluster of linked Cluster is marked as paused. Won't reconcile") return ctrl.Result{}, nil @@ -101,7 +101,7 @@ func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques scope.ClusterScopeParams{ Client: r.Client, Cluster: cluster, - LinodeCluster: LinodeCluster, + LinodeCluster: linodeCluster, }) if err != nil { logger.Info("Failed to create cluster scope", "error", err.Error()) @@ -123,8 +123,9 @@ func (r *LinodeClusterReconciler) reconcile( clusterScope.LinodeCluster.Status.FailureReason = nil clusterScope.LinodeCluster.Status.FailureMessage = util.Pointer("") - // Always close the scope when exiting this function so we can persist any GCPMachine changes. + // Always close the scope when exiting this function so we can persist any LinodeCluster changes. defer func() { + // Filter out any IsNotFound message since client.IgnoreNotFound does not handle aggregate errors if err := clusterScope.Close(); utilerrors.FilterOut(err, apierrors.IsNotFound) != nil && reterr == nil { logger.Error(err, "failed to patch LinodeCluster") reterr = err @@ -139,7 +140,7 @@ func (r *LinodeClusterReconciler) reconcile( controllerutil.AddFinalizer(clusterScope.LinodeCluster, infrav1alpha1.GroupVersion.String()) // Create if clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" { - if err := r.reconcileCreate(ctx, clusterScope, logger); err != nil { + if err := r.reconcileCreate(ctx, logger, clusterScope); err != nil { return res, err } } @@ -149,7 +150,7 @@ func (r *LinodeClusterReconciler) reconcile( r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Load balancer is ready") - return + return ctrl.Result{}, nil } func setFailureReason(clusterScope *scope.ClusterScope, failureReason cerrs.ClusterStatusError, err error, lcr *LinodeClusterReconciler) { @@ -161,7 +162,7 @@ func setFailureReason(clusterScope *scope.ClusterScope, failureReason cerrs.Clus lcr.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeWarning, string(failureReason), err.Error()) } -func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) error { +func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { linodeNB, err := services.CreateNodeBalancer(ctx, clusterScope, logger) if err != nil { setFailureReason(clusterScope, cerrs.CreateClusterError, err, r) @@ -225,9 +226,14 @@ func (r *LinodeClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { return fmt.Errorf("failed to build controller: %w", err) } - return controller.Watch( + err = controller.Watch( source.Kind(mgr.GetCache(), &clusterv1.Cluster{}), handler.EnqueueRequestsFromMapFunc(kutil.ClusterToInfrastructureMapFunc(context.TODO(), infrav1alpha1.GroupVersion.WithKind("LinodeCluster"), mgr.GetClient(), &infrav1alpha1.LinodeCluster{})), predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger()), ) + if err != nil { + return fmt.Errorf("failed adding a watch for ready clusters: %w", err) + } + + return nil } From 044122b69023ab842495d8ab62da8f0dc6b73c22 Mon Sep 17 00:00:00 2001 From: Evan Johnson Date: Mon, 29 Jan 2024 18:25:24 -0500 Subject: [PATCH 10/11] address code review feedback --- controller/linodecluster_controller.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 79a67fc15..b0477c720 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -44,7 +44,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/source" - "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -57,7 +56,6 @@ type LinodeClusterReconciler struct { Recorder record.EventRecorder LinodeApiKey string WatchFilterValue string - Scheme *runtime.Scheme ReconcileTimeout time.Duration } From a1018bd06d22333026bafa9b839c81cdbd54a9d1 Mon Sep 17 00:00:00 2001 From: Evan Johnson Date: Tue, 30 Jan 2024 11:18:43 -0500 Subject: [PATCH 11/11] immediately patch cluster after finalizer is added --- cloud/scope/cluster.go | 23 +++++++++++++------ cmd/main.go | 1 - ...cture.cluster.x-k8s.io_linodeclusters.yaml | 8 +++---- ...uster.x-k8s.io_linodeclustertemplates.yaml | 17 ++++++++++++-- controller/linodecluster_controller.go | 10 ++++---- 5 files changed, 41 insertions(+), 18 deletions(-) diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index e7d9c5511..a71fe3e56 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -22,7 +22,9 @@ import ( "fmt" "net/http" - infrav1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" "github.com/linode/linodego" "golang.org/x/oauth2" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -34,7 +36,7 @@ import ( type ClusterScopeParams struct { Client client.Client Cluster *clusterv1.Cluster - LinodeCluster *infrav1.LinodeCluster + LinodeCluster *infrav1alpha1.LinodeCluster } func validateClusterScopeParams(params ClusterScopeParams) error { @@ -91,15 +93,22 @@ type ClusterScope struct { PatchHelper *patch.Helper LinodeClient *linodego.Client Cluster *clusterv1.Cluster - LinodeCluster *infrav1.LinodeCluster + LinodeCluster *infrav1alpha1.LinodeCluster } // PatchObject persists the cluster configuration and status. -func (s *ClusterScope) PatchObject() error { - return s.PatchHelper.Patch(context.TODO(), s.LinodeCluster) +func (s *ClusterScope) PatchObject(ctx context.Context) error { + return s.PatchHelper.Patch(ctx, s.LinodeCluster) } // Close closes the current scope persisting the cluster configuration and status. -func (s *ClusterScope) Close() error { - return s.PatchObject() +func (s *ClusterScope) Close(ctx context.Context) error { + return s.PatchObject(ctx) +} + +// AddFinalizer adds a finalizer and immediately patches the object to avoid any race conditions +func (s *ClusterScope) AddFinalizer(ctx context.Context) error { + controllerutil.AddFinalizer(s.LinodeCluster, infrav1alpha1.GroupVersion.String()) + + return s.Close(ctx) } diff --git a/cmd/main.go b/cmd/main.go index 720857027..5e30a5726 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -106,7 +106,6 @@ func main() { if err = (&controller2.LinodeClusterReconciler{ Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), Recorder: mgr.GetEventRecorderFor("LinodeClusterReconciler"), WatchFilterValue: clusterWatchFilter, LinodeApiKey: linodeToken, diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml index ba92feda4..6ea8e63a2 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclusters.yaml @@ -141,13 +141,13 @@ spec: type: array failureMessage: description: FailureMessage will be set in the event that there is - a terminal problem reconciling the Machine and will contain a more - verbose string suitable for logging and human consumption. + a terminal problem reconciling the LinodeCluster and will contain + a more verbose string suitable for logging and human consumption. type: string failureReason: description: FailureReason will be set in the event that there is - a terminal problem reconciling the Machine and will contain a succinct - value suitable for machine interpretation. + a terminal problem reconciling the LinodeCluster and will contain + a succinct value suitable for machine interpretation. type: string ready: description: Ready denotes that the cluster (infrastructure) is ready. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclustertemplates.yaml index 9e4b85d41..afb4e5fd6 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodeclustertemplates.yaml @@ -67,8 +67,21 @@ spec: description: NetworkSpec encapsulates all things related to Linode network. properties: - nodebalancerID: - description: NodebalancerID is the id of apiserver Nodebalancer. + loadBalancerPort: + description: LoadBalancerPort used by the api server. + It must be valid ports range (1-65535). If omitted, + default value is 6443. + maximum: 65535 + minimum: 1 + type: integer + loadBalancerType: + description: LoadBalancerType is the type of load balancer + to use, defaults to NodeBalancer if not otherwise set + enum: + - NodeBalancer + type: string + nodeBalancerID: + description: NodeBalancerID is the id of api server NodeBalancer. type: integer type: object region: diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index b0477c720..cf73c8e4a 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -124,7 +124,7 @@ func (r *LinodeClusterReconciler) reconcile( // Always close the scope when exiting this function so we can persist any LinodeCluster changes. defer func() { // Filter out any IsNotFound message since client.IgnoreNotFound does not handle aggregate errors - if err := clusterScope.Close(); utilerrors.FilterOut(err, apierrors.IsNotFound) != nil && reterr == nil { + if err := clusterScope.Close(ctx); utilerrors.FilterOut(err, apierrors.IsNotFound) != nil && reterr == nil { logger.Error(err, "failed to patch LinodeCluster") reterr = err } @@ -132,10 +132,12 @@ func (r *LinodeClusterReconciler) reconcile( // Handle deleted clusters if !clusterScope.LinodeCluster.DeletionTimestamp.IsZero() { - return ctrl.Result{}, r.reconcileDelete(ctx, logger, clusterScope) + return res, r.reconcileDelete(ctx, logger, clusterScope) } - controllerutil.AddFinalizer(clusterScope.LinodeCluster, infrav1alpha1.GroupVersion.String()) + if err := clusterScope.AddFinalizer(ctx); err != nil { + return res, err + } // Create if clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" { if err := r.reconcileCreate(ctx, logger, clusterScope); err != nil { @@ -148,7 +150,7 @@ func (r *LinodeClusterReconciler) reconcile( r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Load balancer is ready") - return ctrl.Result{}, nil + return res, nil } func setFailureReason(clusterScope *scope.ClusterScope, failureReason cerrs.ClusterStatusError, err error, lcr *LinodeClusterReconciler) {