Address comments from siyuanfoundation

Author: henrybear327

tests/e2e/cluster_downgrade_test.go

@@ -17,14 +17,12 @@ package e2e
 import (
 	"context"
 	"fmt"
-	"math/rand"
 	"testing"
 	"time"
 
 	"github.com/coreos/go-semver/semver"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"go.uber.org/zap"
 
 	"go.etcd.io/etcd/api/v3/version"
 	"go.etcd.io/etcd/client/pkg/v3/fileutil"
@@ -65,19 +63,19 @@ func TestDowngradeUpgradeClusterOf3WithSnapshot(t *testing.T) {
 }
 
 func TestDowngradeCancellationWithoutEnablingClusterOf1(t *testing.T) {
-	testDowngradeUpgrade(t, 1, 1, false, cancelRightBeforeEnable)
+	testDowngradeUpgrade(t, 0, 1, false, cancelRightBeforeEnable)
 }
 
 func TestDowngradeCancellationRightAfterEnablingClusterOf1(t *testing.T) {
-	testDowngradeUpgrade(t, 1, 1, false, cancelRightAfterEnable)
+	testDowngradeUpgrade(t, 0, 1, false, cancelRightAfterEnable)
 }
 
 func TestDowngradeCancellationWithoutEnablingClusterOf3(t *testing.T) {
-	testDowngradeUpgrade(t, 3, 3, false, cancelRightBeforeEnable)
+	testDowngradeUpgrade(t, 0, 3, false, cancelRightBeforeEnable)
 }
 
 func TestDowngradeCancellationRightAfterEnablingClusterOf3(t *testing.T) {
-	testDowngradeUpgrade(t, 3, 3, false, cancelRightAfterEnable)
+	testDowngradeUpgrade(t, 0, 3, false, cancelRightAfterEnable)
 }
 
 func TestDowngradeCancellationAfterDowngrading1InClusterOf3(t *testing.T) {
@@ -88,7 +86,7 @@ func TestDowngradeCancellationAfterDowngrading2InClusterOf3(t *testing.T) {
 	testDowngradeUpgrade(t, 2, 3, false, cancelAfterDowngrading)
 }
 
-func testDowngradeUpgrade(t *testing.T, cancellationSize int, clusterSize int, triggerSnapshot bool, triggerCancellation CancellationState) {
+func testDowngradeUpgrade(t *testing.T, numberOfMembersToDowngrade int, clusterSize int, triggerSnapshot bool, triggerCancellation CancellationState) {
 	currentEtcdBinary := e2e.BinPath.Etcd
 	lastReleaseBinary := e2e.BinPath.EtcdLastRelease
 	if !fileutil.Exist(lastReleaseBinary) {
@@ -159,13 +157,10 @@ func testDowngradeUpgrade(t *testing.T, cancellationSize int, clusterSize int, t
 		return // No need to perform downgrading, end the test here
 	}
 
-	membersToChange := rand.Perm(len(epc.Procs))[:cancellationSize]
-	t.Logf(fmt.Sprintln("Elect members for operations"), zap.Any("members", membersToChange))
-
 	t.Logf("Starting downgrade process to %q", lastVersionStr)
-	err = e2e.DowngradeUpgradeMembersByID(t, nil, epc, membersToChange, currentVersion, lastClusterVersion)
+	membersChanged, err := e2e.DowngradeUpgradeMembers(t, nil, epc, numberOfMembersToDowngrade, currentVersion, lastClusterVersion)
 	require.NoError(t, err)
-	if len(membersToChange) == len(epc.Procs) {
+	if len(membersChanged) == len(epc.Procs) {
 		e2e.AssertProcessLogs(t, leader(t, epc), "the cluster has been downgraded")
 	}
 
@@ -182,7 +177,7 @@ func testDowngradeUpgrade(t *testing.T, cancellationSize int, clusterSize int, t
 	if triggerCancellation == cancelAfterDowngrading {
 		e2e.DowngradeCancel(t, epc)
 		t.Log("Downgrade cancelled, validating if cluster is in the right state")
-		e2e.ValidateMemberVersions(t, epc, generatePartialCancellationVersions(clusterSize, membersToChange, lastClusterVersion))
+		e2e.ValidateMemberVersions(t, epc, generatePartialCancellationVersions(clusterSize, membersChanged, lastClusterVersion))
 	}
 
 	t.Log("Adding learner to test membership, but avoid breaking quorum")
@@ -199,7 +194,7 @@ func testDowngradeUpgrade(t *testing.T, cancellationSize int, clusterSize int, t
 	beforeMembers, beforeKV = getMembersAndKeys(t, cc)
 
 	t.Logf("Starting upgrade process to %q", currentVersionStr)
-	err = e2e.DowngradeUpgradeMembersByID(t, nil, epc, membersToChange, lastClusterVersion, currentVersion)
+	membersChanged, err = e2e.DowngradeUpgradeMembers(t, nil, epc, numberOfMembersToDowngrade, lastClusterVersion, currentVersion)
 	require.NoError(t, err)
 	t.Log("Upgrade complete")
 

tests/framework/e2e/downgrade.go

@@ -64,17 +64,11 @@ func DowngradeCancel(t *testing.T, epc *EtcdProcessCluster) {
 	t.Log("Cluster downgrade cancellation is completed")
 }
 
-func DowngradeUpgradeMembers(t *testing.T, lg *zap.Logger, clus *EtcdProcessCluster, numberOfMembersToChange int, currentVersion, targetVersion *semver.Version) error {
-	membersToChange := rand.Perm(len(clus.Procs))[:numberOfMembersToChange]
-	t.Logf(fmt.Sprintln("Elect members for operations"), zap.Any("members", membersToChange))
-
-	return DowngradeUpgradeMembersByID(t, lg, clus, membersToChange, currentVersion, targetVersion)
-}
-
-func DowngradeUpgradeMembersByID(t *testing.T, lg *zap.Logger, clus *EtcdProcessCluster, membersToChange []int, currentVersion, targetVersion *semver.Version) error {
+func DowngradeUpgradeMembers(t *testing.T, lg *zap.Logger, clus *EtcdProcessCluster, numberOfMembersToChange int, currentVersion, targetVersion *semver.Version) (membersChanged []int, err error) {
 	if lg == nil {
 		lg = clus.lg
 	}
+
 	isDowngrade := targetVersion.LessThan(*currentVersion)
 	opString := "upgrading"
 	newExecPath := BinPath.Etcd
@@ -83,29 +77,32 @@ func DowngradeUpgradeMembersByID(t *testing.T, lg *zap.Logger, clus *EtcdProcess
 		newExecPath = BinPath.EtcdLastRelease
 	}
 
+	membersChanged = rand.Perm(len(clus.Procs))[:numberOfMembersToChange]
+	t.Logf(fmt.Sprintln("Elect members for operations"), zap.Any("members", membersChanged))
+
 	// Need to wait health interval for cluster to prepare for downgrade/upgrade
 	time.Sleep(etcdserver.HealthInterval)
 
-	for _, memberID := range membersToChange {
+	for _, memberID := range membersChanged {
 		member := clus.Procs[memberID]
 		if member.Config().ExecPath == newExecPath {
-			return fmt.Errorf("member:%s is already running with the %s target binary - %s", member.Config().Name, opString, member.Config().ExecPath)
+			return nil, fmt.Errorf("member:%s is already running with the %s target binary - %s", member.Config().Name, opString, member.Config().ExecPath)
 		}
 		lg.Info(fmt.Sprintf("%s member", opString), zap.String("member", member.Config().Name))
 		if err := member.Stop(); err != nil {
-			return err
+			return nil, err
 		}
 		member.Config().ExecPath = newExecPath
 		lg.Info("Restarting member", zap.String("member", member.Config().Name))
 		err := member.Start(context.TODO())
 		if err != nil {
-			return err
+			return nil, err
 		}
 	}
 	lg.Info("Validating versions")
-	for _, memberID := range membersToChange {
+	for _, memberID := range membersChanged {
 		member := clus.Procs[memberID]
-		if isDowngrade || len(membersToChange) == len(clus.Procs) {
+		if isDowngrade || len(membersChanged) == len(clus.Procs) {
 			ValidateVersion(t, clus.Cfg, member, version.Versions{
 				Cluster: targetVersion.String(),
 				Server:  targetVersion.String(),
@@ -117,7 +114,7 @@ func DowngradeUpgradeMembersByID(t *testing.T, lg *zap.Logger, clus *EtcdProcess
 			})
 		}
 	}
-	return nil
+	return membersChanged, nil
 }
 
 func ValidateMemberVersions(t *testing.T, epc *EtcdProcessCluster, expect []*version.Versions) {

tests/robustness/failpoint/cluster.go

@@ -174,7 +174,7 @@ func (f memberDowngrade) Inject(ctx context.Context, t *testing.T, lg *zap.Logge
 	time.Sleep(etcdserver.HealthInterval)
 	e2e.DowngradeEnable(t, clus, lastVersion)
 
-	err = e2e.DowngradeUpgradeMembers(t, lg, clus, numberOfMembersToDowngrade, currentVersion, lastVersion)
+	_, err = e2e.DowngradeUpgradeMembers(t, lg, clus, numberOfMembersToDowngrade, currentVersion, lastVersion)
 	time.Sleep(etcdserver.HealthInterval)
 	return nil, err
 }
@@ -230,13 +230,13 @@ func (f memberDowngradeUpgrade) Inject(ctx context.Context, t *testing.T, lg *za
 	time.Sleep(etcdserver.HealthInterval)
 	e2e.DowngradeEnable(t, clus, lastVersion)
 	// downgrade all members first
-	err = e2e.DowngradeUpgradeMembers(t, lg, clus, len(clus.Procs), currentVersion, lastVersion)
+	_, err = e2e.DowngradeUpgradeMembers(t, lg, clus, len(clus.Procs), currentVersion, lastVersion)
 	if err != nil {
 		return nil, err
 	}
 	// partial upgrade the cluster
 	numberOfMembersToUpgrade := rand.Int()%len(clus.Procs) + 1
-	err = e2e.DowngradeUpgradeMembers(t, lg, clus, numberOfMembersToUpgrade, lastVersion, currentVersion)
+	_, err = e2e.DowngradeUpgradeMembers(t, lg, clus, numberOfMembersToUpgrade, lastVersion, currentVersion)
 	time.Sleep(etcdserver.HealthInterval)
 	return nil, err
 }