-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[sonic_debian_extension] add docker script to SONiC filesystem #5935
[sonic_debian_extension] add docker script to SONiC filesystem #5935
Conversation
Later this script will be used to start dockerd in chroot environment on SONiC Signed-off-by: Stepan Blyshchak <stepanb@nvidia.com>
retest broadcom please |
@qiluo-msft Could you please review? |
@@ -512,6 +514,10 @@ sudo LANG=C chroot $FILESYSTEM_ROOT docker $SONIC_NATIVE_DOCKERD_FOR_DOCKERFS ta | |||
{% endif %} | |||
{% endfor %} | |||
|
|||
# Copy docker start script to be able to start docker in chroot | |||
sudo mkdir -p $FILESYSTEM_ROOT_USR_LIB/docker | |||
sudo cp $DOCKER_SCRIPTS_DIR/docker $FILESYSTEM_ROOT_USR_LIB/docker/docker.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice it is already copied. Could you use this one?
sudo cp files/docker/docker $FILESYSTEM_ROOT/etc/init.d/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is later removed by:
sudo rm $FILESYSTEM_ROOT/etc/init.d/docker
And I don't want to put it in /etc/init.d/ as this is not expected to be used as init.d script, docker is a systemd service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the insight. That change is introduced by https://github.com/Azure/sonic-buildimage/pull/2417/files
The purpose of the that PR and this PR is similar, and I see that PR overwrite the vanilla file and later remove it, so the vanilla /etc/init.d/docker
file is missing. I suggest unify the 2 effort and I agree /etc/init.d/docker
may be not a good place.
Your argument "docker is a systemd service" does not relating because you are not using systemctl
to start service in your use case
root@sonic:~$ /usr/lib/docker/docker.sh start
In reply to: 570580898 [](ancestors = 570580898)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I see what you mean. I made a change to copy it once and not remove.
My comment was an argument against /etc/init.d/, as later docker debian packages removed this script and putting it back to /etc/init.d/ gives SONiC users wrong idea that user do: "service docker restart" but I doubt that will work correctly, at least won't consider systemd dependencies, so I choose /usr/lib/docker to hide this script from regular user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As comment
@lguohan could you also help review this PR? |
Signed-off-by: Stepan Blyshchak <stepanb@nvidia.com>
/AzurePipelines run |
Commenter does not have sufficient privileges for PR 5935 in repo Azure/sonic-buildimage |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Stepan Blyshchak <stepanb@nvidia.com>
@jleveque and @qiluo-msft comments were addressed. can you please review and approve? |
@jleveque as your previous approval was dismissed following the feedback changes requested by Qi. Can you please approve so we can move forward and merge? |
…-net#5935) - Why I did it To allow SONiC Package Migration during SONiC-2-SONiC upgrade we need to start docker daemon in chroot-ed environment in new SONiC filesystem. Later this script will be used to start dockerd in chroot environment on SONiC - How I did it Install a docker service script into /usr/lib/docker/ in SONiC filesystem. - How to verify it Install SONiC image on the switch, mount squashfs to some directory, mount overlay rw layer over squashfs, mount procfs and sysfs, mount docker library. Start the docker using: root@sonic:~$ /usr/lib/docker/docker.sh start Signed-off-by: Stepan Blyshchak <stepanb@nvidia.com>
…-net#5935) - Why I did it To allow SONiC Package Migration during SONiC-2-SONiC upgrade we need to start docker daemon in chroot-ed environment in new SONiC filesystem. Later this script will be used to start dockerd in chroot environment on SONiC - How I did it Install a docker service script into /usr/lib/docker/ in SONiC filesystem. - How to verify it Install SONiC image on the switch, mount squashfs to some directory, mount overlay rw layer over squashfs, mount procfs and sysfs, mount docker library. Start the docker using: root@sonic:~$ /usr/lib/docker/docker.sh start Signed-off-by: Stepan Blyshchak <stepanb@nvidia.com>
Later this script will be used to start dockerd in chroot environment on SONiC
Signed-off-by: Stepan Blyshchak stepanb@nvidia.com
This PR is part of SONiC Application Extension
- Why I did it
To allow SONiC Package Migration during SONiC-2-SONiC upgrade we need to start docker daemon in chroot-ed environment in new SONiC filesystem.
- How I did it
Install a docker service script into /usr/lib/docker/ in SONiC filesystem.
- How to verify it
Install SONiC image on the switch, mount squashfs to some directory, mount overlay rw layer over squashfs, mount procfs and sysfs, mount docker library. Start the docker using:
- Which release branch to backport (provide reason below if selected)
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)