Skip to content

Commit

Permalink
[fix] add support for filling in controlPlaneEndpoint if only network…
Browse files Browse the repository at this point in the history 
… info is specified (#395)

* only create the nodebalancer and config if not already set, switch to getNB instead of listNB

* clean up unused listNB function
  • Loading branch information
@AshleyDumaine
AshleyDumaine authored Jul 8, 2024
1 parent e039a73 commit 7db8837
Show file tree
Hide file tree
Showing 7 changed files with 273 additions and 200 deletions.
3 changes: 2 additions & 1 deletion clients/clients.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ type LinodeVPCClient interface {

// LinodeNodeBalancerClient defines the methods that interact with Linode's Node Balancer service.
type LinodeNodeBalancerClient interface {
ListNodeBalancers(ctx context.Context, opts *linodego.ListOptions) ([]linodego.NodeBalancer, error)
CreateNodeBalancer(ctx context.Context, opts linodego.NodeBalancerCreateOptions) (*linodego.NodeBalancer, error)
GetNodeBalancer(ctx context.Context, nodebalancerID int) (*linodego.NodeBalancer, error)
GetNodeBalancerConfig(ctx context.Context, nodebalancerID int, configID int) (*linodego.NodeBalancerConfig, error)
CreateNodeBalancerConfig(ctx context.Context, nodebalancerID int, opts linodego.NodeBalancerConfigCreateOptions) (*linodego.NodeBalancerConfig, error)
DeleteNodeBalancerNode(ctx context.Context, nodebalancerID int, configID int, nodeID int) error
DeleteNodeBalancer(ctx context.Context, nodebalancerID int) error
Expand Down
104 changes: 39 additions & 65 deletions cloud/services/loadbalancers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"errors"
"fmt"
"net/http"
"slices"

"github.com/go-logr/logr"
"github.com/linode/linodego"
Expand All @@ -20,85 +19,69 @@ const (
DefaultKonnectivityLBPort = 8132
)

// 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 linodeNB *linodego.NodeBalancer

NBLabel := clusterScope.LinodeCluster.Name
clusterUID := string(clusterScope.LinodeCluster.UID)
tags := []string{string(clusterScope.LinodeCluster.UID)}
listFilter := util.Filter{
ID: clusterScope.LinodeCluster.Spec.Network.NodeBalancerID,
Label: NBLabel,
Tags: tags,
}
filter, err := listFilter.String()
if err != nil {
return nil, err
}
linodeNBs, err := clusterScope.LinodeClient.ListNodeBalancers(ctx, linodego.NewListOptions(1, filter))
if err != nil {
logger.Info("Failed to list NodeBalancers", "error", err.Error())

return nil, err
}
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")
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]))
// EnsureNodeBalancer creates a new NodeBalancer if one doesn't exist or returns the existing NodeBalancer
func EnsureNodeBalancer(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (*linodego.NodeBalancer, error) {
nbID := clusterScope.LinodeCluster.Spec.Network.NodeBalancerID
if nbID != nil && *nbID != 0 {
res, err := clusterScope.LinodeClient.GetNodeBalancer(ctx, *nbID)
if err != nil {
logger.Info("Failed to get NodeBalancer", "error", err.Error())

return nil, err
}

return &linodeNBs[0], nil
return res, nil
}

logger.Info(fmt.Sprintf("Creating NodeBalancer %s", clusterScope.LinodeCluster.Name))
createConfig := linodego.NodeBalancerCreateOptions{
Label: util.Pointer(clusterScope.LinodeCluster.Name),
Region: clusterScope.LinodeCluster.Spec.Region,
Tags: tags,
Tags: []string{string(clusterScope.LinodeCluster.UID)},
}

linodeNB, err = clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig)
if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
return nil, err
}
if linodeNB != nil {
logger.Info("Linode NodeBalancer already exists", "existing", linodeNB.Label)
}

return linodeNB, nil
return clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig)
}

// CreateNodeBalancerConfigs creates NodeBalancer configs if it does not exist
func CreateNodeBalancerConfigs(
// EnsureNodeBalancerConfigs creates NodeBalancer configs if it does not exist or returns the existing NodeBalancerConfig
func EnsureNodeBalancerConfigs(
ctx context.Context,
clusterScope *scope.ClusterScope,
logger logr.Logger,
) ([]*linodego.NodeBalancerConfig, error) {
nbConfigs := []*linodego.NodeBalancerConfig{}
var apiserverLinodeNBConfig *linodego.NodeBalancerConfig
var err error
apiLBPort := DefaultApiserverLBPort
if clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort != 0 {
apiLBPort = clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort
}
apiserverCreateConfig := linodego.NodeBalancerConfigCreateOptions{
Port: apiLBPort,
Protocol: linodego.ProtocolTCP,
Algorithm: linodego.AlgorithmRoundRobin,
Check: linodego.CheckConnection,
}

apiserverLinodeNBConfig, err := clusterScope.LinodeClient.CreateNodeBalancerConfig(
ctx,
*clusterScope.LinodeCluster.Spec.Network.NodeBalancerID,
apiserverCreateConfig,
)
if err != nil {
logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error())
return nil, err
if clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID != nil {
apiserverLinodeNBConfig, err = clusterScope.LinodeClient.GetNodeBalancerConfig(
ctx,
*clusterScope.LinodeCluster.Spec.Network.NodeBalancerID,
*clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID)
if err != nil {
logger.Info("Failed to get Linode NodeBalancer config", "error", err.Error())
return nil, err
}
} else {
apiserverLinodeNBConfig, err = clusterScope.LinodeClient.CreateNodeBalancerConfig(
ctx,
*clusterScope.LinodeCluster.Spec.Network.NodeBalancerID,
linodego.NodeBalancerConfigCreateOptions{
Port: apiLBPort,
Protocol: linodego.ProtocolTCP,
Algorithm: linodego.AlgorithmRoundRobin,
Check: linodego.CheckConnection,
},
)
if err != nil {
logger.Info("Failed to create Linode NodeBalancer config", "error", err.Error())
return nil, err
}
}

nbConfigs = append(nbConfigs, apiserverLinodeNBConfig)

// return if additional ports should not be configured
Expand Down Expand Up @@ -180,11 +163,6 @@ func AddNodeToNB(
return err
}

// return if additional ports should not be configured
if len(machineScope.LinodeCluster.Spec.Network.AdditionalPorts) == 0 {
return nil
}

for _, portConfig := range machineScope.LinodeCluster.Spec.Network.AdditionalPorts {
_, err = machineScope.LinodeClient.CreateNodeBalancerNode(
ctx,
Expand Down Expand Up @@ -234,10 +212,6 @@ func DeleteNodeFromNB(
return err
}

if len(machineScope.LinodeCluster.Spec.Network.AdditionalPorts) == 0 {
return nil
}

for _, portConfig := range machineScope.LinodeCluster.Spec.Network.AdditionalPorts {
err = machineScope.LinodeClient.DeleteNodeBalancerNode(
ctx,
Expand Down
109 changes: 56 additions & 53 deletions cloud/services/loadbalancers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"github.com/linode/cluster-api-provider-linode/mock"
)

func TestCreateNodeBalancer(t *testing.T) {
func TestEnsureNodeBalancer(t *testing.T) {
t.Parallel()
tests := []struct {
name string
Expand Down Expand Up @@ -50,8 +50,7 @@ func TestCreateNodeBalancer(t *testing.T) {
},
},
expects: func(mockClient *mock.MockLinodeClient) {
mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancer{}, nil)
mockClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()).Return(&linodego.NodeBalancer{
mockClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()).Return(&linodego.NodeBalancer{
ID: 1234,
}, nil)
},
Expand All @@ -60,7 +59,7 @@ func TestCreateNodeBalancer(t *testing.T) {
},
},
{
name: "Success - List NodeBalancers returns one nodebalancer and we return that",
name: "Success - Get NodeBalancers returns one nodebalancer and we return that",
clusterScope: &scope.ClusterScope{
LinodeCluster: &infrav1alpha2.LinodeCluster{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -75,12 +74,10 @@ func TestCreateNodeBalancer(t *testing.T) {
},
},
expects: func(mockClient *mock.MockLinodeClient) {
mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancer{
{
ID: 1234,
Label: ptr.To("test"),
Tags: []string{"test-uid"},
},
mockClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()).Return(&linodego.NodeBalancer{
ID: 1234,
Label: ptr.To("test"),
Tags: []string{"test-uid"},
}, nil)
},
expectedNodeBalancer: &linodego.NodeBalancer{
Expand All @@ -90,38 +87,7 @@ func TestCreateNodeBalancer(t *testing.T) {
},
},
{
name: "Error - List NodeBalancers returns one nodebalancer but there is a nodebalancer conflict",
clusterScope: &scope.ClusterScope{
LinodeCluster: &infrav1alpha2.LinodeCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
UID: "test-uid",
},
Spec: infrav1alpha2.LinodeClusterSpec{
Network: infrav1alpha2.NetworkSpec{
NodeBalancerID: ptr.To(1234),
},
},
},
},
expects: func(mockClient *mock.MockLinodeClient) {
mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancer{
{
ID: 1234,
Label: ptr.To("test"),
Tags: []string{"test"},
},
}, nil)
},
expectedNodeBalancer: &linodego.NodeBalancer{
ID: 1234,
Label: ptr.To("test"),
Tags: []string{"test"},
},
expectedError: fmt.Errorf("NodeBalancer conflict"),
},
{
name: "Error - List NodeBalancers returns an error",
name: "Error - Get NodeBalancer returns an error",
clusterScope: &scope.ClusterScope{
LinodeCluster: &infrav1alpha2.LinodeCluster{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -136,9 +102,9 @@ func TestCreateNodeBalancer(t *testing.T) {
},
},
expects: func(mockClient *mock.MockLinodeClient) {
mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("Unable to list NodeBalancers"))
mockClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("Unable to get NodeBalancer"))
},
expectedError: fmt.Errorf("Unable to list NodeBalancers"),
expectedError: fmt.Errorf("Unable to get NodeBalancer"),
},
{
name: "Error - Create NodeBalancer returns an error",
Expand All @@ -148,15 +114,10 @@ func TestCreateNodeBalancer(t *testing.T) {
Name: "test-cluster",
UID: "test-uid",
},
Spec: infrav1alpha2.LinodeClusterSpec{
Network: infrav1alpha2.NetworkSpec{
NodeBalancerID: ptr.To(1234),
},
},
Spec: infrav1alpha2.LinodeClusterSpec{},
},
},
expects: func(mockClient *mock.MockLinodeClient) {
mockClient.EXPECT().ListNodeBalancers(gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancer{}, nil)
mockClient.EXPECT().CreateNodeBalancer(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("Unable to create NodeBalancer"))
},
expectedError: fmt.Errorf("Unable to create NodeBalancer"),
Expand All @@ -176,7 +137,7 @@ func TestCreateNodeBalancer(t *testing.T) {

testcase.expects(MockLinodeClient)

got, err := CreateNodeBalancer(context.Background(), testcase.clusterScope, logr.Discard())
got, err := EnsureNodeBalancer(context.Background(), testcase.clusterScope, logr.Discard())
if testcase.expectedError != nil {
assert.ErrorContains(t, err, testcase.expectedError.Error())
} else {
Expand All @@ -187,7 +148,7 @@ func TestCreateNodeBalancer(t *testing.T) {
}
}

func TestCreateNodeBalancerConfigs(t *testing.T) {
func TestEnsureNodeBalancerConfigs(t *testing.T) {
t.Parallel()

tests := []struct {
Expand Down Expand Up @@ -232,6 +193,48 @@ func TestCreateNodeBalancerConfigs(t *testing.T) {
}, nil)
},
},
{
name: "Success - Get NodeBalancerConfig",
clusterScope: &scope.ClusterScope{
LinodeClient: nil,
LinodeCluster: &infrav1alpha2.LinodeCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
UID: "test-uid",
},
Spec: infrav1alpha2.LinodeClusterSpec{
Network: infrav1alpha2.NetworkSpec{
NodeBalancerID: ptr.To(1234),
ApiserverNodeBalancerConfigID: ptr.To(2),
},
ControlPlaneEndpoint: clusterv1.APIEndpoint{
Host: "",
Port: 0,
},
},
},
},
expectedConfigs: []*linodego.NodeBalancerConfig{
{
Port: DefaultApiserverLBPort,
Protocol: linodego.ProtocolTCP,
Algorithm: linodego.AlgorithmRoundRobin,
Check: linodego.CheckConnection,
NodeBalancerID: 1234,
ID: 2,
},
},
expects: func(mockClient *mock.MockLinodeClient) {
mockClient.EXPECT().GetNodeBalancerConfig(gomock.Any(), gomock.Any(), gomock.Any()).Return(&linodego.NodeBalancerConfig{
ID: 2,
Port: DefaultApiserverLBPort,
Protocol: linodego.ProtocolTCP,
Algorithm: linodego.AlgorithmRoundRobin,
Check: linodego.CheckConnection,
NodeBalancerID: 1234,
}, nil)
},
},
{
name: "Success - Create NodeBalancerConfig using assigned LB ports",
clusterScope: &scope.ClusterScope{
Expand Down Expand Up @@ -396,7 +399,7 @@ func TestCreateNodeBalancerConfigs(t *testing.T) {

testcase.expects(MockLinodeClient)

got, err := CreateNodeBalancerConfigs(context.Background(), testcase.clusterScope, logr.Discard())
got, err := EnsureNodeBalancerConfigs(context.Background(), testcase.clusterScope, logr.Discard())
if testcase.expectedError != nil {
assert.ErrorContains(t, err, testcase.expectedError.Error())
} else {
Expand Down
Loading

0 comments on commit 7db8837

Please sign in to comment.