Skip to content

[mgmt docker] Clean up docker-sonic-mgmt dockerfile#4759

Merged
daall merged 3 commits intosonic-net:masterfrom
daall:mgmt_docker_cleanup
Jun 16, 2020
Merged

[mgmt docker] Clean up docker-sonic-mgmt dockerfile#4759
daall merged 3 commits intosonic-net:masterfrom
daall:mgmt_docker_cleanup

Conversation

@daall
Copy link
Copy Markdown
Contributor

@daall daall commented Jun 11, 2020

  • Alphabetize dependencies to prevent duplicates
  • Remove uneccesary git clone

Signed-off-by: Danny Allen daall@microsoft.com

- Why I did it
I did it to reduce the size of the sonic-mgmt docker image as well as try to improve build performance.

- How I did it
I did 3 main things:

  • I alphabetized the list of apt and pip dependencies to remove duplicates and hopefully prevent duplicates from being added in the future.
  • Before we were installing cffi from apt, then purging some of its dependencies, then re-installing it from pip. So, I simplified this process by skipping the step to install cffi from apt and just install it in pip.
  • I removed the git clone of sonic-mgmt at the end. Since people mount their own sonic-mgmt for testing, this step is unnecessary and just adds a lot to the size of the docker image.

- How to verify it
Built and ran locally, no issues so far.

- Description for the changelog

Cleans up docker-sonic-mgmt dockerfile

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

@daall
[mgmt docker] Clean-up mgmt dockerfile
4907228 
- Alphabetize dependencies to prevent duplicates
- Remove unneccesary git clone

Signed-off-by: Danny Allen <daall@microsoft.com>
@daall daall requested review from lguohan and yxieca June 11, 2020 20:23
yxieca
yxieca reviewed Jun 12, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@yxieca yxieca left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just read your PR comments again. Not a big deal, but if I do it, I would create 3 PRs.

Or you can stick with one PR but 3 commits to make review easier.

You don't need to split now.

yxieca
yxieca previously approved these changes Jun 12, 2020
View reviewed changes
@daall daall marked this pull request as ready for review June 15, 2020 20:56
@daall
Copy link
Copy Markdown
Contributor Author

daall commented Jun 15, 2020

retest mellanox please

@daall
Copy link
Copy Markdown
Contributor Author

daall commented Jun 16, 2020

retest vsimage please

@daall daall merged commit 364511a into sonic-net:master Jun 16, 2020
@daall daall deleted the mgmt_docker_cleanup branch June 16, 2020 18:32
rlhui pushed a commit that referenced this pull request Jul 7, 2020
@daall @rlhui
[mgmt docker] Clean up docker-sonic-mgmt dockerfile (#4759)
fbdd775 
- Alphabetize dependencies to prevent duplicates
- Remove unneccesary git clone

Signed-off-by: Danny Allen <daall@microsoft.com>
praveen-li pushed a commit to praveen-li/sonic-buildimage that referenced this pull request May 28, 2021
@daall @samaity
[mgmt docker] Clean up docker-sonic-mgmt dockerfile (sonic-net#4759)
29baefb 
- Alphabetize dependencies to prevent duplicates
- Remove unneccesary git clone

Signed-off-by: Danny Allen <daall@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yxieca yxieca yxieca left review comments

@lguohan lguohan Awaiting requested review from lguohan lguohan is a code owner

+1 more reviewer

@jleveque jleveque jleveque approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Cleanup 💧

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@daall @jleveque @yxieca