From e6dc8b0ffd418205b1f3cf749b01e7190380a254 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Wed, 1 Aug 2018 14:41:03 -0700 Subject: [PATCH] Refactored remote test framework for easy usage. Added teardown methods --- test/e2e/tests/multi_zone_e2e_test.go | 7 +- test/e2e/tests/setup_e2e_test.go | 4 +- test/e2e/tests/single_zone_e2e_test.go | 15 +- test/e2e/utils/utils.go | 63 ++------ test/{binremote => remote}/archiver.go | 2 +- .../client-wrappers.go} | 2 +- test/{binremote => remote}/instance.go | 2 +- test/{binremote => remote}/runner.go | 34 ++++- test/remote/setup-teardown.go | 144 ++++++++++++++++++ test/{binremote => remote}/ssh.go | 2 +- 10 files changed, 206 insertions(+), 69 deletions(-) rename test/{binremote => remote}/archiver.go (99%) rename test/{e2e/utils/client_wrappers.go => remote/client-wrappers.go} (99%) rename test/{binremote => remote}/instance.go (99%) rename test/{binremote => remote}/runner.go (64%) create mode 100644 test/remote/setup-teardown.go rename test/{binremote => remote}/ssh.go (99%) diff --git a/test/e2e/tests/multi_zone_e2e_test.go b/test/e2e/tests/multi_zone_e2e_test.go index 7bd41dc0d..11255f556 100644 --- a/test/e2e/tests/multi_zone_e2e_test.go +++ b/test/e2e/tests/multi_zone_e2e_test.go @@ -19,6 +19,7 @@ import ( . "github.com/onsi/gomega" "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common" testutils "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/e2e/utils" + remote "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/remote" ) var _ = Describe("GCE PD CSI Driver Multi-Zone", func() { @@ -30,8 +31,12 @@ var _ = Describe("GCE PD CSI Driver Multi-Zone", func() { It("Should get reasonable topology from nodes with NodeGetInfo", func() { for _, instance := range testInstances { - testContext, err := testutils.SetupNewDriverAndClient(instance) + testContext, err := testutils.GCEClientAndDriverSetup(instance) Expect(err).To(BeNil(), "Set up new Driver and Client failed with error") + defer func() { + err := remote.TeardownDriverAndClient(testContext) + Expect(err).To(BeNil(), "Teardown Driver and Client failed with error") + }() resp, err := testContext.Client.NodeGetInfo() Expect(err).To(BeNil()) diff --git a/test/e2e/tests/setup_e2e_test.go b/test/e2e/tests/setup_e2e_test.go index b9440c268..0c688938d 100644 --- a/test/e2e/tests/setup_e2e_test.go +++ b/test/e2e/tests/setup_e2e_test.go @@ -24,8 +24,8 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" compute "google.golang.org/api/compute/v1" - remote "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/binremote" testutils "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/e2e/utils" + remote "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/remote" ) var ( @@ -63,7 +63,7 @@ var _ = BeforeSuite(func() { Expect(*project).ToNot(BeEmpty(), "Project should not be empty") Expect(*serviceAccount).ToNot(BeEmpty(), "Service account should not be empty") - i, err := testutils.SetupInstance(*project, zone, nodeID, *serviceAccount, computeService) + i, err := remote.SetupInstance(*project, zone, nodeID, *serviceAccount, computeService) Expect(err).To(BeNil()) testInstances = append(testInstances, i) diff --git a/test/e2e/tests/single_zone_e2e_test.go b/test/e2e/tests/single_zone_e2e_test.go index e9cf1ecfe..eef3a169e 100644 --- a/test/e2e/tests/single_zone_e2e_test.go +++ b/test/e2e/tests/single_zone_e2e_test.go @@ -23,6 +23,7 @@ import ( "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common" gce "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider/compute" testutils "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/e2e/utils" + remote "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/remote" csi "github.com/container-storage-interface/spec/lib/go/csi/v0" . "github.com/onsi/ginkgo" @@ -44,8 +45,13 @@ var _ = Describe("GCE PD CSI Driver", func() { // Create new driver and client // TODO: Should probably actual have some object that includes both client and instance so we can relate the two?? Expect(testInstances).NotTo(BeEmpty()) - testContext, err := testutils.SetupNewDriverAndClient(testInstances[0]) + testContext, err := testutils.GCEClientAndDriverSetup(testInstances[0]) Expect(err).To(BeNil(), "Set up new Driver and Client failed with error") + defer func() { + err := remote.TeardownDriverAndClient(testContext) + Expect(err).To(BeNil(), "Teardown Driver and Client failed with error") + }() + p, z, _ := testContext.Instance.GetIdentity() client := testContext.Client instance := testContext.Instance @@ -135,8 +141,13 @@ var _ = Describe("GCE PD CSI Driver", func() { It("Should create disks in correct zones when topology is specified", func() { /// Expect(testInstances).NotTo(BeEmpty()) - testContext, err := testutils.SetupNewDriverAndClient(testInstances[0]) + testContext, err := testutils.GCEClientAndDriverSetup(testInstances[0]) Expect(err).To(BeNil(), "Failed to set up new driver and client") + defer func() { + err := remote.TeardownDriverAndClient(testContext) + Expect(err).To(BeNil(), "Teardown Driver and Client failed with error") + }() + p, _, _ := testContext.Instance.GetIdentity() zones := []string{"us-central1-c", "us-central1-b", "us-central1-a"} diff --git a/test/e2e/utils/utils.go b/test/e2e/utils/utils.go index 55a9c5945..b6348dee6 100644 --- a/test/e2e/utils/utils.go +++ b/test/e2e/utils/utils.go @@ -25,40 +25,15 @@ import ( "github.com/golang/glog" "golang.org/x/oauth2/google" cloudresourcemanager "google.golang.org/api/cloudresourcemanager/v1" - compute "google.golang.org/api/compute/v1" boskosclient "k8s.io/test-infra/boskos/client" - remote "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/binremote" + remote "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/remote" ) var ( boskos = boskosclient.NewClient(os.Getenv("JOB_NAME"), "http://boskos") ) -const ( - archiveName = "e2e_gce_pd_test.tar.gz" -) - -type TestContext struct { - Instance *remote.InstanceInfo - Client *CsiClient -} - -func SetupInstance(instanceProject, instanceZone, instanceName, instanceServiceAccount string, cs *compute.Service) (*remote.InstanceInfo, error) { - // Create the instance in the requisite zone - instance, err := remote.CreateInstanceInfo(instanceProject, instanceZone, instanceName, cs) - if err != nil { - return nil, err - } - - err = instance.CreateOrGetInstance(instanceServiceAccount) - if err != nil { - return nil, err - } - return instance, nil -} - -// TODO: Need a function to clean up this driver from the instance -func SetupNewDriverAndClient(instance *remote.InstanceInfo) (*TestContext, error) { +func GCEClientAndDriverSetup(instance *remote.InstanceInfo) (*remote.TestContext, error) { port := fmt.Sprintf("%v", 1024+rand.Intn(10000)) goPath, ok := os.LookupEnv("GOPATH") if !ok { @@ -66,40 +41,22 @@ func SetupNewDriverAndClient(instance *remote.InstanceInfo) (*TestContext, error } pkgPath := path.Join(goPath, "src/sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/") binPath := path.Join(pkgPath, "bin/gce-pd-csi-driver") - archivePath, err := remote.CreateDriverArchive(archiveName, pkgPath, binPath) - if err != nil { - return nil, err - } - defer func() { - err = os.Remove(archivePath) - if err != nil { - glog.Warningf("Failed to remove archive file %s: %v", archivePath, err) - } - }() - // Upload archive to instance and run binaries endpoint := fmt.Sprintf("tcp://localhost:%s", port) + workspace := remote.NewWorkspaceDir("gce-pd-e2e-") driverRunCmd := fmt.Sprintf("sh -c '/usr/bin/nohup %s/gce-pd-csi-driver --endpoint=%s --nodeid=%s > %s/prog.out 2> %s/prog.err < /dev/null &'", workspace, endpoint, instance.GetName(), workspace, workspace) - err = instance.UploadAndRun(archivePath, workspace, driverRunCmd) - if err != nil { - return nil, err - } - // Create an SSH tunnel from port to port - res, err := instance.CreateSSHTunnel(port, port) - if err != nil { - return nil, fmt.Errorf("SSH Tunnel pid %v encountered error: %v", res, err) - } - - client := CreateCSIClient(fmt.Sprintf("localhost:%s", port)) - err = client.AssertCSIConnection() - if err != nil { - return nil, fmt.Errorf("asserting csi connection failed with: %v", err) + config := &remote.ClientConfig{ + PkgPath: pkgPath, + BinPath: binPath, + WorkspaceDir: workspace, + RunDriverCmd: driverRunCmd, + Port: port, } - return &TestContext{Instance: instance, Client: client}, nil + return remote.SetupNewDriverAndClient(instance, config) } func SetupProwConfig() (project, serviceAccount string) { diff --git a/test/binremote/archiver.go b/test/remote/archiver.go similarity index 99% rename from test/binremote/archiver.go rename to test/remote/archiver.go index 60733e4ce..f31ff95ab 100644 --- a/test/binremote/archiver.go +++ b/test/remote/archiver.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package binremote +package remote import ( "fmt" diff --git a/test/e2e/utils/client_wrappers.go b/test/remote/client-wrappers.go similarity index 99% rename from test/e2e/utils/client_wrappers.go rename to test/remote/client-wrappers.go index 25daeb8f7..e5bc253aa 100644 --- a/test/e2e/utils/client_wrappers.go +++ b/test/remote/client-wrappers.go @@ -12,7 +12,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package utils +package remote import ( "context" diff --git a/test/binremote/instance.go b/test/remote/instance.go similarity index 99% rename from test/binremote/instance.go rename to test/remote/instance.go index 95e301711..581935c8c 100644 --- a/test/binremote/instance.go +++ b/test/remote/instance.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package binremote +package remote import ( "context" diff --git a/test/binremote/runner.go b/test/remote/runner.go similarity index 64% rename from test/binremote/runner.go rename to test/remote/runner.go index 5ca5e7bec..f0d81a141 100644 --- a/test/binremote/runner.go +++ b/test/remote/runner.go @@ -14,17 +14,19 @@ See the License for the specific language governing permissions and limitations under the License. */ -package binremote +package remote import ( "fmt" "path" "path/filepath" + "strconv" + "strings" "github.com/golang/glog" ) -func (i *InstanceInfo) UploadAndRun(archivePath, remoteWorkspace, driverRunCmd string) error { +func (i *InstanceInfo) UploadAndRun(archivePath, remoteWorkspace, driverRunCmd string) (int, error) { // Create the temp staging directory glog.V(4).Infof("Staging test binaries on %q", i.name) @@ -32,13 +34,13 @@ func (i *InstanceInfo) UploadAndRun(archivePath, remoteWorkspace, driverRunCmd s // Do not sudo here, so that we can use scp to copy test archive to the directdory. if output, err := i.SSHNoSudo("mkdir", remoteWorkspace); err != nil { // Exit failure with the error - return fmt.Errorf("failed to create remoteWorkspace directory %q on i.name %q: %v output: %q", remoteWorkspace, i.name, err, output) + return -1, fmt.Errorf("failed to create remoteWorkspace directory %q on i.name %q: %v output: %q", remoteWorkspace, i.name, err, output) } // Copy the archive to the staging directory if output, err := runSSHCommand("scp", archivePath, fmt.Sprintf("%s:%s/", i.GetSSHTarget(), remoteWorkspace)); err != nil { // Exit failure with the error - return fmt.Errorf("failed to copy test archive: %v, output: %q", err, output) + return -1, fmt.Errorf("failed to copy test archive: %v, output: %q", err, output) } // Extract the archive @@ -52,22 +54,40 @@ func (i *InstanceInfo) UploadAndRun(archivePath, remoteWorkspace, driverRunCmd s // we want the extracted files to be owned by the current user. if output, err := i.SSHNoSudo("sh", "-c", cmd); err != nil { // Exit failure with the error - return fmt.Errorf("failed to extract test archive: %v, output: %q", err, output) + return -1, fmt.Errorf("failed to extract test archive: %v, output: %q", err, output) } glog.V(4).Infof("Starting driver on %q", i.name) // When the process is killed the driver should close the TCP endpoint, then we want to download the logs output, err := i.SSH(driverRunCmd) + if err != nil { + // Exit failure with the error + return -1, fmt.Errorf("failed start driver, got output: %v, error: %v", output, err) + } + // Get the driver PID + // ps -aux | grep /tmp/gce-pd-e2e-0180801T114407/gce-pd-csi-driver | awk '{print $2}' + driverPIDCmd := getSSHCommand(" | ", + "ps -aux", + fmt.Sprintf("grep %s", remoteWorkspace), + "grep -v grep", + // All ye who try to deal with escaped/non-escaped quotes with exec beware. + //`awk "{print \$2}"`, + ) + driverPIDString, err := i.SSHNoSudo("sh", "-c", driverPIDCmd) if err != nil { // Exit failure with the error - return fmt.Errorf("failed start GCE PD driver, got output: %v, error: %v", output, err) + return -1, fmt.Errorf("failed to get PID of driver, got output: %v, error: %v", output, err) } + driverPID, err := strconv.Atoi(strings.Fields(driverPIDString)[1]) + if err != nil { + return -1, fmt.Errorf("failed to convert driver PID from string %s to int: %v", driverPIDString, err) + } // TODO: return the PID so that we can kill the driver later // Actually just do a pkill -f my_pattern - return nil + return driverPID, nil } func NewWorkspaceDir(workspaceDirPrefix string) string { diff --git a/test/remote/setup-teardown.go b/test/remote/setup-teardown.go new file mode 100644 index 000000000..04f21bbff --- /dev/null +++ b/test/remote/setup-teardown.go @@ -0,0 +1,144 @@ +/* +Copyright 2018 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 remote + +import ( + "fmt" + "os" + + "github.com/golang/glog" + compute "google.golang.org/api/compute/v1" +) + +const ( + archiveName = "e2e_driver_binaries.tar.gz" +) + +// TestContext holds the CSI Client handle to a remotely connected Driver +// as well as a handle to the Instance that the driver is running on +type TestContext struct { + Instance *InstanceInfo + Client *CsiClient + proc *processes +} + +// ClientConfig contains all the parameters required to package a new +// driver and run it remotely on a GCE Instance +type ClientConfig struct { + // Absolute path of the package + PkgPath string + // Absolute path of the driver binary to copy remotely + BinPath string + // Path on remote instance workspace + WorkspaceDir string + // Command to run on remote instance to start the driver + RunDriverCmd string + // Port to use as SSH tunnel on both remote and local side. + Port string +} + +type processes struct { + sshTunnel int + remoteDriver int +} + +// SetupInstance sets up the specified GCE Instance for E2E testing and returns a handle to the instance object for future use. +func SetupInstance(instanceProject, instanceZone, instanceName, instanceServiceAccount string, cs *compute.Service) (*InstanceInfo, error) { + // Create the instance in the requisite zone + instance, err := CreateInstanceInfo(instanceProject, instanceZone, instanceName, cs) + if err != nil { + return nil, err + } + + err = instance.CreateOrGetInstance(instanceServiceAccount) + if err != nil { + return nil, err + } + return instance, nil +} + +// SetupNewDriverAndClient gets the driver binary, runs it on the provided instance and connects +// a CSI client to it through SHH tunnelling. It returns a TestContext with both a handle to the instance +// that the driver is on and the CSI Client object to make CSI calls to the remote driver. +func SetupNewDriverAndClient(instance *InstanceInfo, config *ClientConfig) (*TestContext, error) { + archivePath, err := CreateDriverArchive(archiveName, config.PkgPath, config.BinPath) + if err != nil { + return nil, err + } + defer func() { + err = os.Remove(archivePath) + if err != nil { + glog.Warningf("Failed to remove archive file %s: %v", archivePath, err) + } + }() + + // Upload archive to instance and run binaries + driverPID, err := instance.UploadAndRun(archivePath, config.WorkspaceDir, config.RunDriverCmd) + if err != nil { + return nil, err + } + + // Create an SSH tunnel from port to port + sshPID, err := instance.CreateSSHTunnel(config.Port, config.Port) + if err != nil { + return nil, fmt.Errorf("SSH Tunnel pid %v encountered error: %v", sshPID, err) + } + + client := CreateCSIClient(fmt.Sprintf("localhost:%s", config.Port)) + err = client.AssertCSIConnection() + if err != nil { + return nil, fmt.Errorf("asserting csi connection failed with: %v", err) + } + + return &TestContext{ + Instance: instance, + Client: client, + proc: &processes{ + sshTunnel: sshPID, + remoteDriver: driverPID, + }, + }, nil +} + +// TeardownDriverAndClient closes the CSI Client connection, closes the SSH tunnel +// Kills the driver process on the GCE instance, and cleans up the remote driver workspace +func TeardownDriverAndClient(context *TestContext) error { + // Close the client connection + err := context.Client.CloseConn() + if err != nil { + return fmt.Errorf("failed to close CSI Client connection: %v", err) + } + // Close the SSH tunnel + proc, err := os.FindProcess(context.proc.sshTunnel) + if err != nil { + return fmt.Errorf("unable to efind process for ssh tunnel %v: %v", context.proc.sshTunnel, err) + } + if err = proc.Kill(); err != nil { + return fmt.Errorf("failed to kill ssh tunnel process %v: %v", context.proc.sshTunnel, err) + } + + // Kill the driver process on remote + cmd := fmt.Sprintf("kill %v", context.proc.remoteDriver) + output, err := context.Instance.SSH(cmd) + if err != nil { + return fmt.Errorf("failed to kill driver on remote instance, got output %s: %v", output, err) + } + + // TODO: Cleanup driver workspace on remote + + return nil +} diff --git a/test/binremote/ssh.go b/test/remote/ssh.go similarity index 99% rename from test/binremote/ssh.go rename to test/remote/ssh.go index 5b39b2df2..01f86f821 100644 --- a/test/binremote/ssh.go +++ b/test/remote/ssh.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package binremote +package remote import ( "fmt"