From 2d4fa5840045f08989abaf25a25764961144e6fa Mon Sep 17 00:00:00 2001
From: Marek Siarkowicz <siarkowicz@google.com>
Date: Mon, 21 Feb 2022 15:46:36 +0100
Subject: [PATCH] tests: Apply suggestions from code-review

---
 tests/e2e/cluster_downgrade_test.go | 38 ++++++++++++++++++++++-------
 tests/framework/e2e/cluster.go      | 24 ------------------
 2 files changed, 29 insertions(+), 33 deletions(-)

diff --git a/tests/e2e/cluster_downgrade_test.go b/tests/e2e/cluster_downgrade_test.go
index 9458ec78ea52..eecf4c86213a 100644
--- a/tests/e2e/cluster_downgrade_test.go
+++ b/tests/e2e/cluster_downgrade_test.go
@@ -15,6 +15,7 @@
 package e2e
 
 import (
+	"context"
 	"fmt"
 	"testing"
 	"time"
@@ -22,6 +23,7 @@ import (
 	"github.com/coreos/go-semver/semver"
 	"go.etcd.io/etcd/api/v3/version"
 	"go.etcd.io/etcd/client/pkg/v3/fileutil"
+	clientv3 "go.etcd.io/etcd/client/v3"
 	"go.etcd.io/etcd/tests/v3/framework/e2e"
 	"go.etcd.io/etcd/tests/v3/framework/testutils"
 )
@@ -54,7 +56,7 @@ func testDowngradeUpgrade(t *testing.T, clusterSize int) {
 	}
 	t.Logf("Cluster created")
 
-	t.Logf("etcdctl downgrade enable %s", lastVersion)
+	t.Logf("etcdctl downgrade enable %s", lastVersionStr)
 	downgradeEnable(t, epc, lastVersion)
 
 	t.Log("Downgrade enabled, validating if cluster is ready for downgrade")
@@ -64,9 +66,9 @@ func testDowngradeUpgrade(t *testing.T, clusterSize int) {
 	}
 	t.Log("Cluster is ready for downgrade")
 
-	t.Log("Starting downgrade process")
+	t.Logf("Starting downgrade process to %q", lastVersionStr)
 	for i := 0; i < len(epc.Procs); i++ {
-		t.Logf("Downgrading member %d", i)
+		t.Logf("Downgrading member %d by running %s binary", i, lastReleaseBinary)
 		stopEtcd(t, epc.Procs[i])
 		startEtcd(t, epc.Procs[i], lastReleaseBinary)
 	}
@@ -77,11 +79,14 @@ func testDowngradeUpgrade(t *testing.T, clusterSize int) {
 	}
 	t.Log("Downgrade complete")
 
-	t.Log("Starting upgrade process")
+	t.Logf("Starting upgrade process to %q", currentVersionStr)
 	for i := 0; i < len(epc.Procs); i++ {
 		t.Logf("Upgrading member %d", i)
 		stopEtcd(t, epc.Procs[i])
 		startEtcd(t, epc.Procs[i], currentEtcdBinary)
+		if i+1 < len(epc.Procs) {
+			validateVersion(t, epc.Cfg, epc.Procs[i], version.Versions{Cluster: lastVersionStr, Server: currentVersionStr})
+		}
 	}
 	t.Log("All members upgraded, validating upgrade")
 	for i := 0; i < len(epc.Procs); i++ {
@@ -172,10 +177,25 @@ func expectLog(t *testing.T, ep e2e.EtcdProcess, expectLog string) {
 
 func leader(t *testing.T, epc *e2e.EtcdProcessCluster) e2e.EtcdProcess {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
-	leader, err := epc.Leader(ctx)
-	cancel()
-	if err != nil {
-		t.Fatal(err)
+	defer cancel()
+	for i := 0; i < len(epc.Procs); i++ {
+		endpoints := epc.Procs[i].EndpointsV3()
+		cli, err := clientv3.New(clientv3.Config{
+			Endpoints:   endpoints,
+			DialTimeout: 3 * time.Second,
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer cli.Close()
+		resp, err := cli.Status(ctx, endpoints[0])
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.Header.GetMemberId() == resp.Leader {
+			return epc.Procs[i]
+		}
 	}
-	return leader
+	t.Fatal("Leader not found")
+	return nil
 }
diff --git a/tests/framework/e2e/cluster.go b/tests/framework/e2e/cluster.go
index fe608f7444e7..4b1daf93d784 100644
--- a/tests/framework/e2e/cluster.go
+++ b/tests/framework/e2e/cluster.go
@@ -15,7 +15,6 @@
 package e2e
 
 import (
-	"context"
 	"fmt"
 	"net/url"
 	"os"
@@ -24,7 +23,6 @@ import (
 	"testing"
 	"time"
 
-	clientv3 "go.etcd.io/etcd/client/v3"
 	"go.etcd.io/etcd/server/v3/etcdserver"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zaptest"
@@ -515,25 +513,3 @@ func (epc *EtcdProcessCluster) WithStopSignal(sig os.Signal) (ret os.Signal) {
 	}
 	return ret
 }
-func (epc *EtcdProcessCluster) Leader(ctx context.Context) (EtcdProcess, error) {
-	for i := 0; i < len(epc.Procs); i++ {
-		endpoints := epc.Procs[i].EndpointsV3()
-		cli, err := clientv3.New(clientv3.Config{
-			Endpoints:   endpoints,
-			DialTimeout: 3 * time.Second,
-		})
-		if err != nil {
-			return nil, err
-		}
-		defer cli.Close()
-		resp, err := cli.Status(ctx, endpoints[0])
-		if err != nil {
-			return nil, err
-		}
-		if resp.Header.GetMemberId() == resp.Leader {
-			return epc.Procs[i], nil
-		}
-	}
-
-	return nil, fmt.Errorf("Leader not found")
-}