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.

Sign up for GitHub

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

Jump to bottom

Fix rsyslog plugin slim image logic #19034

Merged
merged 3 commits into from
Jun 19, 2024
Merged

Fix rsyslog plugin slim image logic #19034

merged 3 commits into from
Jun 19, 2024

Conversation

zbud-msft
Copy link
Contributor

Why I did it

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

How I did it

Added checks after redirect to frr/bgpd.log

How to verify it

Manual test

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 May 22, 2024 00:04
@lguohan
Copy link
Collaborator

lguohan commented May 24, 2024

@qiluo-msft , can you review?

saiarcot895
saiarcot895 reviewed May 29, 2024
View reviewed changes
files/image_config/rsyslog/rsyslog.d/00-sonic.conf.j2 Outdated
if re_match($programname, "bgp[0-9]*#(frr|zebra|staticd|watchfrr)") then {
/var/log/frr/zebra.log
{% if include_system_eventd == "y" %}
{% if build_reduce_image_size != "y" or sonic_asic_platform != "broadcom" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have a ASIC check for Broadcom here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

eventd should not be enabled on 7050qx and 7060cx3, so we check flags build_reduce_image_size for slim image enabled and also for broadcom asic.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why single flag is not enough? for example build_reduce_image_size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initial requirement was for disabling eventd in 7050qx/7060 which is why I added a tighter check.

@zbud-msft zbud-msft requested a review from qiluo-msft May 29, 2024 23:12
@zbud-msft zbud-msft marked this pull request as draft June 5, 2024 18:16
@zbud-msft zbud-msft marked this pull request as ready for review June 7, 2024 20:56
@zbud-msft
Copy link
Contributor Author

/azpw ms_conflict

@StormLiangMS StormLiangMS merged commit 0ffdf4e into sonic-net:master Jun 19, 2024
20 checks passed
@StormLiangMS
Copy link
Contributor

@yxieca for 202311 cherry pick, this one would be nice to have for next image build.

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Jun 20, 2024
@zbud-msft @mssonicbld
Fix rsyslog plugin slim image logic (sonic-net#19034)
5ca57a0 
* Fix rsyslog plugin slim image logic

* Remove asic check for broadcom when checking for eventd enabled and slim image
@mssonicbld mssonicbld mentioned this pull request Jun 20, 2024
Merged
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #19363

@TafkaMax
Copy link

TafkaMax commented Jun 21, 2024 •
edited
Loading

I am building master branch for broadcom and I got this error:

It seems to be related? eventd is the codeword here.

Assertion failed: _term_acks > 0 (src/own.cpp:180)
make[2]: *** [Makefile:60: eventd-tests] Aborted (core dumped)
make[2]: *** Waiting for unfinished jobs....
Finished building: rsyslog_plugin_tests/rsyslog_plugin_ut.cpp
 
make[2]: Leaving directory '/sonic/src/sonic-eventd'
dh_auto_build: error: make -j8 "INSTALL=install --strip-program=true" returned exit code 2
make[1]: *** [debian/rules:6: build] Error 25
make[1]: Leaving directory '/sonic/src/sonic-eventd'
dpkg-buildpackage: error: debian/rules build subprocess returned exit status 2
[  FAIL LOG END  ] [ target/debs/bullseye/sonic-eventd_1.0.0-0_amd64.deb ]
make: *** [slave.mk:775: target/debs/bullseye/sonic-eventd_1.0.0-0_amd64.deb] Error 1
make: *** Waiting for unfinished jobs....
[ finished ] [ target/python-wheels/bullseye/sonic_py_common-1.0-py3-none-any.whl ] 
[ finished ] [ target/debs/bullseye/libsairedis_1.0.0_amd64.deb ] 
make[1]: *** [Makefile.work:600: bullseye] Error 2
make[1]: Leaving directory '/home/gitlab-runner/builds/EzyZQ4s5/0/sysadm/sonic-builder/sonic-buildimage'
make: *** [Makefile:46: target/sonic-broadcom.bin] Error 2

EDIT: Building from commit 44ae9e7 to see if it fixes it.

@zbud-msft
Copy link
Contributor Author

@TafkaMax This change should not affect build of sonic-eventd deb package.

@TafkaMax
Copy link

@TafkaMax This change should not affect build of sonic-eventd deb package.

Yes, it builds now, must have been something else. Sorry for bothering.

@mssonicbld
Copy link
Collaborator

@zbud-msft cherry pick PR didn't pass PR checker. Please check!!!
#19363

yxieca pushed a commit that referenced this pull request Jun 24, 2024
@mssonicbld @zbud-msft
Fix rsyslog plugin slim image logic (#19034) (#19363)
a1215f4 
* Fix rsyslog plugin slim image logic

* Remove asic check for broadcom when checking for eventd enabled and slim image

Co-authored-by: Zain Budhwani <[email protected]>
arun1355492 pushed a commit to arun1355492/sonic-buildimage that referenced this pull request Jul 26, 2024
@zbud-msft @arun1355492
Fix rsyslog plugin slim image logic (sonic-net#19034)
f66cd5d 
* Fix rsyslog plugin slim image logic

* Remove asic check for broadcom when checking for eventd enabled and slim image
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Aug 23, 2024
@zbud-msft @mssonicbld
Fix rsyslog plugin slim image logic (sonic-net#19034)
6518488 
* Fix rsyslog plugin slim image logic

* Remove asic check for broadcom when checking for eventd enabled and slim image
@mssonicbld mssonicbld mentioned this pull request Aug 23, 2024
Merged
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #20009

mssonicbld pushed a commit that referenced this pull request Aug 26, 2024
@zbud-msft @mssonicbld
Fix rsyslog plugin slim image logic (#19034)
754fe20 
* Fix rsyslog plugin slim image logic

* Remove asic check for broadcom when checking for eventd enabled and slim image
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saiarcot895 saiarcot895 saiarcot895 left review comments

@qiluo-msft qiluo-msft qiluo-msft approved these changes

@lguohan lguohan Awaiting requested review from lguohan lguohan is a code owner

@StormLiangMS StormLiangMS Awaiting requested review from StormLiangMS

Assignees
No one assigned
Labels
Approved for 202311 Branch Approved for 202405 Branch Included in 202311 Branch Included in 202405 Branch Request for 202311 Branch Request for 202405 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@zbud-msft @lguohan @StormLiangMS @mssonicbld @TafkaMax @saiarcot895 @qiluo-msft @yxieca @bingwang-ms