From 24fc95ff62188b3eee55a9d61fa49e2f5af63589 Mon Sep 17 00:00:00 2001 From: Tasdik Rahman Date: Fri, 14 Jun 2024 12:39:15 +0200 Subject: [PATCH 1/3] fix: Prevent GCPManagedCluster reconciler to not crash if Network.Name and Network.Subnets is not passed to it --- cloud/services/container/clusters/errors.go | 3 +++ cloud/services/container/clusters/reconcile.go | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/cloud/services/container/clusters/errors.go b/cloud/services/container/clusters/errors.go index 6b25e36b6..2d9fbabbd 100644 --- a/cloud/services/container/clusters/errors.go +++ b/cloud/services/container/clusters/errors.go @@ -23,6 +23,9 @@ import ( // ErrAutopilotClusterMachinePoolsNotAllowed is used when there are machine pools specified for an autopilot enabled cluster. var ErrAutopilotClusterMachinePoolsNotAllowed = errors.New("cannot use machine pools with an autopilot enabled cluster") +// ErrGCPManagedClusterHasNoNetworkDefined is used when there is no Network definition provided in a GCPManagedCluster. +var ErrGCPManagedClusterHasNoNetworkDefined = errors.New("cannot reconcile GCPManagedCluster if Network is not defined") + // NewErrUnexpectedClusterStatus creates a new error for an unexpected cluster status. func NewErrUnexpectedClusterStatus(status string) error { return &UnexpectedClusterStatusError{status} diff --git a/cloud/services/container/clusters/reconcile.go b/cloud/services/container/clusters/reconcile.go index 5a3f8cb08..edf56df4c 100644 --- a/cloud/services/container/clusters/reconcile.go +++ b/cloud/services/container/clusters/reconcile.go @@ -30,6 +30,7 @@ import ( "github.com/googleapis/gax-go/v2/apierror" "github.com/pkg/errors" "google.golang.org/grpc/codes" + infrav1 "sigs.k8s.io/cluster-api-provider-gcp/api/v1beta1" infrav1exp "sigs.k8s.io/cluster-api-provider-gcp/exp/api/v1beta1" "sigs.k8s.io/cluster-api-provider-gcp/util/reconciler" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -55,6 +56,14 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) { s.scope.GCPManagedControlPlane.Status.Initialized = false s.scope.GCPManagedControlPlane.Status.Ready = false + if !isNetworkSpecValid(s.scope.GCPManagedCluster.Spec.Network) { + log.Error(ErrGCPManagedClusterHasNoNetworkDefined, "Error Reconciling GCPManagedControlPlane", "name", s.scope.ClusterName()) + conditions.MarkFalse(s.scope.ConditionSetter(), clusterv1.ReadyCondition, infrav1exp.GKEControlPlaneReconciliationFailedReason, clusterv1.ConditionSeverityError, ErrGCPManagedClusterHasNoNetworkDefined.Error()) + conditions.MarkFalse(s.scope.ConditionSetter(), infrav1exp.GKEControlPlaneReadyCondition, infrav1exp.GKEControlPlaneReconciliationFailedReason, clusterv1.ConditionSeverityError, ErrGCPManagedClusterHasNoNetworkDefined.Error()) + conditions.MarkFalse(s.scope.ConditionSetter(), infrav1exp.GKEControlPlaneCreatingCondition, infrav1exp.GKEControlPlaneReconciliationFailedReason, clusterv1.ConditionSeverityError, ErrGCPManagedClusterHasNoNetworkDefined.Error()) + return ctrl.Result{}, ErrGCPManagedClusterHasNoNetworkDefined + } + nodePools, _, err := s.scope.GetAllNodePools(ctx) if err != nil { conditions.MarkFalse(s.scope.ConditionSetter(), clusterv1.ReadyCondition, infrav1exp.GKEControlPlaneReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error()) @@ -449,3 +458,12 @@ func compareMasterAuthorizedNetworksConfig(a, b *containerpb.MasterAuthorizedNet } return true } + +// isNetworkSpecValid checks for the Name and Subnets field inside infrav1.NetworkSpec and returns true +// if both are present. As of now only checking for Name and Subnets field as that is what is being used in the createCluster flow. +func isNetworkSpecValid(network infrav1.NetworkSpec) bool { + if network.Name == nil || len(network.Subnets) == 0 { + return false + } + return true +} From 59e5ba3efc8e8cebe5e5580921c8083ff8871447 Mon Sep 17 00:00:00 2001 From: Tasdik Rahman Date: Mon, 8 Jul 2024 14:08:39 +0200 Subject: [PATCH 2/3] feat(managedmachinepool|managedcontrolplane): Assert for GCPManagedClusterHasNoNetworkDefined when reconciling Also introduces Container interface which implements bits of https://github.com/googleapis/google-cloud-go/blob/a187451a912835703078e5b6a339c514edebe5de/container/apiv1/cluster_manager_client.go#L468 since it's an internal interface. --- cloud/interfaces.go | 21 ++ cloud/scope/managedcontrolplane.go | 14 +- cloud/scope/managedmachinepool.go | 7 +- cloud/scope/mocks.go | 28 ++ cloud/scope/mocks/container.go | 276 ++++++++++++++++++ .../container/clusters/clusters_suite_test.go | 13 + .../container/clusters/reconcile_test.go | 122 ++++++++ go.mod | 1 + go.sum | 2 + 9 files changed, 472 insertions(+), 12 deletions(-) create mode 100644 cloud/scope/mocks.go create mode 100644 cloud/scope/mocks/container.go create mode 100644 cloud/services/container/clusters/clusters_suite_test.go create mode 100644 cloud/services/container/clusters/reconcile_test.go diff --git a/cloud/interfaces.go b/cloud/interfaces.go index e2652a3c0..968d633a1 100644 --- a/cloud/interfaces.go +++ b/cloud/interfaces.go @@ -19,6 +19,10 @@ package cloud import ( "context" + "cloud.google.com/go/container/apiv1/containerpb" + + "github.com/googleapis/gax-go/v2" + ctrl "sigs.k8s.io/controller-runtime" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" @@ -49,6 +53,23 @@ type Client interface { NetworkCloud() Cloud } +// Container is an interface which implements bits of internalClusterManagerClient from since there is no Public Interface for it. +// ref: https://github.com/googleapis/google-cloud-go/blob/a187451a912835703078e5b6a339c514edebe5de/container/apiv1/cluster_manager_client.go#L468 +type Container interface { + Close() error + CreateCluster(context.Context, *containerpb.CreateClusterRequest, ...gax.CallOption) (*containerpb.Operation, error) + UpdateCluster(context.Context, *containerpb.UpdateClusterRequest, ...gax.CallOption) (*containerpb.Operation, error) + DeleteCluster(context.Context, *containerpb.DeleteClusterRequest, ...gax.CallOption) (*containerpb.Operation, error) + GetCluster(context.Context, *containerpb.GetClusterRequest, ...gax.CallOption) (*containerpb.Cluster, error) + ListNodePools(context.Context, *containerpb.ListNodePoolsRequest, ...gax.CallOption) (*containerpb.ListNodePoolsResponse, error) + GetNodePool(context.Context, *containerpb.GetNodePoolRequest, ...gax.CallOption) (*containerpb.NodePool, error) + CreateNodePool(context.Context, *containerpb.CreateNodePoolRequest, ...gax.CallOption) (*containerpb.Operation, error) + DeleteNodePool(context.Context, *containerpb.DeleteNodePoolRequest, ...gax.CallOption) (*containerpb.Operation, error) + SetNodePoolSize(context.Context, *containerpb.SetNodePoolSizeRequest, ...gax.CallOption) (*containerpb.Operation, error) + SetNodePoolAutoscaling(context.Context, *containerpb.SetNodePoolAutoscalingRequest, ...gax.CallOption) (*containerpb.Operation, error) + UpdateNodePool(context.Context, *containerpb.UpdateNodePoolRequest, ...gax.CallOption) (*containerpb.Operation, error) +} + // ClusterGetter is an interface which can get cluster information. type ClusterGetter interface { Client diff --git a/cloud/scope/managedcontrolplane.go b/cloud/scope/managedcontrolplane.go index 8e730bd90..e1a89c7f5 100644 --- a/cloud/scope/managedcontrolplane.go +++ b/cloud/scope/managedcontrolplane.go @@ -20,17 +20,15 @@ import ( "context" "fmt" - "sigs.k8s.io/cluster-api-provider-gcp/util/location" - - "sigs.k8s.io/cluster-api/util/conditions" - - container "cloud.google.com/go/container/apiv1" credentials "cloud.google.com/go/iam/credentials/apiv1" resourcemanager "cloud.google.com/go/resourcemanager/apiv3" "github.com/pkg/errors" + "sigs.k8s.io/cluster-api-provider-gcp/cloud" infrav1exp "sigs.k8s.io/cluster-api-provider-gcp/exp/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-gcp/util/location" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" clusterv1exp "sigs.k8s.io/cluster-api/exp/api/v1beta1" + "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -43,7 +41,7 @@ const ( // ManagedControlPlaneScopeParams defines the input parameters used to create a new Scope. type ManagedControlPlaneScopeParams struct { CredentialsClient *credentials.IamCredentialsClient - ManagedClusterClient *container.ClusterManagerClient + ManagedClusterClient cloud.Container TagBindingsClient *resourcemanager.TagBindingsClient Client client.Client Cluster *clusterv1.Cluster @@ -118,7 +116,7 @@ type ManagedControlPlaneScope struct { Cluster *clusterv1.Cluster GCPManagedCluster *infrav1exp.GCPManagedCluster GCPManagedControlPlane *infrav1exp.GCPManagedControlPlane - mcClient *container.ClusterManagerClient + mcClient cloud.Container tagBindingsClient *resourcemanager.TagBindingsClient credentialsClient *credentials.IamCredentialsClient credential *Credential @@ -159,7 +157,7 @@ func (s *ManagedControlPlaneScope) Client() client.Client { } // ManagedControlPlaneClient returns a client used to interact with GKE. -func (s *ManagedControlPlaneScope) ManagedControlPlaneClient() *container.ClusterManagerClient { +func (s *ManagedControlPlaneScope) ManagedControlPlaneClient() cloud.Container { return s.mcClient } diff --git a/cloud/scope/managedmachinepool.go b/cloud/scope/managedmachinepool.go index 16ee491da..9093f0798 100644 --- a/cloud/scope/managedmachinepool.go +++ b/cloud/scope/managedmachinepool.go @@ -29,7 +29,6 @@ import ( "sigs.k8s.io/cluster-api/util/conditions" compute "cloud.google.com/go/compute/apiv1" - container "cloud.google.com/go/container/apiv1" "cloud.google.com/go/container/apiv1/containerpb" "github.com/pkg/errors" infrav1exp "sigs.k8s.io/cluster-api-provider-gcp/exp/api/v1beta1" @@ -41,7 +40,7 @@ import ( // ManagedMachinePoolScopeParams defines the input parameters used to create a new Scope. type ManagedMachinePoolScopeParams struct { - ManagedClusterClient *container.ClusterManagerClient + ManagedClusterClient cloud.Container InstanceGroupManagersClient *compute.InstanceGroupManagersClient Client client.Client Cluster *clusterv1.Cluster @@ -112,7 +111,7 @@ type ManagedMachinePoolScope struct { GCPManagedCluster *infrav1exp.GCPManagedCluster GCPManagedControlPlane *infrav1exp.GCPManagedControlPlane GCPManagedMachinePool *infrav1exp.GCPManagedMachinePool - mcClient *container.ClusterManagerClient + mcClient cloud.Container migClient *compute.InstanceGroupManagersClient } @@ -142,7 +141,7 @@ func (s *ManagedMachinePoolScope) ConditionSetter() conditions.Setter { } // ManagedMachinePoolClient returns a client used to interact with GKE. -func (s *ManagedMachinePoolScope) ManagedMachinePoolClient() *container.ClusterManagerClient { +func (s *ManagedMachinePoolScope) ManagedMachinePoolClient() cloud.Container { return s.mcClient } diff --git a/cloud/scope/mocks.go b/cloud/scope/mocks.go new file mode 100644 index 000000000..c0ae60a6d --- /dev/null +++ b/cloud/scope/mocks.go @@ -0,0 +1,28 @@ +package scope + +import ( + "context" + + "github.com/pkg/errors" +) + +// NewMockManagedControlPlaneScope creates a new Scope from the supplied parameters. +func NewMockManagedControlPlaneScope(_ context.Context, params ManagedControlPlaneScopeParams) (*ManagedControlPlaneScope, error) { + if params.Cluster == nil { + return nil, errors.New("failed to generate new scope from nil Cluster") + } + if params.GCPManagedCluster == nil { + return nil, errors.New("failed to generate new scope from nil GCPManagedCluster") + } + if params.GCPManagedControlPlane == nil { + return nil, errors.New("failed to generate new scope from nil GCPManagedControlPlane") + } + + return &ManagedControlPlaneScope{ + client: params.Client, + Cluster: params.Cluster, + GCPManagedCluster: params.GCPManagedCluster, + GCPManagedControlPlane: params.GCPManagedControlPlane, + mcClient: params.ManagedClusterClient, + }, nil +} diff --git a/cloud/scope/mocks/container.go b/cloud/scope/mocks/container.go new file mode 100644 index 000000000..7257f39f1 --- /dev/null +++ b/cloud/scope/mocks/container.go @@ -0,0 +1,276 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: sigs.k8s.io/cluster-api-provider-gcp/cloud (interfaces: Container) +// +// Generated by this command: +// +// mockgen -destination=cloud/scope/mocks/container.go -package=mocks sigs.k8s.io/cluster-api-provider-gcp/cloud Container +// + +// Package mocks is a generated GoMock package. +package mocks + +import ( + context "context" + reflect "reflect" + + containerpb "cloud.google.com/go/container/apiv1/containerpb" + gax "github.com/googleapis/gax-go/v2" + gomock "go.uber.org/mock/gomock" +) + +// MockContainer is a mock of Container interface. +type MockContainer struct { + ctrl *gomock.Controller + recorder *MockContainerMockRecorder +} + +// MockContainerMockRecorder is the mock recorder for MockContainer. +type MockContainerMockRecorder struct { + mock *MockContainer +} + +// NewMockContainer creates a new mock instance. +func NewMockContainer(ctrl *gomock.Controller) *MockContainer { + mock := &MockContainer{ctrl: ctrl} + mock.recorder = &MockContainerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockContainer) EXPECT() *MockContainerMockRecorder { + return m.recorder +} + +// Close mocks base method. +func (m *MockContainer) Close() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Close") + ret0, _ := ret[0].(error) + return ret0 +} + +// Close indicates an expected call of Close. +func (mr *MockContainerMockRecorder) Close() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockContainer)(nil).Close)) +} + +// CreateCluster mocks base method. +func (m *MockContainer) CreateCluster(arg0 context.Context, arg1 *containerpb.CreateClusterRequest, arg2 ...gax.CallOption) (*containerpb.Operation, error) { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "CreateCluster", varargs...) + ret0, _ := ret[0].(*containerpb.Operation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateCluster indicates an expected call of CreateCluster. +func (mr *MockContainerMockRecorder) CreateCluster(arg0, arg1 any, arg2 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateCluster", reflect.TypeOf((*MockContainer)(nil).CreateCluster), varargs...) +} + +// CreateNodePool mocks base method. +func (m *MockContainer) CreateNodePool(arg0 context.Context, arg1 *containerpb.CreateNodePoolRequest, arg2 ...gax.CallOption) (*containerpb.Operation, error) { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "CreateNodePool", varargs...) + ret0, _ := ret[0].(*containerpb.Operation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateNodePool indicates an expected call of CreateNodePool. +func (mr *MockContainerMockRecorder) CreateNodePool(arg0, arg1 any, arg2 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateNodePool", reflect.TypeOf((*MockContainer)(nil).CreateNodePool), varargs...) +} + +// DeleteCluster mocks base method. +func (m *MockContainer) DeleteCluster(arg0 context.Context, arg1 *containerpb.DeleteClusterRequest, arg2 ...gax.CallOption) (*containerpb.Operation, error) { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "DeleteCluster", varargs...) + ret0, _ := ret[0].(*containerpb.Operation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DeleteCluster indicates an expected call of DeleteCluster. +func (mr *MockContainerMockRecorder) DeleteCluster(arg0, arg1 any, arg2 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteCluster", reflect.TypeOf((*MockContainer)(nil).DeleteCluster), varargs...) +} + +// DeleteNodePool mocks base method. +func (m *MockContainer) DeleteNodePool(arg0 context.Context, arg1 *containerpb.DeleteNodePoolRequest, arg2 ...gax.CallOption) (*containerpb.Operation, error) { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "DeleteNodePool", varargs...) + ret0, _ := ret[0].(*containerpb.Operation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DeleteNodePool indicates an expected call of DeleteNodePool. +func (mr *MockContainerMockRecorder) DeleteNodePool(arg0, arg1 any, arg2 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteNodePool", reflect.TypeOf((*MockContainer)(nil).DeleteNodePool), varargs...) +} + +// GetCluster mocks base method. +func (m *MockContainer) GetCluster(arg0 context.Context, arg1 *containerpb.GetClusterRequest, arg2 ...gax.CallOption) (*containerpb.Cluster, error) { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetCluster", varargs...) + ret0, _ := ret[0].(*containerpb.Cluster) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetCluster indicates an expected call of GetCluster. +func (mr *MockContainerMockRecorder) GetCluster(arg0, arg1 any, arg2 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCluster", reflect.TypeOf((*MockContainer)(nil).GetCluster), varargs...) +} + +// GetNodePool mocks base method. +func (m *MockContainer) GetNodePool(arg0 context.Context, arg1 *containerpb.GetNodePoolRequest, arg2 ...gax.CallOption) (*containerpb.NodePool, error) { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetNodePool", varargs...) + ret0, _ := ret[0].(*containerpb.NodePool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetNodePool indicates an expected call of GetNodePool. +func (mr *MockContainerMockRecorder) GetNodePool(arg0, arg1 any, arg2 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNodePool", reflect.TypeOf((*MockContainer)(nil).GetNodePool), varargs...) +} + +// ListNodePools mocks base method. +func (m *MockContainer) ListNodePools(arg0 context.Context, arg1 *containerpb.ListNodePoolsRequest, arg2 ...gax.CallOption) (*containerpb.ListNodePoolsResponse, error) { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "ListNodePools", varargs...) + ret0, _ := ret[0].(*containerpb.ListNodePoolsResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListNodePools indicates an expected call of ListNodePools. +func (mr *MockContainerMockRecorder) ListNodePools(arg0, arg1 any, arg2 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListNodePools", reflect.TypeOf((*MockContainer)(nil).ListNodePools), varargs...) +} + +// SetNodePoolAutoscaling mocks base method. +func (m *MockContainer) SetNodePoolAutoscaling(arg0 context.Context, arg1 *containerpb.SetNodePoolAutoscalingRequest, arg2 ...gax.CallOption) (*containerpb.Operation, error) { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "SetNodePoolAutoscaling", varargs...) + ret0, _ := ret[0].(*containerpb.Operation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// SetNodePoolAutoscaling indicates an expected call of SetNodePoolAutoscaling. +func (mr *MockContainerMockRecorder) SetNodePoolAutoscaling(arg0, arg1 any, arg2 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetNodePoolAutoscaling", reflect.TypeOf((*MockContainer)(nil).SetNodePoolAutoscaling), varargs...) +} + +// SetNodePoolSize mocks base method. +func (m *MockContainer) SetNodePoolSize(arg0 context.Context, arg1 *containerpb.SetNodePoolSizeRequest, arg2 ...gax.CallOption) (*containerpb.Operation, error) { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "SetNodePoolSize", varargs...) + ret0, _ := ret[0].(*containerpb.Operation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// SetNodePoolSize indicates an expected call of SetNodePoolSize. +func (mr *MockContainerMockRecorder) SetNodePoolSize(arg0, arg1 any, arg2 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetNodePoolSize", reflect.TypeOf((*MockContainer)(nil).SetNodePoolSize), varargs...) +} + +// UpdateCluster mocks base method. +func (m *MockContainer) UpdateCluster(arg0 context.Context, arg1 *containerpb.UpdateClusterRequest, arg2 ...gax.CallOption) (*containerpb.Operation, error) { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "UpdateCluster", varargs...) + ret0, _ := ret[0].(*containerpb.Operation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateCluster indicates an expected call of UpdateCluster. +func (mr *MockContainerMockRecorder) UpdateCluster(arg0, arg1 any, arg2 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateCluster", reflect.TypeOf((*MockContainer)(nil).UpdateCluster), varargs...) +} + +// UpdateNodePool mocks base method. +func (m *MockContainer) UpdateNodePool(arg0 context.Context, arg1 *containerpb.UpdateNodePoolRequest, arg2 ...gax.CallOption) (*containerpb.Operation, error) { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1} + for _, a := range arg2 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "UpdateNodePool", varargs...) + ret0, _ := ret[0].(*containerpb.Operation) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateNodePool indicates an expected call of UpdateNodePool. +func (mr *MockContainerMockRecorder) UpdateNodePool(arg0, arg1 any, arg2 ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1}, arg2...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateNodePool", reflect.TypeOf((*MockContainer)(nil).UpdateNodePool), varargs...) +} diff --git a/cloud/services/container/clusters/clusters_suite_test.go b/cloud/services/container/clusters/clusters_suite_test.go new file mode 100644 index 000000000..3e801a9d4 --- /dev/null +++ b/cloud/services/container/clusters/clusters_suite_test.go @@ -0,0 +1,13 @@ +package clusters + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestClusters(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Clusters Suite") +} diff --git a/cloud/services/container/clusters/reconcile_test.go b/cloud/services/container/clusters/reconcile_test.go new file mode 100644 index 000000000..531bb410f --- /dev/null +++ b/cloud/services/container/clusters/reconcile_test.go @@ -0,0 +1,122 @@ +/* +Copyright 2021 The Kubernetes Authors. + +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 clusters + +import ( + "context" + + "cloud.google.com/go/container/apiv1/containerpb" + + "go.uber.org/mock/gomock" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + + infrav1 "sigs.k8s.io/cluster-api-provider-gcp/api/v1beta1" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + + cloudScope "sigs.k8s.io/cluster-api-provider-gcp/cloud/scope" + mockContainer "sigs.k8s.io/cluster-api-provider-gcp/cloud/scope/mocks" + infrav1exp "sigs.k8s.io/cluster-api-provider-gcp/exp/api/v1beta1" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func init() { + _ = clusterv1.AddToScheme(scheme.Scheme) + _ = infrav1.AddToScheme(scheme.Scheme) +} + +var ( + fakeCluster *clusterv1.Cluster + fakeGCPManagedCluster *infrav1exp.GCPManagedCluster + fakeGCPManagedControlPlane *infrav1exp.GCPManagedControlPlane + ctx context.Context +) + +var _ = Describe("ManagedControlPlaneScope Reconcile", func() { + BeforeEach(func() { + fakeCluster = &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + Spec: clusterv1.ClusterSpec{}, + } + + fakeGCPManagedCluster = &infrav1exp.GCPManagedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + Spec: infrav1exp.GCPManagedClusterSpec{ + CredentialsRef: &infrav1.ObjectReference{ + Namespace: "default", + Name: "my-cluster", + }, + }, + } + + fakeGCPManagedControlPlane = &infrav1exp.GCPManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + Spec: infrav1exp.GCPManagedControlPlaneSpec{ + Location: "us-west1-a", + Project: "foo-project", + ClusterName: "my-cluster", + }, + } + }) + + Context("When GCPManagedCluster passed doesn't have a name present in the network spec", func() { + It("should log an error message and return from the reconcile", func() { + fakec := fake.NewClientBuilder(). + WithScheme(scheme.Scheme). + Build() + + mockCtrl := gomock.NewController(GinkgoT()) + defer mockCtrl.Finish() + + mockInternalClusterManagerClient := mockContainer.NewMockContainer(mockCtrl) + getClusterRequest := &containerpb.GetClusterRequest{ + Name: "projects/foo-project/locations/us-west1/clusters/my-cluster", + } + mockInternalClusterManagerClient.EXPECT().GetCluster(ctx, getClusterRequest).Return(nil, nil).Times(1) + + testManagedControlPlaneScope, err := cloudScope.NewMockManagedControlPlaneScope(ctx, cloudScope.ManagedControlPlaneScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPManagedCluster: fakeGCPManagedCluster, + GCPManagedControlPlane: fakeGCPManagedControlPlane, + ManagedClusterClient: mockInternalClusterManagerClient, + }) + + reconcile := New(testManagedControlPlaneScope) + result, err := reconcile.Reconcile(ctx) + + Expect(result).To(Equal(ctrl.Result{})) + Expect(err).NotTo(BeNil()) + Expect(err).To(Equal(ErrGCPManagedClusterHasNoNetworkDefined)) + }) + }) +}) diff --git a/go.mod b/go.mod index 3a751baed..6250d5633 100644 --- a/go.mod +++ b/go.mod @@ -70,6 +70,7 @@ require ( go.opentelemetry.io/otel/sdk v1.24.0 // indirect go.opentelemetry.io/otel/trace v1.24.0 // indirect go.opentelemetry.io/proto/otlp v1.0.0 // indirect + go.uber.org/mock v0.4.0 // indirect golang.org/x/exp v0.0.0-20230905200255-921286631fa9 // indirect golang.org/x/sync v0.7.0 // indirect sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.28.0 // indirect diff --git a/go.sum b/go.sum index c77617745..dc978a6da 100644 --- a/go.sum +++ b/go.sum @@ -372,6 +372,8 @@ go.opentelemetry.io/proto/otlp v1.0.0 h1:T0TX0tmXU8a3CbNXzEKGeU5mIVOdf0oykP+u2lI go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v80hjKIs5JXpM= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= +go.uber.org/mock v0.4.0 h1:VcM4ZOtdbR4f6VXfiOpwpVJDL6lCReaZ6mw31wqh7KU= +go.uber.org/mock v0.4.0/go.mod h1:a6FSlNadKUHUa9IP5Vyt1zh4fC7uAwxMutEAscFbkZc= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= From bf6e61f53c0b3e2f0845ea3cdb149d98259c09db Mon Sep 17 00:00:00 2001 From: Tasdik Rahman Date: Mon, 8 Jul 2024 14:12:48 +0200 Subject: [PATCH 3/3] fix: lint issues for container clusters reconcile tests --- cloud/services/container/clusters/reconcile_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cloud/services/container/clusters/reconcile_test.go b/cloud/services/container/clusters/reconcile_test.go index 531bb410f..02a93e890 100644 --- a/cloud/services/container/clusters/reconcile_test.go +++ b/cloud/services/container/clusters/reconcile_test.go @@ -103,7 +103,7 @@ var _ = Describe("ManagedControlPlaneScope Reconcile", func() { } mockInternalClusterManagerClient.EXPECT().GetCluster(ctx, getClusterRequest).Return(nil, nil).Times(1) - testManagedControlPlaneScope, err := cloudScope.NewMockManagedControlPlaneScope(ctx, cloudScope.ManagedControlPlaneScopeParams{ + testManagedControlPlaneScope, _ := cloudScope.NewMockManagedControlPlaneScope(ctx, cloudScope.ManagedControlPlaneScopeParams{ Client: fakec, Cluster: fakeCluster, GCPManagedCluster: fakeGCPManagedCluster, @@ -115,7 +115,6 @@ var _ = Describe("ManagedControlPlaneScope Reconcile", func() { result, err := reconcile.Reconcile(ctx) Expect(result).To(Equal(ctrl.Result{})) - Expect(err).NotTo(BeNil()) Expect(err).To(Equal(ErrGCPManagedClusterHasNoNetworkDefined)) }) })