[docker_img_ctl.j2] make tmpfs mounts optional and add ability to run container by image id #6439
Conversation
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
|
what is sae dockers? |
|
@lguohan SAE - SONiC Application Extension |
There was a problem hiding this comment.
I recommend we find a method do the following:
- All dockers mount /tmp if a certain variable is not defined or not set to a certain value
- Dockers which don't want /tmp to be mounted, will define the variable
- The variable is passed to the j2 environment while rendering the docker_image_ctl.j2
This will allow us to minimize the places where /tmp mount logic needs to be specified.
This reverts commit f908b82.
Signed-off-by: Stepan Blyshchak <stepanb@nvidia.com>
|
retest this please |
provided Signed-off-by: Stepan Blyshchak <stepanb@nvidia.com>
| {%- if docker_image_name is defined %} | ||
| {{docker_image_name}}:latest \ | ||
| {%- else %} | ||
| {{docker_image_id}} \ |
There was a problem hiding this comment.
Why add docker_image_id? I don't find any clue in your PR description. #Closed
There was a problem hiding this comment.
I updated title and PR description.
With coming app ext feature docker image might not have a 'latest' tag or it might not have a name at all.
So this template is required to accept more general docker_image_id, but preserving compatibility if docker_image_name was passed.
| @@ -388,13 +390,23 @@ start() { | |||
| $REDIS_MNT \ | |||
| -v /usr/share/sonic/device/$PLATFORM:/usr/share/sonic/platform:ro \ | |||
| {%- if sonic_asic_platform != "mellanox" %} | |||
| {%- if mount_default_tmpfs is defined and mount_default_tmpfs == "y" %} | |||
There was a problem hiding this comment.
This and condition is verbose. Is it possible to simplify? #Closed
There was a problem hiding this comment.
Indeed, please check the new way:
{%- if mount_default_tmpfs|default("n") == "y" %}
stepanblyschak
changed the title
Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
|
@rajendra-dendukuri comments were handled. Can you please review and approve? |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@rajendra-dendukuri your approval is needed. Please provide your feedback. |
… container by image id (sonic-net#6439) - Why I did it I made the docker_img_ctl.j2 applicable for more dockers (including application extensions dockers) by adding an option not to mount tmpfs on /tmp/ and /var/tmp/. In some applications /tmp/ is a different docker volume which can't be tmpfs. Also, I added and ability to pass REPO[:TAG]|[@digest]/IMAGE_ID instead of just REPO name. - How I did it Modified docker_img_ctl.j2 and docker makefiles. - How to verify it Run it on the switch.
… container by image id (sonic-net#6439) - Why I did it I made the docker_img_ctl.j2 applicable for more dockers (including application extensions dockers) by adding an option not to mount tmpfs on /tmp/ and /var/tmp/. In some applications /tmp/ is a different docker volume which can't be tmpfs. Also, I added and ability to pass REPO[:TAG]|[@digest]/IMAGE_ID instead of just REPO name. - How I did it Modified docker_img_ctl.j2 and docker makefiles. - How to verify it Run it on the switch.
Signed-off-by: Stepan Blyschak stepanb@nvidia.com
- Why I did it
I made the docker_img_ctl.j2 applicable for more dockers (including application extensions dockers) by adding an option not to mount tmpfs on /tmp/ and /var/tmp/. In some applications /tmp/ is a different docker volume which can't be tmpfs.
Also, I added and ability to pass REPO[:TAG]|[@digest]/IMAGE_ID instead of just REPO name.
- How I did it
Modified docker_img_ctl.j2 and docker makefiles.
- How to verify it
Run it on the switch.
- Which release branch to backport (provide reason below if selected)
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)