Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

fix: automatically get updated apt keys via CSE #2022

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions parts/k8s/cloud-init/artifacts/cse_helpers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,21 @@ apt_get_dist_upgrade() {
echo Executed apt-get dist-upgrade $i times
wait_for_apt_locks
}
apt_fix_keys() {
retries=10
output=/tmp/apt-fix-keys.out
for i in $(seq 1 $retries); do
wait_for_apt_locks
! (apt-get update | tee $output | grep NO_PUBKEY) && \
Copy link
Contributor

@CecileRobertMichon CecileRobertMichon Sep 24, 2019

Choose a reason for hiding this comment

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

you could do

for K in $(apt-key list | grep expired | cut -d'/' -f2 | cut -d' ' -f1); do sudo apt-key adv --recv-keys --keyserver keyserver.ubuntu.com $K; done

Copy link
Contributor

Choose a reason for hiding this comment

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

running two extra apt-get update operations in every single CSE (even with VHD) would be a de-optimization

Copy link
Member Author

Choose a reason for hiding this comment

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

The current nvidia scenario does not show up in apt-key list | grep expired. It's only when you run apt-get update that you are able to derive the key that needs fixing.

Copy link
Contributor

Choose a reason for hiding this comment

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

in that case should we only do the update in nvidia node? I think doing an update for everyone will take a hit on provisioning latency.

Copy link
Member Author

Choose a reason for hiding this comment

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

it's attempting the apt-get update in the background (and it's best-effort, it won't error out), so it won't affect provisioning latency

cat $output && break || \
apt-key adv --keyserver keyserver.ubuntu.com --recv-keys $(apt-get update | grep NO_PUBKEY -m 1 | awk -F "NO_PUBKEY" '{print $2}')
Copy link
Member Author

Choose a reason for hiding this comment

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

@palma21 Can we add keyserver.ubuntu.com to the "no egress" whitelist? This URL is used to remotely retrieve expired/missing keys in the apt repo configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

if we have a cluster with no egress lock down, I assume it doesn't need apt operation, why we need this whitelist?

Copy link
Member Author

Choose a reason for hiding this comment

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

My understanding is there are apt sources in the whitelist to allow no egress clusters to update themselves (kernel, azsecpack, etc).

If not, then AKS Engine is definitely delivering an apt configuration that doesn't make sense in the no egress scenario.

if [ $i -eq $retries ]; then
return 1
else sleep 1
fi
done
echo Executed apt-get update NO_PUBKEY fix $i times
}
systemctl_restart() {
retries=$1; wait_sleep=$2; timeout=$3 svcname=$4
for i in $(seq 1 $retries); do
Expand Down
2 changes: 2 additions & 0 deletions parts/k8s/cloud-init/artifacts/cse_main.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ done
sed -i "/#HELPERSEOF/d" $script_lib
source $script_lib

apt_fix_keys &

install_script=/opt/azure/containers/provision_installs.sh
wait_for_file 3600 1 $install_script || exit $ERR_FILE_WATCH_TIMEOUT
source $install_script
Expand Down
17 changes: 17 additions & 0 deletions pkg/engine/templates_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.