gvforwarder as a systemd service by praveenkumar · Pull Request #1052 · crc-org/snc

praveenkumar · 2025-04-29T11:33:51Z

Create a tap device using nmcli with a hardcoded mac address
Start gvforwarder systemd service which will use this device

Summary by Sourcery

Configure gvforwarder (gvisor-tap-vsock) to run as a native systemd service instead of a container.

Build:

Create a tap0 network interface using nmcli with a fixed MAC address during VM setup.
Install the gvforwarder binary directly onto the host system.
Configure and enable a native systemd service (gv-user-network@.service) to manage the gvforwarder process.
Remove the previous container-based systemd service for gvisor-tap-vsock.

sourcery-ai · 2025-04-29T11:33:55Z

Reviewer's Guide by Sourcery

This pull request refactors the setup for the gvisor-tap-vsock forwarder. It adds the creation of a tap device using nmcli before setting up the service. The approach for running the forwarder is changed from generating a systemd service directly from a podman container to extracting the forwarder binary from the container and running it via a custom systemd service template unit.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change	Details	Files
Create a tap device interface using nmcli.	Add nmcli command to create a 'tun' type connection named 'tap0' with a specific MAC address.	`createdisk.sh`
Refactor gvisor-tap-vsock setup to use a custom systemd service template.	Remove old podman create and generate systemd commands. Add podman create command to get the image. Add podman cp command to copy the gvforwarder binary. Add podman rm command to clean up the temporary container. Add tee command to write the custom gv-user-network@.service unit file. Remove old systemctl enable for the podman-generated service. Add systemctl enable for the new gv-user-network@tap0.service instance.	`createdisk.sh`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey @praveenkumar - I've reviewed your changes - here's some feedback:

Overall Comments:

Consider making the hardcoded MAC address 5A:94:EF:E4:0C:EE configurable instead of embedding it directly in the script.
Extracting the gvforwarder binary from a container might be fragile; consider alternative methods like packaging or building from source.
Creating the systemd unit file via a tee heredoc could be replaced by copying a dedicated unit file for better readability.

Here's what I looked at during the review

🟡 General issues: 1 issue found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

sourcery-ai · 2025-04-29T11:35:13Z

createdisk.sh

+# create the tap device interface with specified mac address
+# this mac address is used to allocate a specific IP to the VM
+# when tap device is in use.
+${SSH} core@${VM_IP} 'sudo bash -x -s' <<EOF
+  nmcli connection add type tun ifname tap0 con-name tap0 mode tap autoconnect yes 802-3-ethernet.cloned-mac-address 5A:94:EF:E4:0C:EE
+EOF


suggestion (bug_risk): Consider handling potential errors when adding the tap device.

Capture and check the nmcli command’s exit status (e.g., check $?) so any failures in creating the tap interface are detected and handled.

Suggested change

# create the tap device interface with specified mac address

# this mac address is used to allocate a specific IP to the VM

# when tap device is in use.

${SSH} core@${VM_IP} 'sudo bash -x -s' <<EOF

nmcli connection add type tun ifname tap0 con-name tap0 mode tap autoconnect yes 802-3-ethernet.cloned-mac-address 5A:94:EF:E4:0C:EE

EOF

# create the tap device interface with specified mac address

# this mac address is used to allocate a specific IP to the VM

# when tap device is in use.

${SSH} core@${VM_IP} 'sudo bash -x -s' <<EOF

nmcli connection add type tun ifname tap0 con-name tap0 mode tap autoconnect yes 802-3-ethernet.cloned-mac-address 5A:94:EF:E4:0C:EE

status=\$?

if [ \$status -ne 0 ]; then

echo "Error: Failed to add tap device interface. nmcli exit status: \$status"

exit \$status

fi

EOF

praveenkumar · 2025-04-29T15:26:55Z

/retest

cfergeau · 2025-04-30T08:48:46Z

Is it the same as #1003 but against a different branch? or are there some differences?

praveenkumar · 2025-04-30T09:59:34Z

Is it the same as #1003 but against a different branch? or are there some differences?

It is same but against release-4.18 branch.

praveenkumar · 2025-05-01T10:50:56Z

/retest

cfergeau · 2025-05-05T09:31:01Z

createdisk.sh

+Description=gvisor-tap-vsock Network Traffic Forwarder
+After=NetworkManager.service
+BindsTo=sys-devices-virtual-net-%i.device
+After=sys-devices-virtual-net-%i.device


Do we need to add Before=nodeip-configuration.service as in #1054 ?

I don't know but I can try to do that also.

anjannath · 2025-05-12T06:32:29Z

createdisk.sh

+  tee /etc/systemd/system/gv-user-network@.service <<TEE
+[Unit]
+Description=gvisor-tap-vsock Network Traffic Forwarder
+After=NetworkManager.service


Should this be Before=NetworkManager.service ? otherwise it starts very late after boot

@anjannath networkManager is responsible to activate tun/tap device so it that is not even active how would this work.

Should this be Before=NetworkManager.service ? otherwise it starts very late after boot

Is it NetworkManager.service which would start late? or network-online?
Since we have After=sys-devices-virtual-net-%i.device , do we even need the reference to NetworkManager.service?

I spoke with Anjan, and the TAP creation should be independent of the network which configures. But the current, After is too undetermined, as it can start at any point after; anything with priority or dependency goes first.

In short, rather use Before than After statements, as that ensures the order.

openshift-ci · 2025-05-13T10:15:19Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from praveenkumar. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

- Create a tap device using nmcli with a hardcoded mac address - Start gvforwarder systemd service which will use this device Signed-off-by: vyasgun <vyasgun20@gmail.com>

Signed-off-by: Praveen Kumar <kumarpraveen.nitdgp@gmail.com>

praveenkumar · 2025-05-14T04:23:23Z

/retest

gbraad · 2025-05-14T06:33:24Z

createdisk.sh

 BASE_DOMAIN=$(${JQ} -r .clusterInfo.baseDomain $INSTALL_DIR/crc-bundle-info.json)
 BUNDLE_TYPE=$(${JQ} -r .type $INSTALL_DIR/crc-bundle-info.json)
-ADDITIONAL_PACKAGES="cloud-init"
+ADDITIONAL_PACKAGES="cloud-init gvisor-tap-vsock-gvforwarder"


This does mean we need to keep this package up to date along with Podman's timeline/schedule.

@gbraad This is subpackage and I think as of now it is not used by podman machine but build everytime there is release of gvisor-tap-vsock so we can just use it without maintaining (like building ourself).

praveenkumar · 2025-05-16T05:47:18Z

/cherry-pick release-4.19

openshift-cherrypick-robot · 2025-05-16T05:47:21Z

@praveenkumar: once the present PR merges, I will cherry-pick it on top of release-4.19 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.19

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

praveenkumar · 2025-05-16T05:47:27Z

/cherry-pick master

openshift-cherrypick-robot · 2025-05-16T05:47:30Z

@praveenkumar: once the present PR merges, I will cherry-pick it on top of master in a new PR and assign it to you.

Details

In response to this:

/cherry-pick master

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-cherrypick-robot · 2025-05-16T05:48:09Z

@praveenkumar: new pull request created: #1060

Details

In response to this:

/cherry-pick release-4.19

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-cherrypick-robot · 2025-05-16T05:48:44Z

@praveenkumar: new pull request created: #1061

Details

In response to this:

/cherry-pick master

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

openshift-ci bot added the do-not-merge/work-in-progress label Apr 29, 2025

openshift-ci bot requested review from cfergeau and gbraad April 29, 2025 11:33

sourcery-ai bot reviewed Apr 29, 2025

View reviewed changes

praveenkumar force-pushed the gvforwarder-service branch from 0c68848 to 19d1eff Compare April 30, 2025 06:54

cfergeau reviewed May 5, 2025

View reviewed changes

praveenkumar self-assigned this May 7, 2025

praveenkumar added this to Project planning: crc May 7, 2025

praveenkumar moved this to Work In Progress in Project planning: crc May 7, 2025

openshift-merge-robot added the needs-rebase label May 7, 2025

anjannath reviewed May 12, 2025

View reviewed changes

gbraad mentioned this pull request May 13, 2025

make: add support for building multi-arch image manifest containers/gvisor-tap-vsock#508

Closed

praveenkumar force-pushed the gvforwarder-service branch from 19d1eff to 6c6f66f Compare May 13, 2025 10:15

openshift-merge-robot removed the needs-rebase label May 13, 2025

praveenkumar force-pushed the gvforwarder-service branch from 6c6f66f to 7669205 Compare May 13, 2025 12:28

vyasgun and others added 2 commits May 13, 2025 21:03

gvforwarder as a systemd service

3879ea2

- Create a tap device using nmcli with a hardcoded mac address - Start gvforwarder systemd service which will use this device Signed-off-by: vyasgun <vyasgun20@gmail.com>

Use gvisor-tap-vsock-gvforwarder package instead image

f04603b

Signed-off-by: Praveen Kumar <kumarpraveen.nitdgp@gmail.com>

praveenkumar force-pushed the gvforwarder-service branch from 7669205 to f04603b Compare May 13, 2025 15:34

gbraad reviewed May 14, 2025

View reviewed changes

praveenkumar changed the title ~~[WIP] gvforwarder as a systemd service~~ gvforwarder as a systemd service May 15, 2025

openshift-ci bot removed the do-not-merge/work-in-progress label May 15, 2025

praveenkumar merged commit 4b18682 into crc-org:release-4.18 May 16, 2025
3 of 4 checks passed

github-project-automation bot moved this from Work In Progress to Done in Project planning: crc May 16, 2025

openshift-cherrypick-robot mentioned this pull request May 16, 2025

[release-4.19] gvforwarder as a systemd service #1060

Merged

openshift-cherrypick-robot mentioned this pull request May 16, 2025

[master] gvforwarder as a systemd service #1061

Merged

This was referenced May 16, 2025

gvforwarder as a systemd service #1003

Closed

Network manager restart hangs the VM #1068

Closed

Conversation

praveenkumar commented Apr 29, 2025 • edited by sourcery-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide by Sourcery

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

praveenkumar commented Apr 29, 2025

Uh oh!

cfergeau commented Apr 30, 2025

Uh oh!

praveenkumar commented Apr 30, 2025

Uh oh!

praveenkumar commented May 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gbraad May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented May 13, 2025

Uh oh!

praveenkumar commented May 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gbraad May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

praveenkumar commented May 16, 2025

Uh oh!

openshift-cherrypick-robot commented May 16, 2025

Uh oh!

praveenkumar commented May 16, 2025

Uh oh!

openshift-cherrypick-robot commented May 16, 2025

Uh oh!

Uh oh!

openshift-cherrypick-robot commented May 16, 2025

Uh oh!

openshift-cherrypick-robot commented May 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

praveenkumar commented Apr 29, 2025 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Apr 29, 2025 •

edited

Loading

gbraad May 12, 2025 •

edited

Loading

gbraad May 14, 2025 •

edited

Loading