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

[eventd] Ignore control character event if it comes in UT #21200

Conversation

zbud-msft
Copy link
Contributor

@zbud-msft zbud-msft commented Dec 17, 2024

Why I did it

Fixes #21140

In #20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in #20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

Work item tracking
  • Microsoft ADO (number only):28728116

How I did it

Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it

Manual test/pipeline

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@zbud-msft zbud-msft requested a review from lguohan as a code owner December 17, 2024 23:37
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dgsudharsan
Copy link
Collaborator

dgsudharsan commented Dec 24, 2024

@lguohan @qiluo-msft Can we merge this PR. This will help us reduce instability in builds because of eventd UT failure

@zbud-msft
Copy link
Contributor Author

Ran ~25 image builds, unable to see failure a single time building eventd

@qiluo-msft qiluo-msft merged commit 943a4aa into sonic-net:master Dec 24, 2024
21 checks passed
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Dec 24, 2024
Why I did it
Fixes sonic-net#21140

In sonic-net#20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in sonic-net#20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #21275

mssonicbld pushed a commit that referenced this pull request Dec 25, 2024
Why I did it
Fixes #21140

In #20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in #20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
github-actions bot pushed a commit to bradh352/sonic-buildimage that referenced this pull request Jan 2, 2025
Why I did it
Fixes sonic-net#21140

In sonic-net#20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in sonic-net#20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
github-actions bot pushed a commit to bradh352/sonic-buildimage that referenced this pull request Jan 2, 2025
Why I did it
Fixes sonic-net#21140

In sonic-net#20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in sonic-net#20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
github-actions bot pushed a commit to bradh352/sonic-buildimage that referenced this pull request Jan 2, 2025
Why I did it
Fixes sonic-net#21140

In sonic-net#20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in sonic-net#20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
VladimirKuk pushed a commit to Marvell-switching/sonic-buildimage that referenced this pull request Jan 21, 2025
Why I did it
Fixes sonic-net#21140

In sonic-net#20024 and sonic-net/sonic-swss-common#906, we made the change that when a control character is read, zmq_message_read will return with rc 0, which will create an empty internal event.

As part of eventd design, empty structured events are dropped, which leads to control characters being a no-op which is the expected behavior.

In UT, we are still always expecting a control character to be the first message to be read by zmq which is not the case as described in sonic-net#20024. We are also not ignoring empty events that are read which is being done by eventd.

With this change, control characters are properly ignored if it does after the first test event.

How I did it
Ignore empty structured events and not expect first zmq_message_read to be control character.

How to verify it
Manual test/pipeline
stepanblyschak pushed a commit to stepanblyschak/sonic-buildimage that referenced this pull request Jan 27, 2025
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202411: #21539

dgsudharsan pushed a commit to dgsudharsan/sonic-buildimage that referenced this pull request Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[eventd] eventd unit test is not stable
6 participants