Revert "tests/robustness: check for compaction before prevKV validation"

This reverts commit 5d7f58d14be1da5135d158dad6fc43391cbf6283.

Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>

tests/robustness/main_test.go

@@ -16,7 +16,6 @@ package robustness
 
 import (
 	"context"
-	"strings"
 	"testing"
 	"time"
 
@@ -24,7 +23,6 @@ import (
 	"go.uber.org/zap/zaptest"
 	"golang.org/x/sync/errgroup"
 
-	"go.etcd.io/etcd/pkg/v3/expect"
 	"go.etcd.io/etcd/tests/v3/framework"
 	"go.etcd.io/etcd/tests/v3/framework/e2e"
 	"go.etcd.io/etcd/tests/v3/robustness/failpoint"
@@ -91,18 +89,11 @@ func testRobustness(ctx context.Context, t *testing.T, lg *zap.Logger, s testSce
 		report.Report(t, panicked)
 	}()
 	report.Client = s.run(ctx, t, lg, report.Cluster)
-	compaction, err := checkForCompaction(report.Cluster)
-	if err != nil {
-		t.Fatalf("failed checking for compaction: %v", err)
-	}
 	forcestopCluster(report.Cluster)
 
 	watchProgressNotifyEnabled := report.Cluster.Cfg.ServerConfig.ExperimentalWatchProgressNotifyInterval != 0
 	validateGotAtLeastOneProgressNotify(t, report.Client, s.watch.requestProgress || watchProgressNotifyEnabled)
-	validateConfig := validate.Config{
-		ExpectRevisionUnique: s.traffic.ExpectUniqueRevision(),
-		AssumeCompaction:     compaction,
-	}
+	validateConfig := validate.Config{ExpectRevisionUnique: s.traffic.ExpectUniqueRevision()}
 	report.Visualize = validate.ValidateAndReturnVisualize(t, lg, validateConfig, report.Client)
 
 	panicked = false
@@ -167,30 +158,3 @@ func forcestopCluster(clus *e2e.EtcdProcessCluster) error {
 	}
 	return clus.ConcurrentStop()
 }
-
-func checkForCompaction(clus *e2e.EtcdProcessCluster) (bool, error) {
-	req := e2e.CURLReq{
-		Endpoint: "/metrics",
-		Expected: expect.ExpectedResponse{
-			// Filter out the etcd_debugging_mvcc_db_compaction_keys_total
-			// metric if it has a value greater than 0 from the response returned
-			// by the /metrics endpoint.
-			Value:         `etcd_debugging_mvcc_db_compaction_keys_total\s+[1-9][0-9]*`,
-			IsRegularExpr: true,
-		},
-	}
-
-	err := e2e.CURLGet(clus, req)
-	if err == nil {
-		return true, nil
-	}
-
-	// If no match was found, compaction did not take place.
-	noCompaction := strings.Contains(err.Error(), "match not found.") ||
-		strings.Contains(err.Error(), "context deadline exceeded")
-	if noCompaction {
-		return false, nil
-	}
-
-	return false, err
-}

tests/robustness/validate/validate.go

@@ -49,7 +49,6 @@ func ValidateAndReturnVisualize(t *testing.T, lg *zap.Logger, cfg Config, report
 
 type Config struct {
 	ExpectRevisionUnique bool
-	AssumeCompaction     bool
 }
 
 func mergeWatchEventHistory(reports []report.ClientReport) ([]model.PersistedEvent, error) {

tests/robustness/validate/watch.go

@@ -34,7 +34,7 @@ func validateWatch(t *testing.T, lg *zap.Logger, cfg Config, reports []report.Cl
 		if eventHistory != nil {
 			validateReliable(t, eventHistory, r)
 			validateResumable(t, eventHistory, r)
-			validatePrevKV(t, r, eventHistory, cfg.AssumeCompaction)
+			validatePrevKV(t, r, eventHistory)
 			validateCreateEvent(t, r, eventHistory)
 		}
 	}
@@ -148,7 +148,7 @@ func validateResumable(t *testing.T, events []model.PersistedEvent, report repor
 
 // validatePrevKV ensures that a watch response (if configured with WithPrevKV()) returns
 // the appropriate response.
-func validatePrevKV(t *testing.T, report report.ClientReport, history []model.PersistedEvent, compactionOccured bool) {
+func validatePrevKV(t *testing.T, report report.ClientReport, history []model.PersistedEvent) {
 	replay := model.NewReplay(history)
 	for _, op := range report.Watch {
 		if !op.Request.WithPrevKV {
@@ -161,16 +161,16 @@ func validatePrevKV(t *testing.T, report report.ClientReport, history []model.Pe
 				if err != nil {
 					t.Error(err)
 				}
-
+				// TODO(MadhavJivrajani): check if compaction has been run as part
+				// of failpoint injection. If compaction has run, prevKV can be nil
+				// even if it is not a create event.
+				//
 				// Considering that Kubernetes opens watches to etcd using WithPrevKV()
-				// option, we would want to explicitly check the condition that
+				// option, ideally we would want to explicitly check the condition that
 				// Kubernetes does while parsing events received from etcd:
 				// https://github.com/kubernetes/kubernetes/blob/a9e4f5b7862e84c4152eabe2e960f3f6fb9a4867/staging/src/k8s.io/apiserver/pkg/storage/etcd3/event.go#L59
-				// i.e. prevKV is nil iff the event is a create event, we can reliably
-				// check that only if compaction has not occured.
-				if !compactionOccured && event.PrevValue == nil && !event.IsCreate {
-					t.Errorf("PrevKV - without compaction, PrevValue cannot be nil if the event is not a create event, event: %+v", event)
-				}
+				// i.e. prevKV is nil iff the event is a create event, we cannot reliably
+				// check that without knowing if compaction has run.
 
 				// We allow PrevValue to be nil since in the face of compaction, etcd does not
 				// guarantee its presence.