Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Option to upgrade packages and reboot if necessary #2119

Merged
merged 1 commit into from
Jan 19, 2024
Merged

Option to upgrade packages and reboot if necessary #2119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions examples/default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,11 @@ caCerts:
# YOUR-ORGS-TRUSTED-CA-CERT-HERE
# -----END CERTIFICATE-----

# Upgrade the instance on boot
# Reboot after upgrade if required
# 🟢 Builtin default: false
upgradePackages: null

Copy link
Member

Choose a reason for hiding this comment

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

To avoid inflating the numbers of the limayaml fields, can we consider to support templating cloud-init yaml in limayaml?

Copy link
Member Author

Choose a reason for hiding this comment

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

Should have opened an issue first, before starting on the solution. The user just wants up-to-date packages.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want the user to have to learn cloud-init, since they sort of use lima.yaml to not have to do that

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 cloud-init support is a good feature for advanced users, just not required for this particular feature

Copy link
Member

Choose a reason for hiding this comment

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

I agree, I'm in favour of approving this PR; @AkihiroSuda are you going to veto it?

Copy link
Member

Choose a reason for hiding this comment

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

The user just wants up-to-date packages.

You can already do this with a dependency provisioning script (untested):

provision:
- mode: dependency
  script: |
    apt-get update
    apt-get upgrade
    if [ -f /var/run/reboot-required ]; then
        reboot
    fi

This script is executed as part of 30-install-packages.sh, just before additional packages are installed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was naively hoping this to be a oneliner, both in the lima.yaml and in the cloud-init.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, I'm in favour of approving this PR; @AkihiroSuda are you going to veto it?

@AkihiroSuda can you please let us know if you are opposed to making this change, and want to wait until we have some generic cloudInit mechanism? Because then we can stop arguing about this PR if it has no chance of getting merged. 😃

I think this PR provides reasonable functionality to expose via Lima directly without requiring the user to learn about cloud-init config files. We also simplify this to a single setting.

Also, contrary to my earlier comments, I don't think we should provide the packages field. So the upgrade_packages would be the only key I would expect to be added to lima.yaml.

containerd:
# Enable system-wide (aka rootful) containerd and its dependencies (BuildKit, Stargz Snapshotter)
# Note that `nerdctl.lima` only works in rootless mode; you have to use `lima sudo nerdctl ...`
Expand Down
15 changes: 15 additions & 0 deletions pkg/cidata/cidata.TEMPLATE.d/boot/00-reboot-if-required.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/bin/sh
set -eux

# Check if cloud-init forgot to reboot_if_required
# (only implemented for apt at the moment, not dnf)

if command -v dnf >/dev/null 2>&1; then
# dnf-utils needs to be installed, for needs-restarting
if dnf -h needs-restarting >/dev/null 2>&1; then
# needs-restarting returns "false" if needed (!)
if ! dnf needs-restarting -r >/dev/null 2>&1; then
systemctl reboot
fi
fi
fi
6 changes: 6 additions & 0 deletions pkg/cidata/cidata.TEMPLATE.d/user-data
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ growpart:
mode: auto
devices: ['/']

{{- if .UpgradePackages }}
package_update: true
package_upgrade: true
package_reboot_if_required: true
{{- end }}

{{- if or (eq .MountType "9p") (eq .MountType "virtiofs") }}
{{- if .Mounts }}
mounts:
Expand Down
1 change: 1 addition & 0 deletions pkg/cidata/cidata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ func GenerateISO9660(instDir, name string, y *limayaml.LimaYAML, udpDNSLocalPort
UID: uid,
Home: fmt.Sprintf("/home/%s.linux", u.Username),
GuestInstallPrefix: *y.GuestInstallPrefix,
UpgradePackages: *y.UpgradePackages,
Containerd: Containerd{System: *y.Containerd.System, User: *y.Containerd.User},
SlirpNICName: networks.SlirpNICName,

Expand Down
1 change: 1 addition & 0 deletions pkg/cidata/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type TemplateArgs struct {
MountType string
Disks []Disk
GuestInstallPrefix string
UpgradePackages bool
Containerd Containerd
Networks []Network
SlirpNICName string
Expand Down
10 changes: 10 additions & 0 deletions pkg/limayaml/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,16 @@ func FillDefault(y, d, o *LimaYAML, filePath string) {
y.GuestInstallPrefix = ptr.Of(defaultGuestInstallPrefix())
}

if y.UpgradePackages == nil {
y.UpgradePackages = d.UpgradePackages
}
if o.UpgradePackages != nil {
y.UpgradePackages = o.UpgradePackages
}
if y.UpgradePackages == nil {
y.UpgradePackages = ptr.Of(false)
}

if y.Containerd.System == nil {
y.Containerd.System = d.Containerd.System
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/limayaml/defaults_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ func TestFillDefault(t *testing.T) {
Memory: ptr.Of(defaultMemoryAsString()),
Disk: ptr.Of(defaultDiskSizeAsString()),
GuestInstallPrefix: ptr.Of(defaultGuestInstallPrefix()),
UpgradePackages: ptr.Of(false),
Containerd: Containerd{
System: ptr.Of(false),
User: ptr.Of(true),
Expand Down Expand Up @@ -309,6 +310,7 @@ func TestFillDefault(t *testing.T) {
{Name: "data"},
},
GuestInstallPrefix: ptr.Of("/opt"),
UpgradePackages: ptr.Of(true),
Containerd: Containerd{
System: ptr.Of(true),
User: ptr.Of(false),
Expand Down Expand Up @@ -498,6 +500,7 @@ func TestFillDefault(t *testing.T) {
{Name: "test"},
},
GuestInstallPrefix: ptr.Of("/usr"),
UpgradePackages: ptr.Of(true),
Containerd: Containerd{
System: ptr.Of(true),
User: ptr.Of(false),
Expand Down
1 change: 1 addition & 0 deletions pkg/limayaml/limayaml.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type LimaYAML struct {
Audio Audio `yaml:"audio,omitempty" json:"audio,omitempty"`
Video Video `yaml:"video,omitempty" json:"video,omitempty"`
Provision []Provision `yaml:"provision,omitempty" json:"provision,omitempty"`
UpgradePackages *bool `yaml:"upgradePackages,omitempty" json:"upgradePackages,omitempty"`
Containerd Containerd `yaml:"containerd,omitempty" json:"containerd,omitempty"`
GuestInstallPrefix *string `yaml:"guestInstallPrefix,omitempty" json:"guestInstallPrefix,omitempty"`
Probes []Probe `yaml:"probes,omitempty" json:"probes,omitempty"`
Expand Down
Loading