-
Notifications
You must be signed in to change notification settings - Fork 592
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
chore(agw): cleanup apt installs in agw Dockerfiles #13932
chore(agw): cleanup apt installs in agw Dockerfiles #13932
Conversation
Thanks for opening a PR! 💯
Howto
More infoPlease take a moment to read through the Magma project's
If this is your first Magma PR, also consider reading
|
openvswitch-common \ | ||
openvswitch-datapath-dkms \ | ||
openvswitch-switch |
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.
Do we want a && rm -rf /var/lib/apt/lists/*
here?
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 added it.
libopenvswitch \ | ||
openvswitch-datapath-dkms \ | ||
openvswitch-common \ | ||
openvswitch-switch \ | ||
bcc-tools \ | ||
td-agent-bit \ | ||
wireguard |
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.
Do we want a && rm -rf /var/lib/apt/lists/*
here?
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 added it.
nitpick: personally I prefer meaningful |
prometheus-cpp-dev \ | ||
liblfds710 | ||
RUN rm /etc/apt/sources.list.d/magma.list | ||
&& rm /etc/apt/sources.list.d/magma.list |
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 discussed offlne: The RUN statement will remain untouched, and the rm will target /var/lib/apt/lists/*
instead.
0578127
to
753d705
Compare
Sort all installed packages alphabetically, remove duplicates, and add explicit cleanup after installing Signed-off-by: Sebastian Wolf <sebastian.wolf@tngtech.com>
753d705
to
a1367cd
Compare
Signed-off-by: Sebastian Wolf sebastian.wolf@tngtech.com
Summary
This PR sorts all installed packages alphanumerically for better maintainability. Duplicate packages are removed. An explicit cleanup is introduced after every install, where it was not done yet.
Test Plan
Additional Information