From 9a088c661422f0d10ebc4b83aca19fc205413a6c 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 | 36 +++++++++++++++++++++--------
 tests/framework/e2e/cluster.go      | 24 -------------------
 2 files changed, 27 insertions(+), 33 deletions(-)

diff --git a/tests/e2e/cluster_downgrade_test.go b/tests/e2e/cluster_downgrade_test.go
index 161d5c079138..95a4ac27cd20 100644
--- a/tests/e2e/cluster_downgrade_test.go
+++ b/tests/e2e/cluster_downgrade_test.go
@@ -55,7 +55,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")
@@ -65,9 +65,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)
 	}
@@ -78,11 +78,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++ {
@@ -180,10 +183,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 cdb7b4adb378..242ebe0d6a7d 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"
@@ -493,25 +491,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")
-}