Skip to content

🐛 loadbalancer: resolve ControlPlaneEndpoint.Host when needed #1738

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 33 additions & 5 deletions pkg/cloud/services/loadbalancer/loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ limitations under the License.
package loadbalancer

import (
"context"
"errors"
"fmt"
"net"
"slices"
"time"

Expand All @@ -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"

Expand All @@ -42,10 +44,28 @@ 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()
ips, err := net.DefaultResolver.LookupHost(ctx, host)
if err != nil {
return "", err
}
if ip := net.ParseIP(ips[0]); ip == nil {
return "", fmt.Errorf("failed to resolve IP address for host %s", host)
}
return ips[0], nil
}

// ReconcileLoadBalancer reconciles the load balancer for the given cluster.
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)
Expand All @@ -57,13 +77,18 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust
}

var fixedIPAddress string
var err error

switch {
case lbStatus.InternalIP != "":
fixedIPAddress = lbStatus.InternalIP
case openStackCluster.Spec.APIServerFixedIP != "":
fixedIPAddress = openStackCluster.Spec.APIServerFixedIP
case openStackCluster.Spec.DisableAPIServerFloatingIP && openStackCluster.Spec.ControlPlaneEndpoint.IsValid():
fixedIPAddress = openStackCluster.Spec.ControlPlaneEndpoint.Host
fixedIPAddress, err = lookupHost(openStackCluster.Spec.ControlPlaneEndpoint.Host)
if err != nil {
return false, fmt.Errorf("lookup host: %w", err)
}
}

providers, err := s.loadbalancerClient.ListLoadBalancerProviders()
Expand Down Expand Up @@ -108,7 +133,10 @@ 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
floatingIPAddress, err = lookupHost(openStackCluster.Spec.ControlPlaneEndpoint.Host)
if err != nil {
return false, fmt.Errorf("lookup host: %w", err)
}
}
fp, err := s.networkingService.GetOrCreateFloatingIP(openStackCluster, openStackCluster, clusterName, floatingIPAddress)
if err != nil {
Expand Down Expand Up @@ -308,9 +336,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)
Expand Down
18 changes: 18 additions & 0 deletions pkg/cloud/services/loadbalancer/loadbalancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package loadbalancer

import (
"errors"
"net"
"testing"

"github.com/go-logr/logr"
Expand All @@ -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/v1alpha8"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock"
Expand All @@ -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 host, nil
} else if host == "api.test-cluster.test" {
ips := []string{"192.168.100.10"}
return ips[0], nil
}
return "", 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{
Expand Down