diff --git a/api/v1alpha2/linodefirewall_types.go b/api/v1alpha2/linodefirewall_types.go index f1817519e..1bfff3186 100644 --- a/api/v1alpha2/linodefirewall_types.go +++ b/api/v1alpha2/linodefirewall_types.go @@ -41,6 +41,7 @@ type LinodeFirewallSpec struct { // +optional InboundRules []FirewallRule `json:"inboundRules,omitempty"` + // InboundPolicy determines if traffic by default should be ACCEPTed or DROPped. Defaults to ACCEPT if not defined. // +kubebuilder:validation:Enum=ACCEPT;DROP // +kubebuilder:default=ACCEPT // +optional @@ -49,6 +50,7 @@ type LinodeFirewallSpec struct { // +optional OutboundRules []FirewallRule `json:"outboundRules,omitempty"` + // OutboundPolicy determines if traffic by default should be ACCEPTed or DROPped. Defaults to ACCEPT if not defined. // +kubebuilder:validation:Enum=ACCEPT;DROP // +kubebuilder:default=ACCEPT // +optional diff --git a/api/v1alpha2/linodemachine_types.go b/api/v1alpha2/linodemachine_types.go index 7e27f82a8..4455f3a84 100644 --- a/api/v1alpha2/linodemachine_types.go +++ b/api/v1alpha2/linodemachine_types.go @@ -68,7 +68,6 @@ type LinodeMachineSpec struct { // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable" Tags []string `json:"tags,omitempty"` // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable" - // +kubebuilder:deprecatedversion:warning="Firewalls should be referenced via FirewallRef" FirewallID int `json:"firewallID,omitempty"` // OSDisk is configuration for the root disk that includes the OS, // if not specified this defaults to whatever space is not taken up by the DataDisks diff --git a/clients/clients.go b/clients/clients.go index 93ff0d2a9..bba1801c1 100644 --- a/clients/clients.go +++ b/clients/clients.go @@ -64,6 +64,7 @@ type LinodeVPCClient interface { type LinodeNodeBalancerClient interface { CreateNodeBalancer(ctx context.Context, opts linodego.NodeBalancerCreateOptions) (*linodego.NodeBalancer, error) GetNodeBalancer(ctx context.Context, nodebalancerID int) (*linodego.NodeBalancer, error) + ListNodeBalancerNodes(ctx context.Context, nodebalancerID int, configID int, opts *linodego.ListOptions) ([]linodego.NodeBalancerNode, 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 diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 6f51330dd..b9ccb1c78 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -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 { @@ -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 } @@ -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 { @@ -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. diff --git a/cloud/scope/cluster_test.go b/cloud/scope/cluster_test.go index ef3195cde..9e969ff57 100644 --- a/cloud/scope/cluster_test.go +++ b/cloud/scope/cluster_test.go @@ -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 { @@ -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", @@ -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) @@ -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 @@ -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{}, @@ -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{}, @@ -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{ @@ -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) {}, @@ -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{}, @@ -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{}, @@ -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()) @@ -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 { @@ -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", @@ -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", @@ -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) diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index c6e8a0855..a6d82133d 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -6,11 +6,8 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/util/retry" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - kutil "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -28,16 +25,13 @@ type MachineScopeParams struct { } type MachineScope struct { - Client K8sClient - MachinePatchHelper *patch.Helper - ClusterPatchHelper *patch.Helper - Cluster *clusterv1.Cluster - Machine *clusterv1.Machine - LinodeClient LinodeClient - LinodeDomainsClient LinodeClient - AkamaiDomainsClient AkamClient - LinodeCluster *infrav1alpha2.LinodeCluster - LinodeMachine *infrav1alpha2.LinodeMachine + Client K8sClient + PatchHelper *patch.Helper + Cluster *clusterv1.Cluster + Machine *clusterv1.Machine + LinodeClient LinodeClient + LinodeCluster *infrav1alpha2.LinodeCluster + LinodeMachine *infrav1alpha2.LinodeMachine } func validateMachineScopeParams(params MachineScopeParams) error { @@ -57,7 +51,7 @@ func validateMachineScopeParams(params MachineScopeParams) error { return nil } -func NewMachineScope(ctx context.Context, linodeClientConfig, dnsClientConfig ClientConfig, params MachineScopeParams) (*MachineScope, error) { +func NewMachineScope(ctx context.Context, linodeClientConfig ClientConfig, params MachineScopeParams) (*MachineScope, error) { if err := validateMachineScopeParams(params); err != nil { return nil, err } @@ -89,12 +83,6 @@ func NewMachineScope(ctx context.Context, linodeClientConfig, dnsClientConfig Cl return nil, fmt.Errorf("credentials from secret ref: %w", err) } linodeClientConfig.Token = string(apiToken) - - dnsToken, err := getCredentialDataFromRef(ctx, params.Client, *credentialRef, defaultNamespace, "dnsToken") - if err != nil || len(dnsToken) == 0 { - dnsToken = apiToken - } - dnsClientConfig.Token = string(dnsToken) } linodeClient, err := CreateLinodeClient(linodeClientConfig, @@ -103,98 +91,37 @@ func NewMachineScope(ctx context.Context, linodeClientConfig, dnsClientConfig Cl if err != nil { return nil, fmt.Errorf("failed to create linode client: %w", err) } - linodeDomainsClient, err := CreateLinodeClient(dnsClientConfig, - WithRetryCount(0), - ) - if err != nil { - return nil, fmt.Errorf("failed to create linode client: %w", err) - } - - akamDomainsClient, err := setUpEdgeDNSInterface() - if err != nil { - return nil, fmt.Errorf("failed to create akamai dns client: %w", err) - } - - machineHelper, err := patch.NewHelper(params.LinodeMachine, params.Client) + helper, err := patch.NewHelper(params.LinodeMachine, params.Client) if err != nil { - return nil, fmt.Errorf("failed to init machine patch helper: %w", err) - } - - clusterHelper, err := patch.NewHelper(params.LinodeCluster, params.Client) - if err != nil { - return nil, fmt.Errorf("failed to init cluster patch helper: %w", err) + return nil, fmt.Errorf("failed to init patch helper: %w", err) } return &MachineScope{ - Client: params.Client, - MachinePatchHelper: machineHelper, - ClusterPatchHelper: clusterHelper, - Cluster: params.Cluster, - Machine: params.Machine, - LinodeClient: linodeClient, - LinodeDomainsClient: linodeDomainsClient, - AkamaiDomainsClient: akamDomainsClient, - LinodeCluster: params.LinodeCluster, - LinodeMachine: params.LinodeMachine, + Client: params.Client, + PatchHelper: helper, + Cluster: params.Cluster, + Machine: params.Machine, + LinodeClient: linodeClient, + LinodeCluster: params.LinodeCluster, + LinodeMachine: params.LinodeMachine, }, nil } -// CloseAll persists the linodemachine and linodecluster configuration and status. -func (s *MachineScope) CloseAll(ctx context.Context) error { - if err := s.MachineClose(ctx); err != nil { - return err - } - if err := s.ClusterClose(ctx); err != nil { - return err - } - return nil -} - -// MachineClose persists the linodemachine configuration and status. -func (s *MachineScope) MachineClose(ctx context.Context) error { - return retry.OnError(retry.DefaultRetry, apierrors.IsConflict, func() error { - return s.MachinePatchHelper.Patch(ctx, s.LinodeMachine) - }) +// PatchObject persists the machine configuration and status. +func (s *MachineScope) PatchObject(ctx context.Context) error { + return s.PatchHelper.Patch(ctx, s.LinodeMachine) } -// ClusterClose persists the linodecluster configuration and status. -func (s *MachineScope) ClusterClose(ctx context.Context) error { - return retry.OnError(retry.DefaultRetry, apierrors.IsConflict, func() error { - return s.ClusterPatchHelper.Patch(ctx, s.LinodeCluster) - }) +// Close closes the current scope persisting the machine configuration and status. +func (s *MachineScope) Close(ctx context.Context) error { + return s.PatchObject(ctx) } // AddFinalizer adds a finalizer if not present and immediately patches the // object to avoid any race conditions. func (s *MachineScope) AddFinalizer(ctx context.Context) error { if controllerutil.AddFinalizer(s.LinodeMachine, infrav1alpha2.MachineFinalizer) { - return s.MachineClose(ctx) - } - - return nil -} - -// AddLinodeClusterFinalizer adds a finalizer if not present and immediately patches the -// object to avoid any race conditions. -func (s *MachineScope) AddLinodeClusterFinalizer(ctx context.Context) error { - if !kutil.IsControlPlaneMachine(s.Machine) { - return nil - } - if controllerutil.AddFinalizer(s.LinodeCluster, s.LinodeMachine.Name) { - return s.ClusterClose(ctx) - } - - return nil -} - -// RemoveLinodeClusterFinalizer adds a finalizer if not present and immediately patches the -// object to avoid any race conditions. -func (s *MachineScope) RemoveLinodeClusterFinalizer(ctx context.Context) error { - if !kutil.IsControlPlaneMachine(s.Machine) { - return nil - } - if controllerutil.RemoveFinalizer(s.LinodeCluster, s.LinodeMachine.Name) { - return s.ClusterClose(ctx) + return s.Close(ctx) } return nil diff --git a/cloud/scope/machine_test.go b/cloud/scope/machine_test.go index 1c6eca37e..4241c982d 100644 --- a/cloud/scope/machine_test.go +++ b/cloud/scope/machine_test.go @@ -17,7 +17,6 @@ import ( "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" infrav1alpha2 "github.com/linode/cluster-api-provider-linode/api/v1alpha2" "github.com/linode/cluster-api-provider-linode/mock" @@ -25,8 +24,6 @@ import ( . "github.com/linode/cluster-api-provider-linode/mock/mocktest" ) -const isControlPlane = "true" - func TestValidateMachineScopeParams(t *testing.T) { t.Parallel() type args struct { @@ -136,7 +133,6 @@ func TestMachineScopeAddFinalizer(t *testing.T) { mScope, err := NewMachineScope( ctx, ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, MachineScopeParams{ Client: mck.K8sClient, Cluster: &clusterv1.Cluster{}, @@ -164,7 +160,6 @@ func TestMachineScopeAddFinalizer(t *testing.T) { mScope, err := NewMachineScope( ctx, ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, MachineScopeParams{ Client: mck.K8sClient, Cluster: &clusterv1.Cluster{}, @@ -186,7 +181,6 @@ func TestMachineScopeAddFinalizer(t *testing.T) { mScope, err := NewMachineScope( ctx, ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, MachineScopeParams{ Client: mck.K8sClient, Cluster: &clusterv1.Cluster{}, @@ -203,249 +197,6 @@ func TestMachineScopeAddFinalizer(t *testing.T) { ) } -func TestLinodeClusterFinalizer(t *testing.T) { - t.Parallel() - - NewSuite(t, mock.MockK8sClient{}).Run( - Call("scheme 1", func(ctx context.Context, mck Mock) { - mck.K8sClient.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme { - s := runtime.NewScheme() - infrav1alpha2.AddToScheme(s) - return s - }).AnyTimes() - }), - OneOf( - Path(Call("scheme 2", func(ctx context.Context, mck Mock) { - mck.K8sClient.EXPECT().Scheme().DoAndReturn(func() *runtime.Scheme { - s := runtime.NewScheme() - infrav1alpha2.AddToScheme(s) - return s - }).AnyTimes() - })), - Path(Result("has finalizer", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope( - ctx, - ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, - MachineScopeParams{ - Client: mck.K8sClient, - Cluster: &clusterv1.Cluster{}, - Machine: &clusterv1.Machine{}, - LinodeMachine: &infrav1alpha2.LinodeMachine{}, - LinodeCluster: &infrav1alpha2.LinodeCluster{ - ObjectMeta: metav1.ObjectMeta{ - Finalizers: []string{"test"}, - }, - }, - }) - require.NoError(t, err) - require.NoError(t, mScope.AddLinodeClusterFinalizer(ctx)) - require.Len(t, mScope.LinodeCluster.Finalizers, 1) - assert.Equal(t, "test", mScope.LinodeCluster.Finalizers[0]) - })), - Path( - Call("remove finalizers", func(ctx context.Context, mck Mock) { - mck.K8sClient.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - }), - Result("remove finalizer", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope( - ctx, - ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, - MachineScopeParams{ - Client: mck.K8sClient, - Cluster: &clusterv1.Cluster{}, - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Labels: make(map[string]string), - }, - }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - }, - LinodeCluster: &infrav1alpha2.LinodeCluster{ - ObjectMeta: metav1.ObjectMeta{ - Finalizers: []string{"test"}, - }, - }, - }) - mScope.Machine.Labels[clusterv1.MachineControlPlaneLabel] = isControlPlane - require.NoError(t, err) - require.Len(t, mScope.LinodeCluster.Finalizers, 1) - assert.Equal(t, "test", mScope.LinodeCluster.Finalizers[0]) - require.NoError(t, mScope.RemoveLinodeClusterFinalizer(ctx)) - require.Empty(t, mScope.LinodeCluster.Finalizers) - }), - ), - Path( - Call("success patch helper", func(ctx context.Context, mck Mock) { - mck.K8sClient.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - }), - Result("remove finalizer", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope( - ctx, - ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, - MachineScopeParams{ - Client: mck.K8sClient, - Cluster: &clusterv1.Cluster{}, - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Labels: make(map[string]string), - }, - }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Finalizers: []string{"test"}, - }, - }, - LinodeCluster: &infrav1alpha2.LinodeCluster{ - ObjectMeta: metav1.ObjectMeta{ - Finalizers: []string{"test"}, - }, - }, - }) - mScope.Machine.Labels[clusterv1.MachineControlPlaneLabel] = isControlPlane - require.NoError(t, err) - require.Len(t, mScope.LinodeCluster.Finalizers, 1) - assert.Equal(t, "test", mScope.LinodeCluster.Finalizers[0]) - controllerutil.RemoveFinalizer(mScope.LinodeCluster, mScope.LinodeMachine.Name) - controllerutil.RemoveFinalizer(mScope.LinodeMachine, mScope.LinodeMachine.Name) - require.NoError(t, mScope.CloseAll(ctx)) - require.Empty(t, mScope.LinodeCluster.Finalizers) - require.Empty(t, mScope.LinodeMachine.Finalizers) - }), - ), - Path( - Call("fail patch helper", func(ctx context.Context, mck Mock) { - mck.K8sClient.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()).Return(errors.New("failed to patch")).AnyTimes() - }), - Result("remove finalizer", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope( - ctx, - ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, - MachineScopeParams{ - Client: mck.K8sClient, - Cluster: &clusterv1.Cluster{}, - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Labels: make(map[string]string), - }, - }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Finalizers: []string{"test"}, - }, - }, - LinodeCluster: &infrav1alpha2.LinodeCluster{ - ObjectMeta: metav1.ObjectMeta{ - Finalizers: []string{"test"}, - }, - }, - }) - mScope.Machine.Labels[clusterv1.MachineControlPlaneLabel] = isControlPlane - require.NoError(t, err) - require.Len(t, mScope.LinodeCluster.Finalizers, 1) - assert.Equal(t, "test", mScope.LinodeCluster.Finalizers[0]) - controllerutil.RemoveFinalizer(mScope.LinodeCluster, mScope.LinodeMachine.Name) - controllerutil.RemoveFinalizer(mScope.LinodeMachine, mScope.LinodeMachine.Name) - require.ErrorContains(t, mScope.CloseAll(ctx), "failed to patch") - }), - ), - ), - OneOf( - Path( - Call("able to patch", func(ctx context.Context, mck Mock) { - mck.K8sClient.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - }), - Result("finalizer added when it is a control plane node", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope( - ctx, - ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, - MachineScopeParams{ - Client: mck.K8sClient, - Cluster: &clusterv1.Cluster{}, - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Labels: make(map[string]string), - }, - }, - LinodeCluster: &infrav1alpha2.LinodeCluster{}, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - }, - }) - mScope.Machine.Labels[clusterv1.MachineControlPlaneLabel] = isControlPlane - require.NoError(t, err) - require.NoError(t, mScope.AddLinodeClusterFinalizer(ctx)) - require.Len(t, mScope.LinodeCluster.Finalizers, 1) - assert.Equal(t, mScope.LinodeMachine.Name, mScope.LinodeCluster.Finalizers[0]) - }), - ), - Path( - Result("no finalizer added when it is a worker node", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope( - ctx, - ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, - MachineScopeParams{ - Client: mck.K8sClient, - Cluster: &clusterv1.Cluster{}, - Machine: &clusterv1.Machine{}, - LinodeCluster: &infrav1alpha2.LinodeCluster{}, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - }, - }) - require.NoError(t, err) - require.NoError(t, mScope.AddLinodeClusterFinalizer(ctx)) - require.Empty(t, mScope.LinodeMachine.Finalizers) - }), - ), - Path( - Call("unable to patch when it is a control plane node", func(ctx context.Context, mck Mock) { - mck.K8sClient.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()).Return(errors.New("fail")).AnyTimes() - }), - Result("error", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope( - ctx, - ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, - MachineScopeParams{ - Client: mck.K8sClient, - Cluster: &clusterv1.Cluster{}, - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Labels: make(map[string]string), - }, - }, - LinodeCluster: &infrav1alpha2.LinodeCluster{}, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - }, - }) - mScope.Machine.Labels[clusterv1.MachineControlPlaneLabel] = isControlPlane - require.NoError(t, err) - - assert.Error(t, mScope.AddLinodeClusterFinalizer(ctx)) - }), - ), - ), - ) -} - func TestNewMachineScope(t *testing.T) { t.Parallel() @@ -455,14 +206,13 @@ func TestNewMachineScope(t *testing.T) { mScope, err := NewMachineScope( ctx, ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, MachineScopeParams{}, ) require.ErrorContains(t, err, "is required") assert.Nil(t, mScope) })), Path(Result("no token", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope(ctx, ClientConfig{Token: ""}, ClientConfig{Token: ""}, MachineScopeParams{ + mScope, err := NewMachineScope(ctx, ClientConfig{Token: ""}, MachineScopeParams{ Client: mck.K8sClient, Cluster: &clusterv1.Cluster{}, Machine: &clusterv1.Machine{}, @@ -477,7 +227,7 @@ func TestNewMachineScope(t *testing.T) { mck.K8sClient.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "example")) }), Result("error", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope(ctx, ClientConfig{Token: ""}, ClientConfig{Token: ""}, MachineScopeParams{ + mScope, err := NewMachineScope(ctx, ClientConfig{Token: ""}, MachineScopeParams{ Client: mck.K8sClient, Cluster: &clusterv1.Cluster{}, Machine: &clusterv1.Machine{}, @@ -509,14 +259,14 @@ func TestNewMachineScope(t *testing.T) { mck.K8sClient.EXPECT().Scheme().Return(runtime.NewScheme()).AnyTimes() }), Result("cannot init patch helper", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope(ctx, ClientConfig{Token: "apiToken"}, ClientConfig{Token: "dnsToken"}, MachineScopeParams{ + mScope, err := NewMachineScope(ctx, ClientConfig{Token: "apiToken"}, MachineScopeParams{ Client: mck.K8sClient, Cluster: &clusterv1.Cluster{}, Machine: &clusterv1.Machine{}, LinodeCluster: &infrav1alpha2.LinodeCluster{}, LinodeMachine: &infrav1alpha2.LinodeMachine{}, }) - require.ErrorContains(t, err, "failed to init machine patch helper") + require.ErrorContains(t, err, "failed to init patch helper") assert.Nil(t, mScope) }), ), @@ -534,7 +284,7 @@ func TestNewMachineScope(t *testing.T) { }).AnyTimes() })), Path(Result("default credentials", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope(ctx, ClientConfig{Token: "apiToken"}, ClientConfig{Token: "dnsToken"}, MachineScopeParams{ + mScope, err := NewMachineScope(ctx, ClientConfig{Token: "apiToken"}, MachineScopeParams{ Client: mck.K8sClient, Cluster: &clusterv1.Cluster{}, Machine: &clusterv1.Machine{}, @@ -547,7 +297,7 @@ func TestNewMachineScope(t *testing.T) { ), OneOf( Path(Result("credentials from LinodeMachine credentialsRef", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope(ctx, ClientConfig{Token: ""}, ClientConfig{Token: ""}, MachineScopeParams{ + mScope, err := NewMachineScope(ctx, ClientConfig{Token: ""}, MachineScopeParams{ Client: mck.K8sClient, Cluster: &clusterv1.Cluster{}, Machine: &clusterv1.Machine{}, @@ -565,7 +315,7 @@ func TestNewMachineScope(t *testing.T) { assert.NotNil(t, mScope) })), Path(Result("credentials from LinodeCluster credentialsRef", func(ctx context.Context, mck Mock) { - mScope, err := NewMachineScope(ctx, ClientConfig{Token: "apiToken"}, ClientConfig{Token: "dnsToken"}, MachineScopeParams{ + mScope, err := NewMachineScope(ctx, ClientConfig{Token: "apiToken"}, MachineScopeParams{ Client: mck.K8sClient, Cluster: &clusterv1.Cluster{}, Machine: &clusterv1.Machine{}, @@ -733,7 +483,6 @@ func TestMachineAddCredentialsRefFinalizer(t *testing.T) { mScope, err := NewMachineScope( context.Background(), ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, MachineScopeParams{ Client: mockK8sClient, Cluster: &clusterv1.Cluster{}, @@ -827,7 +576,6 @@ func TestMachineRemoveCredentialsRefFinalizer(t *testing.T) { mScope, err := NewMachineScope( context.Background(), ClientConfig{Token: "apiToken"}, - ClientConfig{Token: "dnsToken"}, MachineScopeParams{ Client: mockK8sClient, Cluster: &clusterv1.Cluster{}, diff --git a/cloud/services/domains.go b/cloud/services/domains.go index c99ff46c9..aceef0706 100644 --- a/cloud/services/domains.go +++ b/cloud/services/domains.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "net" "net/netip" "strings" "sync" @@ -12,8 +13,8 @@ import ( "github.com/linode/linodego" "golang.org/x/exp/slices" "sigs.k8s.io/cluster-api/api/v1beta1" - kutil "sigs.k8s.io/cluster-api/util" + "github.com/linode/cluster-api-provider-linode/clients" "github.com/linode/cluster-api-provider-linode/cloud/scope" rutil "github.com/linode/cluster-api-provider-linode/util/reconciler" ) @@ -31,101 +32,136 @@ type DNSOptions struct { } // EnsureDNSEntries ensures the domainrecord on Linode Cloud Manager is created, updated, or deleted based on operation passed -func EnsureDNSEntries(ctx context.Context, mscope *scope.MachineScope, operation string) error { - // Check if instance is a control plane node - if !kutil.IsControlPlaneMachine(mscope.Machine) { - return nil - } - +func EnsureDNSEntries(ctx context.Context, cscope *scope.ClusterScope, operation string) error { // Get the public IP that was assigned var dnss DNSEntries - dnsEntries, err := dnss.getDNSEntriesToEnsure(mscope) + dnsEntries, err := dnss.getDNSEntriesToEnsure(cscope) if err != nil { return err } - if mscope.LinodeCluster.Spec.Network.DNSProvider == "akamai" { - return EnsureAkamaiDNSEntries(ctx, mscope, operation, dnsEntries) + if len(dnsEntries) == 0 { + return nil + } + + if cscope.LinodeCluster.Spec.Network.DNSProvider == "akamai" { + for _, dnsEntry := range dnsEntries { + if err := EnsureAkamaiDNSEntries(ctx, cscope, operation, dnsEntry); err != nil { + return err + } + } + } else { + for _, dnsEntry := range dnsEntries { + if err := EnsureLinodeDNSEntries(ctx, cscope, operation, dnsEntry); err != nil { + return err + } + } } - return EnsureLinodeDNSEntries(ctx, mscope, operation, dnsEntries) + return nil } // EnsureLinodeDNSEntries ensures the domainrecord on Linode Cloud Manager is created, updated, or deleted based on operation passed -func EnsureLinodeDNSEntries(ctx context.Context, mscope *scope.MachineScope, operation string, dnsEntries []DNSOptions) error { +func EnsureLinodeDNSEntries(ctx context.Context, cscope *scope.ClusterScope, operation string, dnsEntry DNSOptions) error { // Get domainID from domain name - domainID, err := GetDomainID(ctx, mscope) + domainID, err := GetDomainID(ctx, cscope) if err != nil { return err } - for _, dnsEntry := range dnsEntries { - if operation == "delete" { - if err := DeleteDomainRecord(ctx, mscope, domainID, dnsEntry); err != nil { - return err - } - continue - } - if err := CreateDomainRecord(ctx, mscope, domainID, dnsEntry); err != nil { - return err - } + if operation == "delete" { + return DeleteDomainRecord(ctx, cscope, domainID, dnsEntry) } - - return nil + return CreateDomainRecord(ctx, cscope, domainID, dnsEntry) } // EnsureAkamaiDNSEntries ensures the domainrecord on Akamai EDGE DNS is created, updated, or deleted based on operation passed -func EnsureAkamaiDNSEntries(ctx context.Context, mscope *scope.MachineScope, operation string, dnsEntries []DNSOptions) error { - linodeCluster := mscope.LinodeCluster +func EnsureAkamaiDNSEntries(ctx context.Context, cscope *scope.ClusterScope, operation string, dnsEntry DNSOptions) error { + linodeCluster := cscope.LinodeCluster linodeClusterNetworkSpec := linodeCluster.Spec.Network rootDomain := linodeClusterNetworkSpec.DNSRootDomain - akaDNSClient := mscope.AkamaiDomainsClient - fqdn := getSubDomain(mscope) + "." + rootDomain + akaDNSClient := cscope.AkamaiDomainsClient + fqdn := getSubDomain(cscope) + "." + rootDomain - for _, dnsEntry := range dnsEntries { - recordBody, err := akaDNSClient.GetRecord(ctx, rootDomain, fqdn, string(dnsEntry.DNSRecordType)) - if err != nil { - if !strings.Contains(err.Error(), "Not Found") { - return err - } - if operation == "create" { - if err := akaDNSClient.CreateRecord( - ctx, - &dns.RecordBody{ - Name: fqdn, - RecordType: string(dnsEntry.DNSRecordType), - TTL: dnsEntry.DNSTTLSec, - Target: []string{dnsEntry.Target}, - }, rootDomain); err != nil { - return err - } - } - continue + // Get the record for the root domain and fqdn + recordBody, err := akaDNSClient.GetRecord(ctx, rootDomain, fqdn, string(dnsEntry.DNSRecordType)) + + if err != nil { + if !strings.Contains(err.Error(), "Not Found") { + return err } - if operation == "delete" { - switch { - case len(recordBody.Target) > 1: - recordBody.Target = removeElement( - recordBody.Target, - strings.Replace(dnsEntry.Target, "::", ":0:0:", 8), //nolint:mnd // 8 for 8 octest - ) - if err := akaDNSClient.UpdateRecord(ctx, recordBody, rootDomain); err != nil { - return err - } - continue - default: - if err := akaDNSClient.DeleteRecord(ctx, recordBody, rootDomain); err != nil { - return err - } + // Record was not found - if operation is not "create", nothing to do + if operation != "create" { + return nil + } + // Create record + return createAkamaiEntry(ctx, akaDNSClient, dnsEntry, fqdn, rootDomain) + } + if recordBody == nil { + return fmt.Errorf("akamai dns returned empty dns record") + } + + // if operation is delete and we got the record, delete it + if operation == "delete" { + return deleteAkamaiEntry(ctx, cscope, recordBody, dnsEntry) + } + // if operation is create and we got the record, update it + // Check if the target already exists in the target list + for _, target := range recordBody.Target { + if recordBody.RecordType == "TXT" { + if strings.Contains(target, dnsEntry.Target) { + return nil } } else { - recordBody.Target = append(recordBody.Target, dnsEntry.Target) - if err := akaDNSClient.UpdateRecord(ctx, recordBody, rootDomain); err != nil { - return err + if slices.Equal(net.ParseIP(target), net.ParseIP(dnsEntry.Target)) { + return nil } } } - return nil + // Target doesn't exist so lets append it to the existing list and update it + recordBody.Target = append(recordBody.Target, dnsEntry.Target) + return akaDNSClient.UpdateRecord(ctx, recordBody, rootDomain) +} + +func createAkamaiEntry(ctx context.Context, client clients.AkamClient, dnsEntry DNSOptions, fqdn, rootDomain string) error { + return client.CreateRecord( + ctx, + &dns.RecordBody{ + Name: fqdn, + RecordType: string(dnsEntry.DNSRecordType), + TTL: dnsEntry.DNSTTLSec, + Target: []string{dnsEntry.Target}, + }, + rootDomain, + ) +} + +func deleteAkamaiEntry(ctx context.Context, cscope *scope.ClusterScope, recordBody *dns.RecordBody, dnsEntry DNSOptions) error { + linodeCluster := cscope.LinodeCluster + linodeClusterNetworkSpec := linodeCluster.Spec.Network + rootDomain := linodeClusterNetworkSpec.DNSRootDomain + // If record is A/AAAA type, verify ownership + if dnsEntry.DNSRecordType != linodego.RecordTypeTXT { + isOwner, err := IsAkamaiDomainRecordOwner(ctx, cscope) + if err != nil { + return err + } + if !isOwner { + return fmt.Errorf("the domain record is not owned by this entity. wont delete") + } + } + switch { + case len(recordBody.Target) > 1: + recordBody.Target = removeElement( + recordBody.Target, + // Linode DNS API formats the IPv6 IPs using :: for :0:0: while the address from the LinodeMachine status keeps it as is + // So we need to match that + strings.Replace(dnsEntry.Target, "::", ":0:0:", 8), //nolint:mnd // 8 for 8 octest + ) + return cscope.AkamaiDomainsClient.UpdateRecord(ctx, recordBody, rootDomain) + default: + return cscope.AkamaiDomainsClient.DeleteRecord(ctx, recordBody, rootDomain) + } } func removeElement(stringList []string, elemToRemove string) []string { @@ -139,46 +175,48 @@ func removeElement(stringList []string, elemToRemove string) []string { } // getDNSEntriesToEnsure return DNS entries to create/delete -func (d *DNSEntries) getDNSEntriesToEnsure(mscope *scope.MachineScope) ([]DNSOptions, error) { +func (d *DNSEntries) getDNSEntriesToEnsure(cscope *scope.ClusterScope) ([]DNSOptions, error) { d.mux.Lock() defer d.mux.Unlock() dnsTTLSec := rutil.DefaultDNSTTLSec - if mscope.LinodeCluster.Spec.Network.DNSTTLSec != 0 { - dnsTTLSec = mscope.LinodeCluster.Spec.Network.DNSTTLSec + if cscope.LinodeCluster.Spec.Network.DNSTTLSec != 0 { + dnsTTLSec = cscope.LinodeCluster.Spec.Network.DNSTTLSec } - if mscope.LinodeMachine.Status.Addresses == nil { - return nil, fmt.Errorf("no addresses available on the LinodeMachine resource") - } - subDomain := getSubDomain(mscope) + subDomain := getSubDomain(cscope) - for _, IPs := range mscope.LinodeMachine.Status.Addresses { - recordType := linodego.RecordTypeA - if IPs.Type != v1beta1.MachineExternalIP { - continue - } - addr, err := netip.ParseAddr(IPs.Address) - if err != nil { - return nil, fmt.Errorf("not a valid IP %w", err) + for _, eachMachine := range cscope.LinodeMachines.Items { + for _, IPs := range eachMachine.Status.Addresses { + recordType := linodego.RecordTypeA + if IPs.Type != v1beta1.MachineExternalIP { + continue + } + addr, err := netip.ParseAddr(IPs.Address) + if err != nil { + return nil, fmt.Errorf("not a valid IP %w", err) + } + if !addr.Is4() { + recordType = linodego.RecordTypeAAAA + } + d.options = append(d.options, DNSOptions{subDomain, IPs.Address, recordType, dnsTTLSec}) } - if !addr.Is4() { - recordType = linodego.RecordTypeAAAA + if len(d.options) == 0 { + continue } - d.options = append(d.options, DNSOptions{subDomain, IPs.Address, recordType, dnsTTLSec}) } - d.options = append(d.options, DNSOptions{subDomain, mscope.LinodeMachine.Name, linodego.RecordTypeTXT, dnsTTLSec}) + d.options = append(d.options, DNSOptions{subDomain, cscope.LinodeCluster.Name, linodego.RecordTypeTXT, dnsTTLSec}) return d.options, nil } // GetDomainID gets the domains linode id -func GetDomainID(ctx context.Context, mscope *scope.MachineScope) (int, error) { - rootDomain := mscope.LinodeCluster.Spec.Network.DNSRootDomain +func GetDomainID(ctx context.Context, cscope *scope.ClusterScope) (int, error) { + rootDomain := cscope.LinodeCluster.Spec.Network.DNSRootDomain filter, err := json.Marshal(map[string]string{"domain": rootDomain}) if err != nil { return 0, err } - domains, err := mscope.LinodeDomainsClient.ListDomains(ctx, linodego.NewListOptions(0, string(filter))) + domains, err := cscope.LinodeDomainsClient.ListDomains(ctx, linodego.NewListOptions(0, string(filter))) if err != nil { return 0, err } @@ -189,21 +227,21 @@ func GetDomainID(ctx context.Context, mscope *scope.MachineScope) (int, error) { return domains[0].ID, nil } -func CreateDomainRecord(ctx context.Context, mscope *scope.MachineScope, domainID int, dnsEntry DNSOptions) error { +func CreateDomainRecord(ctx context.Context, cscope *scope.ClusterScope, domainID int, dnsEntry DNSOptions) error { // Check if domain record exists for this IP and name combo filter, err := json.Marshal(map[string]interface{}{"name": dnsEntry.Hostname, "target": dnsEntry.Target, "type": dnsEntry.DNSRecordType}) if err != nil { return err } - domainRecords, err := mscope.LinodeDomainsClient.ListDomainRecords(ctx, domainID, linodego.NewListOptions(0, string(filter))) + domainRecords, err := cscope.LinodeDomainsClient.ListDomainRecords(ctx, domainID, linodego.NewListOptions(0, string(filter))) if err != nil { return err } // If record doesnt exist, create it if len(domainRecords) == 0 { - if _, err := mscope.LinodeDomainsClient.CreateDomainRecord( + if _, err := cscope.LinodeDomainsClient.CreateDomainRecord( ctx, domainID, linodego.DomainRecordCreateOptions{ @@ -219,14 +257,14 @@ func CreateDomainRecord(ctx context.Context, mscope *scope.MachineScope, domainI return nil } -func DeleteDomainRecord(ctx context.Context, mscope *scope.MachineScope, domainID int, dnsEntry DNSOptions) error { +func DeleteDomainRecord(ctx context.Context, cscope *scope.ClusterScope, domainID int, dnsEntry DNSOptions) error { // Check if domain record exists for this IP and name combo filter, err := json.Marshal(map[string]interface{}{"name": dnsEntry.Hostname, "target": dnsEntry.Target, "type": dnsEntry.DNSRecordType}) if err != nil { return err } - domainRecords, err := mscope.LinodeDomainsClient.ListDomainRecords(ctx, domainID, linodego.NewListOptions(0, string(filter))) + domainRecords, err := cscope.LinodeDomainsClient.ListDomainRecords(ctx, domainID, linodego.NewListOptions(0, string(filter))) if err != nil { return err } @@ -238,7 +276,7 @@ func DeleteDomainRecord(ctx context.Context, mscope *scope.MachineScope, domainI // If record is A/AAAA type, verify ownership if dnsEntry.DNSRecordType != linodego.RecordTypeTXT { - isOwner, err := IsDomainRecordOwner(ctx, mscope, dnsEntry.Hostname, domainID) + isOwner, err := IsLinodeDomainRecordOwner(ctx, cscope, dnsEntry.Hostname, domainID) if err != nil { return err } @@ -248,41 +286,52 @@ func DeleteDomainRecord(ctx context.Context, mscope *scope.MachineScope, domainI } // Delete record - if deleteErr := mscope.LinodeDomainsClient.DeleteDomainRecord(ctx, domainID, domainRecords[0].ID); deleteErr != nil { - return deleteErr - } - return nil + return cscope.LinodeDomainsClient.DeleteDomainRecord(ctx, domainID, domainRecords[0].ID) } -func IsDomainRecordOwner(ctx context.Context, mscope *scope.MachineScope, hostname string, domainID int) (bool, error) { +func IsLinodeDomainRecordOwner(ctx context.Context, cscope *scope.ClusterScope, hostname string, domainID int) (bool, error) { // Check if domain record exists - filter, err := json.Marshal(map[string]interface{}{"name": hostname, "target": mscope.LinodeMachine.Name, "type": linodego.RecordTypeTXT}) + filter, err := json.Marshal(map[string]interface{}{"name": hostname, "type": linodego.RecordTypeTXT}) if err != nil { return false, err } - domainRecords, err := mscope.LinodeDomainsClient.ListDomainRecords(ctx, domainID, linodego.NewListOptions(0, string(filter))) + domainRecords, err := cscope.LinodeDomainsClient.ListDomainRecords(ctx, domainID, linodego.NewListOptions(0, string(filter))) if err != nil { return false, err } // If record exists, update it if len(domainRecords) == 0 { - return false, fmt.Errorf("no txt record %s found with value %s for machine %s", hostname, mscope.LinodeMachine.Name, mscope.LinodeMachine.Name) + return false, fmt.Errorf("no txt record %s found", hostname) + } + + return true, nil +} + +func IsAkamaiDomainRecordOwner(ctx context.Context, cscope *scope.ClusterScope) (bool, error) { + linodeCluster := cscope.LinodeCluster + linodeClusterNetworkSpec := linodeCluster.Spec.Network + rootDomain := linodeClusterNetworkSpec.DNSRootDomain + akaDNSClient := cscope.AkamaiDomainsClient + fqdn := getSubDomain(cscope) + "." + rootDomain + recordBody, err := akaDNSClient.GetRecord(ctx, rootDomain, fqdn, string(linodego.RecordTypeTXT)) + if err != nil || recordBody == nil { + return false, fmt.Errorf("no txt record %s found", fqdn) } return true, nil } -func getSubDomain(mscope *scope.MachineScope) (subDomain string) { - if mscope.LinodeCluster.Spec.Network.DNSSubDomainOverride != "" { - subDomain = mscope.LinodeCluster.Spec.Network.DNSSubDomainOverride +func getSubDomain(cscope *scope.ClusterScope) (subDomain string) { + if cscope.LinodeCluster.Spec.Network.DNSSubDomainOverride != "" { + subDomain = cscope.LinodeCluster.Spec.Network.DNSSubDomainOverride } else { uniqueID := "" - if mscope.LinodeCluster.Spec.Network.DNSUniqueIdentifier != "" { - uniqueID = "-" + mscope.LinodeCluster.Spec.Network.DNSUniqueIdentifier + if cscope.LinodeCluster.Spec.Network.DNSUniqueIdentifier != "" { + uniqueID = "-" + cscope.LinodeCluster.Spec.Network.DNSUniqueIdentifier } - subDomain = mscope.LinodeCluster.Name + uniqueID + subDomain = cscope.LinodeCluster.Name + uniqueID } return subDomain } diff --git a/cloud/services/domains_test.go b/cloud/services/domains_test.go index 5edebd9a0..291626463 100644 --- a/cloud/services/domains_test.go +++ b/cloud/services/domains_test.go @@ -23,23 +23,14 @@ func TestAddIPToEdgeDNS(t *testing.T) { t.Parallel() tests := []struct { name string - machineScope *scope.MachineScope + clusterScope *scope.ClusterScope expects func(*mock.MockAkamClient) expectK8sClient func(*mock.MockK8sClient) expectedError error }{ { name: "Success - If DNS Provider is akamai", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -60,23 +51,28 @@ func TestAddIPToEdgeDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), }, - { - Type: "ExternalIP", - Address: "fd00::", + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -93,16 +89,7 @@ func TestAddIPToEdgeDNS(t *testing.T) { }, { name: "Faiure - Error in creating records", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -123,23 +110,28 @@ func TestAddIPToEdgeDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", }, - { - Type: "ExternalIP", - Address: "fd00::", + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -164,16 +156,18 @@ func TestAddIPToEdgeDNS(t *testing.T) { defer ctrl.Finish() MockAkamClient := mock.NewMockAkamClient(ctrl) - testcase.machineScope.AkamaiDomainsClient = MockAkamClient + testcase.clusterScope.AkamaiDomainsClient = MockAkamClient testcase.expects(MockAkamClient) MockK8sClient := mock.NewMockK8sClient(ctrl) - testcase.machineScope.Client = MockK8sClient + testcase.clusterScope.Client = MockK8sClient testcase.expectK8sClient(MockK8sClient) - err := EnsureDNSEntries(context.Background(), testcase.machineScope, "create") - if err != nil || testcase.expectedError != nil { + err := EnsureDNSEntries(context.Background(), testcase.clusterScope, "create") + if testcase.expectedError != nil { require.ErrorContains(t, err, testcase.expectedError.Error()) + } else { + assert.NoError(t, err) } }) } @@ -185,23 +179,14 @@ func TestRemoveIPFromEdgeDNS(t *testing.T) { name string listOfIPS []string expectedList []string - machineScope *scope.MachineScope + clusterScope *scope.ClusterScope expects func(*mock.MockAkamClient) expectK8sClient func(*mock.MockK8sClient) expectedError error }{ { name: "Success - If DNS Provider is akamai", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -222,23 +207,28 @@ func TestRemoveIPFromEdgeDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), }, - { - Type: "ExternalIP", - Address: "fd00::", + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -252,6 +242,7 @@ func TestRemoveIPFromEdgeDNS(t *testing.T) { TTL: 30, Target: []string{"10.10.10.10"}, }, nil).AnyTimes() + mockClient.EXPECT().UpdateRecord(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() mockClient.EXPECT().DeleteRecord(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() }, expectedError: nil, @@ -262,16 +253,7 @@ func TestRemoveIPFromEdgeDNS(t *testing.T) { }, { name: "Failure - API Error", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -292,23 +274,28 @@ func TestRemoveIPFromEdgeDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", }, - { - Type: "ExternalIP", - Address: "fd00::", + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -335,14 +322,14 @@ func TestRemoveIPFromEdgeDNS(t *testing.T) { defer ctrl.Finish() MockAkamClient := mock.NewMockAkamClient(ctrl) - testcase.machineScope.AkamaiDomainsClient = MockAkamClient + testcase.clusterScope.AkamaiDomainsClient = MockAkamClient testcase.expects(MockAkamClient) MockK8sClient := mock.NewMockK8sClient(ctrl) - testcase.machineScope.Client = MockK8sClient + testcase.clusterScope.Client = MockK8sClient testcase.expectK8sClient(MockK8sClient) - err := EnsureDNSEntries(context.Background(), testcase.machineScope, "delete") + err := EnsureDNSEntries(context.Background(), testcase.clusterScope, "delete") if err != nil || testcase.expectedError != nil { require.ErrorContains(t, err, testcase.expectedError.Error()) } @@ -355,7 +342,7 @@ func TestAddIPToDNS(t *testing.T) { t.Parallel() tests := []struct { name string - machineScope *scope.MachineScope + clusterScope *scope.ClusterScope expects func(*mock.MockLinodeClient) expectK8sClient func(*mock.MockK8sClient) expectedDomainRecord *linodego.DomainRecord @@ -363,16 +350,7 @@ func TestAddIPToDNS(t *testing.T) { }{ { name: "Success - If the machine is a control plane node, add the IP to the Domain", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -392,23 +370,28 @@ func TestAddIPToDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), }, - { - Type: "ExternalIP", - Address: "fd00::", + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -436,16 +419,7 @@ func TestAddIPToDNS(t *testing.T) { }, { name: "Success - use custom dnsttlsec", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -466,23 +440,28 @@ func TestAddIPToDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), }, - { - Type: "ExternalIP", - Address: "fd00::", + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -510,16 +489,7 @@ func TestAddIPToDNS(t *testing.T) { }, { name: "Error - CreateDomainRecord() returns an error", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -539,23 +509,28 @@ func TestAddIPToDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), }, - { - Type: "ExternalIP", - Address: "fd00::", + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -578,16 +553,7 @@ func TestAddIPToDNS(t *testing.T) { }, { name: "Success - If the machine is a control plane node and record already exists, leave it alone", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -607,23 +573,28 @@ func TestAddIPToDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", }, - { - Type: "ExternalIP", - Address: "fd00::", + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -652,16 +623,7 @@ func TestAddIPToDNS(t *testing.T) { }, { name: "Failure - Failed to get domain records", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -681,23 +643,28 @@ func TestAddIPToDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), }, - { - Type: "ExternalIP", - Address: "fd00::", + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -719,16 +686,7 @@ func TestAddIPToDNS(t *testing.T) { }, { name: "Error - no public ip set", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -748,16 +706,21 @@ func TestAddIPToDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: nil, + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: nil, + }, + }, }, }, }, @@ -768,24 +731,23 @@ func TestAddIPToDNS(t *testing.T) { Domain: "lkedevs.net", }, }, nil).AnyTimes() + mockClient.EXPECT().ListDomainRecords(gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.DomainRecord{ + { + ID: 1234, + Type: "A", + Name: "test-cluster", + TTLSec: 30, + }, + }, nil).AnyTimes() }, - expectedError: fmt.Errorf("no addresses available on the LinodeMachine resource"), + expectedError: nil, expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() }, }, { name: "Error - no domain found when creating", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -805,23 +767,28 @@ func TestAddIPToDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), }, - { - Type: "ExternalIP", - Address: "fd00::", + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -852,17 +819,17 @@ func TestAddIPToDNS(t *testing.T) { MockLinodeClient := mock.NewMockLinodeClient(ctrl) MockLinodeDomainsClient := mock.NewMockLinodeClient(ctrl) - testcase.machineScope.LinodeClient = MockLinodeClient - testcase.machineScope.LinodeDomainsClient = MockLinodeClient + testcase.clusterScope.LinodeClient = MockLinodeClient + testcase.clusterScope.LinodeDomainsClient = MockLinodeClient testcase.expects(MockLinodeClient) testcase.expects(MockLinodeDomainsClient) MockK8sClient := mock.NewMockK8sClient(ctrl) - testcase.machineScope.Client = MockK8sClient + testcase.clusterScope.Client = MockK8sClient testcase.expectK8sClient(MockK8sClient) - err := EnsureDNSEntries(context.Background(), testcase.machineScope, "create") + err := EnsureDNSEntries(context.Background(), testcase.clusterScope, "create") if testcase.expectedError != nil { assert.ErrorContains(t, err, testcase.expectedError.Error()) } @@ -874,23 +841,14 @@ func TestDeleteIPFromDNS(t *testing.T) { t.Parallel() tests := []struct { name string - machineScope *scope.MachineScope + clusterScope *scope.ClusterScope expects func(*mock.MockLinodeClient) expectK8sClient func(*mock.MockK8sClient) expectedError error }{ { name: "Success - Deleted the record", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -910,23 +868,28 @@ func TestDeleteIPFromDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), }, - { - Type: "ExternalIP", - Address: "fd00::", + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -956,16 +919,7 @@ func TestDeleteIPFromDNS(t *testing.T) { }, { name: "Failure - Deleting the record fails", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -985,23 +939,28 @@ func TestDeleteIPFromDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", }, - { - Type: "ExternalIP", - Address: "fd00::", + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -1030,17 +989,8 @@ func TestDeleteIPFromDNS(t *testing.T) { }, }, { - name: "Error - failed to get machine ip", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + name: "Error - failed to get machine", + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -1060,34 +1010,32 @@ func TestDeleteIPFromDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", + }, + expects: func(mockClient *mock.MockLinodeClient) { + mockClient.EXPECT().ListDomains(gomock.Any(), gomock.Any()).Return([]linodego.Domain{ + { + ID: 1, + Domain: "lkedevs.net", }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), + }, nil).AnyTimes() + mockClient.EXPECT().ListDomainRecords(gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.DomainRecord{ + { + ID: 1234, + Type: "A", + Name: "test-cluster", + TTLSec: 30, }, - }, + }, nil).AnyTimes() + mockClient.EXPECT().DeleteDomainRecord(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() }, - expects: func(mockClient *mock.MockLinodeClient) {}, - expectedError: fmt.Errorf("no addresses available on the LinodeMachine resource"), + expectedError: nil, expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() }, }, { name: "Error - failure in getting domain", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -1107,23 +1055,28 @@ func TestDeleteIPFromDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", }, - { - Type: "ExternalIP", - Address: "fd00::", + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -1139,16 +1092,7 @@ func TestDeleteIPFromDNS(t *testing.T) { }, { name: "Error - no domain found when deleting", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -1168,23 +1112,28 @@ func TestDeleteIPFromDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), }, - { - Type: "ExternalIP", - Address: "fd00::", + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -1205,16 +1154,7 @@ func TestDeleteIPFromDNS(t *testing.T) { }, { name: "Error - error listing domains when deleting", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -1234,23 +1174,28 @@ func TestDeleteIPFromDNS(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - Status: infrav1alpha2.LinodeMachineStatus{ - Addresses: []clusterv1.MachineAddress{ - { - Type: "ExternalIP", - Address: "10.10.10.10", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), }, - { - Type: "ExternalIP", - Address: "fd00::", + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "ExternalIP", + Address: "10.10.10.10", + }, + { + Type: "ExternalIP", + Address: "fd00::", + }, + }, }, }, }, @@ -1282,17 +1227,17 @@ func TestDeleteIPFromDNS(t *testing.T) { MockLinodeClient := mock.NewMockLinodeClient(ctrl) MockLinodeDomainsClient := mock.NewMockLinodeClient(ctrl) - testcase.machineScope.LinodeClient = MockLinodeClient - testcase.machineScope.LinodeDomainsClient = MockLinodeClient + testcase.clusterScope.LinodeClient = MockLinodeClient + testcase.clusterScope.LinodeDomainsClient = MockLinodeClient testcase.expects(MockLinodeClient) testcase.expects(MockLinodeDomainsClient) MockK8sClient := mock.NewMockK8sClient(ctrl) - testcase.machineScope.Client = MockK8sClient + testcase.clusterScope.Client = MockK8sClient testcase.expectK8sClient(MockK8sClient) - err := EnsureDNSEntries(context.Background(), testcase.machineScope, "delete") + err := EnsureDNSEntries(context.Background(), testcase.clusterScope, "delete") if testcase.expectedError != nil { assert.ErrorContains(t, err, testcase.expectedError.Error()) } diff --git a/cloud/services/loadbalancers.go b/cloud/services/loadbalancers.go index a545a3974..b205c5bc4 100644 --- a/cloud/services/loadbalancers.go +++ b/cloud/services/loadbalancers.go @@ -5,11 +5,13 @@ import ( "errors" "fmt" "net/http" + "strings" "github.com/go-logr/logr" "github.com/linode/linodego" - kutil "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/cluster-api/api/v1beta1" + "github.com/linode/cluster-api-provider-linode/api/v1alpha2" "github.com/linode/cluster-api-provider-linode/cloud/scope" "github.com/linode/cluster-api-provider-linode/util" ) @@ -111,128 +113,109 @@ func EnsureNodeBalancerConfigs( return nbConfigs, nil } -// AddNodeToNB adds a backend Node on the Node Balancer configuration -func AddNodeToNB( - ctx context.Context, - logger logr.Logger, - machineScope *scope.MachineScope, -) error { - // Update the NB backend with the new instance if it's a control plane node - if !kutil.IsControlPlaneMachine(machineScope.Machine) { - return nil - } - - instanceID, err := util.GetInstanceID(machineScope.LinodeMachine.Spec.ProviderID) - if err != nil { - logger.Error(err, "Failed to parse instance ID from provider ID") - return err - } - // Get the private IP that was assigned - addresses, err := machineScope.LinodeClient.GetInstanceIPAddresses(ctx, instanceID) - if err != nil { - logger.Error(err, "Failed get instance IP addresses") - - return err - } - if len(addresses.IPv4.Private) == 0 { - err := errors.New("no private IP address") - logger.Error(err, "no private IPV4 addresses set for LinodeInstance") - - return err - } - +// AddNodesToNB adds backend Nodes on the Node Balancer configuration +func AddNodesToNB(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope, eachMachine v1alpha2.LinodeMachine) error { apiserverLBPort := DefaultApiserverLBPort - if machineScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort != 0 { - apiserverLBPort = machineScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort + if clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort != 0 { + apiserverLBPort = clusterScope.LinodeCluster.Spec.Network.ApiserverLoadBalancerPort } - if machineScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID == nil { - err := errors.New("nil NodeBalancer Config ID") - logger.Error(err, "config ID for NodeBalancer is nil") - - return err + if clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID == nil { + return errors.New("nil NodeBalancer Config ID") } - _, err = machineScope.LinodeClient.CreateNodeBalancerNode( + nodeBalancerNodes, err := clusterScope.LinodeClient.ListNodeBalancerNodes( ctx, - *machineScope.LinodeCluster.Spec.Network.NodeBalancerID, - *machineScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID, - linodego.NodeBalancerNodeCreateOptions{ - Label: machineScope.Cluster.Name, - Address: fmt.Sprintf("%s:%d", addresses.IPv4.Private[0].Address, apiserverLBPort), - Mode: linodego.ModeAccept, - }, + *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, + *clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID, + &linodego.ListOptions{}, ) if err != nil { - logger.Error(err, "Failed to update Node Balancer") return err } + internalIPFound := false + for _, IPs := range eachMachine.Status.Addresses { + if IPs.Type != v1beta1.MachineInternalIP || !strings.Contains(IPs.Address, "192.168") { + continue + } + internalIPFound = true - for _, portConfig := range machineScope.LinodeCluster.Spec.Network.AdditionalPorts { - _, err = machineScope.LinodeClient.CreateNodeBalancerNode( - ctx, - *machineScope.LinodeCluster.Spec.Network.NodeBalancerID, - *portConfig.NodeBalancerConfigID, - linodego.NodeBalancerNodeCreateOptions{ - Label: machineScope.Cluster.Name, - Address: fmt.Sprintf("%s:%d", addresses.IPv4.Private[0].Address, portConfig.Port), - Mode: linodego.ModeAccept, - }, - ) - if err != nil { - logger.Error(err, "Failed to update Node Balancer") - return err + // Set the port number and NB config ID for standard ports + portsToBeAdded := make([]map[string]int, 0) + standardPort := map[string]int{"configID": *clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID, "port": apiserverLBPort} + portsToBeAdded = append(portsToBeAdded, standardPort) + + // Set the port number and NB config ID for any additional ports + for _, portConfig := range clusterScope.LinodeCluster.Spec.Network.AdditionalPorts { + portsToBeAdded = append(portsToBeAdded, map[string]int{"configID": *portConfig.NodeBalancerConfigID, "port": portConfig.Port}) + } + + // Cycle through all ports to be added + for _, ports := range portsToBeAdded { + ipPortComboExists := false + for _, nodes := range nodeBalancerNodes { + // Create the node if the IP:Port combination does not exist + if nodes.Address == fmt.Sprintf("%s:%d", IPs.Address, ports["port"]) { + ipPortComboExists = true + break + } + } + if !ipPortComboExists { + _, err := clusterScope.LinodeClient.CreateNodeBalancerNode( + ctx, + *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, + ports["configID"], + linodego.NodeBalancerNodeCreateOptions{ + Label: clusterScope.Cluster.Name, + Address: fmt.Sprintf("%s:%d", IPs.Address, ports["port"]), + Mode: linodego.ModeAccept, + }, + ) + if err != nil { + return err + } + } } } + if !internalIPFound { + return errors.New("no private IP address") + } return nil } -// DeleteNodeFromNB removes a backend Node from the Node Balancer configuration -func DeleteNodeFromNB( - ctx context.Context, - logger logr.Logger, - machineScope *scope.MachineScope, -) error { - // Update the NB to remove the node if it's a control plane node - if !kutil.IsControlPlaneMachine(machineScope.Machine) { - return nil - } - - if machineScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" { +// DeleteNodesFromNB removes backend Nodes from the Node Balancer configuration +func DeleteNodesFromNB(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { + if clusterScope.LinodeCluster.Spec.ControlPlaneEndpoint.Host == "" { logger.Info("NodeBalancer already deleted, no NodeBalancer backend Node to remove") - return nil } - instanceID, err := util.GetInstanceID(machineScope.LinodeMachine.Spec.ProviderID) - if err != nil { - logger.Error(err, "Failed to parse instance ID from provider ID") - return err - } - err = machineScope.LinodeClient.DeleteNodeBalancerNode( - ctx, - *machineScope.LinodeCluster.Spec.Network.NodeBalancerID, - *machineScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID, - instanceID, - ) - if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil { - logger.Error(err, "Failed to update Node Balancer") - - return err - } - - for _, portConfig := range machineScope.LinodeCluster.Spec.Network.AdditionalPorts { - err = machineScope.LinodeClient.DeleteNodeBalancerNode( + for _, eachMachine := range clusterScope.LinodeMachines.Items { + err := clusterScope.LinodeClient.DeleteNodeBalancerNode( ctx, - *machineScope.LinodeCluster.Spec.Network.NodeBalancerID, - *portConfig.NodeBalancerConfigID, - instanceID, + *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, + *clusterScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID, + *eachMachine.Spec.InstanceID, ) if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil { logger.Error(err, "Failed to update Node Balancer") + return err } + + for _, portConfig := range clusterScope.LinodeCluster.Spec.Network.AdditionalPorts { + err = clusterScope.LinodeClient.DeleteNodeBalancerNode( + ctx, + *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID, + *portConfig.NodeBalancerConfigID, + *eachMachine.Spec.InstanceID, + ) + if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil { + logger.Error(err, "Failed to update Node Balancer") + return err + } + } } return nil diff --git a/cloud/services/loadbalancers_test.go b/cloud/services/loadbalancers_test.go index 28ca3f1b0..e22089aac 100644 --- a/cloud/services/loadbalancers_test.go +++ b/cloud/services/loadbalancers_test.go @@ -414,23 +414,14 @@ func TestAddNodeToNBConditions(t *testing.T) { tests := []struct { name string - machineScope *scope.MachineScope + clusterScope *scope.ClusterScope expectedError error expects func(*mock.MockLinodeClient) expectK8sClient func(*mock.MockK8sClient) }{ { name: "Error - ApiserverNodeBalancerConfigID is not set", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ LinodeCluster: &infrav1alpha2.LinodeCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -444,91 +435,61 @@ func TestAddNodeToNBConditions(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - }, - }, - expectedError: fmt.Errorf("nil NodeBalancer Config ID"), - expects: func(mockClient *mock.MockLinodeClient) { - mockClient.EXPECT().GetInstanceIPAddresses(gomock.Any(), gomock.Any()).Return(&linodego.InstanceIPAddressResponse{ - IPv4: &linodego.InstanceIPv4Response{ - Private: []*linodego.InstanceIP{ - { - Address: "1.2.3.4", + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), }, }, }, - }, nil) + }, }, + expectedError: fmt.Errorf("nil NodeBalancer Config ID"), + expects: func(mockClient *mock.MockLinodeClient) {}, expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() }, }, { name: "Error - No private IP addresses were set", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ + clusterScope: &scope.ClusterScope{ + LinodeCluster: &infrav1alpha2.LinodeCluster{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", + Name: "test-cluster", UID: "test-uid", }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - }, - }, - expectedError: fmt.Errorf("no private IP address"), - expects: func(mockClient *mock.MockLinodeClient) { - mockClient.EXPECT().GetInstanceIPAddresses(gomock.Any(), gomock.Any()).Return(&linodego.InstanceIPAddressResponse{ - IPv4: &linodego.InstanceIPv4Response{ - Private: []*linodego.InstanceIP{}, - }, - }, nil) - }, - expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { - mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() - }, - }, - { - name: "Error - GetInstanceIPAddresses() returns an error", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", + Spec: infrav1alpha2.LinodeClusterSpec{ + Network: infrav1alpha2.NetworkSpec{ + NodeBalancerID: ptr.To(1234), + ApiserverNodeBalancerConfigID: ptr.To(1234), + ApiserverLoadBalancerPort: DefaultApiserverLBPort, }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, + }, }, }, }, - expectedError: fmt.Errorf("could not get instance IP addresses"), + expectedError: fmt.Errorf("no private IP address"), expects: func(mockClient *mock.MockLinodeClient) { - mockClient.EXPECT().GetInstanceIPAddresses(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("could not get instance IP addresses")) + mockClient.EXPECT().ListNodeBalancerNodes(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancerNode{}, nil) }, expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() @@ -543,16 +504,18 @@ func TestAddNodeToNBConditions(t *testing.T) { defer ctrl.Finish() MockLinodeClient := mock.NewMockLinodeClient(ctrl) - testcase.machineScope.LinodeClient = MockLinodeClient + testcase.clusterScope.LinodeClient = MockLinodeClient testcase.expects(MockLinodeClient) MockK8sClient := mock.NewMockK8sClient(ctrl) - testcase.machineScope.Client = MockK8sClient + testcase.clusterScope.Client = MockK8sClient testcase.expectK8sClient(MockK8sClient) - err := AddNodeToNB(context.Background(), logr.Discard(), testcase.machineScope) - if testcase.expectedError != nil { - assert.ErrorContains(t, err, testcase.expectedError.Error()) + for _, eachMachine := range testcase.clusterScope.LinodeMachines.Items { + err := AddNodesToNB(context.Background(), logr.Discard(), testcase.clusterScope, eachMachine) + if testcase.expectedError != nil { + assert.ErrorContains(t, err, testcase.expectedError.Error()) + } } }) } @@ -563,44 +526,14 @@ func TestAddNodeToNBFullWorkflow(t *testing.T) { tests := []struct { name string - machineScope *scope.MachineScope + clusterScope *scope.ClusterScope expectedError error expects func(*mock.MockLinodeClient) expectK8sClient func(*mock.MockK8sClient) }{ - { - name: "If the machine is not a control plane node, do nothing", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - }, - Cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - UID: "test-uid", - }, - }, - }, - expects: func(*mock.MockLinodeClient) {}, - expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { - mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() - }, - }, { name: "Success - If the machine is a control plane node, add the node to the NodeBalancer", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -625,27 +558,24 @@ func TestAddNodeToNBFullWorkflow(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, + }, }, }, }, expects: func(mockClient *mock.MockLinodeClient) { - mockClient.EXPECT().GetInstanceIPAddresses(gomock.Any(), gomock.Any()).Return(&linodego.InstanceIPAddressResponse{ - IPv4: &linodego.InstanceIPv4Response{ - Private: []*linodego.InstanceIP{ - { - Address: "1.2.3.4", - }, - }, - }, - }, nil) - mockClient.EXPECT().CreateNodeBalancerNode(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(2).Return(&linodego.NodeBalancerNode{}, nil) + mockClient.EXPECT().ListNodeBalancerNodes(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancerNode{}, nil) + mockClient.EXPECT().CreateNodeBalancerNode(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(&linodego.NodeBalancerNode{}, nil) }, expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() @@ -653,16 +583,7 @@ func TestAddNodeToNBFullWorkflow(t *testing.T) { }, { name: "Error - CreateNodeBalancerNode() returns an error", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", - }, - }, - }, + clusterScope: &scope.ClusterScope{ Cluster: &clusterv1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -681,28 +602,33 @@ func TestAddNodeToNBFullWorkflow(t *testing.T) { }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{ + { + Type: "InternalIP", + Address: "192.168.10.10", + }, + }, + }, + }, }, }, }, - expectedError: fmt.Errorf("could not create node balancer node"), + expectedError: nil, expects: func(mockClient *mock.MockLinodeClient) { - mockClient.EXPECT().GetInstanceIPAddresses(gomock.Any(), gomock.Any()).Return(&linodego.InstanceIPAddressResponse{ - IPv4: &linodego.InstanceIPv4Response{ - Private: []*linodego.InstanceIP{ - { - Address: "1.2.3.4", - }, - }, - }, - }, nil) - mockClient.EXPECT().CreateNodeBalancerNode(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("could not create node balancer node")) + mockClient.EXPECT().ListNodeBalancerNodes(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancerNode{}, nil) + mockClient.EXPECT().CreateNodeBalancerNode(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() }, expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() @@ -717,16 +643,18 @@ func TestAddNodeToNBFullWorkflow(t *testing.T) { defer ctrl.Finish() MockLinodeClient := mock.NewMockLinodeClient(ctrl) - testcase.machineScope.LinodeClient = MockLinodeClient + testcase.clusterScope.LinodeClient = MockLinodeClient testcase.expects(MockLinodeClient) MockK8sClient := mock.NewMockK8sClient(ctrl) - testcase.machineScope.Client = MockK8sClient + testcase.clusterScope.Client = MockK8sClient testcase.expectK8sClient(MockK8sClient) - err := AddNodeToNB(context.Background(), logr.Discard(), testcase.machineScope) - if testcase.expectedError != nil { - assert.ErrorContains(t, err, testcase.expectedError.Error()) + for _, eachMachine := range testcase.clusterScope.LinodeMachines.Items { + err := AddNodesToNB(context.Background(), logr.Discard(), testcase.clusterScope, eachMachine) + if testcase.expectedError != nil { + assert.ErrorContains(t, err, testcase.expectedError.Error()) + } } }) } @@ -737,54 +665,29 @@ func TestDeleteNodeFromNB(t *testing.T) { tests := []struct { name string - machineScope *scope.MachineScope + clusterScope *scope.ClusterScope expectedError error expects func(*mock.MockLinodeClient) expectK8sClient func(*mock.MockK8sClient) }{ // TODO: Add test cases. - { - name: "If the machine is not a control plane node, do nothing", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - }, - Cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - UID: "test-uid", - }, - }, - }, - expects: func(*mock.MockLinodeClient) {}, - expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { - mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() - }, - }, { name: "NodeBalancer is already deleted", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", + clusterScope: &scope.ClusterScope{ + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - }, LinodeCluster: &infrav1alpha2.LinodeCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -802,25 +705,21 @@ func TestDeleteNodeFromNB(t *testing.T) { }, { name: "Success - Delete Node from NodeBalancer", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", + clusterScope: &scope.ClusterScope{ + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - }, LinodeCluster: &infrav1alpha2.LinodeCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -851,25 +750,21 @@ func TestDeleteNodeFromNB(t *testing.T) { }, { name: "Error - Deleting Apiserver Node from NodeBalancer", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", + clusterScope: &scope.ClusterScope{ + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - }, LinodeCluster: &infrav1alpha2.LinodeCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -894,25 +789,21 @@ func TestDeleteNodeFromNB(t *testing.T) { }, { name: "Error - Deleting Konnectivity Node from NodeBalancer", - machineScope: &scope.MachineScope{ - Machine: &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - Labels: map[string]string{ - clusterv1.MachineControlPlaneLabel: "true", + clusterScope: &scope.ClusterScope{ + LinodeMachines: infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + UID: "test-uid", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + InstanceID: ptr.To(123), + }, }, }, }, - LinodeMachine: &infrav1alpha2.LinodeMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-machine", - UID: "test-uid", - }, - Spec: infrav1alpha2.LinodeMachineSpec{ - ProviderID: ptr.To("linode://123"), - }, - }, LinodeCluster: &infrav1alpha2.LinodeCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", @@ -952,14 +843,14 @@ func TestDeleteNodeFromNB(t *testing.T) { defer ctrl.Finish() MockLinodeClient := mock.NewMockLinodeClient(ctrl) - testcase.machineScope.LinodeClient = MockLinodeClient + testcase.clusterScope.LinodeClient = MockLinodeClient testcase.expects(MockLinodeClient) MockK8sClient := mock.NewMockK8sClient(ctrl) - testcase.machineScope.Client = MockK8sClient + testcase.clusterScope.Client = MockK8sClient testcase.expectK8sClient(MockK8sClient) - err := DeleteNodeFromNB(context.Background(), logr.Discard(), testcase.machineScope) + err := DeleteNodesFromNB(context.Background(), logr.Discard(), testcase.clusterScope) if testcase.expectedError != nil { assert.ErrorContains(t, err, testcase.expectedError.Error()) } diff --git a/cmd/main.go b/cmd/main.go index 02af91ec1..c0e70fbf3 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -59,13 +59,12 @@ var ( ) const ( - controllerName = "cluster-api-provider-linode.linode.com" - envK8sNodeName = "K8S_NODE_NAME" - envK8sPodName = "K8S_POD_NAME" - concurrencyDefault = 10 - linodeMachineConcurrencyDefault = 1 - qpsDefault = 20 - burstDefault = 30 + controllerName = "cluster-api-provider-linode.linode.com" + envK8sNodeName = "K8S_NODE_NAME" + envK8sPodName = "K8S_POD_NAME" + concurrencyDefault = 10 + qpsDefault = 20 + burstDefault = 30 ) func init() { @@ -116,7 +115,7 @@ func main() { "Maximum number of queries that should be allowed in one burst from the controller client to the Kubernetes API server. Default 30") flag.IntVar(&linodeClusterConcurrency, "linodecluster-concurrency", concurrencyDefault, "Number of LinodeClusters to process simultaneously. Default 10") - flag.IntVar(&linodeMachineConcurrency, "linodemachine-concurrency", linodeMachineConcurrencyDefault, + flag.IntVar(&linodeMachineConcurrency, "linodemachine-concurrency", concurrencyDefault, "Number of LinodeMachines to process simultaneously. Default 10") flag.IntVar(&linodeObjectStorageBucketConcurrency, "linodeobjectstoragebucket-concurrency", concurrencyDefault, "Number of linodeObjectStorageBuckets to process simultaneously. Default 10") @@ -180,6 +179,7 @@ func main() { Recorder: mgr.GetEventRecorderFor("LinodeClusterReconciler"), WatchFilterValue: clusterWatchFilter, LinodeClientConfig: linodeClientConfig, + DnsClientConfig: dnsClientConfig, }).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: linodeClusterConcurrency}); err != nil { setupLog.Error(err, "unable to create controller", "controller", "LinodeCluster") os.Exit(1) @@ -190,7 +190,6 @@ func main() { Recorder: mgr.GetEventRecorderFor("LinodeMachineReconciler"), WatchFilterValue: machineWatchFilter, LinodeClientConfig: linodeClientConfig, - DnsClientConfig: dnsClientConfig, }).SetupWithManager(mgr, crcontroller.Options{MaxConcurrentReconciles: linodeMachineConcurrency}); err != nil { setupLog.Error(err, "unable to create controller", "controller", "LinodeMachine") os.Exit(1) diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodefirewalls.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodefirewalls.yaml index 6718da454..8adf0f14f 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_linodefirewalls.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_linodefirewalls.yaml @@ -72,6 +72,8 @@ spec: type: integer inboundPolicy: default: ACCEPT + description: InboundPolicy determines if traffic by default should + be ACCEPTed or DROPped. Defaults to ACCEPT if not defined. enum: - ACCEPT - DROP @@ -114,6 +116,8 @@ spec: type: array outboundPolicy: default: ACCEPT + description: OutboundPolicy determines if traffic by default should + be ACCEPTed or DROPped. Defaults to ACCEPT if not defined. enum: - ACCEPT - DROP diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index bf2080b10..9f8aa3cb2 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -25,6 +25,7 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/tools/record" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -49,11 +50,16 @@ import ( "github.com/linode/cluster-api-provider-linode/util/reconciler" ) +const ( + ConditionLoadBalancing clusterv1.ConditionType = "ConditionLoadBalancing" +) + // LinodeClusterReconciler reconciles a LinodeCluster object type LinodeClusterReconciler struct { client.Client Recorder record.EventRecorder LinodeClientConfig scope.ClientConfig + DnsClientConfig scope.ClientConfig WatchFilterValue string ReconcileTimeout time.Duration } @@ -92,10 +98,12 @@ func (r *LinodeClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques clusterScope, err := scope.NewClusterScope( ctx, r.LinodeClientConfig, + r.DnsClientConfig, scope.ClusterScopeParams{ - Client: r.TracedClient(), - Cluster: cluster, - LinodeCluster: linodeCluster, + Client: r.TracedClient(), + Cluster: cluster, + LinodeCluster: linodeCluster, + LinodeMachineList: infrav1alpha2.LinodeMachineList{}, }, ) @@ -127,6 +135,14 @@ func (r *LinodeClusterReconciler) reconcile( } }() + labels := map[string]string{ + clusterv1.ClusterNameLabel: clusterScope.LinodeCluster.Name, + clusterv1.MachineControlPlaneLabel: "", + } + if err := r.TracedClient().List(ctx, &clusterScope.LinodeMachines, client.InNamespace(clusterScope.LinodeCluster.Namespace), client.MatchingLabels(labels)); err != nil { + return res, err + } + // Handle deleted clusters if !clusterScope.LinodeCluster.DeletionTimestamp.IsZero() { if err := r.reconcileDelete(ctx, logger, clusterScope); err != nil { @@ -159,6 +175,19 @@ func (r *LinodeClusterReconciler) reconcile( clusterScope.LinodeCluster.Status.Ready = true conditions.MarkTrue(clusterScope.LinodeCluster, clusterv1.ReadyCondition) + for _, eachMachine := range clusterScope.LinodeMachines.Items { + if len(eachMachine.Status.Addresses) == 0 { + return res, nil + } + } + + err := r.addMachineToLB(ctx, clusterScope) + if err != nil { + logger.Error(err, "Failed to add Linode machine to loadbalancer option") + return retryIfTransient(err) + } + conditions.MarkTrue(clusterScope.LinodeCluster, ConditionLoadBalancing) + return res, nil } @@ -181,7 +210,7 @@ func (r *LinodeClusterReconciler) reconcileCreate(ctx context.Context, logger lo // handle creation for the loadbalancer for the control plane if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "dns" { r.handleDNS(clusterScope) - } else if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "NodeBalancer" || clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "" { + } else { if err := r.handleNBCreate(ctx, logger, clusterScope); err != nil { return err } @@ -256,7 +285,7 @@ func (r *LinodeClusterReconciler) handleDNS(clusterScope *scope.ClusterScope) { func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { logger.Info("deleting cluster") - if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == nil { + if clusterScope.LinodeCluster.Spec.Network.NodeBalancerID == nil && !reconciler.ConditionTrue(clusterScope.LinodeCluster, ConditionLoadBalancing) { logger.Info("NodeBalancer ID is missing, nothing to do") if err := clusterScope.RemoveCredentialsRefFinalizer(ctx); err != nil { @@ -270,11 +299,18 @@ func (r *LinodeClusterReconciler) reconcileDelete(ctx context.Context, logger lo return nil } - err := clusterScope.LinodeClient.DeleteNodeBalancer(ctx, *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID) - if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil { - logger.Error(err, "failed to delete NodeBalancer") - setFailureReason(clusterScope, cerrs.DeleteClusterError, err, r) - return err + if err := r.removeMachineFromLB(ctx, logger, clusterScope); err != nil { + return fmt.Errorf("remove machine from loadbalancer: %w", err) + } + conditions.MarkFalse(clusterScope.LinodeCluster, ConditionLoadBalancing, "cleared loadbalancer", clusterv1.ConditionSeverityInfo, "") + + if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType != "dns" && clusterScope.LinodeCluster.Spec.Network.NodeBalancerID != nil { + err := clusterScope.LinodeClient.DeleteNodeBalancer(ctx, *clusterScope.LinodeCluster.Spec.Network.NodeBalancerID) + if util.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil { + logger.Error(err, "failed to delete NodeBalancer") + setFailureReason(clusterScope, cerrs.DeleteClusterError, err, r) + return err + } } conditions.MarkFalse(clusterScope.LinodeCluster, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "Load balancer deleted") @@ -314,6 +350,10 @@ func (r *LinodeClusterReconciler) SetupWithManager(mgr ctrl.Manager, options crc kutil.ClusterToInfrastructureMapFunc(context.TODO(), infrav1alpha2.GroupVersion.WithKind("LinodeCluster"), mgr.GetClient(), &infrav1alpha2.LinodeCluster{}), ), builder.WithPredicates(predicates.ClusterUnpausedAndInfrastructureReady(mgr.GetLogger())), + ). + Watches( + &infrav1alpha2.LinodeMachine{}, + handler.EnqueueRequestsFromMapFunc(r.linodeMachineToLinodeCluster(mgr.GetLogger())), ).Complete(wrappedruntimereconciler.NewRuntimeReconcilerWithTracing(r, wrappedruntimereconciler.DefaultDecorator())) if err != nil { return fmt.Errorf("failed to build controller: %w", err) @@ -325,3 +365,49 @@ func (r *LinodeClusterReconciler) SetupWithManager(mgr ctrl.Manager, options crc func (r *LinodeClusterReconciler) TracedClient() client.Client { return wrappedruntimeclient.NewRuntimeClientWithTracing(r.Client, wrappedruntimereconciler.DefaultDecorator()) } + +func (r *LinodeClusterReconciler) linodeMachineToLinodeCluster(logger logr.Logger) handler.MapFunc { + logger = logger.WithName("LinodeClusterReconciler").WithName("linodeMachineToLinodeCluster") + + return func(ctx context.Context, o client.Object) []ctrl.Request { + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultMappingTimeout) + defer cancel() + + linodeMachine, ok := o.(*infrav1alpha2.LinodeMachine) + if !ok { + logger.Info("Failed to cast object to LinodeMachine") + return nil + } + + // We only need control plane machines to trigger reconciliation + machine, err := getOwnerMachine(ctx, r.TracedClient(), *linodeMachine, logger) + if err != nil || machine == nil { + return nil + } + if !kutil.IsControlPlaneMachine(machine) { + return nil + } + + linodeCluster := infrav1alpha2.LinodeCluster{} + if err := r.TracedClient().Get( + ctx, + types.NamespacedName{ + Name: linodeMachine.ObjectMeta.Labels[clusterv1.ClusterNameLabel], + Namespace: linodeMachine.Namespace, + }, + &linodeCluster); err != nil { + logger.Info("Failed to get LinodeCluster") + return nil + } + + result := make([]ctrl.Request, 0, 1) + result = append(result, ctrl.Request{ + NamespacedName: client.ObjectKey{ + Namespace: linodeCluster.Namespace, + Name: linodeCluster.Name, + }, + }) + + return result + } +} diff --git a/controller/linodecluster_controller_helpers.go b/controller/linodecluster_controller_helpers.go new file mode 100644 index 000000000..fbfe1c491 --- /dev/null +++ b/controller/linodecluster_controller_helpers.go @@ -0,0 +1,42 @@ +package controller + +import ( + "context" + + "github.com/go-logr/logr" + + "github.com/linode/cluster-api-provider-linode/cloud/scope" + "github.com/linode/cluster-api-provider-linode/cloud/services" +) + +func (r *LinodeClusterReconciler) addMachineToLB(ctx context.Context, clusterScope *scope.ClusterScope) error { + logger := logr.FromContextOrDiscard(ctx) + if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType != "dns" { + for _, eachMachine := range clusterScope.LinodeMachines.Items { + if err := services.AddNodesToNB(ctx, logger, clusterScope, eachMachine); err != nil { + return err + } + } + } else { + if err := services.EnsureDNSEntries(ctx, clusterScope, "create"); err != nil { + return err + } + } + + return nil +} + +func (r *LinodeClusterReconciler) removeMachineFromLB(ctx context.Context, logger logr.Logger, clusterScope *scope.ClusterScope) error { + if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "NodeBalancer" { + if err := services.DeleteNodesFromNB(ctx, logger, clusterScope); err != nil { + logger.Error(err, "Failed to remove node from Node Balancer backend") + return err + } + } else if clusterScope.LinodeCluster.Spec.Network.LoadBalancerType == "dns" { + if err := services.EnsureDNSEntries(ctx, clusterScope, "delete"); err != nil { + logger.Error(err, "Failed to remove IP from DNS") + return err + } + } + return nil +} diff --git a/controller/linodecluster_controller_test.go b/controller/linodecluster_controller_test.go index b25497c32..d0fc95042 100644 --- a/controller/linodecluster_controller_test.go +++ b/controller/linodecluster_controller_test.go @@ -25,6 +25,7 @@ import ( "github.com/linode/linodego" "go.uber.org/mock/gomock" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" @@ -99,6 +100,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc }), OneOf( Path(Result("create requeues", func(ctx context.Context, mck Mock) { + reconciler.Client = k8sClient res, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).NotTo(HaveOccurred()) Expect(res.RequeueAfter).To(Equal(rec.DefaultClusterControllerReconcileDelay)) @@ -107,6 +109,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc Path(Result("create nb error - timeout error", func(ctx context.Context, mck Mock) { tempTimeout := reconciler.ReconcileTimeout reconciler.ReconcileTimeout = time.Nanosecond + reconciler.Client = k8sClient _, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("failed to ensure nodebalancer")) @@ -122,6 +125,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc }), OneOf( Path(Result("create requeues", func(ctx context.Context, mck Mock) { + reconciler.Client = k8sClient res, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).NotTo(HaveOccurred()) Expect(res.RequeueAfter).To(Equal(rec.DefaultClusterControllerReconcileDelay)) @@ -130,6 +134,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc Path(Result("create nb error - timeout error", func(ctx context.Context, mck Mock) { tempTimeout := reconciler.ReconcileTimeout reconciler.ReconcileTimeout = time.Nanosecond + reconciler.Client = k8sClient _, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("nodeBalancer created was nil")) @@ -150,6 +155,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc ID: nodebalancerID, IPv4: &controlPlaneEndpointHost, }, nil) + reconciler.Client = k8sClient res, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).NotTo(HaveOccurred()) Expect(res.RequeueAfter).To(Equal(rec.DefaultClusterControllerReconcileDelay)) @@ -163,6 +169,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc }, nil) tempTimeout := reconciler.ReconcileTimeout + reconciler.Client = k8sClient reconciler.ReconcileTimeout = time.Nanosecond _, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).To(HaveOccurred()) @@ -174,6 +181,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc Path( Call("cluster is not created because there was an error getting nb config", func(ctx context.Context, mck Mock) { cScope.LinodeClient = mck.LinodeClient + reconciler.Client = k8sClient cScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID = nbConfigID mck.LinodeClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()). Return(&linodego.NodeBalancer{ @@ -185,6 +193,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc }), OneOf( Path(Result("create requeues", func(ctx context.Context, mck Mock) { + reconciler.Client = k8sClient res, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).NotTo(HaveOccurred()) Expect(res.RequeueAfter).To(Equal(rec.DefaultClusterControllerReconcileDelay)) @@ -193,6 +202,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc Path(Result("create nb error - timeout error", func(ctx context.Context, mck Mock) { tempTimeout := reconciler.ReconcileTimeout reconciler.ReconcileTimeout = time.Nanosecond + reconciler.Client = k8sClient _, err := reconciler.reconcile(ctx, cScope, mck.Logger()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("failed to get nodebalancer config")) @@ -217,6 +227,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc Call("cluster is created", func(ctx context.Context, mck Mock) { cScope.LinodeClient = mck.LinodeClient cScope.LinodeCluster.Spec.Network.ApiserverNodeBalancerConfigID = nil + mck.LinodeClient.EXPECT().ListNodeBalancerNodes(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.NodeBalancerNode{}, nil).AnyTimes() getNB := mck.LinodeClient.EXPECT().GetNodeBalancer(gomock.Any(), gomock.Any()). Return(&linodego.NodeBalancer{ ID: nodebalancerID, @@ -231,6 +242,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc }, nil) }), Result("cluster created", func(ctx context.Context, mck Mock) { + reconciler.Client = k8sClient _, err := reconciler.reconcile(ctx, cScope, logr.Logger{}) Expect(err).NotTo(HaveOccurred()) @@ -238,7 +250,7 @@ var _ = Describe("cluster-lifecycle", Ordered, Label("cluster", "cluster-lifecyc clusterKey := client.ObjectKeyFromObject(&linodeCluster) Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed()) Expect(linodeCluster.Status.Ready).To(BeTrue()) - Expect(linodeCluster.Status.Conditions).To(HaveLen(1)) + Expect(linodeCluster.Status.Conditions).To(HaveLen(2)) Expect(linodeCluster.Status.Conditions[0].Type).To(Equal(clusterv1.ReadyCondition)) By("checking NB id") @@ -270,6 +282,19 @@ var _ = Describe("cluster-lifecycle-dns", Ordered, Label("cluster", "cluster-lif OwnerReferences: ownerRefs, } + linodeMachine := infrav1alpha2.LinodeMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName + "-control-plane", + Namespace: defaultNamespace, + UID: "12345", + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + Type: "g6-nanode-1", + Image: rec.DefaultMachineControllerLinodeImage, + DiskEncryption: string(linodego.InstanceDiskEncryptionEnabled), + }, + } + linodeCluster := infrav1alpha2.LinodeCluster{ ObjectMeta: metadata, Spec: infrav1alpha2.LinodeClusterSpec{ @@ -292,6 +317,7 @@ var _ = Describe("cluster-lifecycle-dns", Ordered, Label("cluster", "cluster-lif BeforeAll(func(ctx SpecContext) { cScope.Client = k8sClient Expect(k8sClient.Create(ctx, &linodeCluster)).To(Succeed()) + Expect(k8sClient.Create(ctx, &linodeMachine)).To(Succeed()) }) ctlrSuite.BeforeEach(func(ctx context.Context, mck Mock) { @@ -312,8 +338,24 @@ var _ = Describe("cluster-lifecycle-dns", Ordered, Label("cluster", "cluster-lif Path( Call("cluster with dns loadbalancing is created", func(ctx context.Context, mck Mock) { cScope.LinodeClient = mck.LinodeClient + cScope.LinodeDomainsClient = mck.LinodeClient + mck.LinodeClient.EXPECT().ListDomains(gomock.Any(), gomock.Any()).Return([]linodego.Domain{ + { + ID: 1, + Domain: "lkedevs.net", + }, + }, nil).AnyTimes() + mck.LinodeClient.EXPECT().ListDomainRecords(gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.DomainRecord{ + { + ID: 1234, + Type: "A", + Name: "test-cluster", + TTLSec: 30, + }, + }, nil).AnyTimes() }), Result("cluster created", func(ctx context.Context, mck Mock) { + reconciler.Client = k8sClient _, err := reconciler.reconcile(ctx, cScope, logr.Logger{}) Expect(err).NotTo(HaveOccurred()) @@ -321,7 +363,7 @@ var _ = Describe("cluster-lifecycle-dns", Ordered, Label("cluster", "cluster-lif clusterKey := client.ObjectKeyFromObject(&linodeCluster) Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed()) Expect(linodeCluster.Status.Ready).To(BeTrue()) - Expect(linodeCluster.Status.Conditions).To(HaveLen(1)) + Expect(linodeCluster.Status.Conditions).To(HaveLen(2)) Expect(linodeCluster.Status.Conditions[0].Type).To(Equal(clusterv1.ReadyCondition)) By("checking controlPlaneEndpoint/NB host and port") @@ -354,7 +396,8 @@ var _ = Describe("cluster-delete", Ordered, Label("cluster", "cluster-delete"), Spec: infrav1alpha2.LinodeClusterSpec{ Region: "us-ord", Network: infrav1alpha2.NetworkSpec{ - NodeBalancerID: &nodebalancerID, + LoadBalancerType: "NodeBalancer", + NodeBalancerID: &nodebalancerID, }, }, } @@ -380,7 +423,7 @@ var _ = Describe("cluster-delete", Ordered, Label("cluster", "cluster-delete"), Call("cluster is deleted", func(ctx context.Context, mck Mock) { cScope.LinodeClient = mck.LinodeClient cScope.Client = mck.K8sClient - mck.LinodeClient.EXPECT().DeleteNodeBalancer(gomock.Any(), gomock.Any()).Return(nil) + mck.LinodeClient.EXPECT().DeleteNodeBalancer(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() }), ), Path( @@ -401,7 +444,7 @@ var _ = Describe("cluster-delete", Ordered, Label("cluster", "cluster-delete"), cScope.LinodeClient = mck.LinodeClient cScope.Client = mck.K8sClient cScope.LinodeCluster.Spec.Network.NodeBalancerID = &nodebalancerID - mck.LinodeClient.EXPECT().DeleteNodeBalancer(gomock.Any(), gomock.Any()).Return(errors.New("delete NB error")) + mck.LinodeClient.EXPECT().DeleteNodeBalancer(gomock.Any(), gomock.Any()).Return(errors.New("delete NB error")).AnyTimes() }), Result("cluster not deleted because the nb can't be deleted", func(ctx context.Context, mck Mock) { reconciler.Client = mck.K8sClient @@ -436,7 +479,9 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid Namespace: defaultNamespace, OwnerReferences: ownerRefs, } - + cluster := clusterv1.Cluster{ + ObjectMeta: metadata, + } linodeCluster := infrav1alpha2.LinodeCluster{ ObjectMeta: metadata, Spec: infrav1alpha2.LinodeClusterSpec{ @@ -449,6 +494,18 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid }, }, } + linodeMachine := infrav1alpha2.LinodeMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + Namespace: defaultNamespace, + }, + Spec: infrav1alpha2.LinodeMachineSpec{ + ProviderID: ptr.To("linode://123"), + }, + Status: infrav1alpha2.LinodeMachineStatus{ + Addresses: []clusterv1.MachineAddress{}, + }, + } ctlrSuite := NewControllerSuite(GinkgoT(), mock.MockLinodeClient{}) reconciler := LinodeClusterReconciler{} @@ -457,6 +514,7 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid BeforeAll(func(ctx SpecContext) { cScope.Client = k8sClient + Expect(k8sClient.Create(ctx, &cluster)).To(Succeed()) Expect(k8sClient.Create(ctx, &linodeCluster)).To(Succeed()) }) @@ -464,6 +522,7 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid reconciler.Recorder = mck.Recorder() Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed()) + cScope.Cluster = &cluster cScope.LinodeCluster = &linodeCluster // Create patch helper with latest state of resource. @@ -478,8 +537,30 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid Path( Call("cluster with dns loadbalancing is created", func(ctx context.Context, mck Mock) { cScope.LinodeClient = mck.LinodeClient + cScope.LinodeDomainsClient = mck.LinodeClient + cScope.AkamaiDomainsClient = mck.AkamEdgeDNSClient + linodeMachines := infrav1alpha2.LinodeMachineList{ + Items: []infrav1alpha2.LinodeMachine{linodeMachine}, + } + Expect(k8sClient.Create(ctx, &linodeMachine)).To(Succeed()) + cScope.LinodeMachines = linodeMachines + mck.LinodeClient.EXPECT().ListDomains(gomock.Any(), gomock.Any()).Return([]linodego.Domain{ + { + ID: 1, + Domain: "lkedevs.net", + }, + }, nil).AnyTimes() + mck.LinodeClient.EXPECT().ListDomainRecords(gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.DomainRecord{ + { + ID: 1234, + Type: "A", + Name: "test-cluster", + TTLSec: 30, + }, + }, nil).AnyTimes() }), Result("cluster created", func(ctx context.Context, mck Mock) { + reconciler.Client = k8sClient _, err := reconciler.reconcile(ctx, cScope, logr.Logger{}) Expect(err).NotTo(HaveOccurred()) @@ -487,7 +568,7 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid clusterKey := client.ObjectKeyFromObject(&linodeCluster) Expect(k8sClient.Get(ctx, clusterKey, &linodeCluster)).To(Succeed()) Expect(linodeCluster.Status.Ready).To(BeTrue()) - Expect(linodeCluster.Status.Conditions).To(HaveLen(1)) + Expect(linodeCluster.Status.Conditions).To(HaveLen(2)) Expect(linodeCluster.Status.Conditions[0].Type).To(Equal(clusterv1.ReadyCondition)) By("checking controlPlaneEndpoint/NB host and port") diff --git a/controller/linodefirewall_controller_helpers.go b/controller/linodefirewall_controller_helpers.go index 5096d23b2..1a18adb8c 100644 --- a/controller/linodefirewall_controller_helpers.go +++ b/controller/linodefirewall_controller_helpers.go @@ -199,12 +199,10 @@ func processACL(firewall *infrav1alpha2.LinodeFirewall) ( }) } } - if firewall.Spec.InboundPolicy == "ACCEPT" { - // if an allow list is present, we drop everything else. - createOpts.Rules.InboundPolicy = "DROP" - } else { - // if a deny list is present, we accept everything else. + if firewall.Spec.InboundPolicy == "" { createOpts.Rules.InboundPolicy = "ACCEPT" + } else { + createOpts.Rules.InboundPolicy = firewall.Spec.InboundPolicy } // process outbound rules @@ -255,12 +253,11 @@ func processACL(firewall *infrav1alpha2.LinodeFirewall) ( }) } } - if firewall.Spec.OutboundPolicy == "ACCEPT" { - // if an allow list is present, we drop everything else. - createOpts.Rules.OutboundPolicy = "DROP" - } else { - // if a deny list is present, we accept everything else. + + if firewall.Spec.OutboundPolicy == "" { createOpts.Rules.OutboundPolicy = "ACCEPT" + } else { + createOpts.Rules.OutboundPolicy = firewall.Spec.OutboundPolicy } // need to check if we ended up needing to make too many rules diff --git a/controller/linodemachine_controller.go b/controller/linodemachine_controller.go index 939858a94..aa08c6c20 100644 --- a/controller/linodemachine_controller.go +++ b/controller/linodemachine_controller.go @@ -47,7 +47,6 @@ import ( infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" infrav1alpha2 "github.com/linode/cluster-api-provider-linode/api/v1alpha2" "github.com/linode/cluster-api-provider-linode/cloud/scope" - "github.com/linode/cluster-api-provider-linode/cloud/services" wrappedruntimeclient "github.com/linode/cluster-api-provider-linode/observability/wrappers/runtimeclient" wrappedruntimereconciler "github.com/linode/cluster-api-provider-linode/observability/wrappers/runtimereconciler" "github.com/linode/cluster-api-provider-linode/util" @@ -65,8 +64,6 @@ const ( ConditionPreflightAdditionalDisksCreated clusterv1.ConditionType = "PreflightAdditionalDisksCreated" ConditionPreflightConfigured clusterv1.ConditionType = "PreflightConfigured" ConditionPreflightBootTriggered clusterv1.ConditionType = "PreflightBootTriggered" - ConditionPreflightNetworking clusterv1.ConditionType = "PreflightNetworking" - ConditionPreflightLoadBalancing clusterv1.ConditionType = "PreflightLoadbalancing" ConditionPreflightReady clusterv1.ConditionType = "PreflightReady" ) @@ -94,7 +91,6 @@ type LinodeMachineReconciler struct { client.Client Recorder record.EventRecorder LinodeClientConfig scope.ClientConfig - DnsClientConfig scope.ClientConfig WatchFilterValue string ReconcileTimeout time.Duration } @@ -129,7 +125,7 @@ func (r *LinodeMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, err } - machine, err := r.getOwnerMachine(ctx, *linodeMachine, log) + machine, err := getOwnerMachine(ctx, r.TracedClient(), *linodeMachine, log) if err != nil || machine == nil { return ctrl.Result{}, err } @@ -143,7 +139,6 @@ func (r *LinodeMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques machineScope, err := scope.NewMachineScope( ctx, r.LinodeClientConfig, - r.DnsClientConfig, scope.MachineScopeParams{ Client: r.TracedClient(), Cluster: cluster, @@ -184,9 +179,9 @@ func (r *LinodeMachineReconciler) reconcile( r.Recorder.Event(machineScope.LinodeMachine, corev1.EventTypeWarning, string(failureReason), err.Error()) } - // Always close the scope when exiting this function so we can persist any LinodeMachine and LinodeCluster changes. + // Always close the scope when exiting this function so we can persist any LinodeMachine changes. // This ignores any resource not found errors when reconciling deletions. - if patchErr := machineScope.CloseAll(ctx); patchErr != nil && utilerrors.FilterOut(util.UnwrapError(patchErr), apierrors.IsNotFound) != nil { + if patchErr := machineScope.Close(ctx); patchErr != nil && utilerrors.FilterOut(util.UnwrapError(patchErr), apierrors.IsNotFound) != nil { logger.Error(patchErr, "failed to patch LinodeMachine and LinodeCluster") err = errors.Join(err, patchErr) @@ -356,7 +351,6 @@ func (r *LinodeMachineReconciler) reconcileCreate( return r.reconcileInstanceCreate(ctx, logger, machineScope, linodeInstance) } -//nolint:cyclop,gocognit // It is ok for the moment but need larger refactor. func (r *LinodeMachineReconciler) reconcileInstanceCreate( ctx context.Context, logger logr.Logger, @@ -424,79 +418,12 @@ func (r *LinodeMachineReconciler) reconcileInstanceCreate( conditions.MarkTrue(machineScope.LinodeMachine, ConditionPreflightReady) } - if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightNetworking) { - if err := r.addMachineToLB(ctx, machineScope); err != nil { - logger.Error(err, "Failed to add machine to LB") - - if reconciler.RecordDecayingCondition(machineScope.LinodeMachine, - ConditionPreflightNetworking, string(cerrs.CreateMachineError), err.Error(), - reconciler.DefaultTimeout(r.ReconcileTimeout, reconciler.DefaultMachineControllerWaitForPreflightTimeout)) { - return ctrl.Result{}, err - } - - return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerWaitForRunningDelay}, nil - } - conditions.MarkTrue(machineScope.LinodeMachine, ConditionPreflightNetworking) - } - - if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightLoadBalancing) { - // Add the finalizer if not already there - if err := machineScope.AddLinodeClusterFinalizer(ctx); err != nil { - logger.Error(err, "Failed to add linodecluster finalizer") - - if reconciler.RecordDecayingCondition(machineScope.LinodeMachine, - ConditionPreflightLoadBalancing, string(cerrs.CreateMachineError), err.Error(), - reconciler.DefaultTimeout(r.ReconcileTimeout, reconciler.DefaultMachineControllerWaitForPreflightTimeout)) { - return ctrl.Result{}, err - } - return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerRetryDelay}, nil - } - conditions.MarkTrue(machineScope.LinodeMachine, ConditionPreflightLoadBalancing) - } - // Set the instance state to signal preflight process is done machineScope.LinodeMachine.Status.InstanceState = util.Pointer(linodego.InstanceOffline) return ctrl.Result{}, nil } -func (r *LinodeMachineReconciler) addMachineToLB( - ctx context.Context, - machineScope *scope.MachineScope, -) error { - logger := logr.FromContextOrDiscard(ctx) - if machineScope.LinodeCluster.Spec.Network.LoadBalancerType != "dns" { - if err := services.AddNodeToNB(ctx, logger, machineScope); err != nil { - return err - } - } else { - if err := services.EnsureDNSEntries(ctx, machineScope, "create"); err != nil { - return err - } - } - - return nil -} - -func (r *LinodeMachineReconciler) removeMachineFromLB( - ctx context.Context, - logger logr.Logger, - machineScope *scope.MachineScope, -) error { - if machineScope.LinodeCluster.Spec.Network.LoadBalancerType == "NodeBalancer" { - if err := services.DeleteNodeFromNB(ctx, logger, machineScope); err != nil { - logger.Error(err, "Failed to remove node from Node Balancer backend") - return err - } - } else if machineScope.LinodeCluster.Spec.Network.LoadBalancerType == "dns" { - if err := services.EnsureDNSEntries(ctx, machineScope, "delete"); err != nil { - logger.Error(err, "Failed to remove IP from DNS") - return err - } - } - return nil -} - func (r *LinodeMachineReconciler) configureDisks( ctx context.Context, logger logr.Logger, @@ -695,9 +622,6 @@ func (r *LinodeMachineReconciler) reconcileUpdate( conditions.MarkFalse(machineScope.LinodeMachine, clusterv1.ReadyCondition, "missing", clusterv1.ConditionSeverityWarning, "instance not found") } - if err := r.removeMachineFromLB(ctx, logger, machineScope); err != nil { - return res, nil, fmt.Errorf("remove machine from loadbalancer: %w", err) - } return res, nil, err } if _, ok := requeueInstanceStatuses[linodeInstance.Status]; ok { @@ -746,15 +670,6 @@ func (r *LinodeMachineReconciler) reconcileDelete( return ctrl.Result{}, nil } - if err := r.removeMachineFromLB(ctx, logger, machineScope); err != nil { - return ctrl.Result{}, fmt.Errorf("remove machine from loadbalancer: %w", err) - } - - // Add the finalizer if not already there - if err := machineScope.RemoveLinodeClusterFinalizer(ctx); err != nil { - return ctrl.Result{}, fmt.Errorf("Failed to remove linodecluster finalizer %w", err) - } - instanceID, err := util.GetInstanceID(machineScope.LinodeMachine.Spec.ProviderID) if err != nil { logger.Error(err, "Failed to parse instance ID from provider ID") @@ -778,6 +693,9 @@ func (r *LinodeMachineReconciler) reconcileDelete( conditions.MarkFalse(machineScope.LinodeMachine, clusterv1.ReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "instance deleted") r.Recorder.Event(machineScope.LinodeMachine, corev1.EventTypeNormal, clusterv1.DeletedReason, "instance has cleaned up") + if reconciler.ConditionTrue(machineScope.LinodeCluster, ConditionLoadBalancing) { + return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerRetryDelay}, nil + } machineScope.LinodeMachine.Spec.ProviderID = nil machineScope.LinodeMachine.Status.InstanceState = nil diff --git a/controller/linodemachine_controller_helpers.go b/controller/linodemachine_controller_helpers.go index 161589487..755a06a19 100644 --- a/controller/linodemachine_controller_helpers.go +++ b/controller/linodemachine_controller_helpers.go @@ -188,8 +188,8 @@ func (r *LinodeMachineReconciler) buildInstanceAddrs(ctx context.Context, machin return ips, nil } -func (r *LinodeMachineReconciler) getOwnerMachine(ctx context.Context, linodeMachine infrav1alpha2.LinodeMachine, log logr.Logger) (*clusterv1.Machine, error) { - machine, err := kutil.GetOwnerMachine(ctx, r.TracedClient(), linodeMachine.ObjectMeta) +func getOwnerMachine(ctx context.Context, tracedClient client.Client, linodeMachine infrav1alpha2.LinodeMachine, log logr.Logger) (*clusterv1.Machine, error) { + machine, err := kutil.GetOwnerMachine(ctx, tracedClient, linodeMachine.ObjectMeta) if err != nil { if err = client.IgnoreNotFound(err); err != nil { log.Error(err, "Failed to fetch owner machine") diff --git a/controller/linodemachine_controller_test.go b/controller/linodemachine_controller_test.go index 9a7041e44..28359b41b 100644 --- a/controller/linodemachine_controller_test.go +++ b/controller/linodemachine_controller_test.go @@ -198,12 +198,9 @@ var _ = Describe("create", Label("machine", "create"), func() { LinodeMachine: &linodeMachine, } - machinePatchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) + patchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) Expect(err).NotTo(HaveOccurred()) - mScope.MachinePatchHelper = machinePatchHelper - clusterPatchHelper, err := patch.NewHelper(mScope.LinodeCluster, k8sClient) - Expect(err).NotTo(HaveOccurred()) - mScope.ClusterPatchHelper = clusterPatchHelper + mScope.PatchHelper = patchHelper _, err = reconciler.reconcileCreate(ctx, logger, &mScope) Expect(err).NotTo(HaveOccurred()) @@ -262,12 +259,9 @@ var _ = Describe("create", Label("machine", "create"), func() { LinodeMachine: &linodeMachine, } - machinePatchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) - Expect(err).NotTo(HaveOccurred()) - mScope.MachinePatchHelper = machinePatchHelper - clusterPatchHelper, err := patch.NewHelper(mScope.LinodeCluster, k8sClient) + patchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) Expect(err).NotTo(HaveOccurred()) - mScope.ClusterPatchHelper = clusterPatchHelper + mScope.PatchHelper = patchHelper reconciler.ReconcileTimeout = time.Nanosecond @@ -311,12 +305,9 @@ var _ = Describe("create", Label("machine", "create"), func() { LinodeMachine: &linodeMachine, } - machinePatchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) - Expect(err).NotTo(HaveOccurred()) - mScope.MachinePatchHelper = machinePatchHelper - clusterPatchHelper, err := patch.NewHelper(mScope.LinodeCluster, k8sClient) + patchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) Expect(err).NotTo(HaveOccurred()) - mScope.ClusterPatchHelper = clusterPatchHelper + mScope.PatchHelper = patchHelper res, err := reconciler.reconcileCreate(ctx, logger, &mScope) Expect(err).NotTo(HaveOccurred()) @@ -413,7 +404,7 @@ var _ = Describe("create", Label("machine", "create"), func() { Address: "192.168.0.2:6443", Mode: linodego.ModeAccept, }). - After(getAddrs). + After(getAddrs).AnyTimes(). Return(nil, nil) getAddrs = mockLinodeClient.EXPECT(). GetInstanceIPAddresses(ctx, 123). @@ -447,12 +438,9 @@ var _ = Describe("create", Label("machine", "create"), func() { LinodeMachine: &linodeMachine, } - machinePatchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) - Expect(err).NotTo(HaveOccurred()) - mScope.MachinePatchHelper = machinePatchHelper - clusterPatchHelper, err := patch.NewHelper(mScope.LinodeCluster, k8sClient) + patchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) Expect(err).NotTo(HaveOccurred()) - mScope.ClusterPatchHelper = clusterPatchHelper + mScope.PatchHelper = patchHelper Expect(k8sClient.Create(ctx, &linodeCluster)).To(Succeed()) Expect(k8sClient.Create(ctx, &linodeMachine)).To(Succeed()) @@ -543,12 +531,9 @@ var _ = Describe("create", Label("machine", "create"), func() { LinodeMachine: &linodeMachine, } - machinePatchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) - Expect(err).NotTo(HaveOccurred()) - mScope.MachinePatchHelper = machinePatchHelper - clusterPatchHelper, err := patch.NewHelper(mScope.LinodeCluster, k8sClient) + patchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) Expect(err).NotTo(HaveOccurred()) - mScope.ClusterPatchHelper = clusterPatchHelper + mScope.PatchHelper = patchHelper res, err := reconciler.reconcileCreate(ctx, logger, &mScope) Expect(res.RequeueAfter).To(Equal(rutil.DefaultMachineControllerWaitForRunningDelay)) @@ -613,7 +598,7 @@ var _ = Describe("create", Label("machine", "create"), func() { Address: "192.168.0.2:6443", Mode: linodego.ModeAccept, }). - After(getAddrs). + After(getAddrs).AnyTimes(). Return(nil, nil) getAddrs = mockLinodeClient.EXPECT(). GetInstanceIPAddresses(ctx, 123). @@ -801,21 +786,17 @@ var _ = Describe("createDNS", Label("machine", "createDNS"), func() { }}, nil) mScope := scope.MachineScope{ - Client: k8sClient, - LinodeClient: mockLinodeClient, - LinodeDomainsClient: mockLinodeClient, - Cluster: &cluster, - Machine: &machine, - LinodeCluster: &linodeCluster, - LinodeMachine: &linodeMachine, + Client: k8sClient, + LinodeClient: mockLinodeClient, + Cluster: &cluster, + Machine: &machine, + LinodeCluster: &linodeCluster, + LinodeMachine: &linodeMachine, } - machinePatchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) - Expect(err).NotTo(HaveOccurred()) - mScope.MachinePatchHelper = machinePatchHelper - clusterPatchHelper, err := patch.NewHelper(mScope.LinodeCluster, k8sClient) + patchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) Expect(err).NotTo(HaveOccurred()) - mScope.ClusterPatchHelper = clusterPatchHelper + mScope.PatchHelper = patchHelper _, err = reconciler.reconcileCreate(ctx, logger, &mScope) Expect(err).NotTo(HaveOccurred()) @@ -930,13 +911,9 @@ var _ = Describe("machine-lifecycle", Ordered, Label("machine", "machine-lifecyc Expect(k8sClient.Get(ctx, machineKey, linodeMachine)).To(Succeed()) mScope.LinodeMachine = linodeMachine - machinePatchHelper, err := patch.NewHelper(linodeMachine, k8sClient) - Expect(err).NotTo(HaveOccurred()) - mScope.MachinePatchHelper = machinePatchHelper - clusterPatchHelper, err := patch.NewHelper(linodeCluster, k8sClient) + patchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) Expect(err).NotTo(HaveOccurred()) - mScope.ClusterPatchHelper = clusterPatchHelper - + mScope.PatchHelper = patchHelper Expect(k8sClient.Get(ctx, clusterKey, linodeCluster)).To(Succeed()) mScope.LinodeCluster = linodeCluster @@ -1237,13 +1214,10 @@ var _ = Describe("machine-delete", Ordered, Label("machine", "machine-delete"), ctlrSuite.BeforeEach(func(ctx context.Context, mck Mock) { reconciler.Recorder = mck.Recorder() mScope.LinodeMachine = linodeMachine - machinePatchHelper, err := patch.NewHelper(linodeMachine, k8sClient) + patchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) Expect(err).NotTo(HaveOccurred()) - mScope.MachinePatchHelper = machinePatchHelper + mScope.PatchHelper = patchHelper mScope.LinodeCluster = linodeCluster - clusterPatchHelper, err := patch.NewHelper(linodeCluster, k8sClient) - Expect(err).NotTo(HaveOccurred()) - mScope.ClusterPatchHelper = clusterPatchHelper mScope.LinodeClient = mck.LinodeClient reconciler.Client = mck.K8sClient }) @@ -1460,21 +1434,17 @@ var _ = Describe("machine in PlacementGroup", Label("machine", "placementGroup") Expect(err).NotTo(HaveOccurred()) mScope := scope.MachineScope{ - Client: k8sClient, - LinodeClient: mockLinodeClient, - LinodeDomainsClient: mockLinodeClient, - Cluster: &cluster, - Machine: &machine, - LinodeCluster: &linodeCluster, - LinodeMachine: &linodeMachine, + Client: k8sClient, + LinodeClient: mockLinodeClient, + Cluster: &cluster, + Machine: &machine, + LinodeCluster: &linodeCluster, + LinodeMachine: &linodeMachine, } - machinePatchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) - Expect(err).NotTo(HaveOccurred()) - mScope.MachinePatchHelper = machinePatchHelper - clusterPatchHelper, err := patch.NewHelper(mScope.LinodeCluster, k8sClient) + patchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient) Expect(err).NotTo(HaveOccurred()) - mScope.ClusterPatchHelper = clusterPatchHelper + mScope.PatchHelper = patchHelper createOpts, err := reconciler.newCreateConfig(ctx, &mScope, []string{}, logger) Expect(err).NotTo(HaveOccurred()) diff --git a/docs/src/topics/firewalling.md b/docs/src/topics/firewalling.md index a918a06a4..ebf1d7a62 100644 --- a/docs/src/topics/firewalling.md +++ b/docs/src/topics/firewalling.md @@ -1,9 +1,15 @@ # Firewalling -This guide covers how Cilium can be set up to act as a [host firewall](https://docs.cilium.io/en/latest/security/host-firewall/) on CAPL clusters. +This guide covers how Cilium and Cloud Firewalls can be used for firewalling CAPL clusters. -## Default Configuration -By default, the following policies are set to audit mode(without any enforcement) on CAPL clusters +## Cilium Firewalls + +Cilium provides cluster-wide firewalling via [Host Policies](https://docs.cilium.io/en/latest/security/policy/language/#hostpolicies) +which enforce access control over connectivity to and from cluster nodes. +Cilium's [host firewall](https://docs.cilium.io/en/latest/security/host-firewall/) is responsible for enforcing the security policies. + +### Default Cilium Host Firewall Configuration +By default, the following Host Policies are set to audit mode (without any enforcement) on CAPL clusters: * [Kubeadm](./flavors/default.md) cluster allow rules @@ -30,13 +36,13 @@ For kubeadm clusters running outside of VPC, ports 2379 and 2380 are also allowe | 6443 | API Server Traffic | World | | * | In Cluster Communication | Intra Cluster and VPC Traffic | -## Enabling Firewall Enforcement -In order to turn the cilium network policy from audit to enforce mode use the environment variable `FW_AUDIT_ONLY=false` +### Enabling Cilium Host Policy Enforcement +In order to turn the Cilium Host Policies from audit to enforce mode, use the environment variable `FW_AUDIT_ONLY=false` when generating the cluster. This will set the [policy-audit-mode](https://docs.cilium.io/en/latest/security/policy-creation/#creating-policies-from-verdicts) -on the cilium deployment +on the Cilium deployment. -## Adding Additional Rules -Additional rules can be added to the `default-policy` +### Adding Additional Cilium Host Policies +Additional rules can be added to the `default-policy`: ```yaml apiVersion: "cilium.io/v2" kind: CiliumClusterwideNetworkPolicy @@ -57,7 +63,7 @@ spec: - port: "22" # added for SSH Access to the nodes - port: "${APISERVER_PORT:=6443}" ``` -Alternatively, additional rules can be added by creating a new policy +Alternatively, additional rules can be added by creating a new policy: ```yaml apiVersion: "cilium.io/v2" kind: CiliumClusterwideNetworkPolicy @@ -73,3 +79,39 @@ spec: - ports: - port: "22" ``` + +## Cloud Firewalls + +For controlling firewalls via Linode resources, a [Cloud Firewall](https://www.linode.com/products/cloud-firewall/) can +be defined and provisioned via the `LinodeFirewall` resource in CAPL. + +The created Cloud Firewall can be used on a `LinodeMachine` or a `LinodeMachineTemplate` by setting the `firewallRef` field. +Alternatively, the provisioned Cloud Firewall's ID can be used in the `firewallID` field. + +```admonish note +The `firewallRef` and `firewallID` fields are currently immutable for `LinodeMachines` and `LinodeMachineTemplates`. This will +be addressed in a later release. +``` + +Example `LinodeFirewall`: +```yaml +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 +kind: LinodeFirewall +metadata: + name: sample-fw +spec: + enabled: true + inboundPolicy: DROP + inboundRules: + - action: ACCEPT + label: k8s-api + ports: "6443" + protocol: "TCP" + addresses: + ipv4: + - "10.0.0.0/24" + # outboundPolicy: ACCEPT + # outboundRules: [] +``` + +Cloud Firewalls are not automatically created for any CAPL flavor at this time. diff --git a/e2e/linodecluster-controller/minimal-linodecluster/chainsaw-test.yaml b/e2e/linodecluster-controller/minimal-linodecluster/chainsaw-test.yaml index bb1703336..5b65c0d06 100755 --- a/e2e/linodecluster-controller/minimal-linodecluster/chainsaw-test.yaml +++ b/e2e/linodecluster-controller/minimal-linodecluster/chainsaw-test.yaml @@ -68,6 +68,11 @@ spec: results: 1 - name: Delete Cluster resource try: + - delete: + ref: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 + kind: LinodeCluster + name: ($cluster) - delete: ref: apiVersion: cluster.x-k8s.io/v1beta1 diff --git a/e2e/linodemachine-controller/minimal-linodemachine/chainsaw-test.yaml b/e2e/linodemachine-controller/minimal-linodemachine/chainsaw-test.yaml index 6dc870d97..46526395f 100755 --- a/e2e/linodemachine-controller/minimal-linodemachine/chainsaw-test.yaml +++ b/e2e/linodemachine-controller/minimal-linodemachine/chainsaw-test.yaml @@ -69,6 +69,11 @@ spec: results: 1 - name: Delete Cluster resource try: + - delete: + ref: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 + kind: LinodeCluster + name: ($cluster) - delete: ref: apiVersion: cluster.x-k8s.io/v1beta1 diff --git a/e2e/linodemachine-controller/vpc-integration/chainsaw-test.yaml b/e2e/linodemachine-controller/vpc-integration/chainsaw-test.yaml index f346c442f..b786edfc0 100755 --- a/e2e/linodemachine-controller/vpc-integration/chainsaw-test.yaml +++ b/e2e/linodemachine-controller/vpc-integration/chainsaw-test.yaml @@ -106,6 +106,11 @@ spec: - active: true - name: Delete the Cluster & LinodeVPC resource try: + - delete: + ref: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2 + kind: LinodeCluster + name: ($cluster) - delete: ref: apiVersion: cluster.x-k8s.io/v1beta1 diff --git a/go.mod b/go.mod index b9371bf26..31b2e26d5 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,7 @@ require ( k8s.io/api v0.30.3 k8s.io/apimachinery v0.30.3 k8s.io/client-go v0.30.3 - k8s.io/utils v0.0.0-20231127182322-b307cd553661 + k8s.io/utils v0.0.0-20240102154912-e7106e64919e sigs.k8s.io/cluster-api v1.8.0 sigs.k8s.io/controller-runtime v0.18.5 ) @@ -66,12 +66,12 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.19.1 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.55.0 // indirect github.com/prometheus/procfs v0.15.1 // indirect - github.com/spf13/pflag v1.0.5 // indirect + github.com/spf13/pflag v1.0.6-0.20210604193023-d5e0c0615ace // indirect github.com/stretchr/objx v0.5.2 // indirect go.opentelemetry.io/contrib/bridges/prometheus v0.53.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp v0.4.0 // indirect diff --git a/go.sum b/go.sum index d8c574707..8179ad827 100644 --- a/go.sum +++ b/go.sum @@ -156,8 +156,9 @@ github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3I github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= +github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prashantv/gostub v1.1.0 h1:BTyx3RfQjRHnUWaGF9oQos79AlQ5k8WNktv7VGvVH4g= github.com/prashantv/gostub v1.1.0/go.mod h1:A5zLQHz7ieHGG7is6LLXLz7I8+3LZzsrV0P1IAHhP5U= github.com/prometheus/client_golang v1.19.1 h1:wZWJDwK+NameRJuPGDhlnFgx8e8HN3XHQeLaYJFJBOE= @@ -179,8 +180,8 @@ github.com/smartystreets/go-aws-auth v0.0.0-20180515143844-0c1422d1fdb9/go.mod h github.com/smartystreets/gunit v1.0.0/go.mod h1:qwPWnhz6pn0NnRBP++URONOVyNkPyr4SauJk4cUOwJs= github.com/spf13/cast v1.6.0 h1:GEiTHELF+vaR5dhz3VqZfFSzZjYbgeKDpBxQVS4GYJ0= github.com/spf13/cast v1.6.0/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo= -github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= -github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/pflag v1.0.6-0.20210604193023-d5e0c0615ace h1:9PNP1jnUjRhfmGMlkXHjYPishpcw4jpSt/V/xYY3FMA= +github.com/spf13/pflag v1.0.6-0.20210604193023-d5e0c0615ace/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stoewer/go-strcase v1.2.0 h1:Z2iHWqGXH00XYgqDmNgQbIBxf3wrNq0F3feEy0ainaU= github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -399,8 +400,8 @@ k8s.io/klog/v2 v2.120.1 h1:QXU6cPEOIslTGvZaXvFWiP9VKyeet3sawzTOvdXb4Vw= k8s.io/klog/v2 v2.120.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7FjZpUb45WallggurYhKGag= k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98= -k8s.io/utils v0.0.0-20231127182322-b307cd553661 h1:FepOBzJ0GXm8t0su67ln2wAZjbQ6RxQGZDnzuLcrUTI= -k8s.io/utils v0.0.0-20231127182322-b307cd553661/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= +k8s.io/utils v0.0.0-20240102154912-e7106e64919e h1:eQ/4ljkx21sObifjzXwlPKpdGLrCfRziVtos3ofG/sQ= +k8s.io/utils v0.0.0-20240102154912-e7106e64919e/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.0 h1:Tc9rS7JJoZ9sl3OpL4842oIk6lH7gWBb0JOmJ0ute7M= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.0/go.mod h1:1ewhL9l1gkPcU/IU/6rFYfikf+7Y5imWv7ARVbBOzNs= sigs.k8s.io/cluster-api v1.8.0 h1:xdF9svGCbezxOn9Y6QmlVnNaZ0n9QkRJpNuKJkeorUw= diff --git a/mock/client.go b/mock/client.go index 3e20c26e5..7d0127be9 100644 --- a/mock/client.go +++ b/mock/client.go @@ -680,6 +680,21 @@ func (mr *MockLinodeClientMockRecorder) ListInstances(ctx, opts any) *gomock.Cal return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListInstances", reflect.TypeOf((*MockLinodeClient)(nil).ListInstances), ctx, opts) } +// ListNodeBalancerNodes mocks base method. +func (m *MockLinodeClient) ListNodeBalancerNodes(ctx context.Context, nodebalancerID, configID int, opts *linodego.ListOptions) ([]linodego.NodeBalancerNode, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListNodeBalancerNodes", ctx, nodebalancerID, configID, opts) + ret0, _ := ret[0].([]linodego.NodeBalancerNode) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListNodeBalancerNodes indicates an expected call of ListNodeBalancerNodes. +func (mr *MockLinodeClientMockRecorder) ListNodeBalancerNodes(ctx, nodebalancerID, configID, opts any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListNodeBalancerNodes", reflect.TypeOf((*MockLinodeClient)(nil).ListNodeBalancerNodes), ctx, nodebalancerID, configID, opts) +} + // ListPlacementGroups mocks base method. func (m *MockLinodeClient) ListPlacementGroups(ctx context.Context, options *linodego.ListOptions) ([]linodego.PlacementGroup, error) { m.ctrl.T.Helper() @@ -1501,6 +1516,21 @@ func (mr *MockLinodeNodeBalancerClientMockRecorder) GetNodeBalancerConfig(ctx, n return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNodeBalancerConfig", reflect.TypeOf((*MockLinodeNodeBalancerClient)(nil).GetNodeBalancerConfig), ctx, nodebalancerID, configID) } +// ListNodeBalancerNodes mocks base method. +func (m *MockLinodeNodeBalancerClient) ListNodeBalancerNodes(ctx context.Context, nodebalancerID, configID int, opts *linodego.ListOptions) ([]linodego.NodeBalancerNode, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListNodeBalancerNodes", ctx, nodebalancerID, configID, opts) + ret0, _ := ret[0].([]linodego.NodeBalancerNode) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListNodeBalancerNodes indicates an expected call of ListNodeBalancerNodes. +func (mr *MockLinodeNodeBalancerClientMockRecorder) ListNodeBalancerNodes(ctx, nodebalancerID, configID, opts any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListNodeBalancerNodes", reflect.TypeOf((*MockLinodeNodeBalancerClient)(nil).ListNodeBalancerNodes), ctx, nodebalancerID, configID, opts) +} + // MockLinodeObjectStorageClient is a mock of LinodeObjectStorageClient interface. type MockLinodeObjectStorageClient struct { ctrl *gomock.Controller diff --git a/observability/wrappers/linodeclient/linodeclient.gen.go b/observability/wrappers/linodeclient/linodeclient.gen.go index 05a4ed485..319a323af 100644 --- a/observability/wrappers/linodeclient/linodeclient.gen.go +++ b/observability/wrappers/linodeclient/linodeclient.gen.go @@ -1118,6 +1118,33 @@ func (_d LinodeClientWithTracing) ListInstances(ctx context.Context, opts *linod return _d.LinodeClient.ListInstances(ctx, opts) } +// ListNodeBalancerNodes implements clients.LinodeClient +func (_d LinodeClientWithTracing) ListNodeBalancerNodes(ctx context.Context, nodebalancerID int, configID int, opts *linodego.ListOptions) (na1 []linodego.NodeBalancerNode, err error) { + ctx, _span := tracing.Start(ctx, "clients.LinodeClient.ListNodeBalancerNodes") + defer func() { + if _d._spanDecorator != nil { + _d._spanDecorator(_span, map[string]interface{}{ + "ctx": ctx, + "nodebalancerID": nodebalancerID, + "configID": configID, + "opts": opts}, map[string]interface{}{ + "na1": na1, + "err": err}) + } + + if err != nil { + _span.RecordError(err) + _span.SetAttributes( + attribute.String("event", "error"), + attribute.String("message", err.Error()), + ) + } + + _span.End() + }() + return _d.LinodeClient.ListNodeBalancerNodes(ctx, nodebalancerID, configID, opts) +} + // ListPlacementGroups implements clients.LinodeClient func (_d LinodeClientWithTracing) ListPlacementGroups(ctx context.Context, options *linodego.ListOptions) (pa1 []linodego.PlacementGroup, err error) { ctx, _span := tracing.Start(ctx, "clients.LinodeClient.ListPlacementGroups") diff --git a/util/reconciler/defaults.go b/util/reconciler/defaults.go index c3079c624..cb087474b 100644 --- a/util/reconciler/defaults.go +++ b/util/reconciler/defaults.go @@ -31,7 +31,7 @@ const ( // DefaultMachineControllerLinodeImage default image. DefaultMachineControllerLinodeImage = "linode/ubuntu22.04" // DefaultMachineControllerWaitForRunningDelay is the default requeue delay if instance is not running. - DefaultMachineControllerWaitForRunningDelay = 5 * time.Second + DefaultMachineControllerWaitForRunningDelay = 15 * time.Second // DefaultMachineControllerWaitForPreflightTimeout is the default timeout during the preflight phase. DefaultMachineControllerWaitForPreflightTimeout = 5 * time.Minute // DefaultMachineControllerWaitForRunningTimeout is the default timeout if instance is not running.