🌱 Avoid panic if hetznercluster.spec.controlPlaneEndpoint is not set (#1684)

Author: guettli

pkg/services/hcloud/loadbalancer/loadbalancer.go

@@ -50,6 +50,9 @@ func NewService(scope *scope.ClusterScope) *Service {
 // ErrNoLoadBalancerAvailable indicates that no available load balancer could be fond.
 var ErrNoLoadBalancerAvailable = fmt.Errorf("no available load balancer")
 
+// ErrControlPlaneEndpointNotSet indicates that hetznercluster.spec.controlPlaneEndpoint is not set.
+var ErrControlPlaneEndpointNotSet = errors.New("hetznercluster.spec.controlPlaneEndpoint is not set")
+
 // Reconcile implements the life cycle of HCloud load balancers.
 func (s *Service) Reconcile(ctx context.Context) (reconcile.Result, error) {
 	// delete the deprecated condition from existing cluster objects
@@ -68,20 +71,37 @@ func (s *Service) Reconcile(ctx context.Context) (reconcile.Result, error) {
 	}
 
 	if lb == nil {
-		// fixed name is set - we expect a load balancer with this name to exist
-
 		if s.scope.HetznerCluster.Spec.ControlPlaneLoadBalancer.Name != nil {
+			// fixed name is set - we expect a load balancer with this name to exist
 			lb, err = s.ownExistingLoadBalancer(ctx)
-
-			// if load balancer is not found even though we expect it to exist, wait and reconcile until user creates it
-			if errors.Is(err, ErrNoLoadBalancerAvailable) {
-				return reconcile.Result{RequeueAfter: 1 * time.Minute}, nil
+			if err != nil {
+				// if load balancer is not found even though we expect it to exist, wait and reconcile until user creates it
+				if errors.Is(err, ErrNoLoadBalancerAvailable) {
+					return reconcile.Result{RequeueAfter: 1 * time.Minute}, nil
+				}
+				return reconcile.Result{}, fmt.Errorf("failed to own existing load balancer (name=%s): %w", *s.scope.HetznerCluster.Spec.ControlPlaneLoadBalancer.Name, err)
 			}
 		} else {
 			lb, err = s.createLoadBalancer(ctx)
-		}
-		if err != nil {
-			return reconcile.Result{}, fmt.Errorf("failed to own/create load balancer: %w", err)
+			if err != nil {
+				if errors.Is(err, ErrControlPlaneEndpointNotSet) {
+					// When an external ControlPlane Provider gets used (Kamaji), it might
+					// need some time until the endpoint is available.
+					err = fmt.Errorf("requeue, waiting for control-plane endpoint to be set: %w",
+						err)
+					conditions.MarkFalse(
+						s.scope.HetznerCluster,
+						infrav1.LoadBalancerReadyCondition,
+						"MissingControlPlaneEndpoint",
+						clusterv1.ConditionSeverityWarning,
+						"%s",
+						err.Error(),
+					)
+					s.scope.Logger.Info(err.Error())
+					return reconcile.Result{RequeueAfter: 10 * time.Second}, nil
+				}
+				return reconcile.Result{}, fmt.Errorf("failed to create load balancer: %w", err)
+			}
 		}
 	}
 
@@ -287,7 +307,10 @@ func (s *Service) reconcileServices(ctx context.Context, lb *hcloud.LoadBalancer
 }
 
 func (s *Service) createLoadBalancer(ctx context.Context) (*hcloud.LoadBalancer, error) {
-	opts := createOptsFromSpec(s.scope.HetznerCluster)
+	opts, err := createOptsFromSpec(s.scope.HetznerCluster)
+	if err != nil {
+		return nil, err
+	}
 	lb, err := s.scope.HCloudClient.CreateLoadBalancer(ctx, opts)
 	if err != nil {
 		err = fmt.Errorf("failed to create load balancer: %w", err)
@@ -309,7 +332,7 @@ func (s *Service) createLoadBalancer(ctx context.Context) (*hcloud.LoadBalancer,
 	return lb, nil
 }
 
-func createOptsFromSpec(hc *infrav1.HetznerCluster) hcloud.LoadBalancerCreateOpts {
+func createOptsFromSpec(hc *infrav1.HetznerCluster) (hcloud.LoadBalancerCreateOpts, error) {
 	// gather algorithm type
 	algorithmType := hc.Spec.ControlPlaneLoadBalancer.Algorithm.HCloudAlgorithmType()
 
@@ -323,6 +346,10 @@ func createOptsFromSpec(hc *infrav1.HetznerCluster) hcloud.LoadBalancerCreateOpt
 		network = &hcloud.Network{ID: hc.Status.Network.ID}
 	}
 
+	if hc.Spec.ControlPlaneEndpoint == nil {
+		return hcloud.LoadBalancerCreateOpts{}, ErrControlPlaneEndpointNotSet
+	}
+
 	listenPort := int(hc.Spec.ControlPlaneEndpoint.Port)
 	publicInterface := true
 	return hcloud.LoadBalancerCreateOpts{
@@ -341,7 +368,7 @@ func createOptsFromSpec(hc *infrav1.HetznerCluster) hcloud.LoadBalancerCreateOpt
 				Proxyprotocol:   &proxyprotocol,
 			},
 		},
-	}
+	}, nil
 }
 
 // Delete implements the deletion of HCloud load balancers.

pkg/services/hcloud/loadbalancer/loadbalancer_test.go

@@ -17,6 +17,8 @@ limitations under the License.
 package loadbalancer
 
 import (
+	"errors"
+
 	"github.com/go-logr/logr"
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 	. "github.com/onsi/ginkgo/v2"
@@ -122,7 +124,8 @@ var _ = Describe("createOptsFromSpec", func() {
 		hetznerCluster.Status.Network = nil
 		wantCreateOpts.Network = nil
 
-		createOpts := createOptsFromSpec(hetznerCluster)
+		createOpts, err := createOptsFromSpec(hetznerCluster)
+		Expect(err).To(BeNil())
 
 		// ignore random name
 		createOpts.Name = ""
@@ -131,7 +134,8 @@ var _ = Describe("createOptsFromSpec", func() {
 	})
 
 	It("creates specs for cluster with network", func() {
-		createOpts := createOptsFromSpec(hetznerCluster)
+		createOpts, err := createOptsFromSpec(hetznerCluster)
+		Expect(err).To(BeNil())
 
 		// ignore random name
 		createOpts.Name = ""
@@ -142,7 +146,8 @@ var _ = Describe("createOptsFromSpec", func() {
 	It("creates specs for cluster without load balancer name set", func() {
 		hetznerCluster.Spec.ControlPlaneLoadBalancer.Name = nil
 
-		createOpts := createOptsFromSpec(hetznerCluster)
+		createOpts, err := createOptsFromSpec(hetznerCluster)
+		Expect(err).To(BeNil())
 
 		// should generate correct name
 		Expect(createOpts.Name).To(HavePrefix("hetzner-cluster-kube-apiserver-"))
@@ -152,4 +157,11 @@ var _ = Describe("createOptsFromSpec", func() {
 		wantCreateOpts.Name = ""
 		Expect(createOpts).To(Equal(wantCreateOpts))
 	})
+
+	It("returns ErrControlPlaneEndpointNotSet", func() {
+		hetznerCluster.Spec.ControlPlaneEndpoint = nil
+
+		_, err := createOptsFromSpec(hetznerCluster)
+		Expect(errors.Is(err, ErrControlPlaneEndpointNotSet)).To(BeTrue())
+	})
 })