[Comb] Include correct MAC and IPv4/6 address for peer interface.Create DVaaS traffic helper for NSF test infra.Add support for gnmi_config and p4_info to NsfTestParams.Rename Upgrade to ImageCopy to better reflect its functionality.Make error_margin an optional parameter for ValidateTraffic.Implement reboot and gNMI config push helpers.Create flags to skip NSF Upgrade and traffic to only test NSF Reboot.Implement the ImageCopy reusable helper and reuse it in other helpers and NSF test skeleton.Create a reusable Reboot method to perform reboot on the SUT with the given reboot method.

dvaas/BUILD.bazel

@@ -20,6 +20,18 @@ package(
     licenses = ["notice"],
 )
 
+# Since DVaaS's user-facing API is still in development and subject to change, we restrict
+# visibility to an explicit allow list. Send us a CL or come talk to us if you'd like to be added.
+DVAAS_USER_ALLOW_LIST = [
+    "//platforms/networking/gpins/testing/blackbox/p4:__subpackages__",
+    "//platforms/networking/gpins/testing/blackbox/replay:__subpackages__",
+    "//platforms/networking/gpins/testing/ondatra/thinkit/tests:__pkg__",
+    "//tests/forwarding:__pkg__",
+    "//tests/ondatra/thinkit:__pkg__",
+    "//p4testgen:__pkg__",
+    "//platforms/networking/gpins/testing/lib/nsf/traffic_helpers:__subpackages__",
+]
+
 cc_library(
     name = "dataplane_validation",
     srcs = ["dataplane_validation.cc"],

tests/integration/system/nsf/BUILD.bazel

@@ -28,6 +28,7 @@ cc_library(
         "//gutil:overload",
         "//gutil:status",
         "//gutil:testing",
+        "//lib/gnmi:gnmi_helper",
         "//lib/utils:generic_testbed_utils",
         "//lib/validator:validator_lib",
         "//tests/integration/system/nsf/interfaces:component_validator",

tests/integration/system/nsf/interfaces/component_validator.h

@@ -45,6 +45,9 @@ namespace pins_test {
 //
 // Eg. We can use these methods to track performance by calculating the time
 // difference between function calls.
+//
+// Note: The caller is responsible for ensuring that exact same testbed is
+// passed throughout the test.
 class ComponentValidator {
  public:
   virtual ~ComponentValidator() = default;
@@ -67,8 +70,9 @@ class ComponentValidator {
     return absl::OkStatus();
   }
 
-  // Called after an upgrade is performed on the SUT.
-  virtual absl::Status OnUpgrade(absl::string_view version, Testbed& testbed) {
+  // Called after an image copy is performed on the SUT.
+  virtual absl::Status OnImageCopy(absl::string_view version,
+                                   Testbed& testbed) {
     return absl::OkStatus();
   }
 

tests/integration/system/nsf/interfaces/flow_programmer.h

@@ -22,6 +22,9 @@ namespace pins_test {
 
 // Interface to program or clear flows on the SUT of the given `testbed` during
 // NSF integration tests.
+//
+// Note: The caller is responsible for ensuring that exact same testbed is
+// passed throughout the test.
 class FlowProgrammer {
  public:
   virtual ~FlowProgrammer() = default;

tests/integration/system/nsf/interfaces/test_params.h

@@ -34,6 +34,8 @@ namespace pins_test {
 // parameterized NSF integration test.
 struct NsfTestParams {
   std::string name;
+  std::string gnmi_config;
+  p4::config::v1::P4Info p4_info;
   std::function<std::unique_ptr<FlowProgrammer>()> create_flow_programmer;
   std::function<std::unique_ptr<TrafficHelper>()> create_traffic_helper;
   std::function<TestbedInterface()> create_testbed_interface;

tests/integration/system/nsf/interfaces/traffic_helper.h

@@ -22,6 +22,9 @@ namespace pins_test {
 
 // Interface to control traffic in the given `testbed` during NSF integration
 // test.
+//
+// Note: The caller is responsible for ensuring that exact same testbed is
+// passed throughout the test.
 class TrafficHelper {
  public:
   virtual ~TrafficHelper() = default;
@@ -34,9 +37,11 @@ class TrafficHelper {
   virtual absl::Status StopTraffic(Testbed& testbed) = 0;
 
   // Validates traffic in the testbed.
-  // Needs to be called *after* `StopTraffic()` is called.
-  virtual absl::Status ValidateTraffic(int error_percentage,
-                                       Testbed& testbed) = 0;
+  absl::Status ValidateTraffic(Testbed& testbed) {
+    return ValidateTraffic(testbed, 0);
+  };
+  virtual absl::Status ValidateTraffic(Testbed& testbed,
+                                       int error_percentage) = 0;
 };
 
 }  // namespace pins_test

tests/integration/system/nsf/traffic_helpers/BUILD.bazel

@@ -19,17 +19,6 @@ package(
     licenses = ["notice"],
 )
 
-cc_library(
-    name = "dvaas_helper",
-    testonly = True,
-    hdrs = ["dvaas_helper.h"],
-    deps = [
-        "//tests/integration/system/nsf/interfaces:testbed",
-        "//tests/integration/system/nsf/interfaces:traffic_helper",
-        "@com_google_absl//absl/status",
-    ],
-)
-
 cc_library(
     name = "otg_helper",
     testonly = True,

tests/integration/system/nsf/traffic_helpers/otg_helper.cc

@@ -56,8 +56,10 @@ absl::Status OtgHelper::StartTraffic(Testbed& testbed) {
             auto* config = set_config_request.mutable_config();
 
             // Randomly pick source and destination hosts.
-            const std::string otg_src_port = up_links.front().peer_interface;
-            const std::string otg_dst_port = up_links.back().peer_interface;
+            const std::string otg_src_port =
+                up_links.front().peer_traffic_location;
+            const std::string otg_dst_port =
+                up_links.back().peer_traffic_location;
             const std::string otg_src_mac = up_links.front().peer_mac_address;
             const std::string otg_dst_mac = up_links.back().peer_mac_address;
             const std::string otg_src_ip = up_links.front().peer_ipv6_address;
@@ -170,7 +172,8 @@ absl::Status OtgHelper::StopTraffic(Testbed& testbed) {
   return absl::UnimplementedError("Stopping traffic is not implemented.");
 }
 
-absl::Status OtgHelper::ValidateTraffic(int error_margin, Testbed& testbed) {
+absl::Status OtgHelper::ValidateTraffic(Testbed& testbed,
+                                        int error_percentage) {
   return std::visit(
       gutil::Overload{
           [&](std::unique_ptr<thinkit::GenericTestbed>& testbed)
@@ -207,8 +210,8 @@ absl::Status OtgHelper::ValidateTraffic(int error_margin, Testbed& testbed) {
               bool transmission_stopped =
                   flow_metric.transmit() == otg::FlowMetric::Transmit::stopped;
 
-              if (bytes_drop_percent <= error_margin &&
-                  frames_drop_percent <= error_margin)
+              if (bytes_drop_percent <= error_percentage &&
+                  frames_drop_percent <= error_percentage)
                 continue;
 
               errors.push_back(absl::StrCat(

tests/integration/system/nsf/traffic_helpers/otg_helper.h

@@ -25,7 +25,7 @@ class OtgHelper : public TrafficHelper {
  public:
   absl::Status StartTraffic(Testbed& testbed) override;
   absl::Status StopTraffic(Testbed& testbed) override;
-  absl::Status ValidateTraffic(int error_margin, Testbed& testbed) override;
+  absl::Status ValidateTraffic(Testbed& testbed, int error_percentage) override;
 };
 
 }  // namespace pins_test

tests/integration/system/nsf/upgrade_test.cc

@@ -15,6 +15,7 @@
 #include "tests/integration/system/nsf/upgrade_test.h"
 
 #include <memory>
+#include <string>
 #include <vector>
 
 #include "absl/flags/flag.h"
@@ -39,6 +40,12 @@
 
 ABSL_FLAG(pins_test::NsfMilestone, milestone, pins_test::NsfMilestone::kAll,
           "The NSF milestone to test.");
+ABSL_FLAG(bool, do_nsf_upgrade, false,
+          "Specifies whether to perform NSF Upgrade during the test or just an "
+          "NSF Reboot.");
+ABSL_FLAG(bool, include_traffic, false,
+          "Specifies whether to include traffic transmission and validations "
+          "during the test.");
 
 namespace pins_test {
 
@@ -62,8 +69,8 @@ void NsfUpgradeTest::TearDown() { TearDownTestbed(testbed_interface_); }
 
 // Assumption: Valid config (gNMI and P4Info) has been pushed (to avoid
 // duplicate config push)
-absl::Status NsfUpgradeTest::NsfUpgrade(absl::string_view prev_version,
-                                        absl::string_view version) {
+absl::Status NsfUpgradeTest::NsfUpgrade(const std::string& prev_version,
+                                        const std::string& version) {
   RETURN_IF_ERROR(ValidateSutState(prev_version, testbed_, *ssh_client_));
   RETURN_IF_ERROR(ValidateComponents(&ComponentValidator::OnInit,
                                      component_validators_, prev_version,
@@ -79,22 +86,27 @@ absl::Status NsfUpgradeTest::NsfUpgrade(absl::string_view prev_version,
                                      component_validators_, prev_version,
                                      testbed_));
 
-  RETURN_IF_ERROR(traffic_helper_->StartTraffic(testbed_));
-  RETURN_IF_ERROR(ValidateComponents(&ComponentValidator::OnStartTraffic,
-                                     component_validators_, prev_version,
-                                     testbed_));
+  if (absl::GetFlag(FLAGS_include_traffic)) {
+    RETURN_IF_ERROR(traffic_helper_->StartTraffic(testbed_));
+    RETURN_IF_ERROR(ValidateComponents(&ComponentValidator::OnStartTraffic,
+                                       component_validators_, prev_version,
+                                       testbed_));
+  }
 
 
   // P4 Snapshot before Upgrade and NSF reboot.
   ReadResponse p4flow_snapshot2 = TakeP4FlowSnapshot();
 
-  // Copy image to the switch for installation.
-  RETURN_IF_ERROR(Upgrade(version));
-  RETURN_IF_ERROR(ValidateComponents(&ComponentValidator::OnUpgrade,
-                                     component_validators_, version, testbed_));
+  if (absl::GetFlag(FLAGS_do_nsf_upgrade)) {
+    // Copy image to the switch for installation.
+    RETURN_IF_ERROR(ImageCopy(version, testbed_, *ssh_client_));
+    RETURN_IF_ERROR(ValidateComponents(&ComponentValidator::OnImageCopy,
+                                       component_validators_, version,
+                                       testbed_));
+  }
 
   // Perform NSF Reboot.
-  RETURN_IF_ERROR(NsfReboot(testbed_, *ssh_client_));
+  RETURN_IF_ERROR(NsfReboot(testbed_));
   RETURN_IF_ERROR(WaitForReboot(testbed_, *ssh_client_));
 
   // Perform validations after reboot is completed.
@@ -107,21 +119,26 @@ absl::Status NsfUpgradeTest::NsfUpgrade(absl::string_view prev_version,
   ReadResponse p4flow_snapshot3 = TakeP4FlowSnapshot();
 
   // Push the new config and validate.
-  RETURN_IF_ERROR(PushConfig(version));
+  RETURN_IF_ERROR(PushConfig(GetParam().gnmi_config, testbed_, *ssh_client_));
   RETURN_IF_ERROR(ValidateSutState(version, testbed_, *ssh_client_));
   RETURN_IF_ERROR(ValidateComponents(&ComponentValidator::OnConfigPush,
                                      component_validators_, version, testbed_));
 
-  // Wait for transmission duration.
-  LOG(INFO) << "Wait for " << kTrafficRunDuration << " for transmit completion";
-  absl::SleepFor(kTrafficRunDuration);
+  if (absl::GetFlag(FLAGS_include_traffic)) {
+    // Wait for transmission duration.
+    LOG(INFO) << "Wait for " << kTrafficRunDuration
+              << " for transmit completion";
+    absl::SleepFor(kTrafficRunDuration);
 
-  // Stop and validate traffic
-  RETURN_IF_ERROR(traffic_helper_->StopTraffic(testbed_));
+    // Stop and validate traffic
+    RETURN_IF_ERROR(traffic_helper_->StopTraffic(testbed_));
 
-  RETURN_IF_ERROR(traffic_helper_->ValidateTraffic(kErrorPercentage, testbed_));
-  RETURN_IF_ERROR(ValidateComponents(&ComponentValidator::OnStopTraffic,
-                                     component_validators_, version, testbed_));
+    RETURN_IF_ERROR(
+        traffic_helper_->ValidateTraffic(testbed_, kErrorPercentage));
+    RETURN_IF_ERROR(ValidateComponents(&ComponentValidator::OnStopTraffic,
+                                       component_validators_, version,
+                                       testbed_));
+  }
 
   // Selectively clear flows (eg. not clearing nexthop entries for host
   // testbeds).
@@ -132,26 +149,29 @@ absl::Status NsfUpgradeTest::NsfUpgrade(absl::string_view prev_version,
 
   ReadResponse p4flow_snapshot4 = TakeP4FlowSnapshot();
 
-  RETURN_IF_ERROR(CompareP4FlowSnapshots(p4flow_snapshot1, p4flow_snapshot4));
-  return CompareP4FlowSnapshots(p4flow_snapshot2, p4flow_snapshot3);
+  RETURN_IF_ERROR(CompareP4FlowSnapshots(p4flow_snapshot2, p4flow_snapshot3));
+  return CompareP4FlowSnapshots(p4flow_snapshot1, p4flow_snapshot4);
 }
 
 TEST_P(NsfUpgradeTest, UpgradeAndReboot) {
-  static constexpr absl::string_view kThirdLastImage = "third_last_image";
-  static constexpr absl::string_view kSecondLastImage = "second_last_image";
-  static constexpr absl::string_view kLastImage = "last_image";
-  static constexpr absl::string_view kCurrentImage = "current_image";
+  const std::string third_last_image = "third_last_image";
+  const std::string second_last_image = "second_last_image";
+  const std::string last_image = "last_image";
+  const std::string current_image = "current_image";
 
-  ASSERT_OK(InstallRebootPushConfig(kThirdLastImage));
+  ASSERT_OK(InstallRebootPushConfig(third_last_image, GetParam().gnmi_config,
+                                    testbed_, *ssh_client_));
 
-  // NSF Upgrade to N - 2 (once a week)
-  ASSERT_OK(NsfUpgrade(kThirdLastImage, kSecondLastImage));
+  if (absl::GetFlag(FLAGS_do_nsf_upgrade)) {
+    // NSF Upgrade to N - 2 (once a week)
+    ASSERT_OK(NsfUpgrade(third_last_image, second_last_image));
 
-  // NSF Upgrade to N - 1
-  ASSERT_OK(NsfUpgrade(kSecondLastImage, kLastImage));
+    // NSF Upgrade to N - 1
+    ASSERT_OK(NsfUpgrade(second_last_image, last_image));
+  }
 
   // NSF Upgrade to N
-  ASSERT_OK(NsfUpgrade(kLastImage, kCurrentImage));
+  ASSERT_OK(NsfUpgrade(last_image, current_image));
 }
 
 }  // namespace pins_test

tests/integration/system/nsf/upgrade_test.h

@@ -16,16 +16,16 @@
 #define PINS_TESTS_INTEGRATION_SYSTEM_NSF_UPGRADE_TEST_H_
 
 #include <memory>
+#include <string>
 #include <vector>
 
 #include "absl/status/status.h"
-#include "absl/strings/string_view.h"
 #include "gtest/gtest.h"
 #include "tests/integration/system/nsf/interfaces/component_validator.h"
 #include "tests/integration/system/nsf/interfaces/flow_programmer.h"
 #include "tests/integration/system/nsf/interfaces/test_params.h"
+#include "tests/integration/system/nsf/interfaces/testbed.h"
 #include "tests/integration/system/nsf/interfaces/traffic_helper.h"
-#include "tests/integration/system/nsf/util.h"
 #include "thinkit/ssh_client.h"
 
 namespace pins_test {
@@ -36,8 +36,8 @@ class NsfUpgradeTest : public testing::TestWithParam<NsfTestParams> {
   void TearDown() override;
 
   // Assumption: Valid config (gNMI and P4Info) has already been pushed.
-  absl::Status NsfUpgrade(absl::string_view prev_version,
-                          absl::string_view version);
+  absl::Status NsfUpgrade(const std::string& prev_version,
+                          const std::string& version);
 
   std::unique_ptr<FlowProgrammer> flow_programmer_;
   std::unique_ptr<TrafficHelper> traffic_helper_;