From 7ccdd6556c1cc7f44f3dfab702cdbe435c665a10 Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Mon, 30 Oct 2023 11:36:55 -0400 Subject: [PATCH] loadbalancer: resolve ControlPlaneEndpoint.Host when needed `ControlPlaneEndpoint.Host` is not guaranteed to be an IP address, it can also be an hostname. Now we'll try to lookup the hostname if it's not an IP and set that for the LB VipAddress. --- .../services/loadbalancer/loadbalancer.go | 38 ++++++++++++++++--- .../loadbalancer/loadbalancer_test.go | 18 +++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/pkg/cloud/services/loadbalancer/loadbalancer.go b/pkg/cloud/services/loadbalancer/loadbalancer.go index 780f7c5426..b2f5780bb0 100644 --- a/pkg/cloud/services/loadbalancer/loadbalancer.go +++ b/pkg/cloud/services/loadbalancer/loadbalancer.go @@ -17,8 +17,10 @@ limitations under the License. package loadbalancer import ( + "context" "errors" "fmt" + "net" "reflect" "time" @@ -27,7 +29,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/monitors" "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/pools" "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/utils/net" + utilsnet "k8s.io/utils/net" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/util" @@ -42,20 +44,39 @@ import ( const ( networkPrefix string = "k8s-clusterapi" kubeapiLBSuffix string = "kubeapi" + resolvedMsg string = "ControlPlaneEndpoint.Host is not an IP address, using the first resolved IP address" ) const loadBalancerProvisioningStatusActive = "ACTIVE" +// We wrap the LookupHost function in a variable to allow overriding it in unit tests. +// +//nolint:gocritic +var lookupHost = func(host string) ([]string, error) { + ctx, cancel := context.WithTimeout(context.TODO(), 30*time.Second) + defer cancel() + return net.DefaultResolver.LookupHost(ctx, host) +} + func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackCluster, clusterName string, apiServerPort int) (bool, error) { loadBalancerName := getLoadBalancerName(clusterName) s.scope.Logger().Info("Reconciling load balancer", "name", loadBalancerName) var fixedIPAddress string + var err error + switch { case openStackCluster.Spec.APIServerFixedIP != "": fixedIPAddress = openStackCluster.Spec.APIServerFixedIP case openStackCluster.Spec.DisableAPIServerFloatingIP && openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): - fixedIPAddress = openStackCluster.Spec.ControlPlaneEndpoint.Host + ips, err := lookupHost(openStackCluster.Spec.ControlPlaneEndpoint.Host) + if err != nil { + return false, fmt.Errorf("lookup host: %w", err) + } + fixedIPAddress = ips[0] + if net.ParseIP(fixedIPAddress) == nil { + s.scope.Logger().Info(resolvedMsg, "host", openStackCluster.Spec.ControlPlaneEndpoint.Host, "ip", fixedIPAddress) + } } providers, err := s.loadbalancerClient.ListLoadBalancerProviders() @@ -93,7 +114,14 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust case openStackCluster.Spec.APIServerFloatingIP != "": floatingIPAddress = openStackCluster.Spec.APIServerFloatingIP case openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): - floatingIPAddress = openStackCluster.Spec.ControlPlaneEndpoint.Host + ips, err := lookupHost(openStackCluster.Spec.ControlPlaneEndpoint.Host) + if err != nil { + return false, fmt.Errorf("lookup host: %w", err) + } + floatingIPAddress = ips[0] + if net.ParseIP(floatingIPAddress) == nil { + s.scope.Logger().Info(resolvedMsg, "host", openStackCluster.Spec.ControlPlaneEndpoint.Host, "ip", floatingIPAddress) + } } fp, err := s.networkingService.GetOrCreateFloatingIP(openStackCluster, openStackCluster, clusterName, floatingIPAddress) if err != nil { @@ -294,9 +322,9 @@ func validateIPs(openStackCluster *infrav1.OpenStackCluster, definedCIDRs []stri for _, v := range definedCIDRs { switch { - case net.IsIPv4String(v): + case utilsnet.IsIPv4String(v): marshaledCIDRs = append(marshaledCIDRs, v+"/32") - case net.IsIPv4CIDRString(v): + case utilsnet.IsIPv4CIDRString(v): marshaledCIDRs = append(marshaledCIDRs, v) default: record.Warnf(openStackCluster, "FailedIPAddressValidation", "%s is not a valid IPv4 nor CIDR address and will not get applied to allowed_cidrs", v) diff --git a/pkg/cloud/services/loadbalancer/loadbalancer_test.go b/pkg/cloud/services/loadbalancer/loadbalancer_test.go index 6b78f1c1bc..6fac23b557 100644 --- a/pkg/cloud/services/loadbalancer/loadbalancer_test.go +++ b/pkg/cloud/services/loadbalancer/loadbalancer_test.go @@ -17,6 +17,8 @@ limitations under the License. package loadbalancer import ( + "errors" + "net" "testing" "github.com/go-logr/logr" @@ -28,6 +30,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/pools" "github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/providers" . "github.com/onsi/gomega" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7" "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock" @@ -38,9 +41,24 @@ func Test_ReconcileLoadBalancer(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() + // Stub the call to net.LookupHost + lookupHost = func(host string) (addrs []string, err error) { + if net.ParseIP(host) != nil { + return []string{host}, nil + } else if host == "api.test-cluster.test" { + ips := []string{"192.168.100.10"} + return ips, nil + } + return nil, errors.New("Unknown Host " + host) + } + openStackCluster := &infrav1.OpenStackCluster{ Spec: infrav1.OpenStackClusterSpec{ DisableAPIServerFloatingIP: true, + ControlPlaneEndpoint: clusterv1.APIEndpoint{ + Host: "api.test-cluster.test", + Port: 6443, + }, }, Status: infrav1.OpenStackClusterStatus{ ExternalNetwork: &infrav1.NetworkStatus{