Allowing globally knob change in TOML file based test

In commit 99b030c2f63a3c9ad92ed56aa2b5709322a4cb06, it is allowed to set
knob values in TOML file per single test, using syntax

[[test]]
    [[test.knobs]]
    knob_key = knob_value

the knob key/value pairs are changed before the TEST_CASE starts, then
reverted after TEST_CASE completes.

With this patch, it is possible to *globally* update the knob value,
i.e.

[[knobs]]
enable_encryption = true

[[test]]
testTitle = 'EncryptKeyProxy'

    [[test.workload]]
    testName = 'EncryptKeyProxyTest'

This is manually tested by printing out knob key/value pairs. Also
tested using Ata's EncryptKeyProxy test code by enabling
enable_encryption key.

fdbserver/tester.actor.cpp

@@ -1306,17 +1306,58 @@ std::string toml_to_string(const T& value) {
 	}
 }
 
-std::vector<TestSpec> readTOMLTests_(std::string fileName) {
+struct TestSet {
-	TestSpec spec;
+	KnobKeyValuePairs overrideKnobs;
+	std::vector<TestSpec> testSpecs;
+};
+
+namespace {
+
+// In the current TOML scope, look for "knobs" field. If exists, translate all
+// key value pairs into KnobKeyValuePairs
+KnobKeyValuePairs getOverriddenKnobKeyValues(const toml::value& context) {
+	KnobKeyValuePairs result;
+
+	try {
+		const toml::array& overrideKnobs = toml::find(context, "knobs").as_array();
+		for (const toml::value& knob : overrideKnobs) {
+			for (const auto& [key, value_] : knob.as_table()) {
+				const std::string& value = toml_to_string(value_);
+				ParsedKnobValue parsedValue = CLIENT_KNOBS->parseKnobValue(key, value);
+				if (std::get_if<NoKnobFound>(&parsedValue)) {
+					parsedValue = SERVER_KNOBS->parseKnobValue(key, value);
+				}
+				if (std::get_if<NoKnobFound>(&parsedValue)) {
+					TraceEvent(SevError, "TestSpecUnrecognizedKnob")
+					    .detail("KnobName", key)
+					    .detail("OverrideValue", value);
+					continue;
+				}
+				result.set(key, parsedValue);
+			}
+		}
+	} catch (const std::out_of_range&) {
+		// No knobs field in this scope, this is not an error
+	}
+
+	return result;
+}
+
+} // namespace
+
+TestSet readTOMLTests_(std::string fileName) {
 	Standalone<VectorRef<KeyValueRef>> workloadOptions;
-	std::vector<TestSpec> result;
+	TestSet result;
 
 	const toml::value& conf = toml::parse(fileName);
 
+	// Parse the global knob changes
+	result.overrideKnobs = getOverriddenKnobKeyValues(conf);
+
 	// Then parse each test
 	const toml::array& tests = toml::find(conf, "test").as_array();
 	for (const toml::value& test : tests) {
-		spec = TestSpec();
+		TestSpec spec;
 
 		// First handle all test-level settings
 		for (const auto& [k, v] : test.as_table()) {
@@ -1347,29 +1388,9 @@ std::vector<TestSpec> readTOMLTests_(std::string fileName) {
 		}
 
 		// And then copy the knob attributes to spec.overrideKnobs
-		try {
+		spec.overrideKnobs = getOverriddenKnobKeyValues(test);
-			const toml::array& overrideKnobs = toml::find(test, "knobs").as_array();
-			for (const toml::value& knob : overrideKnobs) {
-				for (const auto& [key, value_] : knob.as_table()) {
-					const std::string& value = toml_to_string(value_);
-					ParsedKnobValue parsedValue = CLIENT_KNOBS->parseKnobValue(key, value);
-					if (std::get_if<NoKnobFound>(&parsedValue)) {
-						parsedValue = SERVER_KNOBS->parseKnobValue(key, value);
-					}
-					if (std::get_if<NoKnobFound>(&parsedValue)) {
-						TraceEvent(SevError, "TestSpecUnrecognizedKnob")
-						    .detail("KnobName", key)
-						    .detail("OverrideValue", value);
-						continue;
-					}
-					spec.overrideKnobs.set(key, parsedValue);
-				}
-			}
-		} catch (const std::out_of_range&) {
-			// no knob overridden
-		}
 
-		result.push_back(spec);
+		result.testSpecs.push_back(spec);
 	}
 
 	return result;
@@ -1377,7 +1398,7 @@ std::vector<TestSpec> readTOMLTests_(std::string fileName) {
 
 // A hack to catch and log std::exception, because TOML11 has very useful
 // error messages, but the actor framework can't handle std::exception.
-std::vector<TestSpec> readTOMLTests(std::string fileName) {
+TestSet readTOMLTests(std::string fileName) {
 	try {
 		return readTOMLTests_(fileName);
 	} catch (std::exception& e) {
@@ -1680,7 +1701,8 @@ ACTOR Future<Void> runTests(Reference<IClusterConnectionRecord> connRecord,
                             LocalityData locality,
                             UnitTestParameters testOptions,
                             Optional<TenantName> defaultTenant) {
-	state std::vector<TestSpec> testSpecs;
+	state TestSet testSet;
+	state std::unique_ptr<KnobProtectiveGroup> knobProtectiveGroup(nullptr);
 	auto cc = makeReference<AsyncVar<Optional<ClusterControllerFullInterface>>>();
 	auto ci = makeReference<AsyncVar<Optional<ClusterInterface>>>();
 	std::vector<Future<Void>> actors;
@@ -1711,7 +1733,7 @@ ACTOR Future<Void> runTests(Reference<IClusterConnectionRecord> connRecord,
 		options.push_back_deep(options.arena(),
 		                       KeyValueRef(LiteralStringRef("shuffleShards"), LiteralStringRef("true")));
 		spec.options.push_back_deep(spec.options.arena(), options);
-		testSpecs.push_back(spec);
+		testSet.testSpecs.push_back(spec);
 	} else if (whatToRun == TEST_TYPE_UNIT_TESTS) {
 		TestSpec spec;
 		Standalone<VectorRef<KeyValueRef>> options;
@@ -1727,7 +1749,7 @@ ACTOR Future<Void> runTests(Reference<IClusterConnectionRecord> connRecord,
 			options.push_back_deep(options.arena(), KeyValueRef(kv.first, kv.second));
 		}
 		spec.options.push_back_deep(spec.options.arena(), options);
-		testSpecs.push_back(spec);
+		testSet.testSpecs.push_back(spec);
 	} else {
 		std::ifstream ifs;
 		ifs.open(fileName.c_str(), std::ifstream::in);
@@ -1740,11 +1762,11 @@ ACTOR Future<Void> runTests(Reference<IClusterConnectionRecord> connRecord,
 		}
 		enableClientInfoLogging(); // Enable Client Info logging by default for tester
 		if (boost::algorithm::ends_with(fileName, ".txt")) {
-			testSpecs = readTests(ifs);
+			testSet.testSpecs = readTests(ifs);
 		} else if (boost::algorithm::ends_with(fileName, ".toml")) {
 			// TOML is weird about opening the file as binary on windows, so we
 			// just let TOML re-open the file instead of using ifs.
-			testSpecs = readTOMLTests(fileName);
+			testSet = readTOMLTests(fileName);
 		} else {
 			TraceEvent(SevError, "TestHarnessFail")
 			    .detail("Reason", "unknown tests specification extension")
@@ -1754,6 +1776,7 @@ ACTOR Future<Void> runTests(Reference<IClusterConnectionRecord> connRecord,
 		ifs.close();
 	}
 
+	knobProtectiveGroup = std::make_unique<KnobProtectiveGroup>(testSet.overrideKnobs);
 	Future<Void> tests;
 	if (at == TEST_HERE) {
 		auto db = makeReference<AsyncVar<ServerDBInfo>>();
@@ -1761,10 +1784,10 @@ ACTOR Future<Void> runTests(Reference<IClusterConnectionRecord> connRecord,
 		actors.push_back(
 		    reportErrors(monitorServerDBInfo(cc, LocalityData(), db), "MonitorServerDBInfo")); // FIXME: Locality
 		actors.push_back(reportErrors(testerServerCore(iTesters[0], connRecord, db, locality), "TesterServerCore"));
-		tests = runTests(cc, ci, iTesters, testSpecs, startingConfiguration, locality, defaultTenant);
+		tests = runTests(cc, ci, iTesters, testSet.testSpecs, startingConfiguration, locality, defaultTenant);
 	} else {
 		tests = reportErrors(
-		    runTests(cc, ci, testSpecs, at, minTestersExpected, startingConfiguration, locality, defaultTenant),
+		    runTests(cc, ci, testSet.testSpecs, at, minTestersExpected, startingConfiguration, locality, defaultTenant),
 		    "RunTests");
 	}