Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[fix] [improvement] move apiserver loadbalancing logic from linodemachine controller to linodecluster controller #457

Merged
merged 41 commits into from
Aug 21, 2024
Merged

[fix] [improvement] move apiserver loadbalancing logic from linodemachine controller to linodecluster controller #457

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
3421652
move loadbalancing logic from linodemachine to linodecluster
amold1 Aug 15, 2024
db6088f
update cluster_test
amold1 Aug 15, 2024
830fab3
update machine_test
amold1 Aug 15, 2024
ad1c49a
update domains_test
amold1 Aug 15, 2024
2db6032
update cluster_test and machine_test
amold1 Aug 15, 2024
3a97498
update loadbalancers_test
amold1 Aug 15, 2024
a55a5a2
update linodemachine_controller_test
amold1 Aug 15, 2024
e8ba740
fix linting issues and all tests except linodecluster_controller ones
amold1 Aug 15, 2024
1031ec7
fix nilcheck failure
amold1 Aug 15, 2024
cc0d43b
Merge branch 'main' into dns.cleanup
amold1 Aug 15, 2024
bb66598
add filtering for only controlplane nodes to trigger reconciliation
amold1 Aug 16, 2024
0ceb101
fix lint errors
amold1 Aug 16, 2024
8aceb74
Merge branch 'main' into dns.cleanup
amold1 Aug 16, 2024
ccbc4e2
add support for rke2 and kthrees
amold1 Aug 16, 2024
6bde061
fix ip parsing and deletion failures
amold1 Aug 17, 2024
96963a8
Merge branch 'main' into dns.cleanup
amold1 Aug 17, 2024
bfdda7e
remove controlplane logic
amold1 Aug 17, 2024
8aceba5
remove cp rbac roles
amold1 Aug 17, 2024
4d6636d
fix mock tests
amold1 Aug 20, 2024
7ab659c
Merge branch 'main' into dns.cleanup
amold1 Aug 20, 2024
879c933
address comments from ashley
amold1 Aug 20, 2024
07b5612
fix linter issues
amold1 Aug 20, 2024
e0c929d
directly return err/nil instead of if/else
amold1 Aug 20, 2024
2c5c4e4
check for 429 errors
amold1 Aug 20, 2024
b087a64
fix linodemachine e2e tests
amold1 Aug 20, 2024
694df28
update returned errors and update tests
amold1 Aug 20, 2024
f577ec0
remove commented out code
amold1 Aug 20, 2024
fa40846
remove instanceid from spec in tests
amold1 Aug 20, 2024
64cb394
fix import order for lint
amold1 Aug 20, 2024
9616754
fix minimal-linodecluster e2e test
amold1 Aug 20, 2024
94ecb1f
add debug for minimal-linodecluster e2e test
amold1 Aug 20, 2024
d345348
debugging
amold1 Aug 20, 2024
361c11f
remove debugging
amold1 Aug 20, 2024
59f9edc
debugging
amold1 Aug 21, 2024
c9a154a
add NB backend nodes only if it doesn't already exist
amold1 Aug 21, 2024
edcb049
update NB delete if condition
amold1 Aug 21, 2024
f3aba03
use machine object to determine if linodemachine is a controlplane node
amold1 Aug 21, 2024
f5bcc03
keep the function scope limited to the package and dont export
amold1 Aug 21, 2024
feae180
remove dbug logging
amold1 Aug 21, 2024
ed75083
do not return if ips havent been set yet
amold1 Aug 21, 2024
2dda21f
Merge branch 'main' into dns.cleanup
amold1 Aug 21, 2024
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
49 changes: 35 additions & 14 deletions cloud/scope/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ import (

// ClusterScopeParams defines the input parameters used to create a new Scope.
type ClusterScopeParams struct {
Client K8sClient
Cluster *clusterv1.Cluster
LinodeCluster *infrav1alpha2.LinodeCluster
Client K8sClient
Cluster *clusterv1.Cluster
LinodeCluster *infrav1alpha2.LinodeCluster
LinodeMachineList infrav1alpha2.LinodeMachineList
}

func validateClusterScopeParams(params ClusterScopeParams) error {
Expand All @@ -50,7 +51,7 @@ func validateClusterScopeParams(params ClusterScopeParams) error {

// NewClusterScope creates a new Scope from the supplied parameters.
// This is meant to be called for each reconcile iteration.
func NewClusterScope(ctx context.Context, linodeClientConfig ClientConfig, params ClusterScopeParams) (*ClusterScope, error) {
func NewClusterScope(ctx context.Context, linodeClientConfig, dnsClientConfig ClientConfig, params ClusterScopeParams) (*ClusterScope, error) {
if err := validateClusterScopeParams(params); err != nil {
return nil, err
}
Expand All @@ -63,6 +64,11 @@ func NewClusterScope(ctx context.Context, linodeClientConfig ClientConfig, param
return nil, fmt.Errorf("credentials from secret ref: %w", err)
}
linodeClientConfig.Token = string(apiToken)
dnsToken, err := getCredentialDataFromRef(ctx, params.Client, *params.LinodeCluster.Spec.CredentialsRef, params.LinodeCluster.GetNamespace(), "dnsToken")
if err != nil || len(dnsToken) == 0 {
dnsToken = apiToken
}
dnsClientConfig.Token = string(dnsToken)
}
linodeClient, err := CreateLinodeClient(linodeClientConfig)
if err != nil {
Expand All @@ -74,22 +80,37 @@ func NewClusterScope(ctx context.Context, linodeClientConfig ClientConfig, param
return nil, fmt.Errorf("failed to init patch helper: %w", err)
}

akamDomainsClient, err := setUpEdgeDNSInterface()
if err != nil {
return nil, fmt.Errorf("failed to create akamai dns client: %w", err)
}
linodeDomainsClient, err := CreateLinodeClient(dnsClientConfig, WithRetryCount(0))
if err != nil {
return nil, fmt.Errorf("failed to create linode client: %w", err)
}

return &ClusterScope{
Client: params.Client,
Cluster: params.Cluster,
LinodeClient: linodeClient,
LinodeCluster: params.LinodeCluster,
PatchHelper: helper,
Client: params.Client,
Cluster: params.Cluster,
LinodeClient: linodeClient,
LinodeDomainsClient: linodeDomainsClient,
AkamaiDomainsClient: akamDomainsClient,
LinodeCluster: params.LinodeCluster,
LinodeMachines: params.LinodeMachineList,
PatchHelper: helper,
}, nil
}

// ClusterScope defines the basic context for an actuator to operate upon.
type ClusterScope struct {
Client K8sClient
PatchHelper *patch.Helper
LinodeClient LinodeClient
Cluster *clusterv1.Cluster
LinodeCluster *infrav1alpha2.LinodeCluster
Client K8sClient
PatchHelper *patch.Helper
LinodeClient LinodeClient
Cluster *clusterv1.Cluster
LinodeCluster *infrav1alpha2.LinodeCluster
LinodeMachines infrav1alpha2.LinodeMachineList
AkamaiDomainsClient AkamClient
LinodeDomainsClient LinodeClient
}

// PatchObject persists the cluster configuration and status.
Expand Down
173 changes: 48 additions & 125 deletions cloud/scope/cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,9 @@ func TestValidateClusterScopeParams(t *testing.T) {
func TestClusterScopeMethods(t *testing.T) {
t.Parallel()
type fields struct {
Cluster *clusterv1.Cluster
LinodeCluster *infrav1alpha2.LinodeCluster
Cluster *clusterv1.Cluster
LinodeCluster *infrav1alpha2.LinodeCluster
LinodeMachineList infrav1alpha2.LinodeMachineList
}

tests := []struct {
Expand Down Expand Up @@ -126,7 +127,8 @@ func TestClusterScopeMethods(t *testing.T) {
{
name: "AddFinalizer error - finalizer should not be added to the Linode Cluster object. Function returns nil since it was already present",
fields: fields{
Cluster: &clusterv1.Cluster{},
Cluster: &clusterv1.Cluster{},
LinodeMachineList: infrav1alpha2.LinodeMachineList{},
LinodeCluster: &infrav1alpha2.LinodeCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
Expand Down Expand Up @@ -158,10 +160,12 @@ func TestClusterScopeMethods(t *testing.T) {
cScope, err := NewClusterScope(
context.Background(),
ClientConfig{Token: "test-key"},
ClientConfig{Token: "test-key"},
ClusterScopeParams{
Cluster: testcase.fields.Cluster,
LinodeCluster: testcase.fields.LinodeCluster,
Client: mockK8sClient,
Cluster: testcase.fields.Cluster,
LinodeMachineList: testcase.fields.LinodeMachineList,
LinodeCluster: testcase.fields.LinodeCluster,
Client: mockK8sClient,
})
if err != nil {
t.Errorf("NewClusterScope() error = %v", err)
Expand All @@ -181,8 +185,9 @@ func TestClusterScopeMethods(t *testing.T) {
func TestNewClusterScope(t *testing.T) {
t.Parallel()
type args struct {
apiKey string
params ClusterScopeParams
apiKey string
dnsApiKey string
params ClusterScopeParams
}
tests := []struct {
name string
Expand All @@ -193,7 +198,8 @@ func TestNewClusterScope(t *testing.T) {
{
name: "Success - Pass in valid args and get a valid ClusterScope",
args: args{
apiKey: "test-key",
apiKey: "test-key",
dnsApiKey: "test-key",
params: ClusterScopeParams{
Cluster: &clusterv1.Cluster{},
LinodeCluster: &infrav1alpha2.LinodeCluster{},
Expand All @@ -211,7 +217,8 @@ func TestNewClusterScope(t *testing.T) {
{
name: "Success - Validate getCredentialDataFromRef() returns some apiKey data and we create a valid ClusterScope",
args: args{
apiKey: "test-key",
apiKey: "test-key",
dnsApiKey: "test-key",
params: ClusterScopeParams{
Client: nil,
Cluster: &clusterv1.Cluster{},
Expand All @@ -231,7 +238,7 @@ func TestNewClusterScope(t *testing.T) {
s := runtime.NewScheme()
infrav1alpha2.AddToScheme(s)
return s
})
}).AnyTimes()
mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
cred := corev1.Secret{
Data: map[string][]byte{
Expand All @@ -241,13 +248,23 @@ func TestNewClusterScope(t *testing.T) {
*obj = cred
return nil
})
mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
cred := corev1.Secret{
Data: map[string][]byte{
"dnsToken": []byte("example"),
},
}
*obj = cred
return nil
})
},
},
{
name: "Error - ValidateClusterScopeParams triggers error because ClusterScopeParams is empty",
args: args{
apiKey: "test-key",
params: ClusterScopeParams{},
apiKey: "test-key",
dnsApiKey: "test-key",
params: ClusterScopeParams{},
},
expectedError: fmt.Errorf("cluster is required when creating a ClusterScope"),
expects: func(mock *mock.MockK8sClient) {},
Expand All @@ -269,7 +286,8 @@ func TestNewClusterScope(t *testing.T) {
{
name: "Error - Using getCredentialDataFromRef(), func returns an error. Unable to create a valid ClusterScope",
args: args{
apiKey: "test-key",
apiKey: "test-key",
dnsApiKey: "test-key",
params: ClusterScopeParams{
Client: nil,
Cluster: &clusterv1.Cluster{},
Expand All @@ -291,7 +309,8 @@ func TestNewClusterScope(t *testing.T) {
{
name: "Error - createLinodeCluster throws an error for passing empty apiKey. Unable to create a valid ClusterScope",
args: args{
apiKey: "",
apiKey: "",
dnsApiKey: "",
params: ClusterScopeParams{
Cluster: &clusterv1.Cluster{},
LinodeCluster: &infrav1alpha2.LinodeCluster{},
Expand All @@ -316,7 +335,7 @@ func TestNewClusterScope(t *testing.T) {

testcase.args.params.Client = mockK8sClient

got, err := NewClusterScope(context.Background(), ClientConfig{Token: testcase.args.apiKey}, testcase.args.params)
got, err := NewClusterScope(context.Background(), ClientConfig{Token: testcase.args.apiKey}, ClientConfig{Token: testcase.args.dnsApiKey}, testcase.args.params)

if testcase.expectedError != nil {
assert.ErrorContains(t, err, testcase.expectedError.Error())
Expand All @@ -327,112 +346,12 @@ func TestNewClusterScope(t *testing.T) {
}
}

func TestClusterAddCredentialsRefFinalizer(t *testing.T) {
t.Parallel()
type fields struct {
Cluster *clusterv1.Cluster
LinodeCluster *infrav1alpha2.LinodeCluster
}

tests := []struct {
name string
fields fields
expects func(mock *mock.MockK8sClient)
}{
{
name: "Success - finalizer should be added to the Linode Cluster credentials Secret",
fields: fields{
Cluster: &clusterv1.Cluster{},
LinodeCluster: &infrav1alpha2.LinodeCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
},
Spec: infrav1alpha2.LinodeClusterSpec{
CredentialsRef: &corev1.SecretReference{
Name: "example",
Namespace: "test",
},
},
},
},
expects: func(mock *mock.MockK8sClient) {
mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
s := runtime.NewScheme()
infrav1alpha2.AddToScheme(s)
return s
})
mock.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, key types.NamespacedName, obj *corev1.Secret, opts ...client.GetOption) error {
cred := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "example",
Namespace: "test",
},
Data: map[string][]byte{
"apiToken": []byte("example"),
},
}
*obj = cred

return nil
}).Times(2)
mock.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil)
},
},
{
name: "No-op - no Linode Cluster credentials Secret",
fields: fields{
Cluster: &clusterv1.Cluster{},
LinodeCluster: &infrav1alpha2.LinodeCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
},
},
},
expects: func(mock *mock.MockK8sClient) {
mock.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme {
s := runtime.NewScheme()
infrav1alpha2.AddToScheme(s)
return s
})
},
},
}
for _, tt := range tests {
testcase := tt
t.Run(testcase.name, func(t *testing.T) {
t.Parallel()

ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockK8sClient := mock.NewMockK8sClient(ctrl)

testcase.expects(mockK8sClient)

cScope, err := NewClusterScope(
context.Background(),
ClientConfig{Token: "test-key"},
ClusterScopeParams{
Cluster: testcase.fields.Cluster,
LinodeCluster: testcase.fields.LinodeCluster,
Client: mockK8sClient,
})
if err != nil {
t.Errorf("NewClusterScope() error = %v", err)
}

if err := cScope.AddCredentialsRefFinalizer(context.Background()); err != nil {
t.Errorf("ClusterScope.AddCredentialsRefFinalizer() error = %v", err)
}
})
}
}

func TestRemoveCredentialsRefFinalizer(t *testing.T) {
t.Parallel()
type fields struct {
Cluster *clusterv1.Cluster
LinodeCluster *infrav1alpha2.LinodeCluster
Cluster *clusterv1.Cluster
LinodeCluster *infrav1alpha2.LinodeCluster
LinodeMachineList infrav1alpha2.LinodeMachineList
}

tests := []struct {
Expand All @@ -443,7 +362,8 @@ func TestRemoveCredentialsRefFinalizer(t *testing.T) {
{
name: "Success - finalizer should be removed from the Linode Cluster credentials Secret",
fields: fields{
Cluster: &clusterv1.Cluster{},
Cluster: &clusterv1.Cluster{},
LinodeMachineList: infrav1alpha2.LinodeMachineList{},
LinodeCluster: &infrav1alpha2.LinodeCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
Expand Down Expand Up @@ -475,14 +395,15 @@ func TestRemoveCredentialsRefFinalizer(t *testing.T) {
*obj = cred

return nil
}).Times(2)
}).AnyTimes()
mock.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil)
},
},
{
name: "No-op - no Linode Cluster credentials Secret",
fields: fields{
Cluster: &clusterv1.Cluster{},
Cluster: &clusterv1.Cluster{},
LinodeMachineList: infrav1alpha2.LinodeMachineList{},
LinodeCluster: &infrav1alpha2.LinodeCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
Expand Down Expand Up @@ -513,10 +434,12 @@ func TestRemoveCredentialsRefFinalizer(t *testing.T) {
cScope, err := NewClusterScope(
context.Background(),
ClientConfig{Token: "test-key"},
ClientConfig{Token: "test-key"},
ClusterScopeParams{
Cluster: testcase.fields.Cluster,
LinodeCluster: testcase.fields.LinodeCluster,
Client: mockK8sClient,
Cluster: testcase.fields.Cluster,
LinodeCluster: testcase.fields.LinodeCluster,
LinodeMachineList: testcase.fields.LinodeMachineList,
Client: mockK8sClient,
})
if err != nil {
t.Errorf("NewClusterScope() error = %v", err)
Expand Down
Loading
Loading