Skip to content

wire: wireguard interface#233

Open
sorah wants to merge 1 commit intomasterfrom
wire/wg
Open

wire: wireguard interface#233
sorah wants to merge 1 commit intomasterfrom
wire/wg

Conversation

@sorah
Copy link
Copy Markdown
Member

@sorah sorah commented Apr 12, 2026

@sorah sorah requested a review from hanazuki April 12, 2026 05:24
Comment on lines +1 to +10
[Unit]
Description=rk-move-overlay-wg-iface
After=systemd-nspawn@overlay.service

[Service]
Type=oneshot
ExecStart=/usr/bin/rk-move-overlay-wg-iface

[Install]
WantedBy=systemd-nspawn@overlay.service
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The rk-move-overlay-wg-iface.service does not re-run on container restart, which will break WireGuard tunnels because the interfaces are not moved to the host namespace.
Severity: HIGH

Suggested Fix

To ensure the service's lifecycle is tied to the container's, add RemainAfterExit=yes to the [Service] section and PartOf=systemd-nspawn@overlay.service to the [Install] section of the service file. This will cause the service to be stopped and restarted along with the container.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
itamae/roles/wire/templates/etc/systemd/system/rk-move-overlay-wg-iface.service#L1-L10

Potential issue: The systemd service `rk-move-overlay-wg-iface.service` is configured as
`Type=oneshot` but lacks the `RemainAfterExit=yes` directive and a proper lifecycle
dependency like `PartOf=`. As a result, the service runs only once when the
`systemd-nspawn@overlay.service` container initially starts. If the container restarts
for any reason (e.g., a crash or update), this service will not be re-triggered. This
causes newly created WireGuard interfaces to remain within the container's network
namespace instead of being moved to the host, rendering the WireGuard tunnels
non-functional until a manual intervention or a full host reboot occurs.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +9 to +15
10.times do |i|
if system(*%w(systemd-run --machine overlay --wait --quiet --pipe --collect id))
break
else
sleep 2
end
end
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The retry loop in rk-move-overlay-wg-iface fails silently if the container never comes online, leading to a less informative error message from a subsequent command.
Severity: MEDIUM

Suggested Fix

After the 10.times loop, add a check to verify that the container is actually online. If it is not, raise an explicit exception with a clear error message, such as 'Container failed to come online after 10 attempts'.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: itamae/roles/wire/templates/usr/bin/rk-move-overlay-wg-iface#L9-L15

Potential issue: The script `rk-move-overlay-wg-iface` contains a retry loop that
attempts to connect to the 'overlay' container 10 times. If all attempts fail, the loop
exits silently without raising an error. A subsequent command on line 17 then fails, but
its error message (`Failed to connect to machine 'overlay'`) is less specific than an
explicit error about the container failing to become available after all retries. This
silent failure of the loop degrades diagnostic information, making it harder to debug
why the container failed to start.

Did we get this right? 👍 / 👎 to inform future reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant