Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Commit

Permalink
functional: allow MemberCommand to return stderr
Browse files Browse the repository at this point in the history 
Remove Cluster.MemberCommand(), rename MemberCommandStderr() to
MemberCommand(), and update every call site accordingly.
That way we can clean up leftovers from #1700.
  • Loading branch information
Dongsu Park committed Nov 9, 2016
1 parent 2b2eda2 commit b9f335b
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 87 deletions.
20 changes: 10 additions & 10 deletions functional/connectivity-loss_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,9 @@ func TestSingleNodeConnectivityLoss(t *testing.T) {
// Cut connection to etcd.
//
// We use REJECT here, so fleet knows immediately that it's disconnected, rather than waiting for a timeout.
if _, err = cluster.MemberCommand(m0, "sudo", "iptables", "-I", "OUTPUT", "-p", "tcp", "-m", "multiport", "--dports=2379,4001", "-j", "REJECT"); err != nil {
t.Fatal(err)
stdout, stderr, err := cluster.MemberCommand(m0, "sudo", "iptables", "-I", "OUTPUT", "-p", "tcp", "-m", "multiport", "--dports=2379,4001", "-j", "REJECT")
if err != nil {
t.Fatalf("Failed to cut connection to etcd.\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

// Wait long enough to be reasonably confident that no more state changes will happen.
Expand All @@ -159,9 +160,8 @@ func TestSingleNodeConnectivityLoss(t *testing.T) {
// as fleet is not available to give us this information...
// We have to go deeper, and try to obtain the information from systemd directly.
actualSystemdFiles := map[string]string{}
var stdout string
for name, _ := range expectedSystemdFiles {
stdout, _ := cluster.MemberCommand(m0, "systemctl", "is-enabled", name)
stdout, _, _ := cluster.MemberCommand(m0, "systemctl", "is-enabled", name)
// do not check for error, as systemctl is-enabled returns exit status 1 for linked-runtime.
stdout = strings.TrimSpace(stdout)
if stdout == "" {
Expand All @@ -174,9 +174,9 @@ func TestSingleNodeConnectivityLoss(t *testing.T) {
t.Fatalf("Units files not in expected state after losing connectivity.\nExpected: %v\nActual: %v", expectedSystemdFiles, actualSystemdFiles)
}

stdout, err = cluster.MemberCommand(m0, "systemctl", "list-units", "-t", "service", "--no-legend", "single@*.service", "global@*.service")
stdout, stderr, err = cluster.MemberCommand(m0, "systemctl", "list-units", "-t", "service", "--no-legend", "single@*.service", "global@*.service")
if err != nil {
t.Fatalf("Failed to retrieve systemd unit states: %v", err)
t.Fatalf("Failed to retrieve systemd unit states.\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
stdout = strings.TrimSpace(stdout)
actualSystemdStates := map[string][]string{}
Expand All @@ -191,8 +191,8 @@ func TestSingleNodeConnectivityLoss(t *testing.T) {
}

// Restore etcd connection.
if _, err = cluster.MemberCommand(m0, "sudo", "iptables", "-D", "OUTPUT", "-p", "tcp", "-m", "multiport", "--dports=2379,4001", "-j", "REJECT"); err != nil {
t.Fatal(err)
if stdout, stderr, err = cluster.MemberCommand(m0, "sudo", "iptables", "-D", "OUTPUT", "-p", "tcp", "-m", "multiport", "--dports=2379,4001", "-j", "REJECT"); err != nil {
t.Fatalf("Failed to restore etcd connection.\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

// Again, wait long enough to be reasonably confident that no more state changes will happen.
Expand All @@ -219,9 +219,9 @@ func TestSingleNodeConnectivityLoss(t *testing.T) {
}

// Additionally check the logs of all active units for possible temporary state flapping.
stdout, err = cluster.MemberCommand(m0, "journalctl", "_PID=1")
stdout, stderr, err = cluster.MemberCommand(m0, "journalctl", "_PID=1")
if err != nil {
t.Fatalf("Failed to retrieve journal: %v", err)
t.Fatalf("Failed to retrieve journal.\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
if strings.Contains(stdout, "Stopping single@") || strings.Contains(stdout, "Stopping global@") {
t.Fatalf("Units were unexpectedly stopped at some point:\n%s", stdout)
Expand Down
37 changes: 18 additions & 19 deletions functional/node_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ func TestNodeShutdown(t *testing.T) {
}

// Stop the fleet process on the first member
if stdout, err = cluster.MemberCommand(m0, "sudo", "systemctl", "stop", "fleet"); err != nil {
t.Fatalf("Failed stopping fleet service: %v\nstdout: %s\n", err, stdout)
if stdout, stderr, err = cluster.MemberCommand(m0, "sudo", "systemctl", "stop", "fleet"); err != nil {
t.Fatalf("Failed stopping fleet service:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

// The first member should quickly remove itself from the published
Expand All @@ -81,11 +81,10 @@ func TestNodeShutdown(t *testing.T) {
// NOTE: In case of no units, systemd v230 or older prints out
// "Active: inactive" to stdout, while systemd v231 or newer prints out
// "Unit NAME could not be found" to stderr. So we need to check for
// both cases. Use MemberCommandStderr() to retrieve both stdout and stderr,
// and check for each case.
stdout, stderr, err = cluster.MemberCommandStderr(m0, "systemctl", "status", "hello.service")
// both cases.
stdout, stderr, err = cluster.MemberCommand(m0, "systemctl", "status", "hello.service")
if !strings.Contains(stdout, "Active: inactive") && !strings.Contains(stderr, "could not be found") {
t.Fatalf("Unit hello.service not reported as inactive:\n%s\n", stdout)
t.Fatalf("Unit hello.service not reported as inactive:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
}

Expand Down Expand Up @@ -118,35 +117,35 @@ func TestDetectMachineId(t *testing.T) {

// Restart fleet service, and check if its systemd status is still active.
restartFleetService := func(m platform.Member) error {
stdout, err := cluster.MemberCommand(m, "sudo", "systemctl", "restart", "fleet.service")
stdout, stderr, err := cluster.MemberCommand(m, "sudo", "systemctl", "restart", "fleet.service")
if err != nil {
return fmt.Errorf("Failed to restart fleet service\nstdout: %s\nerr: %v", stdout, err)
return fmt.Errorf("Failed to restart fleet service\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

stdout, err = cluster.MemberCommand(m, "systemctl", "show", "--property=ActiveState", "fleet")
stdout, stderr, err = cluster.MemberCommand(m, "systemctl", "show", "--property=ActiveState", "fleet")
if strings.TrimSpace(stdout) != "ActiveState=active" {
return fmt.Errorf("Fleet unit not reported as active:\nstdout:%s\nerr: %v", stdout, err)
return fmt.Errorf("Fleet unit not reported as active:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
stdout, err = cluster.MemberCommand(m, "systemctl", "show", "--property=Result", "fleet")
stdout, stderr, err = cluster.MemberCommand(m, "systemctl", "show", "--property=Result", "fleet")
if strings.TrimSpace(stdout) != "Result=success" {
return fmt.Errorf("Result for fleet unit not reported as success:\nstdout:%s\nerr: %v", stdout, err)
return fmt.Errorf("Result for fleet unit not reported as success:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
return nil
}

stdout, err := cluster.MemberCommand(m0, "cat", machineIdFile)
stdout, stderr, err := cluster.MemberCommand(m0, "cat", machineIdFile)
if err != nil {
t.Fatalf("Failed to get machine-id\nstdout: %s\nerr: %v", stdout, err)
t.Fatalf("Failed to get machine-id\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
m0_machine_id := strings.TrimSpace(stdout)

// If the two machine IDs are different with each other,
// set the m1's ID to the same one as m0, to intentionally
// trigger an error case of duplication of machine ID.
stdout, err = cluster.MemberCommand(m1,
stdout, stderr, err = cluster.MemberCommand(m1,
"echo", m0_machine_id, "|", "sudo", "tee", machineIdFile)
if err != nil {
t.Fatalf("Failed to replace machine-id\nstdout: %s\nerr: %v", stdout, err)
t.Fatalf("Failed to replace machine-id\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

if err := restartFleetService(m1); err != nil {
Expand All @@ -156,7 +155,7 @@ func TestDetectMachineId(t *testing.T) {
// fleetd should actually be running, but failing to list machines.
// So we should expect a specific error after running fleetctl list-machines,
// like "googlapi: Error 503: fleet server unable to communicate with etcd".
stdout, stderr, err := cluster.Fleetctl(m1, "list-machines", "--no-legend")
stdout, stderr, err = cluster.Fleetctl(m1, "list-machines", "--no-legend")
if err != nil {
if !strings.Contains(err.Error(), "exit status 1") ||
!strings.Contains(stderr, "fleet server unable to communicate with etcd") {
Expand All @@ -171,10 +170,10 @@ func TestDetectMachineId(t *testing.T) {
// Trigger another test case of m0's ID getting different from m1's.
// Then it's expected that m0 and m1 would be working properly with distinct
// machine IDs, after having restarted fleet.service both on m0 and m1.
stdout, err = cluster.MemberCommand(m0,
stdout, stderr, err = cluster.MemberCommand(m0,
"echo", util.NewMachineID(), "|", "sudo", "tee", machineIdFile)
if err != nil {
t.Fatalf("m0: Failed to replace machine-id\nstdout: %s\nerr: %v", stdout, err)
t.Fatalf("m0: Failed to replace machine-id\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

// Restart fleet service on m0, and see that it's still working.
Expand Down
3 changes: 1 addition & 2 deletions functional/platform/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ type Cluster interface {
DestroyMember(Member) error
ReplaceMember(Member) (Member, error)
Members() []Member
MemberCommand(Member, ...string) (string, error)
MemberCommandStderr(Member, ...string) (string, string, error)
MemberCommand(Member, ...string) (string, string, error)
Destroy(t *testing.T) error

// client operations
Expand Down
23 changes: 1 addition & 22 deletions functional/platform/nspawn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,28 +379,7 @@ func (nc *nspawnCluster) Members() []Member {
return ms
}

func (nc *nspawnCluster) MemberCommand(m Member, args ...string) (string, error) {
baseArgs := []string{"-o", "UserKnownHostsFile=/dev/null", "-o", "StrictHostKeyChecking=no", fmt.Sprintf("core@%s", m.IP())}
args = append(baseArgs, args...)
log.Printf("ssh %s", strings.Join(args, " "))
var stdoutBytes bytes.Buffer
cmd := exec.Command("ssh", args...)
cmd.Stdout = &stdoutBytes
err := cmd.Run()
return stdoutBytes.String(), err
}

// MemberCommandStderr() is like MemberCommand(), except that it returns
// both stdout and stderr from the running command. This is definitely useful
// for most cases, especially when the caller needs to check for both stdout
// and stderr.
//
// NOTE: Ideally we should remove MemberCommand() above, and rename
// MemberCommandStderr() to MemberCommand(). For doing that, however, we need
// to change every call site to replace MemberCommand() with MemberCommandStderr().
// Of course that would be a lot of work. I'll leave that to future work.
// - dpark 20161102
func (nc *nspawnCluster) MemberCommandStderr(m Member, args ...string) (string, string, error) {
func (nc *nspawnCluster) MemberCommand(m Member, args ...string) (string, string, error) {
baseArgs := []string{"-o", "UserKnownHostsFile=/dev/null", "-o", "StrictHostKeyChecking=no", fmt.Sprintf("core@%s", m.IP())}
args = append(baseArgs, args...)
log.Printf("ssh %s", strings.Join(args, " "))
Expand Down
20 changes: 10 additions & 10 deletions functional/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ func TestReconfigureServer(t *testing.T) {

// Send a SIGHUP to fleetd, and periodically checks if a message
// "Reloading configuration" appears in fleet's journal, up to timeout (15) seconds.
stdout, _ = cluster.MemberCommand(m0, "sudo", "systemctl", "kill", "-s", "SIGHUP", "fleet")
stdout, stderr, err = cluster.MemberCommand(m0, "sudo", "systemctl", "kill", "-s", "SIGHUP", "fleet")
if strings.TrimSpace(stdout) != "" {
t.Fatalf("Sending SIGHUP to fleetd returned: %s", stdout)
t.Fatalf("Sending SIGHUP to fleetd returned.\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

// Watch the logs if fleet was correctly reloaded
Expand Down Expand Up @@ -97,19 +97,19 @@ func TestReconfigureServer(t *testing.T) {
}

// Check for HTTP listener error looking into the fleetd journal
stdout, _ = cluster.MemberCommand(m0, "journalctl _PID=$(pidof fleetd)")
stdout, stderr, err = cluster.MemberCommand(m0, "journalctl _PID=$(pidof fleetd)")
if strings.Contains(strings.TrimSpace(stdout), "Failed serving HTTP on listener:") {
t.Fatalf("Fleetd log returned error on HTTP listeners: %s", stdout)
t.Fatalf("Fleetd log returned error on HTTP listeners.\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

// Check expected state after reconfiguring fleetd
stdout, _ = cluster.MemberCommand(m0, "systemctl", "show", "--property=ActiveState", "fleet")
stdout, stderr, err = cluster.MemberCommand(m0, "systemctl", "show", "--property=ActiveState", "fleet")
if strings.TrimSpace(stdout) != "ActiveState=active" {
t.Fatalf("Fleet unit not reported as active: %s", stdout)
t.Fatalf("Fleet unit not reported as active.\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
stdout, _ = cluster.MemberCommand(m0, "systemctl", "show", "--property=Result", "fleet")
stdout, stderr, err = cluster.MemberCommand(m0, "systemctl", "show", "--property=Result", "fleet")
if strings.TrimSpace(stdout) != "Result=success" {
t.Fatalf("Result for fleet unit not reported as success: %s", stdout)
t.Fatalf("Result for fleet unit not reported as success.\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
}

Expand All @@ -123,7 +123,7 @@ func waitForReloadConfig(cluster platform.Cluster, m0 platform.Member) (err erro
// "journalctl -u fleet | grep \"Reloading configuration\"" is racy
// in a subtle way, so that it sometimes fails only on semaphoreci.
// - dpark 20160408
stdout, _ := cluster.MemberCommand(m0, "sudo", "journalctl --priority=info _PID=$(pidof fleetd)")
stdout, _, _ := cluster.MemberCommand(m0, "sudo", "journalctl --priority=info _PID=$(pidof fleetd)")
journalfleet := strings.TrimSpace(stdout)
if !strings.Contains(journalfleet, "Reloading configuration") {
fmt.Errorf("Fleetd is not fully reconfigured, retrying... entire fleet journal:\n%v", journalfleet)
Expand All @@ -144,7 +144,7 @@ func waitForReloadConfig(cluster platform.Cluster, m0 platform.Member) (err erro
func waitForFleetdSocket(cluster platform.Cluster, m0 platform.Member) (err error) {
_, err = util.WaitForState(
func() bool {
stdout, _ := cluster.MemberCommand(m0, "test -S /var/run/fleet.sock && echo 1")
stdout, _, _ := cluster.MemberCommand(m0, "test -S /var/run/fleet.sock && echo 1")
if strings.TrimSpace(stdout) == "" {
fmt.Errorf("Fleetd is not fully started, retrying...")
return false
Expand Down
33 changes: 17 additions & 16 deletions functional/shutdown_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,19 @@ func TestShutdown(t *testing.T) {
}

// Stop the fleet process.
if _, err = cluster.MemberCommand(m0, "sudo", "systemctl", "stop", "fleet"); err != nil {
t.Fatal(err)
stdout, stderr, err := cluster.MemberCommand(m0, "sudo", "systemctl", "stop", "fleet")
if err != nil {
t.Fatalf("Failed to stop fleet process.\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

// Check expected state after stop.
stdout, _ := cluster.MemberCommand(m0, "systemctl", "show", "--property=ActiveState", "fleet")
stdout, stderr, err = cluster.MemberCommand(m0, "systemctl", "show", "--property=ActiveState", "fleet")
if strings.TrimSpace(stdout) != "ActiveState=inactive" {
t.Fatalf("Fleet unit not reported as inactive: %s", stdout)
t.Fatalf("Fleet unit not reported as inactive.\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
stdout, _ = cluster.MemberCommand(m0, "systemctl", "show", "--property=Result", "fleet")
stdout, stderr, err = cluster.MemberCommand(m0, "systemctl", "show", "--property=Result", "fleet")
if strings.TrimSpace(stdout) != "Result=success" {
t.Fatalf("Result for fleet unit not reported as success: %s", stdout)
t.Fatalf("Result for fleet unit not reported as success.\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
}

Expand All @@ -74,32 +75,32 @@ func TestShutdownVsMonitor(t *testing.T) {
// Cut connection to etcd.
//
// This will result in a failed health check, and consequently the monitor will attempt a restart.
stdout, err := cluster.MemberCommand(m0, "sudo", "iptables", "-I", "OUTPUT", "-p", "tcp", "-m", "multiport", "--dports=2379,4001", "-j", "DROP")
stdout, stderr, err := cluster.MemberCommand(m0, "sudo", "iptables", "-I", "OUTPUT", "-p", "tcp", "-m", "multiport", "--dports=2379,4001", "-j", "DROP")
if err != nil {
t.Fatalf("Failed inserting iptables rule:\nstdout: %s\nerr: %v", stdout, err)
t.Fatalf("Failed inserting iptables rule:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

// Wait for the monitor to trigger the restart.
//
// This will never complete, as long as there is no connectivity.
stdout, err = cluster.MemberCommand(m0, "sudo", "sh", "-c", `'until journalctl -u fleet | grep -q "Server monitor triggered: Monitor timed out before successful heartbeat"; do sleep 1; done'`)
stdout, stderr, err = cluster.MemberCommand(m0, "sudo", "sh", "-c", `'until journalctl -u fleet | grep -q "Server monitor triggered: Monitor timed out before successful heartbeat"; do sleep 1; done'`)
if err != nil {
t.Fatalf("Failed checking journal message:\nstdout: %s\nerr: %v", stdout, err)
t.Fatalf("Failed checking journal message:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

// Stop fleetd while the restart is still in progress.
stdout, err = cluster.MemberCommand(m0, "sudo", "systemctl", "stop", "fleet")
stdout, stderr, err = cluster.MemberCommand(m0, "sudo", "systemctl", "stop", "fleet")
if err != nil {
t.Fatalf("Failed stopping fleet service:\nstdout: %s\nerr: %v", stdout, err)
t.Fatalf("Failed stopping fleet service:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

// Verify that fleetd was shut down cleanly in spite of the concurrent restart.
stdout, _ = cluster.MemberCommand(m0, "systemctl", "show", "--property=ActiveState", "fleet")
stdout, stderr, err = cluster.MemberCommand(m0, "systemctl", "show", "--property=ActiveState", "fleet")
if strings.TrimSpace(stdout) != "ActiveState=inactive" {
t.Fatalf("Fleet unit not reported as inactive: %s", stdout)
t.Fatalf("Fleet unit not reported as inactive:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
stdout, _ = cluster.MemberCommand(m0, "systemctl", "show", "--property=Result", "fleet")
stdout, stderr, err = cluster.MemberCommand(m0, "systemctl", "show", "--property=Result", "fleet")
if strings.TrimSpace(stdout) != "Result=success" {
t.Fatalf("Result for fleet unit not reported as success: %s", stdout)
t.Fatalf("Result for fleet unit not reported as success:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
}
16 changes: 8 additions & 8 deletions functional/unit_action_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,8 @@ func TestUnitStatus(t *testing.T) {
stdout, stderr, err = cluster.Fleetctl(m,
"--strict-host-key-checking=false", "status", path.Base(unitFile))
if !strings.Contains(stdout, "Loaded: loaded") {
t.Errorf("Could not find expected string in status output:\n%s\nstderr:\n%s",
stdout, stderr)
t.Errorf("Could not find expected string in status output:\nstdout: %s\nstderr:\nerr: %s",
stdout, stderr, err)
}
}

Expand Down Expand Up @@ -345,12 +345,12 @@ func TestListUnitFilesOrder(t *testing.T) {
// make sure that all unit files will show up
_, err = cluster.WaitForNUnitFiles(m, 20)
if err != nil {
t.Fatal("Failed to run list-unit-files: %v", err)
t.Fatalf("Failed to run list-unit-files: %v", err)
}

stdout, stderr, err := cluster.Fleetctl(m, "list-unit-files", "--no-legend", "--fields", "unit")
if err != nil {
t.Fatal("Failed to run list-unit-files:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
t.Fatalf("Failed to run list-unit-files:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}

outUnits := strings.Split(strings.TrimSpace(stdout), "\n")
Expand Down Expand Up @@ -607,13 +607,13 @@ func checkListUnits(cl platform.Cluster, m platform.Member, cmd string, ufs []st

if cmd == "start" {
// Check expected systemd state after starting units
stdout, _ := cl.MemberCommand(m, "systemctl", "show", "--property=ActiveState", ufs[i])
stdout, stderr, err := cl.MemberCommand(m, "systemctl", "show", "--property=ActiveState", ufs[i])
if strings.TrimSpace(stdout) != "ActiveState=active" {
return fmt.Errorf("Fleet unit not reported as active: %s", stdout)
return fmt.Errorf("Fleet unit not reported as active:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
stdout, _ = cl.MemberCommand(m, "systemctl", "show", "--property=Result", ufs[i])
stdout, stderr, err = cl.MemberCommand(m, "systemctl", "show", "--property=Result", ufs[i])
if strings.TrimSpace(stdout) != "Result=success" {
return fmt.Errorf("Result for fleet unit not reported as success: %s", stdout)
return fmt.Errorf("Result for fleet unit not reported as success:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
}
}
Expand Down

0 comments on commit b9f335b

Please sign in to comment.