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.

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

fix: add a check for overlay mounts in installer pre-flight checks #3425

Merged
merged 1 commit into from
Apr 5, 2021

Conversation

smira
Copy link
Member

@smira smira commented Apr 1, 2021

Overlay mount in mountinfo don't show up as mounts for any particular
block device, so the existing check doesn't catch them.

This was discovered as our current master can't upgrade because of
overlay mount for /opt and apid image in /opt/apid (which will be
fixed in a separate PR).

Without the check, installer fails on resetting partition table for the
disk effectively wiping the node (device or resourse busy error).

Signed-off-by: Andrey Smirnov smirnov.andrey@gmail.com

@smira smira added this to the 0.10 milestone Apr 1, 2021
@smira
Copy link
Member Author

smira commented Apr 1, 2021

/approve

@smira
Copy link
Member Author

smira commented Apr 2, 2021

/rebase

smira added a commit to smira/talos that referenced this pull request Apr 2, 2021
Container rootfs should be writeable as containerd mounts standard
filesystems `/proc` et al.

When `/opt` was used as a root of container filesystem this results in a
problem: Talos overlay mounts `/opt` on `/var/system` which means that
as long as `apid` running `/var` can't be unmounted which breaks
upgrades.

So instead use `/system/libexec` as rootfs for the containers, `/system`
is `tmpfs`, and bind-mount actualy executable (`/sbin/init`, machined)
into rootfs.

This fixes upgrades for 0.10.

See also siderolabs#3425

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
smira added a commit to smira/talos that referenced this pull request Apr 2, 2021
Container rootfs should be writeable as containerd mounts standard
filesystems `/proc` et al.

When `/opt` was used as a root of container filesystem this results in a
problem: Talos overlay mounts `/opt` on `/var/system` which means that
as long as `apid` running `/var` can't be unmounted which breaks
upgrades.

So instead use `/system/libexec` as rootfs for the containers, `/system`
is `tmpfs`, and bind-mount actually executable (`/sbin/init`, machined)
into rootfs.

This fixes upgrades for 0.10.

See also siderolabs#3425

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
talos-bot pushed a commit that referenced this pull request Apr 2, 2021
Container rootfs should be writeable as containerd mounts standard
filesystems `/proc` et al.

When `/opt` was used as a root of container filesystem this results in a
problem: Talos overlay mounts `/opt` on `/var/system` which means that
as long as `apid` running `/var` can't be unmounted which breaks
upgrades.

So instead use `/system/libexec` as rootfs for the containers, `/system`
is `tmpfs`, and bind-mount actually executable (`/sbin/init`, machined)
into rootfs.

This fixes upgrades for 0.10.

See also #3425

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
@smira
Copy link
Member Author

smira commented Apr 2, 2021

/rebase

@smira smira marked this pull request as draft April 2, 2021 16:32
Overlay mount in `mountinfo` don't show up as mounts for any particular
block device, so the existing check doesn't catch them.

This was discovered as our current master can't upgrade because of
overlay mount for `/opt` and `apid` image in `/opt/apid` (which will be
fixed in a separate PR).

Without the check, installer fails on resetting partition table for the
disk effectively wiping the node (`device or resource busy` error).

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
@smira smira marked this pull request as ready for review April 5, 2021 19:34
@smira
Copy link
Member Author

smira commented Apr 5, 2021

/lgtm

@talos-bot talos-bot merged commit bd5ae1e into siderolabs:master Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants