-
Notifications
You must be signed in to change notification settings - Fork 558
Kubernetes: explicitly install deps during provisioning #3179
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
/retest |
Codecov Report
@@ Coverage Diff @@
## master #3179 +/- ##
=========================================
Coverage ? 52.32%
=========================================
Files ? 103
Lines ? 15452
Branches ? 0
=========================================
Hits ? 8086
Misses ? 6638
Partials ? 728 |
@andyzhangx FYI |
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.
Lgtm
@@ -153,8 +153,11 @@ function installEtcd() { | |||
retrycmd_if_failure 10 1 5 sudo etcdctl member update $MEMBER ${ETCD_PEER_URL} || exit $ERR_ETCD_CONFIG_FAIL | |||
} | |||
|
|||
function installDeps() { | |||
apt_get_install 20 30 180 apt-transport-https ca-certificates iptables iproute2 socat util-linux mount ebtables ethtool init-system-helpers nfs-common ceph-common conntrack glusterfs-client ipset jq || exit $ERR_APT_INSTALL_TIMEOUT |
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.
some packages are already installed in base image, install existing packages won't fail in this command, right?
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.
and would you also add package list source in comment:
https://github.com/kubernetes/kubernetes/blob/master/build/debian-hyperkube-base/Dockerfile#L25-L44
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.
To your 1st question, correct, if a package is already installed the command won’t exit non-zero
Fixes #3022 |
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.
lgtm
@jackfrancis I am also fixing the flexvolume issue in AKS, it requires
by running following is
|
@andyzhangx apt-get update should run first before install. |
|
@andyzhangx Thanks for the call-out. Just verified jq installation is working on a newly built cluster:
By the way, here are the commands we run for that https://github.com/Azure/acs-engine/blob/master/parts/k8s/kubernetesprovisionsource.sh#L74 |
What this PR does / why we need it: Outcome of this change:
Before:
After:
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
If applicable:
Release note: