[Thinkit] Add a function to get all port ids from a GNMI Config.Chang…

…e OpenConfigInterfaceDescription to contain a string instead of a string_view.Add more context to interface state failure to specify which state it isn't in.Avoid making election ids to become primary unpredictable in testing.gNMI dpb tests.Fix wait for start response.No-Op (sonic-net#398) * Add a function to get all port ids from a GNMI Config.Change OpenConfigInterfaceDescription to contain a string instead of a string_view.Add more context to interface state failure to specify which state it isn't in.Avoid making election ids to become primary unpredictable in testing.gNMI dpb tests.Fix wait for start response.No-Op --------- Co-authored-by: Srikishen Pondicherry Shanmugam <[email protected]> Co-authored-by: smolkaj <[email protected]> Co-authored-by: jonathan-dilorenzo <[email protected]>
divyagayathri-hcl · Jul 29, 2024 · a2421cf · a2421cf
1 parent 878e64d
commit a2421cf
Show file tree

Hide file tree

Showing 7 changed files with 132 additions and 25 deletions.
diff --git a/lib/gnmi/BUILD.bazel b/lib/gnmi/BUILD.bazel
@@ -36,6 +36,7 @@ cc_library(
         "@com_github_google_glog//:glog",
         "@com_github_nlohmann_json//:nlohmann_json",
         "@com_github_p4lang_p4runtime//:p4runtime_cc_grpc",
+        "@com_google_absl//absl/container:btree",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/container:flat_hash_set",
         "@com_google_absl//absl/numeric:int128",

diff --git a/lib/gnmi/gnmi_helper.cc b/lib/gnmi/gnmi_helper.cc
@@ -19,6 +19,7 @@
 #include <utility>
 #include <vector>
 
+#include "absl/container/btree_set.h"
 #include "absl/container/flat_hash_map.h"
 #include "absl/container/flat_hash_set.h"
 #include "absl/numeric/int128.h"
@@ -69,8 +70,8 @@ const LazyRE2 kSplitBreakSquareBraceRE = {R"(([^\[\/]+(\[[^\]]+\])?)\/?)"};
 // `field_type` is the type of open config data this function should parse (e.g.
 // "config" or "state").
 absl::StatusOr<absl::flat_hash_map<std::string, std::string>>
-GetPortNameToIdMapFromJsonString(const std::string& json_string,
-                                 const std::string& field_type) {
+GetPortNameToIdMapFromJsonString(absl::string_view json_string,
+                                 absl::string_view field_type) {
   VLOG(2) << "Getting Port Name -> ID Map from JSON string: " << json_string;
   const nlohmann::basic_json<> response_json = json::parse(json_string);
 
@@ -500,8 +501,8 @@ absl::Status CheckInterfaceOperStateOverGnmi(
 
   if (!unavailable_interfaces.empty()) {
     return absl::UnavailableError(absl::StrCat(
-        "Some interfaces are not in the expected state: \n",
-        absl::StrJoin(unavailable_interfaces, "\n"),
+        "Some interfaces are not in the expected state ", interface_oper_state,
+        ": \n", absl::StrJoin(unavailable_interfaces, "\n"),
         "\n\nInterfaces provided: \n", absl::StrJoin(interfaces, "\n")));
   }
   return absl::OkStatus();
@@ -521,6 +522,9 @@ absl::Status CheckAllInterfaceOperStateOverGnmi(
       continue;
     }
     if (oper_status != interface_oper_state) {
+      LOG(INFO) << "Interface "
+                << interface << " not found in interfaces that are "
+                << interface_oper_state;
       unavailable_interfaces.push_back(interface);
     }
   }
@@ -642,6 +646,11 @@ absl::StatusOr<OperStatus> GetInterfaceOperStatusOverGnmi(
   return OperStatus::kUnknown;
 }
 
+absl::StatusOr<absl::flat_hash_map<std::string, std::string>>
+GetAllInterfaceNameToPortId(absl::string_view gnmi_config) {
+  return GetPortNameToIdMapFromJsonString(gnmi_config, /*field_type=*/"config");
+}
+
 absl::StatusOr<absl::flat_hash_map<std::string, std::string>>
 GetAllInterfaceNameToPortId(gnmi::gNMI::StubInterface& stub) {
   ASSIGN_OR_RETURN(gnmi::GetResponse response,
@@ -655,6 +664,30 @@ GetAllInterfaceNameToPortId(gnmi::gNMI::StubInterface& stub) {
       /*field_type=*/"state");
 }
 
+absl::StatusOr<absl::btree_set<std::string>> GetAllPortIds(
+    absl::string_view gnmi_config) {
+  ASSIGN_OR_RETURN(auto interface_name_to_port_id,
+                   GetAllInterfaceNameToPortId(gnmi_config));
+
+  absl::btree_set<std::string> port_ids;
+  for (const auto& [_, port_id] : interface_name_to_port_id) {
+    port_ids.insert(port_id);
+  }
+  return port_ids;
+}
+
+absl::StatusOr<absl::btree_set<std::string>> GetAllPortIds(
+    gnmi::gNMI::StubInterface& stub) {
+  ASSIGN_OR_RETURN(auto interface_name_to_port_id,
+                   GetAllInterfaceNameToPortId(stub));
+
+  absl::btree_set<std::string> port_ids;
+  for (const auto& [_, port_id] : interface_name_to_port_id) {
+    port_ids.insert(port_id);
+  }
+  return port_ids;
+}
+
 absl::StatusOr<std::vector<std::string>> ParseAlarms(
     const std::string& alarms_json) {
   auto alarms_array = json::parse(alarms_json);

diff --git a/lib/gnmi/gnmi_helper.h b/lib/gnmi/gnmi_helper.h
@@ -19,6 +19,7 @@
 #include <type_traits>
 #include <vector>
 
+#include "absl/container/btree_set.h"
 #include "absl/container/flat_hash_map.h"
 #include "absl/numeric/int128.h"
 #include "absl/status/status.h"
@@ -54,7 +55,7 @@ enum class GnmiFieldType {
 
 // Describes a single interface in a gNMI config.
 struct OpenConfigInterfaceDescription {
-  absl::string_view port_name;
+  std::string port_name;
   int port_id;
 };
 
@@ -180,10 +181,22 @@ absl::StatusOr<std::vector<std::string>> GetUpInterfacesOverGnmi(
 absl::StatusOr<OperStatus> GetInterfaceOperStatusOverGnmi(
     gnmi::gNMI::StubInterface& stub, absl::string_view if_name);
 
-// Gets the interface name to port id map.
+// Returns the interface name to port id map from a gNMI config.
+absl::StatusOr<absl::flat_hash_map<std::string, std::string>>
+GetAllInterfaceNameToPortId(absl::string_view gnmi_config);
+
+// Reads the gNMI state and returns the interface name to port id map.
 absl::StatusOr<absl::flat_hash_map<std::string, std::string>>
 GetAllInterfaceNameToPortId(gnmi::gNMI::StubInterface& stub);
 
+// Returns the ordered set of all port ids mapped by the given gNMI config.
+absl::StatusOr<absl::btree_set<std::string>> GetAllPortIds(
+    absl::string_view gnmi_config);
+
+// Reads the gNMI state and returns the ordered set of all port ids mapped.
+absl::StatusOr<absl::btree_set<std::string>> GetAllPortIds(
+    gnmi::gNMI::StubInterface& stub);
+
 // Gets all system process id over gNMI.
 absl::StatusOr<gnmi::GetResponse> GetAllSystemProcesses(
     gnmi::gNMI::StubInterface& gnmi_stub);
@@ -203,6 +216,5 @@ absl::StatusOr<std::vector<std::string>> GetAlarms(
 
 // Strips the beginning and ending double-quotes from the `string`.
 absl::string_view StripQuotes(absl::string_view string);
-
 }  // namespace pins_test
 #endif  // PINS_LIB_GNMI_GNMI_HELPER_H_
diff --git a/lib/gnmi/gnmi_helper_test.cc b/lib/gnmi/gnmi_helper_test.cc
@@ -17,6 +17,7 @@
 #include <string>
 #include <tuple>
 #include <type_traits>
+#include <vector>
 
 #include "absl/status/status.h"
 #include "absl/strings/escaping.h"
@@ -44,11 +45,13 @@ using ::gutil::IsOkAndHolds;
 using ::gutil::StatusIs;
 using ::testing::_;
 using ::testing::DoAll;
+using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::HasSubstr;
 using ::testing::IsEmpty;
 using ::testing::Return;
 using ::testing::SetArgPointee;
+using ::testing::StrEq;
 using ::testing::UnorderedElementsAre;
 using ::testing::UnorderedPointwise;
 
@@ -191,6 +194,23 @@ TEST(ParseAlarms, InvalidInput) {
               StatusIs(absl::StatusCode::kInvalidArgument));
 }
 
+// Constructs a standard gNMI response using the `oc_path` to construct a
+// path and the `gnmi_config` as the response value.
+gnmi::GetResponse ConstructResponse(absl::string_view oc_path,
+                                    absl::string_view gnmi_config) {
+  std::string response = absl::Substitute(
+      R"pb(notification {
+             timestamp: 1620348032128305716
+             prefix { origin: "openconfig" }
+             update {
+               path { $0 }
+               val { json_ietf_val: "$1" }
+             }
+           })pb",
+      ConvertOCStringToPath(oc_path).DebugString(), absl::CEscape(gnmi_config));
+  return gutil::ParseProtoOrDie<gnmi::GetResponse>(response);
+}
+
 TEST(GetAlarms, FailedRPCReturnsError) {
   gnmi::MockgNMIStub stub;
   EXPECT_CALL(stub, Get(_,
@@ -591,21 +611,15 @@ TEST(ConstructedOpenConfig, CreatesACorrectConfig) {
             nlohmann::json::parse(reference_config_with_two_interfaces));
 }
 
-TEST(GetInterfacePortIdMap, SuccessfullyReturnsInterfacePortIdMap) {
+TEST(GetInterfacePortIdMap, StubSuccessfullyReturnsInterfacePortIdMap) {
   gnmi::MockgNMIStub stub;
-  EXPECT_CALL(stub, Get).WillOnce(DoAll(
-      SetArgPointee<2>(gutil::ParseProtoOrDie<gnmi::GetResponse>(
-          R"pb(notification {
-                 timestamp: 1620348032128305716
-                 prefix { origin: "openconfig" }
-                 update {
-                   path { elem { name: "interfaces" } }
-                   val {
-                     json_ietf_val: "{\"openconfig-interfaces:interfaces\":{\"interface\":[{\"name\":\"Ethernet0\",\"state\":{\"openconfig-p4rt:id\":1}}]}}"
-                   }
-                 }
-               })pb")),
-      Return(grpc::Status::OK)));
+  EXPECT_CALL(stub, Get).WillOnce(
+      DoAll(SetArgPointee<2>(ConstructResponse(
+                /*oc_path=*/"interfaces",
+                /*gnmi_config=*/OpenConfigWithInterfaces(
+                    GnmiFieldType::kState,
+                    {{.port_name = "Ethernet0", .port_id = 1}}))),
+            Return(grpc::Status::OK)));
 
   auto interface_name_to_port_id = GetAllInterfaceNameToPortId(stub);
   ASSERT_OK(interface_name_to_port_id);
@@ -615,6 +629,44 @@ TEST(GetInterfacePortIdMap, SuccessfullyReturnsInterfacePortIdMap) {
               UnorderedPointwise(Eq(), expected_map));
 }
 
+TEST(GetInterfacePortIdMap, ConfigSuccessfullyReturnsInterfacePortIdMap) {
+  auto interface_name_to_port_id =
+      GetAllInterfaceNameToPortId(/*gnmi_config=*/OpenConfigWithInterfaces(
+          GnmiFieldType::kConfig, {{.port_name = "Ethernet0", .port_id = 1}}));
+
+  ASSERT_OK(interface_name_to_port_id);
+  const absl::flat_hash_map<std::string, std::string> expected_map = {
+      {"Ethernet0", "1"}};
+  EXPECT_THAT(*interface_name_to_port_id,
+              UnorderedPointwise(Eq(), expected_map));
+}
+
+TEST(GetAllPortIds, StubSuccessfullyReturnsPortIds) {
+  gnmi::MockgNMIStub stub;
+  EXPECT_CALL(stub, Get).WillOnce(
+      DoAll(SetArgPointee<2>(ConstructResponse(
+                /*oc_path=*/"interfaces",
+                /*gnmi_config=*/OpenConfigWithInterfaces(
+                    GnmiFieldType::kState,
+                    {{.port_name = "Ethernet0", .port_id = 1}}))),
+            Return(grpc::Status::OK)));
+
+  ASSERT_OK_AND_ASSIGN(auto port_ids, GetAllPortIds(stub));
+  EXPECT_THAT(port_ids, ElementsAre("1"));
+}
+
+TEST(GetAllPortIds, ConfigSuccessfullyReturnsPortIds) {
+  ASSERT_OK_AND_ASSIGN(
+      auto port_ids,
+      GetAllPortIds(/*gnmi_config=*/OpenConfigWithInterfaces(
+          GnmiFieldType::kConfig, {
+                                      {.port_name = "Ethernet1", .port_id = 2},
+                                      {.port_name = "Ethernet0", .port_id = 1},
+                                  })));
+
+  EXPECT_THAT(port_ids, ElementsAre("1", "2"));
+}
+
 TEST(GetInterfacePortIdMap, PortIdNotFoundInState) {
   gnmi::MockgNMIStub stub;
   EXPECT_CALL(stub, Get).WillOnce(DoAll(
@@ -1311,6 +1363,5 @@ TEST(WaitForGnmiPortIdConvergenceTest, ConfigDoesNotHaveAResponse) {
           absl::Seconds(1)),
       StatusIs(absl::StatusCode::kInternal));
 }
-
 }  // namespace
 }  // namespace pins_test
diff --git a/lib/ixia_helper.cc b/lib/ixia_helper.cc
@@ -386,7 +386,7 @@ absl::Status StartTraffic(absl::Span<const std::string> trefs,
   ASSIGN_OR_RETURN(thinkit::HttpResponse start_response,
                    generic_testbed.SendRestRequestToIxia(
                        thinkit::RequestType::kPost, start_path, start_json));
-  RETURN_IF_ERROR(WaitForComplete(generate_response, generic_testbed));
+  RETURN_IF_ERROR(WaitForComplete(start_response, generic_testbed));
 
   // GET to /ixnetwork/traffic/trafficItem
   std::string titem_path = "/ixnetwork/traffic/trafficItem";

diff --git a/lib/validator/validator_lib.cc b/lib/validator/validator_lib.cc
@@ -91,7 +91,13 @@ absl::Status SSHable(thinkit::Switch& thinkit_switch,
 
 // Checks if a P4Runtime session could be established.
 absl::Status P4rtAble(thinkit::Switch& thinkit_switch) {
-  return pdpi::P4RuntimeSession::Create(thinkit_switch).status();
+  // Uses a metadata with minimum election id to connect as backup and not
+  // change election id necessary to connect as primary in the future.
+  return pdpi::P4RuntimeSession::Create(
+             thinkit_switch,
+             pdpi::P4RuntimeSessionOptionalArgs{.election_id = 0},
+             /*error_if_not_primary=*/false)
+      .status();
 }
 
 // Checks if a gNMI get all interface request can be sent and a response
@@ -157,12 +163,15 @@ absl::Status SwitchReady(thinkit::Switch& thinkit_switch,
 absl::Status SwitchReadyWithSsh(thinkit::Switch& thinkit_switch,
                                 thinkit::SSHClient& ssh_client,
                                 absl::Span<const std::string> interfaces,
+                                bool check_interfaces_state,
                                 absl::Duration timeout) {
   RETURN_IF_ERROR(Pingable(thinkit_switch));
   RETURN_IF_ERROR(SSHable(thinkit_switch, ssh_client));
   RETURN_IF_ERROR(P4rtAble(thinkit_switch));
   RETURN_IF_ERROR(GnmiAble(thinkit_switch));
-  RETURN_IF_ERROR(PortsUp(thinkit_switch, interfaces));
+  if (check_interfaces_state) {
+    RETURN_IF_ERROR(PortsUp(thinkit_switch, interfaces));
+  }
   RETURN_IF_ERROR(GnoiAble(thinkit_switch));
   return NoAlarms(thinkit_switch);
 }

diff --git a/lib/validator/validator_lib.h b/lib/validator/validator_lib.h
@@ -73,6 +73,7 @@ absl::Status SwitchReady(thinkit::Switch& thinkit_switch,
 absl::Status SwitchReadyWithSsh(thinkit::Switch& thinkit_switch,
                                 thinkit::SSHClient& ssh_client,
                                 absl::Span<const std::string> interfaces = {},
+                                bool check_interfaces_state = true,
                                 absl::Duration timeout = kDefaultTimeout);
 
 }  // namespace pins_test