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

[sonic_debian_extension] add docker script to SONiC filesystem #5935

Merged

Conversation

stepanblyschak
Copy link
Collaborator

Later this script will be used to start dockerd in chroot environment on SONiC

Signed-off-by: Stepan Blyshchak [email protected]

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:

root@sonic:~$ /usr/lib/docker/docker.sh start

- Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006

- Description for the changelog

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

Later this script will be used to start dockerd in chroot environment on SONiC

Signed-off-by: Stepan Blyshchak <[email protected]>
@stepanblyschak
Copy link
Collaborator Author

retest broadcom please

@stepanblyschak
Copy link
Collaborator Author

@qiluo-msft Could you please review?

jleveque
jleveque previously approved these changes Feb 4, 2021
@@ -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
Copy link
Collaborator

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/

Copy link
Collaborator Author

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.

Copy link
Collaborator

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)

Copy link
Collaborator Author

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.

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 comment

@qiluo-msft
Copy link
Collaborator

@lguohan could you also help review this PR?

Signed-off-by: Stepan Blyshchak <[email protected]>
qiluo-msft
qiluo-msft previously approved these changes Feb 12, 2021
@stepanblyschak
Copy link
Collaborator Author

/AzurePipelines run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 5935 in repo Azure/sonic-buildimage

@renukamanavalan
Copy link
Contributor

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Stepan Blyshchak <[email protected]>
@liat-grozovik
Copy link
Collaborator

@jleveque and @qiluo-msft comments were addressed. can you please review and approve?
@lguohan kindly reminder as your feedback was also asked.
I would like to move forward and merge it (as no change effect on existing behaviour)

@liat-grozovik
Copy link
Collaborator

@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?

@liat-grozovik liat-grozovik merged commit 2b8941e into sonic-net:master Mar 14, 2021
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-buildimage that referenced this pull request May 23, 2021
…-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 <[email protected]>
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
…-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 <[email protected]>
@stepanblyschak stepanblyschak deleted the include_docker_script_in_sonic branch September 23, 2022 13:33
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.

5 participants