diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 32bbb0d4f..60d943ebf 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -10,6 +10,7 @@ import ( "github.com/openshift/cluster-api-provider-gcp/pkg/apis" "github.com/openshift/cluster-api-provider-gcp/pkg/cloud/gcp/actuators/machine" machinesetcontroller "github.com/openshift/cluster-api-provider-gcp/pkg/cloud/gcp/actuators/machineset" + computeservice "github.com/openshift/cluster-api-provider-gcp/pkg/cloud/gcp/actuators/services/compute" "github.com/openshift/cluster-api-provider-gcp/pkg/version" "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" capimachine "github.com/openshift/machine-api-operator/pkg/controller/machine" @@ -99,8 +100,9 @@ func main() { // Initialize machine actuator. machineActuator := machine.NewActuator(machine.ActuatorParams{ - CoreClient: mgr.GetClient(), - EventRecorder: mgr.GetEventRecorderFor("gcpcontroller"), + CoreClient: mgr.GetClient(), + EventRecorder: mgr.GetEventRecorderFor("gcpcontroller"), + ComputeClientBuilder: computeservice.NewComputeService, }) if err := apis.AddToScheme(mgr.GetScheme()); err != nil { diff --git a/pkg/cloud/gcp/actuators/machine/actuator.go b/pkg/cloud/gcp/actuators/machine/actuator.go index 117b4a56d..ef7e682ae 100644 --- a/pkg/cloud/gcp/actuators/machine/actuator.go +++ b/pkg/cloud/gcp/actuators/machine/actuator.go @@ -7,6 +7,7 @@ import ( "context" "fmt" + computeservice "github.com/openshift/cluster-api-provider-gcp/pkg/cloud/gcp/actuators/services/compute" machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/record" @@ -15,7 +16,8 @@ import ( ) const ( - scopeFailFmt = "%s: failed to create scope for machine: %v" + scopeFailFmt = "%s: failed to create scope for machine: %w" + reconcilerFailFmt = "%s: reconciler failed to %s machine: %w" createEventAction = "Create" updateEventAction = "Update" deleteEventAction = "Delete" @@ -24,31 +26,34 @@ const ( // Actuator is responsible for performing machine reconciliation. type Actuator struct { - coreClient controllerclient.Client - eventRecorder record.EventRecorder + coreClient controllerclient.Client + eventRecorder record.EventRecorder + computeClientBuilder computeservice.BuilderFuncType } // ActuatorParams holds parameter information for Actuator. type ActuatorParams struct { - CoreClient controllerclient.Client - EventRecorder record.EventRecorder + CoreClient controllerclient.Client + EventRecorder record.EventRecorder + ComputeClientBuilder computeservice.BuilderFuncType } // NewActuator returns an actuator. func NewActuator(params ActuatorParams) *Actuator { return &Actuator{ - coreClient: params.CoreClient, - eventRecorder: params.EventRecorder, + coreClient: params.CoreClient, + eventRecorder: params.EventRecorder, + computeClientBuilder: params.ComputeClientBuilder, } } // Set corresponding event based on error. It also returns the original error // for convenience, so callers can do "return handleMachineError(...)". func (a *Actuator) handleMachineError(machine *machinev1.Machine, err error, eventAction string) error { + klog.Errorf("%v error: %v", machine.GetName(), err) if eventAction != noEventAction { a.eventRecorder.Eventf(machine, corev1.EventTypeWarning, "Failed"+eventAction, "%v", err) } - return err } @@ -56,16 +61,20 @@ func (a *Actuator) handleMachineError(machine *machinev1.Machine, err error, eve func (a *Actuator) Create(ctx context.Context, machine *machinev1.Machine) error { klog.Infof("%s: Creating machine", machine.Name) scope, err := newMachineScope(machineScopeParams{ - coreClient: a.coreClient, - machine: machine, + Context: ctx, + coreClient: a.coreClient, + machine: machine, + computeClientBuilder: a.computeClientBuilder, }) if err != nil { - return a.handleMachineError(machine, err, createEventAction) + fmtErr := fmt.Errorf(scopeFailFmt, machine.GetName(), err) + return a.handleMachineError(machine, fmtErr, createEventAction) } if err := newReconciler(scope).create(); err != nil { // Update machine and machine status in case it was modified scope.Close() - return a.handleMachineError(machine, err, createEventAction) + fmtErr := fmt.Errorf(reconcilerFailFmt, machine.GetName(), createEventAction, err) + return a.handleMachineError(machine, fmtErr, createEventAction) } a.eventRecorder.Eventf(machine, corev1.EventTypeNormal, createEventAction, "Created Machine %v", machine.Name) return scope.Close() @@ -74,8 +83,10 @@ func (a *Actuator) Create(ctx context.Context, machine *machinev1.Machine) error func (a *Actuator) Exists(ctx context.Context, machine *machinev1.Machine) (bool, error) { klog.Infof("%s: Checking if machine exists", machine.Name) scope, err := newMachineScope(machineScopeParams{ - coreClient: a.coreClient, - machine: machine, + Context: ctx, + coreClient: a.coreClient, + machine: machine, + computeClientBuilder: a.computeClientBuilder, }) if err != nil { return false, fmt.Errorf(scopeFailFmt, machine.Name, err) @@ -91,17 +102,20 @@ func (a *Actuator) Exists(ctx context.Context, machine *machinev1.Machine) (bool func (a *Actuator) Update(ctx context.Context, machine *machinev1.Machine) error { klog.Infof("%s: Updating machine", machine.Name) scope, err := newMachineScope(machineScopeParams{ - coreClient: a.coreClient, - machine: machine, + Context: ctx, + coreClient: a.coreClient, + machine: machine, + computeClientBuilder: a.computeClientBuilder, }) if err != nil { - fmtErr := fmt.Sprintf(scopeFailFmt, machine.Name, err) - return a.handleMachineError(machine, fmt.Errorf(fmtErr), updateEventAction) + fmtErr := fmt.Errorf(scopeFailFmt, machine.GetName(), err) + return a.handleMachineError(machine, fmtErr, updateEventAction) } if err := newReconciler(scope).update(); err != nil { // Update machine and machine status in case it was modified scope.Close() - return a.handleMachineError(machine, err, updateEventAction) + fmtErr := fmt.Errorf(reconcilerFailFmt, machine.GetName(), updateEventAction, err) + return a.handleMachineError(machine, fmtErr, updateEventAction) } a.eventRecorder.Eventf(machine, corev1.EventTypeNormal, updateEventAction, "Updated Machine %v", machine.Name) return scope.Close() @@ -110,15 +124,18 @@ func (a *Actuator) Update(ctx context.Context, machine *machinev1.Machine) error func (a *Actuator) Delete(ctx context.Context, machine *machinev1.Machine) error { klog.Infof("%s: Deleting machine", machine.Name) scope, err := newMachineScope(machineScopeParams{ - coreClient: a.coreClient, - machine: machine, + Context: ctx, + coreClient: a.coreClient, + machine: machine, + computeClientBuilder: a.computeClientBuilder, }) if err != nil { - fmtErr := fmt.Sprintf(scopeFailFmt, machine.Name, err) - return a.handleMachineError(machine, fmt.Errorf(fmtErr), deleteEventAction) + fmtErr := fmt.Errorf(scopeFailFmt, machine.GetName(), err) + return a.handleMachineError(machine, fmtErr, deleteEventAction) } if err := newReconciler(scope).delete(); err != nil { - return a.handleMachineError(machine, err, deleteEventAction) + fmtErr := fmt.Errorf(reconcilerFailFmt, machine.GetName(), deleteEventAction, err) + return a.handleMachineError(machine, fmtErr, deleteEventAction) } a.eventRecorder.Eventf(machine, corev1.EventTypeNormal, deleteEventAction, "Deleted machine %v", machine.Name) return nil diff --git a/pkg/cloud/gcp/actuators/machine/actuator_test.go b/pkg/cloud/gcp/actuators/machine/actuator_test.go index dcfe6b96f..a806465ef 100644 --- a/pkg/cloud/gcp/actuators/machine/actuator_test.go +++ b/pkg/cloud/gcp/actuators/machine/actuator_test.go @@ -1,12 +1,34 @@ package machine import ( + "context" + "fmt" + "path/filepath" "testing" + "time" + . "github.com/onsi/gomega" + "github.com/openshift/cluster-api-provider-gcp/pkg/apis/gcpprovider/v1beta1" + computeservice "github.com/openshift/cluster-api-provider-gcp/pkg/cloud/gcp/actuators/services/compute" machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" + + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" - "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client" + controllerfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/controller-runtime/pkg/manager" +) + +var ( + userDataSecretName = "user-data-test" + credentialsSecretName = "credentials-test" + defaultNamespaceName = "test" + credentialsSecretKey = "service_account.json" ) func init() { @@ -14,17 +36,335 @@ func init() { machinev1.AddToScheme(scheme.Scheme) } -func TestActuatorCreate(t *testing.T) { - eventsChannel := make(chan string, 1) - recorder := &record.FakeRecorder{ - Events: eventsChannel, +func TestActuatorEvents(t *testing.T) { + g := NewWithT(t) + timeout := 10 * time.Second + + testEnv := &envtest.Environment{ + CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "..", "..", "config", "crds")}, + } + + cfg, err := testEnv.Start() + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(cfg).ToNot(BeNil()) + defer func() { + g.Expect(testEnv.Stop()).To(Succeed()) + }() + + mgr, err := manager.New(cfg, manager.Options{ + Scheme: scheme.Scheme, + MetricsBindAddress: "0", + }) + if err != nil { + t.Fatal(err) + } + + doneMgr := make(chan struct{}) + defer close(doneMgr) + + go func() { + g.Expect(mgr.Start(doneMgr)).To(Succeed()) + }() + + k8sClient := mgr.GetClient() + eventRecorder := mgr.GetEventRecorderFor("vspherecontroller") + + defaultNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultNamespaceName, + }, } - // Initialize machine actuator. - machineActuator := NewActuator(ActuatorParams{ - CoreClient: fake.NewFakeClient(), - EventRecorder: recorder, + g.Expect(k8sClient.Create(context.Background(), defaultNamespace)).To(Succeed()) + defer func() { + g.Expect(k8sClient.Delete(context.Background(), defaultNamespace)).To(Succeed()) + }() + + userDataSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: userDataSecretName, + Namespace: defaultNamespaceName, + }, + Data: map[string][]byte{ + userDataSecretKey: []byte("userDataBlob"), + }, + } + + g.Expect(k8sClient.Create(context.Background(), userDataSecret)).To(Succeed()) + defer func() { + g.Expect(k8sClient.Delete(context.Background(), userDataSecret)).To(Succeed()) + }() + + credentialsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: credentialsSecretName, + Namespace: defaultNamespaceName, + }, + Data: map[string][]byte{ + credentialsSecretKey: []byte("{\"project_id\": \"test\"}"), + }, + } + + g.Expect(k8sClient.Create(context.Background(), credentialsSecret)).To(Succeed()) + defer func() { + g.Expect(k8sClient.Delete(context.Background(), credentialsSecret)).To(Succeed()) + }() + + providerSpec, err := v1beta1.RawExtensionFromProviderSpec(&v1beta1.GCPMachineProviderSpec{ + CredentialsSecret: &corev1.LocalObjectReference{ + Name: credentialsSecretName, + }, }) - if machineActuator == nil { - t.Errorf("expected machine not nil") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(providerSpec).ToNot(BeNil()) + + cases := []struct { + name string + error string + operation func(actuator *Actuator, machine *machinev1.Machine) + event string + }{ + { + name: "Create machine event failed on invalid machine scope", + operation: func(actuator *Actuator, machine *machinev1.Machine) { + machine.Spec = machinev1.MachineSpec{ + ProviderSpec: machinev1.ProviderSpec{ + Value: &runtime.RawExtension{ + Raw: []byte{'1'}, + }, + }, + } + actuator.Create(context.Background(), machine) + }, + event: "test: failed to create scope for machine: failed to get machine config: error unmarshalling providerSpec: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal number into Go value of type v1beta1.GCPMachineProviderSpec", + }, + { + name: "Create machine event failed, reconciler's create failed", + operation: func(actuator *Actuator, machine *machinev1.Machine) { + machine.Labels[machinev1.MachineClusterIDLabel] = "" + actuator.Create(context.Background(), machine) + }, + event: "test: reconciler failed to Create machine: failed validating machine provider spec: machine is missing \"machine.openshift.io/cluster-api-cluster\" label", + }, + { + name: "Create machine event succeed", + operation: func(actuator *Actuator, machine *machinev1.Machine) { + actuator.Create(context.Background(), machine) + }, + event: "Created Machine test", + }, + { + name: "Update machine event failed on invalid machine scope", + operation: func(actuator *Actuator, machine *machinev1.Machine) { + machine.Spec = machinev1.MachineSpec{ + ProviderSpec: machinev1.ProviderSpec{ + Value: &runtime.RawExtension{ + Raw: []byte{'1'}, + }, + }, + } + actuator.Update(context.Background(), machine) + }, + event: "test: failed to create scope for machine: failed to get machine config: error unmarshalling providerSpec: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal number into Go value of type v1beta1.GCPMachineProviderSpec", + }, + { + name: "Update machine event failed, reconciler's update failed", + operation: func(actuator *Actuator, machine *machinev1.Machine) { + machine.Labels[machinev1.MachineClusterIDLabel] = "" + actuator.Update(context.Background(), machine) + }, + event: "test: reconciler failed to Update machine: failed validating machine provider spec: machine is missing \"machine.openshift.io/cluster-api-cluster\" label", + }, + { + name: "Update machine event succeed and only one event is created", + operation: func(actuator *Actuator, machine *machinev1.Machine) { + actuator.Update(context.Background(), machine) + actuator.Update(context.Background(), machine) + }, + event: "Updated Machine test", + }, + { + name: "Delete machine event failed on invalid machine scope", + operation: func(actuator *Actuator, machine *machinev1.Machine) { + machine.Spec = machinev1.MachineSpec{ + ProviderSpec: machinev1.ProviderSpec{ + Value: &runtime.RawExtension{ + Raw: []byte{'1'}, + }, + }, + } + actuator.Delete(context.Background(), machine) + }, + event: "test: failed to create scope for machine: failed to get machine config: error unmarshalling providerSpec: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal number into Go value of type v1beta1.GCPMachineProviderSpec", + }, + { + name: "Delete machine event failed, reconciler's delete failed", + operation: func(actuator *Actuator, machine *machinev1.Machine) { + actuator.Delete(context.Background(), machine) + }, + event: "test: reconciler failed to Delete machine: requeue in: 20s", + }, + { + name: "Delete machine event succeed", + operation: func(actuator *Actuator, machine *machinev1.Machine) { + actuator.computeClientBuilder = computeservice.MockBuilderFuncTypeNotFound + actuator.Delete(context.Background(), machine) + }, + event: "Deleted machine test", + }, } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + gs := NewWithT(t) + + machine := &machinev1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: defaultNamespaceName, + Labels: map[string]string{ + machinev1.MachineClusterIDLabel: "CLUSTERID", + }, + }, + Spec: machinev1.MachineSpec{ + ProviderSpec: machinev1.ProviderSpec{ + Value: providerSpec, + }, + }} + + // Create the machine + gs.Expect(k8sClient.Create(context.Background(), machine)).To(Succeed()) + defer func() { + gs.Expect(k8sClient.Delete(context.Background(), machine)).To(Succeed()) + }() + + // Ensure the machine has synced to the cache + getMachine := func() error { + machineKey := types.NamespacedName{Namespace: machine.Namespace, Name: machine.Name} + return k8sClient.Get(context.Background(), machineKey, machine) + } + gs.Eventually(getMachine, timeout).Should(Succeed()) + + params := ActuatorParams{ + CoreClient: k8sClient, + EventRecorder: eventRecorder, + ComputeClientBuilder: computeservice.MockBuilderFuncType, + } + + actuator := NewActuator(params) + tc.operation(actuator, machine) + + eventList := &v1.EventList{} + waitForEvent := func() error { + err := k8sClient.List(context.Background(), eventList, client.InNamespace(machine.Namespace)) + if err != nil { + return err + } + + if len(eventList.Items) != 1 { + return fmt.Errorf("expected len 1, got %d", len(eventList.Items)) + } + return nil + } + + gs.Eventually(waitForEvent, timeout).Should(Succeed()) + + gs.Expect(eventList.Items[0].Message).To(Equal(tc.event)) + + for i := range eventList.Items { + gs.Expect(k8sClient.Delete(context.Background(), &eventList.Items[i])).To(Succeed()) + } + }) + } +} + +func TestActuatorExists(t *testing.T) { + userDataSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: userDataSecretName, + Namespace: defaultNamespaceName, + }, + Data: map[string][]byte{ + userDataSecretKey: []byte("userDataBlob"), + }, + } + + credentialsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: credentialsSecretName, + Namespace: defaultNamespaceName, + }, + Data: map[string][]byte{ + credentialsSecretKey: []byte("{\"project_id\": \"test\"}"), + }, + } + + providerSpec, err := v1beta1.RawExtensionFromProviderSpec(&v1beta1.GCPMachineProviderSpec{ + CredentialsSecret: &corev1.LocalObjectReference{ + Name: credentialsSecretName, + }, + }) + if err != nil { + t.Fatal(err) + } + + cases := []struct { + name string + expectError bool + }{ + { + name: "succefuly call reconciler exists", + }, + { + name: "fail to call reconciler exists", + expectError: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + machine := &machinev1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: defaultNamespaceName, + Labels: map[string]string{ + machinev1.MachineClusterIDLabel: "CLUSTERID", + }, + }, + Spec: machinev1.MachineSpec{ + ProviderSpec: machinev1.ProviderSpec{ + Value: providerSpec, + }, + }} + + if tc.expectError { + machine.Spec = machinev1.MachineSpec{ + ProviderSpec: machinev1.ProviderSpec{ + Value: &runtime.RawExtension{ + Raw: []byte{'1'}, + }, + }, + } + } + + params := ActuatorParams{ + CoreClient: controllerfake.NewFakeClient(userDataSecret, credentialsSecret), + ComputeClientBuilder: computeservice.MockBuilderFuncType, + } + + actuator := NewActuator(params) + + _, err := actuator.Exists(nil, machine) + + if tc.expectError { + if err == nil { + t.Fatal("actuator exists expected to return an error") + } + } else { + if err != nil { + t.Fatal("actuator exists is not expected to return an error") + } + } + }) + } + } diff --git a/pkg/cloud/gcp/actuators/machine/machine_scope.go b/pkg/cloud/gcp/actuators/machine/machine_scope.go index 61011bc08..7f8155de3 100644 --- a/pkg/cloud/gcp/actuators/machine/machine_scope.go +++ b/pkg/cloud/gcp/actuators/machine/machine_scope.go @@ -9,7 +9,6 @@ import ( "github.com/openshift/cluster-api-provider-gcp/pkg/cloud/gcp/actuators/util" machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" machineapierros "github.com/openshift/machine-api-operator/pkg/controller/machine" - "google.golang.org/api/compute/v1" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog" @@ -18,12 +17,17 @@ import ( // machineScopeParams defines the input parameters used to create a new MachineScope. type machineScopeParams struct { - coreClient controllerclient.Client - machine *machinev1.Machine + context.Context + + coreClient controllerclient.Client + machine *machinev1.Machine + computeClientBuilder computeservice.BuilderFuncType } // machineScope defines a scope defined around a machine and its cluster. type machineScope struct { + context.Context + coreClient controllerclient.Client projectID string providerID string @@ -45,6 +49,10 @@ type machineScope struct { // newMachineScope creates a new MachineScope from the supplied parameters. // This is meant to be called for each machine actuator operation. func newMachineScope(params machineScopeParams) (*machineScope, error) { + if params.Context == nil { + params.Context = context.Background() + } + providerSpec, err := v1beta1.ProviderSpecFromRawExtension(params.machine.Spec.ProviderSpec.Value) if err != nil { return nil, machineapierros.InvalidMachineConfiguration("failed to get machine config: %v", err) @@ -68,16 +76,12 @@ func newMachineScope(params machineScopeParams) (*machineScope, error) { } } - oauthClient, err := util.CreateOauth2Client(serviceAccountJSON, compute.CloudPlatformScope) - if err != nil { - return nil, machineapierros.InvalidMachineConfiguration("error creating oauth client: %v", err) - } - - computeService, err := computeservice.NewComputeService(oauthClient) + computeService, err := params.computeClientBuilder(serviceAccountJSON) if err != nil { return nil, machineapierros.InvalidMachineConfiguration("error creating compute service: %v", err) } return &machineScope{ + Context: params.Context, coreClient: params.coreClient, projectID: projectID, // https://github.com/kubernetes/kubernetes/blob/8765fa2e48974e005ad16e65cb5c3acf5acff17b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_util.go#L204 @@ -163,7 +167,7 @@ func (s *machineScope) PatchMachine() error { statusCopy := *s.machine.Status.DeepCopy() // patch machine - if err := s.coreClient.Patch(context.Background(), s.machine, s.machineToBePatched); err != nil { + if err := s.coreClient.Patch(s.Context, s.machine, s.machineToBePatched); err != nil { klog.Errorf("Failed to patch machine %q: %v", s.machine.GetName(), err) return err } @@ -171,7 +175,7 @@ func (s *machineScope) PatchMachine() error { s.machine.Status = statusCopy // patch status - if err := s.coreClient.Status().Patch(context.Background(), s.machine, s.machineToBePatched); err != nil { + if err := s.coreClient.Status().Patch(s.Context, s.machine, s.machineToBePatched); err != nil { klog.Errorf("Failed to patch machine status %q: %v", s.machine.GetName(), err) return err } diff --git a/pkg/cloud/gcp/actuators/machine/reconciler.go b/pkg/cloud/gcp/actuators/machine/reconciler.go index bf180e50c..d6afb930f 100644 --- a/pkg/cloud/gcp/actuators/machine/reconciler.go +++ b/pkg/cloud/gcp/actuators/machine/reconciler.go @@ -168,6 +168,10 @@ func (r *Reconciler) create() error { } func (r *Reconciler) update() error { + if err := validateMachine(*r.machine, *r.providerSpec); err != nil { + return machinecontroller.InvalidMachineConfiguration("failed validating machine provider spec: %v", err) + } + // Add target pools, if necessary if err := r.processTargetPools(true, r.addInstanceToTargetPool); err != nil { return err diff --git a/pkg/cloud/gcp/actuators/machineset/controller.go b/pkg/cloud/gcp/actuators/machineset/controller.go index 554fbe1c0..0ee61c5a5 100644 --- a/pkg/cloud/gcp/actuators/machineset/controller.go +++ b/pkg/cloud/gcp/actuators/machineset/controller.go @@ -24,7 +24,6 @@ import ( "github.com/openshift/cluster-api-provider-gcp/pkg/cloud/gcp/actuators/util" machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" mapierrors "github.com/openshift/machine-api-operator/pkg/controller/machine" - "google.golang.org/api/compute/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -168,12 +167,7 @@ func (r *Reconciler) getRealGCPService(namespace string, providerConfig provider return nil, err } - oauthClient, err := util.CreateOauth2Client(serviceAccountJSON, compute.CloudPlatformScope) - if err != nil { - return nil, mapierrors.InvalidMachineConfiguration("error creating oauth client: %v", err) - } - - computeService, err := computeservice.NewComputeService(oauthClient) + computeService, err := computeservice.NewComputeService(serviceAccountJSON) if err != nil { return nil, mapierrors.InvalidMachineConfiguration("error creating compute service: %v", err) } diff --git a/pkg/cloud/gcp/actuators/services/compute/computeservice.go b/pkg/cloud/gcp/actuators/services/compute/computeservice.go index 9fc15ddb5..8abcbb130 100644 --- a/pkg/cloud/gcp/actuators/services/compute/computeservice.go +++ b/pkg/cloud/gcp/actuators/services/compute/computeservice.go @@ -1,8 +1,7 @@ package computeservice import ( - "net/http" - + "github.com/openshift/cluster-api-provider-gcp/pkg/cloud/gcp/actuators/util" "github.com/openshift/cluster-api-provider-gcp/pkg/version" "google.golang.org/api/compute/v1" ) @@ -26,13 +25,22 @@ type computeService struct { service *compute.Service } +// BuilderFuncType is function type for building gcp client +type BuilderFuncType func(serviceAccountJSON string) (GCPComputeService, error) + // NewComputeService return a new computeService -func NewComputeService(oauthClient *http.Client) (*computeService, error) { +func NewComputeService(serviceAccountJSON string) (GCPComputeService, error) { + oauthClient, err := util.CreateOauth2Client(serviceAccountJSON, compute.CloudPlatformScope) + if err != nil { + return nil, err + } + service, err := compute.New(oauthClient) if err != nil { return nil, err } service.UserAgent = "gcpprovider.openshift.io/" + version.Version.String() + return &computeService{ service: service, }, nil diff --git a/pkg/cloud/gcp/actuators/services/compute/computeservice_mock.go b/pkg/cloud/gcp/actuators/services/compute/computeservice_mock.go index 6da2eec81..a0993d826 100644 --- a/pkg/cloud/gcp/actuators/services/compute/computeservice_mock.go +++ b/pkg/cloud/gcp/actuators/services/compute/computeservice_mock.go @@ -2,6 +2,7 @@ package computeservice import ( compute "google.golang.org/api/compute/v1" + "google.golang.org/api/googleapi" ) const ( @@ -37,23 +38,26 @@ func (c *GCPComputeServiceMock) ZoneOperationsGet(project string, zone string, o } func (c *GCPComputeServiceMock) InstancesGet(project string, zone string, instance string) (*compute.Instance, error) { - return &compute.Instance{ - Name: instance, - Zone: zone, - MachineType: "n1-standard-1", - CanIpForward: true, - NetworkInterfaces: []*compute.NetworkInterface{ - { - NetworkIP: "10.0.0.15", - AccessConfigs: []*compute.AccessConfig{ - { - NatIP: "35.243.147.143", + if c.mockInstancesGet == nil { + return &compute.Instance{ + Name: instance, + Zone: zone, + MachineType: "n1-standard-1", + CanIpForward: true, + NetworkInterfaces: []*compute.NetworkInterface{ + { + NetworkIP: "10.0.0.15", + AccessConfigs: []*compute.AccessConfig{ + { + NatIP: "35.243.147.143", + }, }, }, }, - }, - Status: "RUNNING", - }, nil + Status: "RUNNING", + }, nil + } + return c.mockInstancesGet(project, zone, instance) } func (c *GCPComputeServiceMock) ZonesGet(project string, zone string) (*compute.Zone, error) { @@ -110,3 +114,18 @@ func NewComputeServiceMock() (*compute.Instance, *GCPComputeServiceMock) { } return &receivedInstance, &computeServiceMock } + +func MockBuilderFuncType(serviceAccountJSON string) (GCPComputeService, error) { + _, computeSvc := NewComputeServiceMock() + return computeSvc, nil +} + +func MockBuilderFuncTypeNotFound(serviceAccountJSON string) (GCPComputeService, error) { + _, computeSvc := NewComputeServiceMock() + computeSvc.mockInstancesGet = func(project string, zone string, instance string) (*compute.Instance, error) { + return nil, &googleapi.Error{ + Code: 404, + } + } + return computeSvc, nil +}