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

Support for connecting to DB in namespace via TCP port in multi-asic platform. #4779

Merged
merged 5 commits into from
Jun 30, 2020

Conversation

judyjoseph
Copy link
Contributor

@judyjoseph judyjoseph commented Jun 16, 2020

- Why I did it

Currently in multi-asic platform support exists only for using UNIX socket to connect to DB in different namespaces. There are permission restrictions with the redis unix socket file because of which applications needs to either be running as root user or need to specify sudo to be able to use it.

Certain applications like "show commands" are currently being called without sudo and hence cannot use the redis unix socket to connect to DB in other namespaces. Hence introducing this option to connect using TCP port.
Note - redis unix socket is still the preferred way due to better performance !!

- How I did it
Updated the database_config.json.j2 to use HOST_IP which is derived from either loopback interface in case of linux host, or eth0 interface ( which is connected to docker bridge network ) in the other namespaces.
In the namespaces the redis server is started on multiple IP address (loop back IP and eth0 IP ) with the "bind' option. We have to use the loopback IP as there are multiple applications using host=127.0.0.1 as hardcoding in the connector class object.

- How to verify it
Verified with the show/config commands.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

…bridge network ) for applications in multi-asic platform.
Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

As comments

@@ -20,7 +20,7 @@ stderr_logfile=syslog
{% if INSTANCES %}
{% for redis_inst, redis_items in INSTANCES.iteritems() %}
[program: {{ redis_inst }}]
command=/bin/bash -c "{ [[ -s /var/lib/{{ redis_inst }}/dump.rdb ]] || rm -f /var/lib/{{ redis_inst }}/dump.rdb; } && mkdir -p /var/lib/{{ redis_inst }} && exec /usr/bin/redis-server /etc/redis/redis.conf --port {{ redis_items['port'] }} --unixsocket {{ redis_items['unix_socket_path'] }} --pidfile /var/run/redis/{{ redis_inst }}.pid --dir /var/lib/{{ redis_inst }}"
command=/bin/bash -c "{ [[ -s /var/lib/{{ redis_inst }}/dump.rdb ]] || rm -f /var/lib/{{ redis_inst }}/dump.rdb; } && mkdir -p /var/lib/{{ redis_inst }} && exec /usr/bin/redis-server /etc/redis/redis.conf --bind {{ LOOPBACK_IP }} {{ redis_items['hostname'] }} --port {{ redis_items['port'] }} --unixsocket {{ redis_items['unix_socket_path'] }} --pidfile /var/run/redis/{{ redis_inst }}.pid --dir /var/lib/{{ redis_inst }}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

just wonder is there any case both LOOPBACK_IP and HOST_IP are empty, if there is a case , the --bind without parameter will work or NOT ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This case should not happen as the LOOPBACK_IP is hardcoded to 127.0.0.1 , the HOST_IP is either 127.0.0.1 ( in case of linux host namespace ) or the docker0 IP address in case of namespaces. But I can add the the default fallback IP of 127.0.0.1, if this error scenario arises.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the code, so that if we are not able to get the ip address correctly from interface, it is set to loopback ip 127.0.0.1 as the default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dzhangalibaba let me know it this was ok ?

…terface fails.

Moved the localhost loopback IP binding logic into the supervisor.j2 file.
qiluo-msft
qiluo-msft previously approved these changes Jun 26, 2020
dzhangalibaba
dzhangalibaba previously approved these changes Jun 28, 2020
This was causing redis server to not start up as command string was coming in same line as program:
abdosi pushed a commit that referenced this pull request Jul 5, 2020
…platform. (#4779)

* Support for connecting to DB in namespace via IP:port ( using docker bridge network ) for applications in multi-asic platform.

* Added the default IP as 127.0.0.1 if the IPaddress derivation from interface fails.
Moved the localhost loopback IP binding logic into the supervisor.j2 file.
qiluo-msft pushed a commit to qiluo-msft/sonic-buildimage that referenced this pull request Jul 12, 2020
…platform. (sonic-net#4779)

* Support for connecting to DB in namespace via IP:port ( using docker bridge network ) for applications in multi-asic platform.

* Added the default IP as 127.0.0.1 if the IPaddress derivation from interface fails.
Moved the localhost loopback IP binding logic into the supervisor.j2 file.
qiluo-msft pushed a commit to qiluo-msft/sonic-buildimage that referenced this pull request Jul 12, 2020
…platform. (sonic-net#4779)

* Support for connecting to DB in namespace via IP:port ( using docker bridge network ) for applications in multi-asic platform.

* Added the default IP as 127.0.0.1 if the IPaddress derivation from interface fails.
Moved the localhost loopback IP binding logic into the supervisor.j2 file.
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.

5 participants