-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Remove existing packages #92
Conversation
Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tao12345666333 If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hi @tao12345666333. Thanks for your PR. I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@@ -52,8 +52,8 @@ RUN chmod +x /usr/local/bin/clean-install | |||
RUN clean-install \ | |||
apt-transport-https ca-certificates curl software-properties-common gnupg2 lsb-release \ | |||
systemd systemd-sysv libsystemd0 \ | |||
iptables iproute2 ethtool socat util-linux mount ebtables udev kmod aufs-tools \ |
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.
hmm we explicitly need these though, what's the downside to installing them?
we may change the base image again in the future. 🤔
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.
Duplicate installations only add time, but because they already exist, there is nothing else going on.
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.
For the current situation, the base images are built ahead of time and it seems that there will be no problem. It’s just my habit of building an image that will remove unwanted things. 🤣
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 shouldn't add much time though? Ot just needs to look up that they are installed and then skip them.
The base image is intended to only be built when we change it / upgrade a package. Ideally not very often. I might be missing a use case though 🙃
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.
yes. I will close it.
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 again anyhow! always appreciate PRs :-)
…erator Fix deploy_tigera_operator in keos.yaml
Because we have changed the base image to
Ubuntu:18.04
, thebash
andmount
packages are already existed.xref:
#89
Signed-off-by: Jintao Zhang zhangjintao9020@gmail.com