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

Do not mount redis.sock to avoid race condition #229

Closed
wants to merge 1 commit into from

Conversation

Junchao-Mellanox
Copy link
Owner

@Junchao-Mellanox Junchao-Mellanox commented Jun 26, 2024

Why I did it

Service restapi is mounting /var/run/redis/redis.sock by command parameter "-v /var/run/redis/redis.sock:/var/run/redis/redis.sock". This is dangerous since it will create a directory named "/var/run/redis/redis.sock" if it does not exists. It could cause race condition like this:

  1. restapi mount "/var/run/redis/redis.sock:/var/run/redis/redis.sock" and create a "/var/run/redis/redis.sock".

2 .redis server starts and try to bind to unix socket, but the folder already exists, so redis failed with log:

"""
Jun 20 22:52:08.451081 sonic INFO database#supervisord: redis 64:M 20 Jun 2024 22:52:08.442 # Opening Unix socket: bind: Address already in use
"""

Work item tracking
  • Microsoft ADO (number only):

How I did it

Since docker_image_ctl.j2 has already mounted /var/run/redis for all containers, there is no point and not safe to mount /var/run/redis/redis.sock. So, I removed the mount from the make file.

How to verify it

Manual test

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)

@Junchao-Mellanox
Copy link
Owner Author

ci passed 4192

@Junchao-Mellanox Junchao-Mellanox deleted the master-fix-redis-race branch July 17, 2024 02:04
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