Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[orchagent]: Publish identified events via structured-events channel #2446

Merged
merged 18 commits into from
Sep 15, 2022

Conversation

renukamanavalan
Copy link
Contributor

What I did
Ref: sonic-net/SONiC#954
Three events identified are now published via events channel.

Why I did it
To publish events per HLD.

How I verified it
When the event occurs,

  1. A syslog line starting "EVENT_PUBLISHED:...." can be seen.
  2. You may run a gNMI query anbd/or events_tool to capture the events.

Details if related

@renukamanavalan
Copy link
Contributor Author

/azp run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

zbud-msft
zbud-msft previously approved these changes Sep 13, 2022
zbud-msft
zbud-msft previously approved these changes Sep 14, 2022
@@ -97,7 +100,9 @@ bool OrchDaemon::init()

string platform = getenv("platform") ? getenv("platform") : "";

gCrmOrch = new CrmOrch(m_configDb, CFG_CRM_TABLE_NAME);
m_events_handle = events_init_publisher("sonic-events-swss");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please have this as a global variable so that we don't have to pass for each orch and change the signature in all places.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to declare this as extern in orch.h or in the .cpp files that uses it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please have this as a global variable so that we don't have to pass for each orch and change the signature in all places.

This is the right approach. Thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extern in the .cpp file that uses it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -3,7 +3,7 @@

#include "crmorch.h"

CrmOrch::CrmOrch(swss::DBConnector *db, std::string tableName) : Orch(db, std::vector<std::string>{})
CrmOrch::CrmOrch(swss::DBConnector *db, std::string tableName, event_handle_t) : Orch(db, std::vector<std::string>{})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to revert this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -352,7 +353,8 @@ static bool isValidPortTypeForLagMember(const Port& port)
* bridge. By design, SONiC switch starts with all bridge ports removed from
* default VLAN and all ports removed from .1Q bridge.
*/
PortsOrch::PortsOrch(DBConnector *db, DBConnector *stateDb, vector<table_name_with_pri_t> &tableNames, DBConnector *chassisAppDb) :
PortsOrch::PortsOrch(DBConnector *db, DBConnector *stateDb, vector<table_name_with_pri_t> &tableNames,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please revert this change if its not modified?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@zbud-msft zbud-msft merged commit 7cc035f into sonic-net:master Sep 15, 2022
tjchadaga added a commit to tjchadaga/sonic-swss that referenced this pull request Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants