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

[updategraph] add support to use preset config instead of default minigraph #2050

Merged
merged 4 commits into from
Sep 22, 2018

Conversation

taoyl-ms
Copy link
Contributor

- What I did
Each platform now does not need to provide a default minigraph. Instead, it could specify a default SKU and preset name, and updategraph service will generate a preset configuration accordingly.

Depends on #2039.

@lguohan
Copy link
Collaborator

lguohan commented Sep 14, 2018

can you remove the minigraph.xml for all skus?

redis-cli -n $CONFIG_DB_INDEX FLUSHDB
sonic-cfggen -j /etc/sonic/config_db.json --write-to-db
redis-cli -n $CONFIG_DB_INDEX SET "CONFIG_DB_INITIALIZED" "1"
else
Copy link
Collaborator

Choose a reason for hiding this comment

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

i am not sure if we need this path now? what purpose?

elif [ -f /usr/share/sonic/device/$PLATFORM/default_sku ]; then
PRESET=(`head -n 1 /usr/share/sonic/device/$PLATFORM/default_sku`)
sonic-cfggen -H -k ${PRESET[0]} --preset empty > /tmp/device_meta.json
else
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure we need this else. in my opinion, we should get rid of the default minigraph.xml completely from the image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we remove this else block, we'll need to make sure all the new platforms have a default_sku file in there device folder. I can create them for all existing ones, but we'd better communicate with community on this change for the future ones.

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 create a unit test in sonic-buildimage/src/sonic-device-data/ to enforce any rule you want and later it will be added into PR check.

Another option: in stead of fallback to minigraph, how about fallback to the SKU with shortest name?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@taoyl-ms , agree with you about the announcement. why not talk about this next community meeting?

Copy link
Collaborator

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

remove minigraph.xml for all skus.

@lguohan
Copy link
Collaborator

lguohan commented Sep 21, 2018

can you resolve conflict?

@lguohan lguohan merged commit 018b589 into sonic-net:master Sep 22, 2018
@wen587 wen587 mentioned this pull request Mar 7, 2022
6 tasks
wen587 added a commit that referenced this pull request Mar 8, 2022
47c243e [show][muxcable] fix the sudo access error for show muxcable metrics (#2083)
f872516 [muxcable][show] enhance show mux status to show last switchover time (#2067)
d440df7 [warmboot] Migrate 10G ports during warm-reboot on s6100 (#2064)
494c6d7 [counterpoll] Display the correct default poll interval for watermark counters (#2082)
499988e [show][config] add muxcable command line support for retrieve / reset ICMP packet loss data (#2046)
8b01d3e Remove the warning message appear when there are no ports on CONFIG DB (#2050)
ed6e66e [GCU] Supporting Groupings during path-xpath translation (#2044)
25b3455 [ci] Use official build debian pkg instead and parameterize source branch (#2079)
taras-keryk pushed a commit to taras-keryk/sonic-buildimage that referenced this pull request Apr 28, 2022
sonic-net#2050)

#### What I did

Remove the warning of "Configuration database contains no ports" when ports table is empty.
Because when having a system with zero ports, this message appear per each access to ports tables on CONFIG DB

For example:

Run any cli command. For example  "show boot" or "show int st"

Observed behavior

root@r-sn4800-simx:/home/admin# show boot
Configuration database contains no ports
Configuration database contains no ports
Configuration database contains no ports


#### How I did it

Remove the warning and remove the unit test verify this warning 

#### How to verify it

Remove all the ports and verify there is no such warning in case of running "show interface status" CLi command

#### Previous command output (if the output of a command-line utility has changed)

Configuration database contains no ports
  Interface    Lanes    Speed    MTU    FEC    Alias    Vlan    Oper    Admin    Type    Asym PFC
-----------  -------  -------  -----  -----  -------  ------  ------  -------  ------  ----------
 
#### New command output (if the output of a command-line utility has changed)

  Interface    Lanes    Speed    MTU    FEC    Alias    Vlan    Oper    Admin    Type    Asym PFC
-----------  -------  -------  -----  -----  -------  ------  ------  -------  ------  ----------
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.

4 participants