syslog changes Multi NPU platforms #4738

arlakshm · 2020-06-09T16:30:11Z

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan arlakshm@microsoft.com

- Why I did it
Add changes for syslog support for containers running in namespaces on multi NPU platforms.
On Multi NPU platforms

Rsyslog service is only running on the host. There is no rsyslog service running in each namespace.
On multi NPU platforms the rsyslog service on the host will be listening on the docker0 ip address instead of loopback address.
The rsyslog.conf on the containers is modified to have omfwd target ip to be docker0 ipaddress instead of loopback ip

No change done for single ASIC platforms

- How I did it

Add new j2 file for the generating the rsyslog.conf inside the containers
Generate the rsyslog.conf and copy to the containers in the preStartAction function.
Update the rsyslog-config.sh script to update the rsyslog.conf on the host to listen to docker0 ip address in multi NPU platforms.

- How to verify it
Verify the syslogs are generated properly
Sample logs:

admin@sonic:~$ sudo grep -i orchagent /var/log/syslog.1
Jun  9 16:18:21.902661 sonic INFO swss4[612] 2020-06-09 16:18:21,902 INFO spawned: 'orchagent' with pid 42#015
Jun  9 16:18:22.142771 sonic INFO swss0[612] 2020-06-09 16:18:22,142 INFO spawned: 'orchagent' with pid 42#015
Jun  9 16:18:22.200722 sonic INFO swss1[612] 2020-06-09 16:18:22,200 INFO spawned: 'orchagent' with pid 36#015
Jun  9 16:18:22.234121 sonic INFO swss5[612] 2020-06-09 16:18:22,233 INFO spawned: 'orchagent' with pid 38#015
Jun  9 16:18:22.612859 sonic INFO swss3[612] 2020-06-09 16:18:22,611 INFO spawned: 'orchagent' with pid 36#015
Jun  9 16:18:22.802017 sonic INFO swss2[612] 2020-06-09 16:18:22,801 INFO spawned: 'orchagent' with pid 37#015
Jun  9 16:18:22.915873 sonic INFO swss4[612] 2020-06-09 16:18:22,911 INFO success: orchagent entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)#015
Jun  9 16:18:23.146156 sonic INFO swss0[612] 2020-06-09 16:18:23,145 INFO success: orchagent entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)#015
Jun  9 16:18:23.211117 sonic INFO swss1[612] 2020-06-09 16:18:23,207 INFO success: orchagent entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)#015
Jun  9 16:18:23.240845 sonic INFO swss5[612] 2020-06-09 16:18:23,236 INFO success: orchagent entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)#015
Jun  9 16:18:23.615436 sonic INFO swss3[612] 2020-06-09 16:18:23,615 INFO success: orchagent entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)#015
Jun  9 16:18:23.808437 sonic INFO swss2[612] 2020-06-09 16:18:23,806 INFO success: orchagent entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)#015

- Description for the changelog

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

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

SuvarnaMeenakshi · 2020-06-09T17:20:13Z

files/build_templates/docker_image_ctl.j2

@@ -238,7 +242,9 @@ start() {
            redis_dir=`echo $redis_dir_list | cut -d " " -f $id`
            REDIS_MNT=" -v $redis_dir:$redis_dir:rw "
        fi
-
+        # Containers in the docker network will use the syslog-driver as log-driver. This is mainly applicable for Multi NPU system
+        LOG_OPTS="--log-driver=syslog --log-opt syslog-address=udp://127.0.0.1:514 --log-opt syslog-format=rfc5424 \


Should there be a check to make sure docker_image_run_opt does not have--log-driver=json-file ?

@SuvarnaMeenakshi syslogs will not work if the log-driver is json on containers running the docker bridge network, so I think we can ignore --log-driver=json-file in this case.

Could you giver proof or reference that "syslogs will not work if the log-driver is json on containers running the docker bridge network"? #Closed

Logs

docker logs for swss running in namespace asic0.

admin@sonic:~$ docker logs swss0 | grep orchagent 2020-06-10 00:45:01,434 INFO spawned: 'orchagent' with pid 36 2020-06-10 00:45:02,439 INFO success: orchagent entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)

same logs not available in syslog file

admin@sonic:~$ sudo zgrep -i orchagent /var/log/syslog.* admin@sonic:~$

I see your point. Syslogs will not work if the log-driver is json and this is not related to docker bridge network and applicable to previous single ASIC use case.

Do you want to fix both single ASIC and multiple ASIC? #Closed

I didnt change the Single ASIC I didnt want to break any backward compatibility for some applications,

Then you need to check '--log-driver=json-file' in docker_image_run_opt because it will work if user specify it explicitly #Closed

Fixed in the latest commit

files/build_templates/docker_image_ctl.j2

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

qiluo-msft

As comment

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

qiluo-msft · 2020-06-12T17:53:25Z

files/image_config/rsyslog/rsyslog-config.sh

+    intf="lo"
+fi
+
+udp_server_ip=$(ip -o -4 addr list $intf | awk '{print $4}' | cut -d/ -f1)


ip -o -4 addr list $intf [](start = 16, length = 24)

There may be more than one lo interfaces. How do you handle? #Closed

fixed in the latest commit

qiluo-msft · 2020-06-12T17:56:38Z

files/build_templates/docker_image_ctl.j2

+        {%- else %}
+            # Containers in the docker network will use the syslog-driver as log-driver. This is mainly applicable for Multi NPU system
+            # In this case the syslog service on the host is listening on the docker0 address
+            syslog_address=$(ip -o -4 addr list docker0 | awk '{print $4}' | cut -d/ -f1)


Suggested change

syslog_address=$(ip -o -4 addr list docker0 | awk '{print $4}' | cut -d/ -f1)

syslog_address=$(docker network inspect bridge --format='{{(index .IPAM.Config 0).Gateway}}')

``` #Closed

This IP address is actually available in build time. Check files/docker/docker.service.conf

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

This change has been removed

qiluo-msft · 2020-06-12T18:29:31Z

files/build_templates/docker_image_ctl.j2

+        TARGET_IP=$(ip -o -4 addr list docker0 | awk '{print $4}' | cut -d/ -f1)
+        CONTAINER_NAME="{{docker_container_name}}$DEV"
+        TMP_FILE="/tmp/rsyslog.$CONTAINER_NAME.conf"
+        sonic-cfggen  -t /usr/share/sonic/templates/rsyslog-container.conf.j2 -a "{\"target_ip\": \"$TARGET_IP\", \"container_name\": \"$CONTAINER_NAME\" }"  > $TMP_FILE


[](start = 21, length = 1)

One extra blank #Closed

added in the latest commit

Could you remove the extra blank in

sonic-cfggen -t

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

Fixed in the latest commit

qiluo-msft · 2020-06-12T18:30:19Z

files/build_templates/docker_image_ctl.j2

+        TARGET_IP=$(ip -o -4 addr list docker0 | awk '{print $4}' | cut -d/ -f1)
+        CONTAINER_NAME="{{docker_container_name}}$DEV"
+        TMP_FILE="/tmp/rsyslog.$CONTAINER_NAME.conf"
+        sonic-cfggen  -t /usr/share/sonic/templates/rsyslog-container.conf.j2 -a "{\"target_ip\": \"$TARGET_IP\", \"container_name\": \"$CONTAINER_NAME\" }"  > $TMP_FILE


container_name [](start = 116, length = 14)

This field make original code useless.

ARG docker_container_name

Could you remove original related code in Dockerfile(s)? #Pending

Agreed. I will do it in a separate PR. Filed this issue to track this.

qiluo-msft · 2020-06-12T18:32:35Z

files/build_templates/docker_image_ctl.j2

+    # running on the namespace to reach the rsyslog service running on the host
+    # Also update the container name  
+    if [[ ($NUM_ASIC -gt 1) ]]; then
+        TARGET_IP=$(ip -o -4 addr list docker0 | awk '{print $4}' | cut -d/ -f1)


TARGET_IP=$(ip -o -4 addr list docker0 | awk '{print $4}' | cut -d/ -f1) [](start = 8, length = 72)

This IP address is actually available in build time. Check files/docker/docker.service.conf #Closed

Yes, I didn't want to hard code the ip address here. So using this method to get the docker0 ip address

I mean docker0 IP address is actually available in build time. Check files/docker/docker.service.conf. You are ok to use ip command, but build time constant may be easier.

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

I see. Your solution is future proof.

In reply to: 440542831 [](ancestors = 440542831,439581391)

qiluo-msft · 2020-06-12T18:36:21Z

files/build_templates/docker_image_ctl.j2

+    if [[ ($NUM_ASIC -gt 1) ]]; then
+        TARGET_IP=$(ip -o -4 addr list docker0 | awk '{print $4}' | cut -d/ -f1)
+        CONTAINER_NAME="{{docker_container_name}}$DEV"
+        TMP_FILE="/tmp/rsyslog.$CONTAINER_NAME.conf"


TMP_FILE="/tmp/rsyslog.$CONTAINER_NAME.conf" [](start = 8, length = 44)

Could you use Dockerfile ENV and docker create --env to pass the container_name. Then no need to create tmpfile. #Pending

The tmp file is created after rendering this template /usr/share/sonic/templates/rsyslog-container.conf.j2, which has updated syslog.conf file. I don't think the tmp file can be avoided even if we pass the ENV to the container.

In Multi-NPU platforms it is possible that multi containers can be started at the same time, to avoid container using wrong syslog.conf files I am using the container name in the tmp filename

Let's create two ENV:

RSYSLOG_SERVER_IP

CONTAINER_NAME

Pass the ENV values by command docker create ... --env RSYSLOG_SERVER_IP=AA.BB.CC.DD ...

The benefit:

not tmp file management

works with multiple containers from single image

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

@qiluo-msft created a issue to track 4778 to improvements suggested in this comment.

qiluo-msft · 2020-06-12T18:38:39Z

files/build_templates/docker_image_ctl.j2

@@ -228,6 +244,10 @@ start() {
            done
        fi
        {%- endif %}
+        # single NPU systems and container running on the host network continue to use the json-file as log-driver


single NPU systems and container running on the host network continue to use the json-file as log-driver [](start = 8, length = 106)

The different behavior of single NPU vs multiple NPU is confusing and complex deign.

What about moving all use cases to log-driver, or all use cases to json-file as before? #Closed

Changed to use json-file for all cases

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

qiluo-msft · 2020-06-16T01:57:17Z

files/build_templates/docker_image_ctl.j2

@@ -228,6 +245,7 @@ start() {
            done
        fi
        {%- endif %}
+
    else


No need to change this line #Closed

Fixed in latest commit

qiluo-msft · 2020-06-16T01:57:22Z

files/build_templates/docker_image_ctl.j2

@@ -238,7 +256,6 @@ start() {
            redis_dir=`echo $redis_dir_list | cut -d " " -f $id`
            REDIS_MNT=" -v $redis_dir:$redis_dir:rw "
        fi
-
        {%- if docker_container_name == "database" %}


No need to change this line #Closed

Fixed in latest commit

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

qiluo-msft · 2020-06-18T00:32:45Z

files/build_templates/docker_image_ctl.j2

+    # running on the namespace to reach the rsyslog service running on the host
+    # Also update the container name  
+    if [[ ($NUM_ASIC -gt 1) ]]; then
+        TARGET_IP=$(ip -o -4 addr list docker0 | awk '{print $4}' | cut -d/ -f1)


Suggested change

TARGET_IP=$(ip -o -4 addr list docker0 | awk '{print $4}' | cut -d/ -f1)

TARGET_IP=$(docker network inspect bridge --format='{{(index .IPAM.Config 0).Gateway}}')

``` #Closed

Fixed in the latest commit

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

arlakshm · 2020-06-19T14:57:51Z

@qiluo-msft , @SuvarnaMeenakshi , @lguohan
Please help approve if there are no more comments

Add changes for syslog support for containers running in namespaces on multi ASIC platforms. On Multi ASIC platforms Rsyslog service is only running on the host. There is no rsyslog service running in each namespace. On multi ASIC platforms the rsyslog service on the host will be listening on the docker0 ip address instead of loopback address. The rsyslog.conf on the containers is modified to have omfwd target ip to be docker0 ipaddress instead of loopback ip Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

syslog changes Multi NPU platforms

a4a10a7

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

arlakshm marked this pull request as ready for review June 9, 2020 16:30

arlakshm requested review from lguohan, jleveque and SuvarnaMeenakshi June 9, 2020 16:31

jleveque requested a review from qiluo-msft June 9, 2020 16:45

SuvarnaMeenakshi reviewed Jun 9, 2020

View reviewed changes

qiluo-msft reviewed Jun 9, 2020

View reviewed changes

files/build_templates/docker_image_ctl.j2 Outdated Show resolved Hide resolved

indentation fix

795b2b0

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

SuvarnaMeenakshi approved these changes Jun 10, 2020

View reviewed changes

SuvarnaMeenakshi self-requested a review June 10, 2020 16:55

SuvarnaMeenakshi previously approved these changes Jun 10, 2020

View reviewed changes

arlakshm marked this pull request as draft June 10, 2020 17:11

qiluo-msft requested changes Jun 10, 2020

View reviewed changes

Additional changes to support swss_log messages

3616a5f

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

arlakshm dismissed SuvarnaMeenakshi’s stale review via 3616a5f June 12, 2020 06:50

arlakshm marked this pull request as ready for review June 12, 2020 07:08

qiluo-msft reviewed Jun 12, 2020

View reviewed changes

Address review comment.

16c6b3d

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

arlakshm mentioned this pull request Jun 15, 2020

remove syslog.conf related cleanup from dockerfiles #4778

Open

Addressing review comments

1deb8a0

qiluo-msft reviewed Jun 16, 2020

View reviewed changes

Addressing review comments

b760de0

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

qiluo-msft reviewed Jun 18, 2020

View reviewed changes

Addressing review comments

222489d

qiluo-msft previously approved these changes Jun 18, 2020

View reviewed changes

fix build

84aa4b5

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>

arlakshm dismissed qiluo-msft’s stale review via 84aa4b5 June 18, 2020 21:22

qiluo-msft approved these changes Jun 20, 2020

View reviewed changes

arlakshm added Multi-NPU 🔢 Request for 201911 Branch labels Jun 22, 2020

arlakshm merged commit ef994a1 into sonic-net:master Jun 30, 2020

arlakshm deleted the multi_npu_syslog_changes branch June 30, 2020 13:29

arlakshm added the Multi-ASIC label Jun 30, 2020

abdosi added the Included in 201911 Branch label Jul 5, 2020

SuvarnaMeenakshi mentioned this pull request Oct 10, 2023

Container net host remove #15176

Closed

	syslog_address=$(ip -o -4 addr list docker0 \| awk '{print $4}' \| cut -d/ -f1)
	syslog_address=$(docker network inspect bridge --format='{{(index .IPAM.Config 0).Gateway}}')
	``` #Closed

	TARGET_IP=$(ip -o -4 addr list docker0 \| awk '{print $4}' \| cut -d/ -f1)
	TARGET_IP=$(docker network inspect bridge --format='{{(index .IPAM.Config 0).Gateway}}')
	``` #Closed

syslog changes Multi NPU platforms #4738

syslog changes Multi NPU platforms #4738

Conversation

arlakshm commented Jun 9, 2020 • edited

SuvarnaMeenakshi Jun 9, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jun 9, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jun 10, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jun 10, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

qiluo-msft Jun 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jun 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jun 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jun 15, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jun 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jun 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jun 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jun 12, 2020 • edited

Choose a reason for hiding this comment

single NPU systems and container running on the host network continue to use the json-file as log-driver [](start = 8, length = 106)

Choose a reason for hiding this comment

qiluo-msft Jun 16, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jun 16, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jun 18, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arlakshm commented Jun 19, 2020

arlakshm commented Jun 9, 2020 •

edited

SuvarnaMeenakshi Jun 9, 2020 •

edited

qiluo-msft Jun 9, 2020 •

edited

qiluo-msft Jun 10, 2020 •

edited

qiluo-msft Jun 10, 2020 •

edited

qiluo-msft Jun 12, 2020 •

edited

qiluo-msft Jun 12, 2020 •

edited

qiluo-msft Jun 12, 2020 •

edited

qiluo-msft Jun 15, 2020 •

edited

qiluo-msft Jun 12, 2020 •

edited

qiluo-msft Jun 12, 2020 •

edited

qiluo-msft Jun 12, 2020 •

edited

qiluo-msft Jun 12, 2020 •

edited

qiluo-msft Jun 16, 2020 •

edited

qiluo-msft Jun 16, 2020 •

edited

qiluo-msft Jun 18, 2020 •

edited