Revert "[RouteOrch] Record ROUTE_TABLE entry programming status to APPL_STATE_DB (sonic-net#2512)"

This reverts commit 35385ad.

Author: stepanblyschak

orchagent/routeorch.cpp

@@ -48,8 +48,6 @@ RouteOrch::RouteOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames,
 {
     SWSS_LOG_ENTER();
 
-    m_publisher.setBuffered(true);
-
     sai_attribute_t attr;
     attr.id = SAI_SWITCH_ATTR_NUMBER_OF_ECMP_GROUPS;
 
@@ -502,7 +500,7 @@ void RouteOrch::doTask(Consumer& consumer)
 
             auto rc = toBulk.emplace(std::piecewise_construct,
                     std::forward_as_tuple(key, op),
-                    std::forward_as_tuple(key, (op == SET_COMMAND)));
+                    std::forward_as_tuple());
 
             bool inserted = rc.second;
             auto& ctx = rc.first->second;
@@ -633,11 +631,6 @@ void RouteOrch::doTask(Consumer& consumer)
 
                     if (fvField(i) == "seg_src")
                         srv6_source = fvValue(i);
-
-                    if (fvField(i) == "protocol")
-                    {
-                        ctx.protocol = fvValue(i);
-                    }
                 }
 
                 /*
@@ -858,10 +851,6 @@ void RouteOrch::doTask(Consumer& consumer)
                     /* fullmask subnet route is same as ip2me route */
                     else if (ip_prefix.isFullMask() && m_intfsOrch->isPrefixSubnet(ip_prefix, alsv[0]))
                     {
-                        /* The prefix is full mask (/32 or /128) and it is an interface subnet route, so IntfOrch has already
-                         * created an IP2ME route for it and we skip programming such route here as it already exists.
-                         * However, to keep APPL_DB and APPL_STATE_DB consistent we have to publish it. */
-                        publishRouteState(ctx);
                         it = consumer.m_toSync.erase(it);
                     }
                     /* subnet route, vrf leaked route, etc */
@@ -891,9 +880,7 @@ void RouteOrch::doTask(Consumer& consumer)
                 }
                 else
                 {
-                    /* Duplicate entry. Publish route state anyway since there could be multiple DEL, SET operations
-                     * consolidated by ConsumerStateTable leading to orchagent receiving only the last SET update. */
-                    publishRouteState(ctx);
+                    /* Duplicate entry */
                     it = consumer.m_toSync.erase(it);
                 }
 
@@ -2321,9 +2308,6 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey
 
     notifyNextHopChangeObservers(vrf_id, ipPrefix, nextHops, true);
 
-    /* Publish and update APPL STATE DB route entry programming status */
-    publishRouteState(ctx);
-
     /*
      * If the route uses a temporary synced NHG owned by NhgOrch, return false
      * in order to keep trying to update the route in case the NHG is updated,
@@ -2538,9 +2522,6 @@ bool RouteOrch::removeRoutePost(const RouteBulkContext& ctx)
 
     SWSS_LOG_INFO("Remove route %s with next hop(s) %s",
             ipPrefix.to_string().c_str(), it_route->second.nhg_key.to_string().c_str());
-    
-    /* Publish removal status, removes route entry from APPL STATE DB */
-    publishRouteState(ctx);
 
     if (ipPrefix.isDefaultRoute() && vrf_id == gVirtualRouterId)
     {
@@ -2697,22 +2678,3 @@ void RouteOrch::decNhgRefCount(const std::string &nhg_index)
         gCbfNhgOrch->decNhgRefCount(nhg_index);
     }
 }
-
-void RouteOrch::publishRouteState(const RouteBulkContext& ctx, const ReturnCode& status)
-{
-    SWSS_LOG_ENTER();
-
-    std::vector<FieldValueTuple> fvs;
-
-    /* Leave the fvs empty if the operation type is "DEL".
-     * An empty fvs makes ResponsePublisher::publish() remove the state entry from APPL_STATE_DB
-     */
-    if (ctx.is_set)
-    {
-        fvs.emplace_back("protocol", ctx.protocol);
-    }
-
-    const bool replace = false;
-
-    m_publisher.publish(APP_ROUTE_TABLE_NAME, ctx.key, fvs, status, replace);
-}

orchagent/routeorch.h

@@ -122,12 +122,8 @@ struct RouteBulkContext
     // using_temp_nhg will track if the NhgOrch's owned NHG is temporary or not
     bool                                using_temp_nhg;
 
-    std::string                         key;       // Key in database table
-    std::string                         protocol;  // Protocol string
-    bool                                is_set;    // True if set operation
-
-    RouteBulkContext(const std::string& key, bool is_set)
-        : key(key), excp_intfs_flag(false), using_temp_nhg(false), is_set(is_set)
+    RouteBulkContext()
+        : excp_intfs_flag(false), using_temp_nhg(false)
     {
     }
 
@@ -143,8 +139,6 @@ struct RouteBulkContext
         excp_intfs_flag = false;
         vrf_id = SAI_NULL_OBJECT_ID;
         using_temp_nhg = false;
-        key.clear();
-        protocol.clear();
     }
 };
 
@@ -276,8 +270,6 @@ class RouteOrch : public Orch, public Subject
     const NhgBase &getNhg(const std::string& nhg_index);
     void incNhgRefCount(const std::string& nhg_index);
     void decNhgRefCount(const std::string& nhg_index);
-
-    void publishRouteState(const RouteBulkContext& ctx, const ReturnCode& status = ReturnCode(SAI_STATUS_SUCCESS));
 };
 
 #endif /* SWSS_ROUTEORCH_H */

tests/mock_tests/Makefile.am

@@ -191,7 +191,7 @@ tests_intfmgrd_INCLUDES = $(tests_INCLUDES) -I$(top_srcdir)/cfgmgr -I$(top_srcdi
 tests_intfmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI)
 tests_intfmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) $(tests_intfmgrd_INCLUDES)
 tests_intfmgrd_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis \
-        -lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lpthread -lgmock -lgmock_main
+        -lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lpthread
 
 ## teammgrd unit tests
 

tests/mock_tests/fake_response_publisher.cpp

@@ -2,36 +2,19 @@
 #include <vector>
 
 #include "response_publisher.h"
-#include "mock_response_publisher.h"
-
-/* This mock plugs into this fake response publisher implementation
- * when needed to test code that uses response publisher. */
-std::unique_ptr<MockResponsePublisher> gMockResponsePublisher;
 
 ResponsePublisher::ResponsePublisher(bool buffered) : m_db(std::make_unique<swss::DBConnector>("APPL_STATE_DB", 0)), m_buffered(buffered) {}
 
 void ResponsePublisher::publish(
     const std::string& table, const std::string& key,
     const std::vector<swss::FieldValueTuple>& intent_attrs,
     const ReturnCode& status,
-    const std::vector<swss::FieldValueTuple>& state_attrs, bool replace)
-{
-    if (gMockResponsePublisher)
-    {
-        gMockResponsePublisher->publish(table, key, intent_attrs, status, state_attrs, replace);
-    }
-}
+    const std::vector<swss::FieldValueTuple>& state_attrs, bool replace) {}
 
 void ResponsePublisher::publish(
     const std::string& table, const std::string& key,
     const std::vector<swss::FieldValueTuple>& intent_attrs,
-    const ReturnCode& status, bool replace)
-{
-    if (gMockResponsePublisher)
-    {
-        gMockResponsePublisher->publish(table, key, intent_attrs, status, replace);
-    }
-}
+    const ReturnCode& status, bool replace) {}
 
 void ResponsePublisher::writeToDB(
     const std::string& table, const std::string& key,

tests/mock_tests/routeorch_ut.cpp

@@ -7,14 +7,10 @@
 #include "ut_helper.h"
 #include "mock_orchagent_main.h"
 #include "mock_table.h"
-#include "mock_response_publisher.h"
 #include "bulker.h"
 
 extern string gMySwitchType;
 
-extern std::unique_ptr<MockResponsePublisher> gMockResponsePublisher;
-
-using ::testing::_;
 
 namespace routeorch_test
 {
@@ -286,10 +282,6 @@ namespace routeorch_test
                                          {"mac_addr", "00:00:00:00:00:00" }});
             intfTable.set("Ethernet0:10.0.0.1/24", { { "scope", "global" },
                                                      { "family", "IPv4" }});
-            intfTable.set("Ethernet4", { {"NULL", "NULL" },
-                                         {"mac_addr", "00:00:00:00:00:00" }});
-            intfTable.set("Ethernet4:11.0.0.1/32", { { "scope", "global" },
-                                                     { "family", "IPv4" }});
             gIntfsOrch->addExistingData(&intfTable);
             static_cast<Orch *>(gIntfsOrch)->doTask();
 
@@ -430,58 +422,4 @@ namespace routeorch_test
         ASSERT_EQ(current_set_count + 1, set_route_count);
         ASSERT_EQ(sai_fail_count, 0);
     }
-
-    TEST_F(RouteOrchTest, RouteOrchTestSetDelResponse)
-    {
-        gMockResponsePublisher = std::make_unique<MockResponsePublisher>();
-
-        std::deque<KeyOpFieldsValuesTuple> entries;
-        std::string key = "2.2.2.0/24";
-        std::vector<FieldValueTuple> fvs{{"ifname", "Ethernet0,Ethernet0"}, {"nexthop", "10.0.0.2,10.0.0.3"}, {"protocol", "bgp"}};
-        entries.push_back({key, "SET", fvs});
-
-        auto consumer = dynamic_cast<Consumer *>(gRouteOrch->getExecutor(APP_ROUTE_TABLE_NAME));
-        consumer->addToSync(entries);
-
-        EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector<FieldValueTuple>{{"protocol", "bgp"}}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1);
-        static_cast<Orch *>(gRouteOrch)->doTask();
-
-        // add entries again to the consumer queue (in case of rapid DEL/SET operations from fpmsyncd, routeorch just gets the last SET update)
-        consumer->addToSync(entries);
-
-        EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector<FieldValueTuple>{{"protocol", "bgp"}}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1);
-        static_cast<Orch *>(gRouteOrch)->doTask();
-
-        entries.clear();
-
-        // Route deletion
-
-        entries.clear();
-        entries.push_back({key, "DEL", {}});
-
-        consumer->addToSync(entries);
-
-        EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector<FieldValueTuple>{}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1);
-        static_cast<Orch *>(gRouteOrch)->doTask();
-
-        gMockResponsePublisher.reset();
-    }
-
-    TEST_F(RouteOrchTest, RouteOrchSetFullMaskSubnetPrefix)
-    {
-        gMockResponsePublisher = std::make_unique<MockResponsePublisher>();
-
-        std::deque<KeyOpFieldsValuesTuple> entries;
-        std::string key = "11.0.0.1/32";
-        std::vector<FieldValueTuple> fvs{{"ifname", "Ethernet4"}, {"nexthop", "0.0.0.0"}, {"protocol", "bgp"}};
-        entries.push_back({key, "SET", fvs});
-
-        auto consumer = dynamic_cast<Consumer *>(gRouteOrch->getExecutor(APP_ROUTE_TABLE_NAME));
-        consumer->addToSync(entries);
-
-        EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector<FieldValueTuple>{{"protocol", "bgp"}}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1);
-        static_cast<Orch *>(gRouteOrch)->doTask();
-
-        gMockResponsePublisher.reset();
-    }
 }