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

feat: run accelerated unattended-upgrade at node creation time #4217

Merged
merged 4 commits into from
Feb 3, 2021
Merged
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
1 change: 1 addition & 0 deletions docs/topics/clusterdefinitions.md
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,7 @@ A cluster can have 0 to 12 agent pool profiles. Agent Pool Profiles are used for
| adminUsername | yes | Describes the username to be used on all linux clusters |
| ssh.publicKeys[].keyData | yes | The public SSH key used for authenticating access to all Linux nodes in the cluster |
| secrets | no | Specifies an array of key vaults to pull secrets from and what secrets to pull from each |
| runUnattendedUpgradesOnBootstrap | no | Invoke an unattended-upgrade when each Linux node VM comes online for the first time. In practice this is accomplished by performing an `apt-get update`, followed by an `apt-get dist-upgrade`, to fetch updated apt configuration, and install all available downstream package updates, respectively. |
| customSearchDomain.name | no | describes the search domain to be used on all linux clusters |
| customSearchDomain.realmUser | no | describes the realm user with permissions to update dns registries on Windows Server DNS |
| customSearchDomain.realmPassword | no | describes the realm user password to update dns registries on Windows Server DNS |
Expand Down
12 changes: 12 additions & 0 deletions parts/k8s/cloud-init/artifacts/cse_helpers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,18 @@ apt_get_dist_upgrade() {
done
echo Executed apt-get dist-upgrade $i times
}
unattended_upgrade() {
retries=10
for i in $(seq 1 $retries); do
wait_for_apt_locks
/usr/bin/unattended-upgrade && break ||
if [ $i -eq $retries ]; then
return 1
else sleep 5
fi
done
echo Executed unattended-upgrade $i times
}
systemctl_restart() {
retries=$1; wait_sleep=$2; timeout=$3 svcname=$4
for i in $(seq 1 $retries); do
Expand Down
4 changes: 4 additions & 0 deletions parts/k8s/cloud-init/artifacts/cse_main.sh
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,10 @@ if [[ $OS == $UBUNTU_OS_NAME ]]; then
fi
{{end}}

{{- if RunUnattendedUpgrades}}
apt_get_update && apt_get_dist_upgrade && unattended_upgrade
Copy link
Member Author

Choose a reason for hiding this comment

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

In practice (I think) the unattended_upgrade invocation here is superfluous (update and dist-upgrade will effectively do the deed; including it here to be extra explicit.

perhaps @Michael-Sinz can confirm if this is sane

Mainly I trust our apt_get_update and apt_get_dist_upgrade functions to definitively accomplish those tasks over silently calling /usr/bin/unattended-upgrade. The latter (by design) silently fails single invocations (because it knows it'll be invoked again — it's not in a rush) if, for example, various apt locks are held (there are probably other reasons).

Copy link
Collaborator

Choose a reason for hiding this comment

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

The big difference between unattended-upgrades and apt-get dist-upgrade is the list of things it will install.

Unattended upgrades is constrained to the list of updates that are deemed safe and vital for security/reliability. They are not minor feature updates unless that was required for security. (This is the default and recommended configuration for unattended-upgrade)

For example, on a test VM, I just logged in and noticed this right now:

58 packages can be updated.
4 updates are security updates.

After running unattended-upgrades on that machine (which normally cron does for me on regular basis), the login looks like this:

54 packages can be updated.
0 updates are security updates.    

This is very different from a full apt-get update/apt-get upgrade (which itself is less than apt-get dist-upgrade)

The actual ubuntu unattended-upgrade command will return an error if it fails to complete an update. But it is constrained to the security updates.

Another good thing about unattended-upgrades is that it does set the unattended settings for apt/apt-get/dpkg such that it should not hang (albeit, packages can still cause this problems but that is rare in the security patches).

Which to use is really a question of risks. Balancing all of them.

We run unattended-upgrade on a regular basis because we can trust it at scale.

Copy link
Collaborator

Choose a reason for hiding this comment

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

PS - It is redundant to run unattended-upgrade after having done the full upgrade or dist-upgrade.

It may be useful to do unattended-upgrade first just to be sure they complete before getting into the larger set (both from a security standpoint and an ability to complete them)

So I would not run unattended afterwards.

Copy link
Member Author

Choose a reason for hiding this comment

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

This all makes sense. What's perplexing is that, in practice, simply adding a "wait for apt locks and then run unattended-upgrade" during CSE does not in my tests produce the expected /var/run/reboot-required (a symptom of critical security updates arriving) outcome.

I'm going to try apt-get update && unattended-upgrade next.

{{- end}}

if [ -f /var/run/reboot-required ]; then
trace_info "RebootRequired" "reboot=true"
/bin/bash -c "shutdown -r 1 &"
Expand Down
1 change: 1 addition & 0 deletions pkg/api/converterfromapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ func convertLinuxProfileToVLabs(obj *LinuxProfile, vlabsProfile *vlabs.LinuxProf
vlabsProfile.CustomNodesDNS = &vlabs.CustomNodesDNS{}
vlabsProfile.CustomNodesDNS.DNSServer = obj.CustomNodesDNS.DNSServer
}
vlabsProfile.RunUnattendedUpgradesOnBootstrap = obj.RunUnattendedUpgradesOnBootstrap
}

func convertWindowsProfileToVLabs(api *WindowsProfile, vlabsProfile *vlabs.WindowsProfile) {
Expand Down
1 change: 1 addition & 0 deletions pkg/api/convertertoapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ func convertVLabsLinuxProfile(vlabs *vlabs.LinuxProfile, api *LinuxProfile) {
api.CustomNodesDNS = &CustomNodesDNS{}
api.CustomNodesDNS.DNSServer = vlabs.CustomNodesDNS.DNSServer
}
api.RunUnattendedUpgradesOnBootstrap = vlabs.RunUnattendedUpgradesOnBootstrap
}

func convertVLabsWindowsProfile(vlabs *vlabs.WindowsProfile, api *WindowsProfile) {
Expand Down
13 changes: 7 additions & 6 deletions pkg/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,13 @@ type LinuxProfile struct {
SSH struct {
PublicKeys []PublicKey `json:"publicKeys"`
} `json:"ssh"`
Secrets []KeyVaultSecrets `json:"secrets,omitempty"`
Distro Distro `json:"distro,omitempty"`
ScriptRootURL string `json:"scriptroot,omitempty"`
CustomSearchDomain *CustomSearchDomain `json:"customSearchDomain,omitempty"`
CustomNodesDNS *CustomNodesDNS `json:"CustomNodesDNS,omitempty"`
IsSSHKeyAutoGenerated *bool `json:"isSSHKeyAutoGenerated,omitempty"`
Secrets []KeyVaultSecrets `json:"secrets,omitempty"`
Distro Distro `json:"distro,omitempty"`
ScriptRootURL string `json:"scriptroot,omitempty"`
CustomSearchDomain *CustomSearchDomain `json:"customSearchDomain,omitempty"`
CustomNodesDNS *CustomNodesDNS `json:"CustomNodesDNS,omitempty"`
IsSSHKeyAutoGenerated *bool `json:"isSSHKeyAutoGenerated,omitempty"`
RunUnattendedUpgradesOnBootstrap *bool `json:"runUnattendedUpgradesOnBootstrap,omitempty"`
}

// PublicKey represents an SSH key for LinuxProfile
Expand Down
9 changes: 5 additions & 4 deletions pkg/api/vlabs/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,11 @@ type LinuxProfile struct {
SSH struct {
PublicKeys []PublicKey `json:"publicKeys" validate:"required,min=1"`
} `json:"ssh" validate:"required"`
Secrets []KeyVaultSecrets `json:"secrets,omitempty"`
ScriptRootURL string `json:"scriptroot,omitempty"`
CustomSearchDomain *CustomSearchDomain `json:"customSearchDomain,omitempty"`
CustomNodesDNS *CustomNodesDNS `json:"customNodesDNS,omitempty"`
Secrets []KeyVaultSecrets `json:"secrets,omitempty"`
ScriptRootURL string `json:"scriptroot,omitempty"`
CustomSearchDomain *CustomSearchDomain `json:"customSearchDomain,omitempty"`
CustomNodesDNS *CustomNodesDNS `json:"customNodesDNS,omitempty"`
RunUnattendedUpgradesOnBootstrap *bool `json:"runUnattendedUpgradesOnBootstrap,omitempty"`
}

// PublicKey represents an SSH key for LinuxProfile
Expand Down
6 changes: 6 additions & 0 deletions pkg/engine/template_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,12 @@ func getContainerServiceFuncMap(cs *api.ContainerService) template.FuncMap {
"GetLinuxCSELogPath": func() string {
return linuxCSELogPath
},
"RunUnattendedUpgrades": func() bool {
if cs.Properties.LinuxProfile != nil {
return to.Bool(cs.Properties.LinuxProfile.RunUnattendedUpgradesOnBootstrap)
}
return false
},
"OpenBraces": func() string {
return "{{"
},
Expand Down
16 changes: 16 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.

1 change: 1 addition & 0 deletions test/e2e/test_cluster_configs/everything.json
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@
}
],
"linuxProfile": {
"runUnattendedUpgradesOnBootstrap": true,
"adminUsername": "azureuser",
"ssh": {
"publicKeys": [
Expand Down