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

[db_migrator] add required "protocol" field in ROUTE_TABLE #2766

Merged

Conversation

stepanblyschak
Copy link
Contributor

@stepanblyschak stepanblyschak commented Mar 29, 2023

What I did

I added requires "protocol" field due to fpmsyncd reconcile logic (WarmRestartHelper class) requires old fvs keys to match new fvs.

How I did it

Add "protocol" field in db migration.

How to verify it

Upgrade from older branch to new image with supported FIB pending.

The field was added by sonic-net/sonic-swss#2551.

Now, after reboot:

127.0.0.1:6379> hgetall ROUTE_TABLE:0.0.0.0/0
1) "weight"
2) "1,1,1,1,1,1,1,1"
3) "protocol"
4) "bgp"
5) "ifname"
6) "PortChannel102,PortChannel105,PortChannel108,PortChannel1011,PortChannel1014,PortChannel1017,PortChannel1020,PortChannel1023"
7) "nexthop"
8) "10.0.0.1,10.0.0.5,10.0.0.9,10.0.0.13,10.0.0.17,10.0.0.21,10.0.0.25,10.0.0.29"

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

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

Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
self.appDB.set(self.appDB.APPL_DB, route_key, 'weight','')

if 'protocol' not in route_attr:
self.appDB.set(self.appDB.APPL_DB, route_key, 'protocol', 'bgp')
Copy link
Contributor

Choose a reason for hiding this comment

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

Why BGP is assumed? Why can't this be left empty like weight attr? Fpmsyncd reconciliation will add the protocol as it expects. I think the reconciliation failure is only limited to attribute name and not its value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vaibhavhd Fixed

Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
Copy link
Contributor

@vaibhavhd vaibhavhd left a comment

Choose a reason for hiding this comment

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

LGTM, good to have Prince S's review too before merge.

@vaibhavhd
Copy link
Contributor

Please link which swss PR brought in this new field.

Please describe how table looks like before and after this migration in verification part of description.

@stepanblyschak
Copy link
Contributor Author

@vaibhavhd Done

@liat-grozovik
Copy link
Collaborator

@stepanblyschak needed only for master?

@liat-grozovik liat-grozovik merged commit 46fba26 into sonic-net:master Jun 14, 2023
5 checks passed
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Jun 20, 2023
Update sonic-utilities submodule pointer to include the following:
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789))
* 659ba24b [syslog] Adjust runningconfiguration syslog command ([sonic-net#2843](sonic-net/sonic-utilities#2843))
* 46fba26f [db_migrator] add required protocol field in ROUTE_TABLE ([sonic-net#2766](sonic-net/sonic-utilities#2766))
* f186376e Fix issue: show interfaces transceiver eeprom -d should display same entry for CMIS cable ([sonic-net#2864](sonic-net/sonic-utilities#2864))
* de491798 fix precedence in portstat CLI ([sonic-net#2874](sonic-net/sonic-utilities#2874))

Signed-off-by: dprital <drorp@nvidia.com>
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Jun 21, 2023
Update sonic-utilities submodule pointer to include the following:
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789))
* 659ba24b [syslog] Adjust runningconfiguration syslog command ([sonic-net#2843](sonic-net/sonic-utilities#2843))
* 46fba26f [db_migrator] add required protocol field in ROUTE_TABLE ([sonic-net#2766](sonic-net/sonic-utilities#2766))
* f186376e Fix issue: show interfaces transceiver eeprom -d should display same entry for CMIS cable ([sonic-net#2864](sonic-net/sonic-utilities#2864))
* de491798 fix precedence in portstat CLI ([sonic-net#2874](sonic-net/sonic-utilities#2874))

Signed-off-by: dprital <drorp@nvidia.com>
pdhruv-marvell pushed a commit to pdhruv-marvell/sonic-utilities that referenced this pull request Aug 23, 2023
…#2766)

- What I did
I added requires "protocol" field due to fpmsyncd reconcile logic (WarmRestartHelper class) requires old fvs keys to match new fvs.

- How I did it
Add "protocol" field in db migration.

- How to verify it
Upgrade from older branch to new image with supported FIB pending.

The field was added by sonic-net/sonic-swss#2551.

Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants