Skip to content

multiDB : add database_config.json into vs images#3757

Merged
qiluo-msft merged 1 commit intosonic-net:masterfrom
dzhangalibaba:vstest
Nov 20, 2019
Merged

multiDB : add database_config.json into vs images#3757
qiluo-msft merged 1 commit intosonic-net:masterfrom
dzhangalibaba:vstest

Conversation

@dzhangalibaba
Copy link
Collaborator

@dzhangalibaba dzhangalibaba commented Nov 14, 2019

  • like what we did in docker-database running on DUT, on vs image we should have the database_config.json in place as well

  • vs image has a default startup config at /etc/default/sonic-db/

  • after mount /var/run/redis, in start.sh we copy startup config to running config at /var/run/redis/sonic-db/

  • verified when running vs tests, we can see the file mounted at /var/run/redis-vs/{sw-name}/sonic-db on host which is mapping to /var/run/redis/sonic-db/ on vs

dzhang@30-57-186-217:~/MS_VS/sonic-buildimage$ ls /var/run/redis-vs/heuristic_haibt/sonic-db/database_config.json 
/var/run/redis-vs/heuristic_haibt/sonic-db/database_config.json
dzhang@30-57-186-217:~/MS_VS/sonic-buildimage$ 

@dzhangalibaba
Copy link
Collaborator Author

@qiluo-msft after this change, we can do the replacement on C++ APIs and old "redis-cli" wrapper without affect current vs tests running on single instance based on the database_config.json. Later we will change all vs tests to adapt multiple instances case.


mkdir -p /var/run/redis
mkdir -p /var/run/redis/sonic-db
cp /etc/default/sonic-db/database_config.json /var/run/redis/sonic-db/
Copy link
Collaborator

@qiluo-msft qiluo-msft Nov 14, 2019

Choose a reason for hiding this comment

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

database_config.json [](start = 25, length = 20)

redis-server is not reading this file right now. #Pending

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes. Let's replace the core C++ APIs and redis-cli scripts first. Then I'll go back to change the vs images redis-server stuff. For now , this change only make sure the vs tests are not broken when doing the replacement.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still applicable. If I unblock now, it will be forgotten easily.


In reply to: 346558103 [](ancestors = 346558103)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we can open an issue and assign it to me for reminder, at this moment I don't want to do this part first. Later when we change the vs tests, this is a must do stuff, it cannot be forgotten.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd like to finish the core stuff first(those APIs in swss and redis-cli replacement), then the image can be run on DUT and we can start testing on local . After that we can focus on vs tests.

COPY ["files/sonic_version.yml", "/etc/sonic/"]
COPY ["database_config.json", "/etc/default/sonic-db/"]

# Workaround the tcpdump issue
Copy link
Collaborator

@qiluo-msft qiluo-msft Nov 14, 2019

Choose a reason for hiding this comment

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

Will you add

VOLUME /var/run/redis/sonic-db/

#Pending

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't need add VOULME again, it already added when we start the dvs in conftest.py.

Copy link
Collaborator

@qiluo-msft qiluo-msft Nov 14, 2019

Choose a reason for hiding this comment

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

Dockerfile VOLUME is different from docker run -v. If you mark it as VOLUME, anything already there (built into docker image) will be copied out to host when you mount. I believe this is useful in some test cases. #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can verify and use this VOLUME MODE later. For now, there is nothing in /var/run/redis/ built into docker image. And today we only use this /var/run/redis after docker image is running. I prefer not to change current behavior which is not related to multiDB changes.

@dzhangalibaba
Copy link
Collaborator Author

retest this please

1 similar comment
@dzhangalibaba
Copy link
Collaborator Author

retest this please

@dzhangalibaba
Copy link
Collaborator Author

@qiluo-msft Can we make this change merged first ? Then I can submit the C++ API replacement PR and won't break the vs test?

@qiluo-msft qiluo-msft merged commit 1d5005b into sonic-net:master Nov 20, 2019
@dzhangalibaba dzhangalibaba deleted the vstest branch November 22, 2019 08:42
zhenggen-xu pushed a commit to zhenggen-xu/sonic-buildimage that referenced this pull request Jan 10, 2020
mssonicbld added a commit that referenced this pull request Jul 16, 2025
…lly (#23068)

#### Why I did it
src/sonic-swss
```
* 9d74a49 - (HEAD -> master, origin/master, origin/HEAD) [orchagent] CoPP neighbor miss trap and enhancements (#3624) (8 hours ago) [Ravi Minnikanti(Marvell)]
* 41dc0cb - Improve route performance 20% by changing NextHopGroupTable from std::map to std::unordered_map. (#3742) (8 hours ago) [Hua Liu]
* 27391fc - Publish oper_status time to STATE_DB (#3756) (29 hours ago) [Bobby McGonigle]
* ad80fa5 - [trim]: Add Packet Trimming Asym DSCP to OA (#3705) (29 hours ago) [Nazarii Hnydyn]
* dc520a7 - [ssw][ha] fix dpu_state_db connection issue and zmq not supporting dpu_appl_db (31 hours ago) [Jing Zhang]
* 035e1c7 - Added MAX pre-FEC BER for link health monitoring (#3757) (33 hours ago) [Prince George]
* 0c5a6e4 - Skip ref counting standby mux neighbor NHs when added to NH group (#3753) (33 hours ago) [manamand2020]
* f53cc8c - [DASH] Implement PL Redirect Map (#3731) (35 hours ago) [Lawrence Lee]
* c5c360e - Fix counter issue #22775 and #22478 (#3681) (4 days ago) [Stephen Sun]
* bd73705 - [DASH] Support trusted VNIs for appliance and ENI objects (#3728) (4 days ago) [Lawrence Lee]
* cea81b2 - stpd crashes due to wrong no.of stp instance passed from stpmgrd (#3752) (5 days ago) [Divya Kumaran Chandralekha]
* af56a61 - Fix fpmsyncd crash during pfcwd/test_pfcwd_warm_reboot.py worm reboot issue (#3746) (5 days ago) [Hua Liu]
* 80932db - use the exact strings from hld (#3735) (13 days ago) [Jing Zhang]
* f44f6ab - [vs][mirror]: Update test to use the max TC number provided by VS lib (#3712) (2 weeks ago) [Nazarii Hnydyn]
* 55e9bba - remove the logic that skip system neigh task for ASICs that share common hostname (#3718) (2 weeks ago) [Changrong Wu]
* 3356753 - LC buffer errors for local port (#3719) (2 weeks ago) [Vineet Mittal]
* bad2141 - Update INIT_VIEW timeout for marvell-prestera platforms (#3729) (2 weeks ago) [Pavan Prakash]
* eebaf97 - [routeorch] Wait for the VRF to be created (#3652) (2 weeks ago) [Manoharan Sundaramoorthy]
* 7dd3be9 - [fpmsyncd]Fixing the blackhole route removal during warmboot (#3726) (2 weeks ago) [Sudharsan Dhamal Gopalarathnam]
* 575c342 - [routeorch] Handle SAI_STATUS_ITEM_NOT_FOUND when setting route entries (#3713) (3 weeks ago) [Nikola Dancejic]
* 1ae6787 - [portsorch] postpone non-critical port init part in warm/fast-reboot (#3562) (3 weeks ago) [Stepan Blyshchak]
* 889aff6 - add support for local endpoints in vnet_route_tunnel (#3651) (3 weeks ago) [Jing Zhang]
* 1f97afb - [trim]: Add Packet Trimming to OA (#3594) (3 weeks ago) [Nazarii Hnydyn]
* 8c2b337 - Gracefully handle errors when accessing dpu app_state DB on NPU from DPU (#3716) (3 weeks ago) [prabhataravind]
* a0e1953 - Harden module build script by specifying the source version to get (#3723) (3 weeks ago) [Saikrishna Arcot]
* 0081e3a - Improve route orch performance by enable ZMQ (#3632) (3 weeks ago) [Hua Liu]
```
#### How I did it
#### How to verify it
#### Description for the changelog
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.

2 participants