From a5bcde4c7236788249e27f9a535c9b073efa2abc Mon Sep 17 00:00:00 2001 From: Rob Leidle Date: Mon, 16 Apr 2018 15:51:51 -0700 Subject: [PATCH] Refactor GCEClient: wrap compute.Service in an interface for mocking GCP compute This change creates a ComputeService implementation which has a runtime implementation that wraps the compute.Service. The MachineActuator is changed to make use of the ComputeService through a new interface named GCEClientComputeService. This will enable creating tests that mock GCP Compute Service calls to control MachineActuator behavior. --- cloud/google/clients/computeservice.go | 62 ++++++++++ cloud/google/clients/computeservice_test.go | 121 ++++++++++++++++++++ cloud/google/machineactuator.go | 52 +++++---- 3 files changed, 214 insertions(+), 21 deletions(-) create mode 100644 cloud/google/clients/computeservice.go create mode 100644 cloud/google/clients/computeservice_test.go diff --git a/cloud/google/clients/computeservice.go b/cloud/google/clients/computeservice.go new file mode 100644 index 000000000000..0cd514a22bd1 --- /dev/null +++ b/cloud/google/clients/computeservice.go @@ -0,0 +1,62 @@ +package clients + +import ( + compute "google.golang.org/api/compute/v1" + "net/http" + "net/url" +) + +type ComputeService struct { + service *compute.Service +} + +func NewComputeService(client *http.Client) (*ComputeService, error) { + return newComputeService(client) +} + +func NewComputeServiceForURL(client *http.Client, baseURL string) (*ComputeService, error) { + ComputeServiceImpl, err := newComputeService(client) + if err != nil { + return nil, err + } + url, err := url.Parse(ComputeServiceImpl.service.BasePath) + if err != nil { + return nil, err + } + ComputeServiceImpl.service.BasePath = baseURL + url.Path + return ComputeServiceImpl, err +} + +func newComputeService(client *http.Client) (*ComputeService, error) { + service, err := compute.New(client) + if err != nil { + return nil, err + } + return &ComputeService{ + service: service, + }, nil +} + +func (c *ComputeService) ImagesGet(project string, image string) (*compute.Image, error) { + return c.service.Images.Get(project, image).Do() +} + +func (c *ComputeService) ImagesGetFromFamily(project string, family string) (*compute.Image, error) { + return c.service.Images.GetFromFamily(project, family).Do() +} + +func (c *ComputeService) InstancesDelete(project string, zone string, targetInstance string) (*compute.Operation, error) { + return c.service.Instances.Delete(project, zone, targetInstance).Do() +} + +func (c *ComputeService) InstancesGet(project string, zone string, instance string) (*compute.Instance, error) { + return c.service.Instances.Get(project, zone, instance).Do() +} + +func (c *ComputeService) InstancesInsert(project string, zone string, instance *compute.Instance) (*compute.Operation, error) { + return c.service.Instances.Insert(project, zone, instance).Do() +} + +func (c *ComputeService) ZoneOperationsGet(project string, zone string, operation string) (*compute.Operation, error) { + return c.service.ZoneOperations.Get(project, zone, operation).Do() +} diff --git a/cloud/google/clients/computeservice_test.go b/cloud/google/clients/computeservice_test.go new file mode 100644 index 000000000000..f5ccf800af8e --- /dev/null +++ b/cloud/google/clients/computeservice_test.go @@ -0,0 +1,121 @@ +package clients_test + +import ( + compute "google.golang.org/api/compute/v1" + "encoding/json" + "github.com/stretchr/testify/assert" + "google.golang.org/api/googleapi" + "net/http" + "net/http/httptest" + "sigs.k8s.io/cluster-api/cloud/google/clients" + "testing" +) + +func TestImagesGet(t *testing.T) { + mux, server, client := createMuxServerAndClient() + defer server.Close() + responseImage := compute.Image{ + Name: "imageName", + ArchiveSizeBytes: 544, + } + mux.Handle("/compute/v1/projects/projectName/global/images/imageName", handler(nil, &responseImage)) + image, err := client.ImagesGet("projectName", "imageName") + assert.Nil(t, err) + assert.NotNil(t, image) + assert.Equal(t, "imageName", image.Name) + assert.Equal(t, int64(544), image.ArchiveSizeBytes) +} + +func TestImagesGetFromFamily(t *testing.T) { + mux, server, client := createMuxServerAndClient() + defer server.Close() + responseImage := compute.Image{ + Name: "imageName", + ArchiveSizeBytes: 544, + } + mux.Handle("/compute/v1/projects/projectName/global/images/family/familyName", handler(nil, &responseImage)) + image, err := client.ImagesGetFromFamily("projectName", "familyName") + assert.Nil(t, err) + assert.NotNil(t, image) + assert.Equal(t, "imageName", image.Name) + assert.Equal(t, int64(544), image.ArchiveSizeBytes) +} + +func TestInstancesDelete(t *testing.T) { + mux, server, client := createMuxServerAndClient() + defer server.Close() + responseOperation := compute.Operation{ + Id: 4501, + } + mux.Handle("/compute/v1/projects/projectName/zones/zoneName/instances/instanceName", handler(nil, &responseOperation)) + op, err := client.InstancesDelete("projectName", "zoneName", "instanceName") + assert.Nil(t, err) + assert.NotNil(t, op) + assert.Equal(t, uint64(4501), responseOperation.Id) +} + +func TestInstancesGet(t *testing.T) { + mux, server, client := createMuxServerAndClient() + defer server.Close() + responseInstance := compute.Instance{ + Name: "instanceName", + Zone: "zoneName", + } + mux.Handle("/compute/v1/projects/projectName/zones/zoneName/instances/instanceName", handler(nil, &responseInstance)) + instance, err := client.InstancesGet("projectName", "zoneName", "instanceName") + assert.Nil(t, err) + assert.NotNil(t, instance) + assert.Equal(t, "instanceName", instance.Name) + assert.Equal(t, "zoneName", instance.Zone) +} + +func TestInstancesInsert(t *testing.T) { + mux, server, client := createMuxServerAndClient() + defer server.Close() + responseOperation := compute.Operation{ + Id: 3001, + } + mux.Handle("/compute/v1/projects/projectName/zones/zoneName/instances", handler(nil, &responseOperation)) + op, err := client.InstancesInsert("projectName", "zoneName", nil) + assert.Nil(t, err) + assert.NotNil(t, op) + assert.Equal(t, uint64(3001), responseOperation.Id) +} + +func createMuxServerAndClient() (*http.ServeMux, *httptest.Server, *clients.ComputeService) { + mux := http.NewServeMux() + server := httptest.NewServer(mux) + client, _ := clients.NewComputeServiceForURL(server.Client(), server.URL) + return mux, server, client +} + +func handler(err *googleapi.Error, obj interface{}) http.HandlerFunc { + return func(w http.ResponseWriter, req *http.Request) { + handleTestRequest(w, err, obj) + } +} + +func handleTestRequest(w http.ResponseWriter, handleErr *googleapi.Error, obj interface{}) { + if handleErr != nil { + http.Error(w, errMsg(handleErr), handleErr.Code) + return + } + res, err := json.Marshal(obj) + if err != nil { + http.Error(w, "json marshal error", http.StatusInternalServerError) + return + } + w.Write(res) +} + +func errMsg(e *googleapi.Error) string { + res, err := json.Marshal(&errorReply{e}) + if err != nil { + return "json marshal error" + } + return string(res) +} + +type errorReply struct { + Error *googleapi.Error `json:"error"` +} diff --git a/cloud/google/machineactuator.go b/cloud/google/machineactuator.go index 7b6b7beb62b8..40e3e9785d85 100644 --- a/cloud/google/machineactuator.go +++ b/cloud/google/machineactuator.go @@ -42,6 +42,7 @@ import ( apierrors "sigs.k8s.io/cluster-api/errors" clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" client "sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset/typed/cluster/v1alpha1" + "sigs.k8s.io/cluster-api/cloud/google/clients" "sigs.k8s.io/cluster-api/util" ) @@ -58,13 +59,22 @@ type SshCreds struct { privateKeyPath string } +type GCEClientComputeService interface { + ImagesGet(project string, image string) (*compute.Image, error) + ImagesGetFromFamily(project string, family string) (*compute.Image, error) + InstancesDelete(project string, zone string, targetInstance string) (*compute.Operation, error) + InstancesGet(project string, zone string, instance string) (*compute.Instance, error) + InstancesInsert(project string, zone string, instance *compute.Instance) (*compute.Operation, error) + ZoneOperationsGet(project string, zone string, operation string) (*compute.Operation, error) +} + type GCEClient struct { - service *compute.Service - scheme *runtime.Scheme - codecFactory *serializer.CodecFactory - kubeadmToken string - sshCreds SshCreds - machineClient client.MachineInterface + computeService GCEClientComputeService + scheme *runtime.Scheme + codecFactory *serializer.CodecFactory + kubeadmToken string + sshCreds SshCreds + machineClient client.MachineInterface } const ( @@ -80,7 +90,7 @@ func NewMachineActuator(kubeadmToken string, machineClient client.MachineInterfa return nil, err } - service, err := compute.New(client) + computeService, err := clients.NewComputeService(client) if err != nil { return nil, err } @@ -104,10 +114,10 @@ func NewMachineActuator(kubeadmToken string, machineClient client.MachineInterfa } return &GCEClient{ - service: service, - scheme: scheme, - codecFactory: codecFactory, - kubeadmToken: kubeadmToken, + computeService: computeService, + scheme: scheme, + codecFactory: codecFactory, + kubeadmToken: kubeadmToken, sshCreds: SshCreds{ privateKeyPath: privateKeyPath, user: user, @@ -215,12 +225,12 @@ func (gce *GCEClient) Create(cluster *clusterv1.Cluster, machine *clusterv1.Mach } if instance == nil { - labels := map[string]string{} + labels := map[string]string{} if gce.machineClient == nil { labels[BootstrapLabelKey] = "true" } - op, err := gce.service.Instances.Insert(project, zone, &compute.Instance{ + op, err := gce.computeService.InstancesInsert(project, zone, &compute.Instance{ Name: name, MachineType: fmt.Sprintf("zones/%s/machineTypes/%s", zone, config.MachineType), NetworkInterfaces: []*compute.NetworkInterface{ @@ -251,7 +261,7 @@ func (gce *GCEClient) Create(cluster *clusterv1.Cluster, machine *clusterv1.Mach Items: []string{"https-server"}, }, Labels: labels, - }).Do() + }) if err == nil { err = gce.waitForOperation(config, op) @@ -310,7 +320,7 @@ func (gce *GCEClient) Delete(machine *clusterv1.Machine) error { name = machine.ObjectMeta.Name } - op, err := gce.service.Instances.Delete(project, zone, name).Do() + op, err := gce.computeService.InstancesDelete(project, zone, name) if err == nil { err = gce.waitForOperation(config, op) } @@ -405,7 +415,7 @@ func (gce *GCEClient) GetIP(machine *clusterv1.Machine) (string, error) { return "", err } - instance, err := gce.service.Instances.Get(config.Project, config.Zone, machine.ObjectMeta.Name).Do() + instance, err := gce.computeService.InstancesGet(config.Project, config.Zone, machine.ObjectMeta.Name) if err != nil { return "", err } @@ -495,7 +505,7 @@ func (gce *GCEClient) instanceIfExists(machine *clusterv1.Machine) (*compute.Ins return nil, err } - instance, err := gce.service.Instances.Get(config.Project, config.Zone, identifyingMachine.ObjectMeta.Name).Do() + instance, err := gce.computeService.InstancesGet(config.Project, config.Zone, identifyingMachine.ObjectMeta.Name) if err != nil { // TODO: Use formal way to check for error code 404 if strings.Contains(err.Error(), "Error 404") { @@ -546,7 +556,7 @@ func (gce *GCEClient) waitForOperation(c *gceconfig.GCEProviderConfig, op *compu // getOp returns an updated operation. func (gce *GCEClient) getOp(c *gceconfig.GCEProviderConfig, op *compute.Operation) (*compute.Operation, error) { - return gce.service.ZoneOperations.Get(c.Project, path.Base(op.Zone), op.Name).Do() + return gce.computeService.ZoneOperationsGet(c.Project, path.Base(op.Zone), op.Name) } func (gce *GCEClient) checkOp(op *compute.Operation, err error) error { @@ -648,7 +658,7 @@ func (gce *GCEClient) getImage(machine *clusterv1.Machine, config *gceconfig.GCE if matches == nil { // Only the image name was specified in config, so check if it is preloaded in the project specified in config. fullPath := fmt.Sprintf("projects/%s/global/images/%s", project, img) - if _, err := gce.service.Images.Get(project, img).Do(); err == nil { + if _, err := gce.computeService.ImagesGet(project, img); err == nil { return fullPath, false } @@ -661,9 +671,9 @@ func (gce *GCEClient) getImage(machine *clusterv1.Machine, config *gceconfig.GCE project, family, name := matches[1], matches[2], matches[3] var err error if family == "" { - _, err = gce.service.Images.Get(project, name).Do() + _, err = gce.computeService.ImagesGet(project, name) } else { - _, err = gce.service.Images.GetFromFamily(project, name).Do() + _, err = gce.computeService.ImagesGetFromFamily(project, name) } if err == nil {