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

Add bmp container init workflow. #18946

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

FengPan-Frank
Copy link
Contributor

Why I did it

Add bmp container init workflow.

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

How I did it

Add bmp container which is disabled by INCLUDE_BMP = n for now, which would cause container not build by default. This will be enabled as last step.

Add INCLUDE_BMP to indicate whether to build bmp container
Add docker file for bmp, as well as agent used openbmpd/bmpcfgd.
Add template file for bmp container services.
Add entry for bmp to FEATURE table in config_db.

How to verify it

Build image with INCLUDE_BMP = y to verify, Image should be install correctly. By config feature state bmp enabled to enable.

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)

@FengPan-Frank
Copy link
Contributor Author

/azp run Azure.sonic-buildimage

Copy link

Commenter does not have sufficient privileges for PR 18946 in repo sonic-net/sonic-buildimage

@FengPan-Frank
Copy link
Contributor Author

/azpw ms_conflict

> /etc/rsyslog.conf

# Make the script that waits for all interfaces to come up executable
chmod +x /usr/bin/start.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could ensure +x during build time. Do we really need it in runtime?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -152,6 +152,9 @@ INCLUDE_DHCP_RELAY = y
# INCLUDE_DHCP_SERVER - build and install dhcp-server package
INCLUDE_DHCP_SERVER ?= n

# INCLUDE_BMP - build and install sonic-bmp package
Copy link
Collaborator

Choose a reason for hiding this comment

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

build

Build time, we should "include".
Runtime, you can decide by default "enable" or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I will turn this on as last step after bmp/bmpcfgd PR are merged.

@lguohan lguohan added the BMP Bgp Monitor Container label May 18, 2024
@forrestchu
Copy link

Why I did it

Add bmp container init workflow.

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

How I did it

Add bmp container which is disabled by INCLUDE_BMP = n for now, which would cause container not build by default. This will be enabled as last step.

Add INCLUDE_BMP to indicate whether to build bmp container Add docker file for bmp, as well as agent used openbmpd/bmpcfgd. Add template file for bmp container services. Add entry for bmp to FEATURE table in config_db.

How to verify it

Build image with INCLUDE_BMP = y to verify, Image should be install correctly. By config feature state bmp enabled to enable.

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)

Hi Frank, which TCP port will this bmp server use by default? Is it configurable from config_db or CLI?

@@ -60,6 +60,7 @@
{%- if include_sflow == "y" %}{% do features.append(("sflow", "disabled", true, "enabled")) %}{% endif %}
{%- if include_macsec == "y" %}{% do features.append(("macsec", "{% if 'type' in DEVICE_METADATA['localhost'] and DEVICE_METADATA['localhost']['type'] == 'SpineRouter' and DEVICE_RUNTIME_METADATA['MACSEC_SUPPORTED'] %}enabled{% else %}disabled{% endif %}", false, "enabled")) %}{% endif %}
{%- if include_system_gnmi == "y" %}{% do features.append(("gnmi", "enabled", true, "enabled")) %}{% endif %}
{%- if include_bmp == "y" %}{% do features.append(("bmp", "enabled", true, "enabled")) %}{% endif %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should not change init_cfg.json.j2, in order to enable by default in image. This enablement is controlled by golden config generation service outside sonic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BMP Bgp Monitor Container
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants