diff --git a/cloud/scope/client.go b/cloud/scope/client.go index d49d3069b..8393c6292 100644 --- a/cloud/scope/client.go +++ b/cloud/scope/client.go @@ -38,6 +38,9 @@ type LinodeInstanceClient interface { // LinodeVPCClient defines the methods that a Linode client must have to interact with Linode's VPC service. type LinodeVPCClient interface { GetVPC(ctx context.Context, vpcID int) (*linodego.VPC, error) + ListVPCs(ctx context.Context, opts *linodego.ListOptions) ([]linodego.VPC, error) + CreateVPC(ctx context.Context, opts linodego.VPCCreateOptions) (*linodego.VPC, error) + DeleteVPC(ctx context.Context, vpcID int) error } // LinodeNodeBalancerClient defines the methods that a Linode client must have to interact with Linode's Node Balancer service. diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 7769df967..8bd4c1d75 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -84,7 +84,7 @@ func NewClusterScope(ctx context.Context, apiKey string, params ClusterScopePara type ClusterScope struct { Client K8sClient PatchHelper *patch.Helper - LinodeClient LinodeNodeBalancerClient + LinodeClient LinodeMachineClient Cluster *clusterv1.Cluster LinodeCluster *infrav1alpha1.LinodeCluster } diff --git a/cloud/scope/vpc.go b/cloud/scope/vpc.go index 1225978fa..9db300a33 100644 --- a/cloud/scope/vpc.go +++ b/cloud/scope/vpc.go @@ -21,7 +21,6 @@ import ( "errors" "fmt" - "github.com/linode/linodego" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -30,10 +29,10 @@ import ( // VPCScope defines the basic context for an actuator to operate upon. type VPCScope struct { - client K8sClient + Client K8sClient PatchHelper *patch.Helper - LinodeClient *linodego.Client + LinodeClient LinodeVPCClient LinodeVPC *infrav1alpha1.LinodeVPC } @@ -77,7 +76,7 @@ func NewVPCScope(ctx context.Context, apiKey string, params VPCScopeParams) (*VP } return &VPCScope{ - client: params.Client, + Client: params.Client, LinodeClient: linodeClient, LinodeVPC: params.LinodeVPC, PatchHelper: helper, diff --git a/controller/linodevpc_controller.go b/controller/linodevpc_controller.go index 78fdf36f4..0052e67b8 100644 --- a/controller/linodevpc_controller.go +++ b/controller/linodevpc_controller.go @@ -100,13 +100,13 @@ func (r *LinodeVPCReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, fmt.Errorf("failed to create VPC scope: %w", err) } - return r.reconcile(ctx, vpcScope, log) + return r.reconcile(ctx, log, vpcScope) } func (r *LinodeVPCReconciler) reconcile( ctx context.Context, - vpcScope *scope.VPCScope, logger logr.Logger, + vpcScope *scope.VPCScope, ) (res ctrl.Result, err error) { res = ctrl.Result{} @@ -167,12 +167,12 @@ func (r *LinodeVPCReconciler) reconcile( // Create failureReason = infrav1alpha1.CreateVPCError - err = r.reconcileCreate(ctx, vpcScope, logger) + err = r.reconcileCreate(ctx, logger, vpcScope) return } -func (r *LinodeVPCReconciler) reconcileCreate(ctx context.Context, vpcScope *scope.VPCScope, logger logr.Logger) error { +func (r *LinodeVPCReconciler) reconcileCreate(ctx context.Context, logger logr.Logger, vpcScope *scope.VPCScope) error { logger.Info("creating vpc") if err := r.reconcileVPC(ctx, vpcScope, logger); err != nil { diff --git a/controller/linodevpc_controller_test.go b/controller/linodevpc_controller_test.go new file mode 100644 index 000000000..7d69858b5 --- /dev/null +++ b/controller/linodevpc_controller_test.go @@ -0,0 +1,375 @@ +// /* +// Copyright 2023 Akamai Technologies, Inc. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at + +// http://www.apache.org/licenses/LICENSE-2.0 + +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// */ + +package controller + +import ( + "bytes" + "errors" + "time" + + "github.com/go-logr/logr" + infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" + "github.com/linode/cluster-api-provider-linode/cloud/scope" + "github.com/linode/cluster-api-provider-linode/mock" + rec "github.com/linode/cluster-api-provider-linode/util/reconciler" + "github.com/linode/linodego" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "go.uber.org/mock/gomock" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +var _ = Describe("lifecycle", Ordered, Label("vpc", "lifecycle"), func() { + var linodeVPC infrav1alpha1.LinodeVPC + var reconciler *LinodeVPCReconciler + + var mockCtrl *gomock.Controller + var testLogs *bytes.Buffer + var logger logr.Logger + + recorder := record.NewFakeRecorder(10) + + BeforeEach(func(ctx SpecContext) { + reconciler = &LinodeVPCReconciler{ + Recorder: recorder, + } + + vpcSpec := infrav1alpha1.LinodeVPCSpec{ + Region: "us-east", + Subnets: []infrav1alpha1.VPCSubnetCreateOptions{ + {Label: "subnet1", IPv4: "10.0.0.0/8"}, + }, + } + linodeVPC = infrav1alpha1.LinodeVPC{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Labels: make(map[string]string), + }, + Spec: vpcSpec, + } + + mockCtrl = gomock.NewController(GinkgoT()) + testLogs = &bytes.Buffer{} + logger = zap.New( + zap.WriteTo(GinkgoWriter), + zap.WriteTo(testLogs), + zap.UseDevMode(true), + ) + }) + + AfterEach(func(ctx SpecContext) { + mockCtrl.Finish() + for len(recorder.Events) > 0 { + <-recorder.Events + } + }) + + It("creates a vpc", func(ctx SpecContext) { + mockLinodeClient := mock.NewMockLinodeVPCClient(mockCtrl) + + listVPCs := mockLinodeClient.EXPECT(). + ListVPCs(ctx, gomock.Any()). + Return([]linodego.VPC{}, nil) + mockLinodeClient.EXPECT(). + CreateVPC(ctx, gomock.Any()). + After(listVPCs). + Return(&linodego.VPC{ + ID: 1, + Label: "vpc1", + Region: "us-east", + Subnets: []linodego.VPCSubnet{ + {ID: 123, Label: "subnet1", IPv4: "10.0.0.0/8"}, + }, + }, nil) + + vpcScope := scope.VPCScope{ + Client: k8sClient, + LinodeClient: mockLinodeClient, + LinodeVPC: &linodeVPC, + } + + err := reconciler.reconcileCreate(ctx, logger, &vpcScope) + Expect(err).NotTo(HaveOccurred()) + + Expect(*linodeVPC.Spec.VPCID).To(Equal(1)) + Expect(linodeVPC.Spec.Subnets[0].IPv4).To(Equal("10.0.0.0/8")) + Expect(testLogs.String()).NotTo(ContainSubstring("Failed to create VPC")) + }) + + Context("when doing update", func() { + It("successfully updates a vpc", func(ctx SpecContext) { + mockLinodeClient := mock.NewMockLinodeVPCClient(mockCtrl) + + listVPCs := mockLinodeClient.EXPECT(). + ListVPCs(ctx, gomock.Any()). + Return([]linodego.VPC{}, nil) + mockLinodeClient.EXPECT(). + CreateVPC(ctx, gomock.Any()). + After(listVPCs). + Return(&linodego.VPC{ + ID: 1, + Label: "vpc1", + Region: "us-east", + Subnets: []linodego.VPCSubnet{ + {ID: 123, Label: "subnet1", IPv4: "10.0.0.0/8"}, + }, + }, nil) + + vpcScope := scope.VPCScope{ + Client: k8sClient, + LinodeClient: mockLinodeClient, + LinodeVPC: &linodeVPC, + } + + err := reconciler.reconcileUpdate(ctx, logger, &vpcScope) + Expect(err).NotTo(HaveOccurred()) + + Expect(*linodeVPC.Spec.VPCID).To(Equal(1)) + Expect(linodeVPC.Spec.Subnets[0].IPv4).To(Equal("10.0.0.0/8")) + Expect(testLogs.String()).NotTo(ContainSubstring("Failed to create VPC")) + }) + + It("fails if it can't list VPCs", func(ctx SpecContext) { + mockLinodeClient := mock.NewMockLinodeVPCClient(mockCtrl) + + mockLinodeClient.EXPECT(). + ListVPCs(ctx, gomock.Any()). + Return([]linodego.VPC{}, errors.New("failed to make call")) + + vpcScope := scope.VPCScope{ + Client: k8sClient, + LinodeClient: mockLinodeClient, + LinodeVPC: &linodeVPC, + } + + err := reconciler.reconcileUpdate(ctx, logger, &vpcScope) + Expect(err).To(HaveOccurred()) + Expect(testLogs.String()).To(ContainSubstring("Failed to list VPCs")) + }) + + It("fails if it can't create VPC", func(ctx SpecContext) { + mockLinodeClient := mock.NewMockLinodeVPCClient(mockCtrl) + + listVPCs := mockLinodeClient.EXPECT(). + ListVPCs(ctx, gomock.Any()). + Return([]linodego.VPC{}, nil) + mockLinodeClient.EXPECT(). + CreateVPC(ctx, gomock.Any()). + After(listVPCs). + Return(&linodego.VPC{}, errors.New("failed creating VPC")) + + vpcScope := scope.VPCScope{ + Client: k8sClient, + LinodeClient: mockLinodeClient, + LinodeVPC: &linodeVPC, + } + + err := reconciler.reconcileCreate(ctx, logger, &vpcScope) + Expect(err).To(HaveOccurred()) + Expect(testLogs.String()).To(ContainSubstring("Failed to create VPC")) + }) + + It("fails if empty VPC is returned", func(ctx SpecContext) { + mockLinodeClient := mock.NewMockLinodeVPCClient(mockCtrl) + + listVPCs := mockLinodeClient.EXPECT(). + ListVPCs(ctx, gomock.Any()). + Return([]linodego.VPC{}, nil) + mockLinodeClient.EXPECT(). + CreateVPC(ctx, gomock.Any()). + After(listVPCs). + Return(nil, nil) + + vpcScope := scope.VPCScope{ + Client: k8sClient, + LinodeClient: mockLinodeClient, + LinodeVPC: &linodeVPC, + } + + err := reconciler.reconcileCreate(ctx, logger, &vpcScope) + Expect(err).To(HaveOccurred()) + Expect(testLogs.String()).To(ContainSubstring("Panic! Failed to create VPC")) + }) + }) + + Context("when deleting a VPC", func() { + It("succeeds if no error occurs", func(ctx SpecContext) { + mockLinodeClient := mock.NewMockLinodeVPCClient(mockCtrl) + + vpcSpec := infrav1alpha1.LinodeVPCSpec{ + VPCID: ptr.To(1), + Region: "us-east", + Subnets: []infrav1alpha1.VPCSubnetCreateOptions{ + {Label: "subnet1", IPv4: "10.0.0.0/8"}, + }, + } + linodeVPC = infrav1alpha1.LinodeVPC{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Labels: make(map[string]string), + }, + Spec: vpcSpec, + } + + getVPC := mockLinodeClient.EXPECT(). + GetVPC(ctx, gomock.Any()). + Return(&linodego.VPC{ + ID: 1, + Label: "vpc1", + Subnets: []linodego.VPCSubnet{ + {ID: 123, Linodes: []linodego.VPCSubnetLinode{}}, + }, + }, nil) + mockLinodeClient.EXPECT(). + DeleteVPC(ctx, gomock.Any()). + After(getVPC). + Return(nil) + + vpcScope := scope.VPCScope{ + Client: k8sClient, + LinodeClient: mockLinodeClient, + LinodeVPC: &linodeVPC, + } + + _, err := reconciler.reconcileDelete(ctx, logger, &vpcScope) + Expect(err).NotTo(HaveOccurred()) + }) + + It("fails if GetVPC API errors out", func(ctx SpecContext) { + mockLinodeClient := mock.NewMockLinodeVPCClient(mockCtrl) + + vpcSpec := infrav1alpha1.LinodeVPCSpec{ + VPCID: ptr.To(1), + Region: "us-east", + Subnets: []infrav1alpha1.VPCSubnetCreateOptions{ + {Label: "subnet1", IPv4: "10.0.0.0/8"}, + }, + } + linodeVPC = infrav1alpha1.LinodeVPC{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Labels: make(map[string]string), + }, + Spec: vpcSpec, + } + + mockLinodeClient.EXPECT(). + GetVPC(ctx, gomock.Any()). + Return(&linodego.VPC{}, errors.New("service unavailable")) + + vpcScope := scope.VPCScope{ + Client: k8sClient, + LinodeClient: mockLinodeClient, + LinodeVPC: &linodeVPC, + } + + _, err := reconciler.reconcileDelete(ctx, logger, &vpcScope) + Expect(err).To(HaveOccurred()) + Expect(testLogs.String()).To(ContainSubstring("Failed to fetch VPC")) + }) + + It("fails if DeleteVPC API errors out", func(ctx SpecContext) { + mockLinodeClient := mock.NewMockLinodeVPCClient(mockCtrl) + + vpcSpec := infrav1alpha1.LinodeVPCSpec{ + VPCID: ptr.To(1), + Region: "us-east", + Subnets: []infrav1alpha1.VPCSubnetCreateOptions{ + {Label: "subnet1", IPv4: "10.0.0.0/8"}, + }, + } + linodeVPC = infrav1alpha1.LinodeVPC{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Labels: make(map[string]string), + }, + Spec: vpcSpec, + } + + getVPC := mockLinodeClient.EXPECT(). + GetVPC(ctx, gomock.Any()). + Return(&linodego.VPC{ + ID: 1, + Label: "vpc1", + Subnets: []linodego.VPCSubnet{ + {ID: 123, Linodes: []linodego.VPCSubnetLinode{}}, + }, + }, nil) + mockLinodeClient.EXPECT(). + DeleteVPC(ctx, gomock.Any()). + After(getVPC). + Return(errors.New("service unavailable")) + + vpcScope := scope.VPCScope{ + Client: k8sClient, + LinodeClient: mockLinodeClient, + LinodeVPC: &linodeVPC, + } + + _, err := reconciler.reconcileDelete(ctx, logger, &vpcScope) + Expect(err).To(HaveOccurred()) + Expect(testLogs.String()).To(ContainSubstring("Failed to delete VPC")) + }) + + It("requeues for reconciliation if linodes are still attached to VPC", func(ctx SpecContext) { + mockLinodeClient := mock.NewMockLinodeVPCClient(mockCtrl) + + vpcSpec := infrav1alpha1.LinodeVPCSpec{ + VPCID: ptr.To(1), + Region: "us-east", + Subnets: []infrav1alpha1.VPCSubnetCreateOptions{ + {Label: "subnet1", IPv4: "10.0.0.0/8"}, + }, + } + linodeVPC = infrav1alpha1.LinodeVPC{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Labels: make(map[string]string), + }, + Spec: vpcSpec, + } + + mockLinodeClient.EXPECT(). + GetVPC(ctx, gomock.Any()). + Return(&linodego.VPC{ + ID: 1, + Label: "vpc1", + Updated: ptr.To(time.Now()), + Subnets: []linodego.VPCSubnet{ + { + ID: 123, + Linodes: []linodego.VPCSubnetLinode{ + {ID: 2, Interfaces: []linodego.VPCSubnetLinodeInterface{}}, + }}, + }, + }, nil) + + vpcScope := scope.VPCScope{ + Client: k8sClient, + LinodeClient: mockLinodeClient, + LinodeVPC: &linodeVPC, + } + + resp, err := reconciler.reconcileDelete(ctx, logger, &vpcScope) + Expect(err).NotTo(HaveOccurred()) + Expect(resp.RequeueAfter).To(Equal(rec.DefaultVPCControllerWaitForHasNodesDelay)) + }) + }) +}) diff --git a/mock/client.go b/mock/client.go index f2bdd9215..801d9fbdd 100644 --- a/mock/client.go +++ b/mock/client.go @@ -148,6 +148,21 @@ func (mr *MockLinodeMachineClientMockRecorder) CreateStackscript(ctx, opts any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateStackscript", reflect.TypeOf((*MockLinodeMachineClient)(nil).CreateStackscript), ctx, opts) } +// CreateVPC mocks base method. +func (m *MockLinodeMachineClient) CreateVPC(ctx context.Context, opts linodego.VPCCreateOptions) (*linodego.VPC, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateVPC", ctx, opts) + ret0, _ := ret[0].(*linodego.VPC) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateVPC indicates an expected call of CreateVPC. +func (mr *MockLinodeMachineClientMockRecorder) CreateVPC(ctx, opts any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateVPC", reflect.TypeOf((*MockLinodeMachineClient)(nil).CreateVPC), ctx, opts) +} + // DeleteInstance mocks base method. func (m *MockLinodeMachineClient) DeleteInstance(ctx context.Context, linodeID int) error { m.ctrl.T.Helper() @@ -190,6 +205,20 @@ func (mr *MockLinodeMachineClientMockRecorder) DeleteNodeBalancerNode(ctx, nodeb return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteNodeBalancerNode", reflect.TypeOf((*MockLinodeMachineClient)(nil).DeleteNodeBalancerNode), ctx, nodebalancerID, configID, nodeID) } +// DeleteVPC mocks base method. +func (m *MockLinodeMachineClient) DeleteVPC(ctx context.Context, vpcID int) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteVPC", ctx, vpcID) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteVPC indicates an expected call of DeleteVPC. +func (mr *MockLinodeMachineClientMockRecorder) DeleteVPC(ctx, vpcID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteVPC", reflect.TypeOf((*MockLinodeMachineClient)(nil).DeleteVPC), ctx, vpcID) +} + // GetImage mocks base method. func (m *MockLinodeMachineClient) GetImage(ctx context.Context, imageID string) (*linodego.Image, error) { m.ctrl.T.Helper() @@ -340,6 +369,21 @@ func (mr *MockLinodeMachineClientMockRecorder) ListStackscripts(ctx, opts any) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListStackscripts", reflect.TypeOf((*MockLinodeMachineClient)(nil).ListStackscripts), ctx, opts) } +// ListVPCs mocks base method. +func (m *MockLinodeMachineClient) ListVPCs(ctx context.Context, opts *linodego.ListOptions) ([]linodego.VPC, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListVPCs", ctx, opts) + ret0, _ := ret[0].([]linodego.VPC) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListVPCs indicates an expected call of ListVPCs. +func (mr *MockLinodeMachineClientMockRecorder) ListVPCs(ctx, opts any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListVPCs", reflect.TypeOf((*MockLinodeMachineClient)(nil).ListVPCs), ctx, opts) +} + // ResizeInstanceDisk mocks base method. func (m *MockLinodeMachineClient) ResizeInstanceDisk(ctx context.Context, linodeID, diskID, size int) error { m.ctrl.T.Helper() @@ -637,6 +681,35 @@ func (m *MockLinodeVPCClient) EXPECT() *MockLinodeVPCClientMockRecorder { return m.recorder } +// CreateVPC mocks base method. +func (m *MockLinodeVPCClient) CreateVPC(ctx context.Context, opts linodego.VPCCreateOptions) (*linodego.VPC, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateVPC", ctx, opts) + ret0, _ := ret[0].(*linodego.VPC) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateVPC indicates an expected call of CreateVPC. +func (mr *MockLinodeVPCClientMockRecorder) CreateVPC(ctx, opts any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateVPC", reflect.TypeOf((*MockLinodeVPCClient)(nil).CreateVPC), ctx, opts) +} + +// DeleteVPC mocks base method. +func (m *MockLinodeVPCClient) DeleteVPC(ctx context.Context, vpcID int) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteVPC", ctx, vpcID) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteVPC indicates an expected call of DeleteVPC. +func (mr *MockLinodeVPCClientMockRecorder) DeleteVPC(ctx, vpcID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteVPC", reflect.TypeOf((*MockLinodeVPCClient)(nil).DeleteVPC), ctx, vpcID) +} + // GetVPC mocks base method. func (m *MockLinodeVPCClient) GetVPC(ctx context.Context, vpcID int) (*linodego.VPC, error) { m.ctrl.T.Helper() @@ -652,6 +725,21 @@ func (mr *MockLinodeVPCClientMockRecorder) GetVPC(ctx, vpcID any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVPC", reflect.TypeOf((*MockLinodeVPCClient)(nil).GetVPC), ctx, vpcID) } +// ListVPCs mocks base method. +func (m *MockLinodeVPCClient) ListVPCs(ctx context.Context, opts *linodego.ListOptions) ([]linodego.VPC, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListVPCs", ctx, opts) + ret0, _ := ret[0].([]linodego.VPC) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListVPCs indicates an expected call of ListVPCs. +func (mr *MockLinodeVPCClientMockRecorder) ListVPCs(ctx, opts any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListVPCs", reflect.TypeOf((*MockLinodeVPCClient)(nil).ListVPCs), ctx, opts) +} + // MockLinodeNodeBalancerClient is a mock of LinodeNodeBalancerClient interface. type MockLinodeNodeBalancerClient struct { ctrl *gomock.Controller