-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Let bgp docker generate configuration by itself #173
Conversation
@@ -3,11 +3,15 @@ FROM docker-base | |||
COPY deps/quagga_*.deb /deps/ | |||
RUN dpkg_apt() { [ -f $1 ] && { dpkg -i $1 || apt-get -y install -f; } || return 1; } && \ | |||
dpkg_apt /deps/quagga_*.deb && \ | |||
apt-get update && \ | |||
apt-get -y install -f python-jinja2 python-netaddr python-ipaddr python-lxml && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move them to docker base as all the docker are going to use them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
! =========== Managed by Ansible DO NOT EDIT! ======================== | ||
! generated by templates/quagga/bgpd.conf.j2 using minigraph_facts.py | ||
! file: bgpd.conf | ||
! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not true
1aad0a3
to
fe497a9
Compare
apt-get clean -y && apt-get autoclean -y && apt-get autoremove -y && \ | ||
rm -rf /deps | ||
|
||
COPY daemons /etc/quagga/ | ||
COPY ["*.j2", "config.sh", "template_list", "*.py", "/etc/swss/"] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we embed the mountpoint here, e.g.,
VOLUME ['/etc/sonic']
Then, in the base image, we can put minigraph in the /etc/sonic/,
also, the VOLUME is in baseimage as it is shared by all dockers.
@@ -0,0 +1,432 @@ | |||
#!/usr/bin/env python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some concerns about
- duplicate the minigraph parsers in every containers.
- runtime overhead to generate config files on each bootup
The requirement is as:
- there is no change of minigraph.xml after deployment
- so, it is possible to create config files on host, copy to docker containers after they are just created. only the first bootup will be slower (if possible, we can run this step before reboot)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For 1, we are considering move those config-related scripts into docker-base after they are stabilized.
For 2, Guohan has some concern on potential changes of config file schema update container version update, thus we want to keep the capability of generating config file in the container itself.
Maybe Guohan could give more comments on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for 1, we can move it do the docker-base and stablized there.
for 2, configuration generate should be part of the docker container's job. For example, if we switch quagga to gobgp, the configuration template for them is going to be very different, where should be store the config template? I feel should be stored in the container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minigraph_facts.py could be a python library that could be used by other config generators
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree the configuration generation code should be inside each container.
apt-get clean -y && apt-get autoclean -y && apt-get autoremove -y && \ | ||
rm -rf /deps | ||
|
||
COPY daemons /etc/quagga/ | ||
COPY ["*.j2", "config.sh", "template_list", "*.py", "/etc/swss/"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
etc [](start = 54, length = 3)
/etc is not a good place for scripts.
data = parse_xml(minigraph) | ||
|
||
env = jinja2.Environment(loader=jinja2.FileSystemLoader('./'), trim_blocks=True) | ||
env.filters['ipv4'] = is_ipv4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filters [](start = 8, length = 7)
How many kinds of filters will we use? If many, we should consider implementation and maintenance cost. One option is to reuse ansible implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ansible implementation is of GPLv3. There might be copyright concerns.
|
||
cd /etc/swss | ||
|
||
awk '{system("python render_config.py -m /etc/swss/minigraph.xml "$1 ">"$2" && chown "$3" "$2" && chmod "$4" "$2)}' template_list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this command too complex to read?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree, use variable name would make it more readable.
bgpd.conf.j2 /etc/quagga/bgpd.conf quagga:quaggavty 0644 | ||
zebra.conf.j2 /etc/quagga/zebra.conf root:root 0644 | ||
isolate.j2 /usr/sbin/bgp-isolate root:root 0755 | ||
unisolate.j2 /usr/sbin/bgp-unisolate root:root 0755 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nisolate.j2 [](start = 1, length = 11)
suggest implement this feature as:
- make fake config files in Dockerfile, set the correct ownership, groupship and mode. I guess the vanilla ones are already good.
- copy file and maintain the target file attribute. ref: http://serverfault.com/questions/273949/cp-not-want-to-overwrite-permissions/273956
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As comments.
@@ -3,11 +3,15 @@ FROM docker-base | |||
COPY deps/quagga_*.deb /deps/ | |||
RUN dpkg_apt() { [ -f $1 ] && { dpkg -i $1 || apt-get -y install -f; } || return 1; } && \ | |||
dpkg_apt /deps/quagga_*.deb && \ | |||
apt-get update && \ | |||
apt-get -y install -f python-jinja2 python-netaddr python-ipaddr python-lxml && \ | |||
apt-get clean -y && apt-get autoclean -y && apt-get autoremove -y && \ | |||
rm -rf /deps | |||
|
|||
COPY daemons /etc/quagga/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
daemons [](start = 5, length = 7)
where is this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's already there. somebody add it in a previous version.
and, it's actually not a template
No test results found. |
1 similar comment
No test results found. |
Signed-off-by: Shuotian Cheng <[email protected]>
…-temp-access [platform/questone2bd] Remove SPD temperature sensors from platform
[marvell-armhf] Armada A385 soc support sonic-net#176 backport macsec xpn support to 4.19 kernel (sonic-net#174) add internal patch marker (sonic-net#173) Improve netfilter fullclone nat support patch sonic-net#171 Signed-off-by: Sabareesh Kumar Anandan <[email protected]>
[marvell-armhf] Armada A385 soc support #176 backport macsec xpn support to 4.19 kernel (#174) add internal patch marker (#173) Improve netfilter fullclone nat support patch #171 Signed-off-by: Sabareesh Kumar Anandan <[email protected]>
this PR updates the following commits 1673d25 [y_cable] refactor upgrade firmware API's; Fix vendor and part number API's read size for read_eeprom (sonic-net#174) ed93a15 [sonic_platform_base] Proper use of class and instance attributes (sonic-net#173) 691de92 [sonic_y_cable] add stub function for upgrade firmware of Y cable and split the get_part_number and get_vendor API's (sonic-net#171) Signed-off-by: vaibhav-dahiya <[email protected]>
Includes the following commits: 1673d25 [y_cable] refactor upgrade firmware API's; Fix vendor and part number API's read size for read_eeprom (#174) ed93a15 [sonic_platform_base] Proper use of class and instance attributes (#173) 691de92 [sonic_y_cable] add stub function for upgrade firmware of Y cable and split the get_part_number and get_vendor API's (#171) Signed-off-by: vaibhav-dahiya <[email protected]>
Includes the following commits: 1673d25 [y_cable] refactor upgrade firmware API's; Fix vendor and part number API's read size for read_eeprom (#174) ed93a15 [sonic_platform_base] Proper use of class and instance attributes (#173) 691de92 [sonic_y_cable] add stub function for upgrade firmware of Y cable and split the get_part_number and get_vendor API's (#171) Signed-off-by: vaibhav-dahiya <[email protected]>
[muxcable] Fix Redis Selectable Processing (sonic-net#173) Signed-off-by: vaibhav-dahiya <[email protected]>
[muxcable] Fix Redis Selectable Processing (sonic-net#173) Signed-off-by: vaibhav-dahiya <[email protected]>
Includes the following commits: 1673d25 [y_cable] refactor upgrade firmware API's; Fix vendor and part number API's read size for read_eeprom (sonic-net#174) ed93a15 [sonic_platform_base] Proper use of class and instance attributes (sonic-net#173) 691de92 [sonic_y_cable] add stub function for upgrade firmware of Y cable and split the get_part_number and get_vendor API's (sonic-net#171) Signed-off-by: vaibhav-dahiya <[email protected]>
[muxcable] Fix Redis Selectable Processing (sonic-net#173) Signed-off-by: vaibhav-dahiya <[email protected]>
Selectable object can return multiple fields, however current code processes only the first field and wait till the next selectable is triggered. This PR consumes all fields that were returned by a selectable object. This results in reducing processing time from order of seconds to order of milliseconds. signed-off-by: Tamer Ahmed <[email protected]>
Memory Histogram HLD
linkmgrd: * d2227d8 2023-02-22 | [active-standby] Toggle to standby if link down and config auto (sonic-net#173) (HEAD -> 202205) [Longxiang Lyu] Signed-off-by: Ying Xie <[email protected]>
linkmgrd: * d2227d8 2023-02-22 | [active-standby] Toggle to standby if link down and config auto (#173) (HEAD -> 202205) [Longxiang Lyu] Signed-off-by: Ying Xie <[email protected]>
Include commit: ``` 6ea1f03 Jing Zhang Tue Mar 28 08:42:44 2023 -0700 [202012] remove chatty log message for peer link event (#192) 198292d Jing Zhang Tue Mar 21 17:53:11 2023 -0700 [active-standby] avoid unnecessary mux state probe after configuring to `auto` (#183) 47de88e Jing Zhang Mon Mar 20 18:14:25 2023 -0700 [202012] Avoid unnecessary error logs from `handleGetServerMacAddressNotification` #96 (#185) 8a33319 Jing Zhang Mon Mar 6 11:53:27 2023 -0800 loose link down swithcover condition (#178) c2bf08d Jing Zhang Thu Mar 16 18:59:10 2023 -0700 fix ActiveStandbyStateMachine referrence (#186) 99d26af Jing Zhang Thu Mar 16 18:58:48 2023 -0700 [ci] Fix apt-get install unable locate package issue. (#177) (#187) d893be9 Longxiang Lyu Wed Feb 22 12:55:44 2023 +0800 [active-standby] Toggle to standby if link down and config auto (#173) ```
…nic-net#173) #### Description - Use class and instance attributes properly. Only use class attributes for data which should be shared among all instances. - Import all modules in `__init__.py` - Remove unused imports - Clean up whitespace #### Motivation and Context Proper object-oriented programming. This could prevent issues where attributes are shared between instances when they were not meant to be. This also fixes a bug with `PsuBase.psu_master_led_color`, where it is defined as both a class attribute and an instance attribute. With this change, some vendors' APIs can be cleaned up, because they redefined class attributes as instance attributes. That code can be removed. However, note that vendor APIs MUST call the base class initializer in their initializer methods, or the instance attributes will not be available and will raise exceptions.
all internal patches will be added below the marker. This eases internal patch management when public patches change Signed-off-by: Guohan Lu <[email protected]>
…lly (#18574) #### Why I did it src/sonic-gnmi ``` * 4b12af9 - (HEAD -> 202305, origin/202305) Merge pull request #209 from zbud-msft/revert_pfcwd_202305 (2 minutes ago) [StormLiangMS] * 802dbb7 - Revert "Replace PFC_WD_TABLE with PFC_WD (#173)" (28 hours ago) [Zain Budhwani] * e1397c4 - Revert "Account for GLOBAL key in PFC_WD (#178)" (28 hours ago) [Zain Budhwani] ``` #### How I did it #### How to verify it #### Description for the changelog
…lly (#18824) #### Why I did it src/sonic-gnmi ``` * 530c6bb - (HEAD -> 202311, origin/202311) Merge pull request #220 from sonic-net/revert-211-cherry/202311/173 (3 days ago) [Ying Xie] * db6f983 - (origin/revert-211-cherry/202311/173) Revert "Replace PFC_WD_TABLE with PFC_WD (#173)" (8 days ago) [Zain Budhwani] ``` #### How I did it #### How to verify it #### Description for the changelog
…lly (#19731) #### Why I did it src/sonic-gnmi ``` * 4e6f5b1 - (HEAD -> 202405, origin/202405) Merge pull request #277 from zbud-msft/revert_pfcwd_202405 (3 hours ago) [bingwang-ms] * 547241a - Rerun pipeline (25 hours ago) [Zain Budhwani] * 9785246 - Revert "Replace PFC_WD_TABLE with PFC_WD (#173)" (25 hours ago) [Zain Budhwani] ``` #### How I did it #### How to verify it #### Description for the changelog
…lly (sonic-net#20158) #### Why I did it src/sonic-gnmi ``` * 95f4400 - (HEAD -> master, origin/master, origin/HEAD) Revert "Replace PFC_WD_TABLE with PFC_WD (sonic-net#173)" (sonic-net#284) (2 hours ago) [Zain Budhwani] ``` #### How I did it #### How to verify it #### Description for the changelog
…utomatically (#20668) #### Why I did it src/sonic-host-services ``` * 13a5419 - (HEAD -> master, origin/master, origin/HEAD) Correct real time CPU Utilization calculation (#173) (3 hours ago) [Feng-msft] * f95b7cd - Optimize state_db update into batch way. (#176) (3 hours ago) [Feng-msft] ``` #### How I did it #### How to verify it #### Description for the changelog
…utomatically (sonic-net#20668) #### Why I did it src/sonic-host-services ``` * 13a5419 - (HEAD -> master, origin/master, origin/HEAD) Correct real time CPU Utilization calculation (sonic-net#173) (3 hours ago) [Feng-msft] * f95b7cd - Optimize state_db update into batch way. (sonic-net#176) (3 hours ago) [Feng-msft] ``` #### How I did it #### How to verify it #### Description for the changelog
…utomatically (sonic-net#20668) #### Why I did it src/sonic-host-services ``` * 13a5419 - (HEAD -> master, origin/master, origin/HEAD) Correct real time CPU Utilization calculation (sonic-net#173) (3 hours ago) [Feng-msft] * f95b7cd - Optimize state_db update into batch way. (sonic-net#176) (3 hours ago) [Feng-msft] ``` #### How I did it #### How to verify it #### Description for the changelog
A temp solution and call for comments.
Ideally python scripts and config.py should be put into docker-base or another layer of base image as the same logic will be shared by different dockers