Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin' into harden-deletion-logic
Browse files Browse the repository at this point in the history
  • Loading branch information
lubedacht committed May 2, 2024
2 parents 76e94d8 + dd3f67e commit 2584e62
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 21 deletions.
5 changes: 5 additions & 0 deletions api/v1alpha1/ionoscloudcluster_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ var _ = Describe("IonosCloudCluster", func() {
It("should allow creating valid clusters", func() {
Expect(k8sClient.Create(context.Background(), defaultCluster())).To(Succeed())
})
It("should work with a FQDN controlplane endpoint", func() {
cluster := defaultCluster()
cluster.Spec.ControlPlaneEndpoint.Host = "example.org"
Expect(k8sClient.Create(context.Background(), cluster)).To(Succeed())
})
It("should not allow creating clusters with empty credential secret", func() {
cluster := defaultCluster()
cluster.Spec.CredentialsRef.Name = ""
Expand Down
20 changes: 12 additions & 8 deletions docs/quickstart.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,23 @@ clusterctl init --infrastructure=ionoscloud
CAPIC requires several environment variables to be set in order to create a Kubernetes cluster on IONOS Cloud.

```env
## -- Cloud specific environment variables -- ##
## -- Cloud-specific environment variables -- ##
IONOS_TOKEN # The token of the IONOS Cloud account.
IONOS_API_URL # The API URL of the IONOS Cloud account.
# Defaults to https://api.ionos.com/cloudapi/v6
## -- Cluster API related environment variables -- ##
CONTROL_PLANE_ENDPOINT_IP # The IP address of the control plane endpoint.
CONTROL_PLANE_ENDPOINT_PORT # The port of the control plane endpoint.
IONOS_API_URL # The API URL of the IONOS Cloud account (optional).
# Defaults to https://api.ionos.com/cloudapi/v6.
## -- Cluster API-related environment variables -- ##
CONTROL_PLANE_ENDPOINT_HOST # The control plane endpoint host (optional).
# If it's not an IP but an FQDN, the provider must be able to resolve it
# to the value for CONTROL_PLANE_ENDPOINT_IP.
CONTROL_PLANE_ENDPOINT_IP # The IPv4 address of the control plane endpoint.
CONTROL_PLANE_ENDPOINT_PORT # The port of the control plane endpoint (optional).
# Defaults to 6443.
CONTROL_PLANE_ENDPOINT_LOCATION # The location of the control plane endpoint.
CLUSTER_NAME # The name of the cluster.
KUBERNETES_VERSION # The version of Kubernetes to be installed (can also be set via clusterctl).
## -- Kubernetes Cluster related environment variables -- ##
## -- Kubernetes Cluster-related environment variables -- ##
IONOSCLOUD_CONTRACT_NUMBER # The contract number of the IONOS Cloud contract.
IONOSCLOUD_DATACENTER_ID # The datacenter ID where the cluster should be created.
IONOSCLOUD_MACHINE_NUM_CORES # The number of cores.
Expand Down
1 change: 1 addition & 0 deletions envfile.example
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export IONOS_API_URL="https://api.ionos.com/cloudapi/v6"


# Cluster API related environment variables
export CONTROL_PLANE_ENDPOINT_HOST="example.org"
export CONTROL_PLANE_ENDPOINT_IP="192.168.0.1"
export CONTROL_PLANE_ENDPOINT_PORT=6443
export CONTROL_PLANE_ENDPOINT_LOCATION="de/txl"
Expand Down
27 changes: 16 additions & 11 deletions internal/service/cloud/ipblock.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,17 @@ func (s *Service) getControlPlaneEndpointIPBlock(ctx context.Context, cs *scope.
if ipBlock != nil || ignoreNotFound(err) != nil {
return ipBlock, err
}
notFoundError := err

s.logger.Info("IP block not found by ID, trying to find by listing IP blocks instead")
blocks, listErr := s.apiWithDepth(listIPBlocksDepth).ListIPBlocks(ctx)
if listErr != nil {
return nil, fmt.Errorf("failed to list IP blocks: %w", listErr)
blocks, err := s.apiWithDepth(listIPBlocksDepth).ListIPBlocks(ctx)
if err != nil {
return nil, fmt.Errorf("failed to list IP blocks: %w", err)
}

controlPlaneEndpointIP, err := cs.GetControlPlaneEndpointIP(ctx)
if err != nil {
return nil, err
}

var (
Expand All @@ -275,7 +281,7 @@ func (s *Service) getControlPlaneEndpointIPBlock(ctx context.Context, cs *scope.
if err != nil {
return nil, err
}
case s.checkIfUserSetBlock(cs, props):
case s.checkIfUserSetBlock(controlPlaneEndpointIP, props):
// NOTE: this is for when customers set IPs for the control plane endpoint themselves.
foundBlock, err = s.cloudAPIStateInconsistencyWorkaround(ctx, &block)
if err != nil {
Expand All @@ -285,25 +291,24 @@ func (s *Service) getControlPlaneEndpointIPBlock(ctx context.Context, cs *scope.
}
if count > 1 {
return nil, fmt.Errorf(
"cannot determine IP block for Control Plane Endpoint as there are multiple IP blocks with the name %s",
"cannot determine IP block for Control Plane Endpoint, as there are multiple IP blocks with the name %s",
expectedName)
}
}
if count == 0 && cs.GetControlPlaneEndpoint().Host != "" {
if count == 0 && controlPlaneEndpointIP != "" {
return nil, errUserSetIPNotFound
}
if foundBlock != nil {
return foundBlock, nil
}
// if we still can't find an IP block we return the potential
// initial not found error.
return nil, err
return nil, notFoundError
}

func (*Service) checkIfUserSetBlock(cs *scope.Cluster, props *sdk.IpBlockProperties) bool {
ip := cs.GetControlPlaneEndpoint().Host
func (*Service) checkIfUserSetBlock(controlPlaneEndpointIP string, props *sdk.IpBlockProperties) bool {
ips := ptr.Deref(props.GetIps(), nil)
return ip != "" && slices.Contains(ips, ip)
return controlPlaneEndpointIP != "" && slices.Contains(ips, controlPlaneEndpointIP)
}

// cloudAPIStateInconsistencyWorkaround is a workaround for a bug where the API returns different states for the same
Expand All @@ -320,7 +325,7 @@ func (s *Service) cloudAPIStateInconsistencyWorkaround(ctx context.Context, bloc

func (s *Service) getIPBlockByID(ctx context.Context, ipBlockID string) (*sdk.IpBlock, error) {
if ipBlockID == "" {
s.logger.Info("Could not find any IP block by ID as the provider ID is not set.")
s.logger.Info("Could not find any IP block by ID, as the provider ID is not set.")
return nil, nil
}
ipBlock, err := s.ionosClient.GetIPBlock(ctx, ipBlockID)
Expand Down
3 changes: 2 additions & 1 deletion internal/service/cloud/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,8 @@ func (s *Service) retrieveFailoverIPForMachine(
log := s.logger.WithName("retrieveFailoverIPForMachine")

if util.IsControlPlaneMachine(ms.Machine) {
return false, ms.ClusterScope.GetControlPlaneEndpoint().Host, nil
ip, err := ms.ClusterScope.GetControlPlaneEndpointIP(ctx)
return false, ip, err
}

failoverIP = ptr.Deref(ms.IonosMachine.Spec.FailoverIP, "")
Expand Down
36 changes: 36 additions & 0 deletions scope/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ import (
"context"
"errors"
"fmt"
"net"
"net/netip"
"slices"
"time"

"k8s.io/client-go/util/retry"
Expand All @@ -32,10 +35,18 @@ import (
infrav1 "github.com/ionos-cloud/cluster-api-provider-ionoscloud/api/v1alpha1"
)

// resolver is able to look up IP addresses from a given host name.
// The net.Resolver type (found at net.DefaultResolver) implements this interface.
// This is intended for testing.
type resolver interface {
LookupNetIP(ctx context.Context, network, host string) ([]netip.Addr, error)
}

// Cluster defines a basic cluster context for primary use in IonosCloudClusterReconciler.
type Cluster struct {
client client.Client
patchHelper *patch.Helper
resolver resolver
Cluster *clusterv1.Cluster
IonosCluster *infrav1.IonosCloudCluster
}
Expand Down Expand Up @@ -72,6 +83,7 @@ func NewCluster(params ClusterParams) (*Cluster, error) {
Cluster: params.Cluster,
IonosCluster: params.IonosCluster,
patchHelper: helper,
resolver: net.DefaultResolver,
}

return clusterScope, nil
Expand All @@ -82,6 +94,30 @@ func (c *Cluster) GetControlPlaneEndpoint() clusterv1.APIEndpoint {
return c.IonosCluster.Spec.ControlPlaneEndpoint
}

// GetControlPlaneEndpointIP returns the endpoint IP for the IonosCloudCluster.
// If the endpoint host is unset (neither an IP nor an FQDN), it will return an empty string.
func (c *Cluster) GetControlPlaneEndpointIP(ctx context.Context) (string, error) {
host := c.GetControlPlaneEndpoint().Host
if host == "" {
return "", nil
}

if ip, err := netip.ParseAddr(host); err == nil {
return ip.String(), nil
}

// If the host is not an IP, try to resolve it.
ips, err := c.resolver.LookupNetIP(ctx, "ip4", host)
if err != nil {
return "", fmt.Errorf("failed to resolve control plane endpoint IP: %w", err)
}

// Sort IPs to deal with random order intended for load balancing.
slices.SortFunc(ips, func(a, b netip.Addr) int { return a.Compare(b) })

return ips[0].String(), nil
}

// SetControlPlaneEndpointIPBlockID sets the IP block ID in the IonosCloudCluster status.
func (c *Cluster) SetControlPlaneEndpointIPBlockID(id string) {
c.IonosCluster.Status.ControlPlaneEndpointIPBlockID = id
Expand Down
76 changes: 76 additions & 0 deletions scope/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package scope

import (
"context"
"net"
"net/netip"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -84,11 +86,85 @@ func TestNewClusterMissingParams(t *testing.T) {
params, err := NewCluster(test.params)
require.NoError(t, err)
require.NotNil(t, params)
require.Equal(t, net.DefaultResolver, params.resolver)
}
})
}
}

type mockResolver struct {
addrs map[string][]netip.Addr
}

func (m *mockResolver) LookupNetIP(_ context.Context, _, host string) ([]netip.Addr, error) {
return m.addrs[host], nil
}

func resolvesTo(ips ...string) []netip.Addr {
res := make([]netip.Addr, 0, len(ips))
for _, ip := range ips {
res = append(res, netip.MustParseAddr(ip))
}
return res
}

func TestCluster_GetControlPlaneEndpointIP(t *testing.T) {
tests := []struct {
name string
host string
resolver resolver
want string
}{
{
name: "host empty",
host: "",
want: "",
},
{
name: "host is IP",
host: "127.0.0.1",
want: "127.0.0.1",
},
{
name: "host is FQDN with single IP",
host: "localhost",
resolver: &mockResolver{
addrs: map[string][]netip.Addr{
"localhost": resolvesTo("127.0.0.1"),
},
},
want: "127.0.0.1",
},
{
name: "host is FQDN with multiple IPs",
host: "example.org",
resolver: &mockResolver{
addrs: map[string][]netip.Addr{
"example.org": resolvesTo("2.3.4.5", "1.2.3.4"),
},
},
want: "1.2.3.4",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := &Cluster{
resolver: tt.resolver,
IonosCluster: &infrav1.IonosCloudCluster{
Spec: infrav1.IonosCloudClusterSpec{
ControlPlaneEndpoint: clusterv1.APIEndpoint{
Host: tt.host,
},
},
},
}
got, err := c.GetControlPlaneEndpointIP(context.Background())
require.NoError(t, err)
require.Equal(t, tt.want, got)
})
}
}

func TestClusterListMachinesForCluster(t *testing.T) {
scheme := runtime.NewScheme()
require.NoError(t, infrav1.AddToScheme(scheme))
Expand Down
2 changes: 1 addition & 1 deletion templates/cluster-template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ metadata:
name: "${CLUSTER_NAME}"
spec:
controlPlaneEndpoint:
host: ${CONTROL_PLANE_ENDPOINT_IP}
host: ${CONTROL_PLANE_ENDPOINT_HOST:-${CONTROL_PLANE_ENDPOINT_IP}}
port: ${CONTROL_PLANE_ENDPOINT_PORT:-6443}
location: ${CONTROL_PLANE_ENDPOINT_LOCATION}
contractNumber: "${IONOSCLOUD_CONTRACT_NUMBER}"
Expand Down

0 comments on commit 2584e62

Please sign in to comment.