-
Notifications
You must be signed in to change notification settings - Fork 657
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 get local.<instance>.bridged
command
#3357
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3357 +/- ##
==========================================
- Coverage 84.05% 84.04% -0.02%
==========================================
Files 251 251
Lines 13798 13812 +14
==========================================
+ Hits 11598 11608 +10
- Misses 2200 2204 +4 ☔ View full report in Codecov by Sentry. |
fe8ef6f
to
90572c6
Compare
This is a platform function which returns the name of the bridge created to bridge a given network interface.
90572c6
to
7f10f71
Compare
Hey @townsend2010, maybe I misunderstand what you're asking, but the steps to reproduce the issue should create the bridge automatically. The problem was that they did so without first asking the user. |
Hey @ricab, ah, I think I misread the steps. |
That's right Chris, an ethernet device is needed to reproduce that. It may be possible to setup a dummy ethernet... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @luis4a0!
I'm afraid this isn't working for me. I followed the steps for issue #1 and then issued the command in issue #3 and I still get false
.
It's entirely possible I've done something wrong, but I am running this version of the snap:
$ multipass version
multipass 1.14.0-dev.1222.pr3357+g1797cd09
multipassd 1.14.0-dev.1222.pr3357+g1797cd09
$ multipass list
Name State IPv4 Image
test2 Running 10.230.244.14 Ubuntu 22.04 LTS
192.168.8.179
$ multipass get local.test2.bridged
false
Hey @townsend2010! Thanks for taking the time to review. In order to be sure it's failing, please clarify if, after creating the bridge, you changed the value of Thanks! |
Hey @luis4a0, no I did not change the value of
|
Ok, thanks for confirming! I'll see what's happening. |
Well @townsend2010, this is working for me. Absolutely strange. Before investigating the version of the snap you're running, and keep trying to reproduce, can you please share the contents of |
Hey @luis4a0! Sure: {
"test2": {
"deleted": false,
"disk_space": "10737418240",
"extra_interfaces": [
{
"auto_mode": true,
"id": "br-enx586d8fd35",
"mac_address": "52:54:00:5e:f8:7b"
}
],
"mac_addr": "52:54:00:44:29:b8",
"mem_size": "1073741824",
"metadata": {
},
"mounts": [
],
"num_cores": 1,
"run_at_boot": [
],
"ssh_username": "ubuntu",
"state": 4
}
} |
Is it perhaps because the |
Yes, you're absolutely right! So there should be a maximum length of the bridge names. The code checks for |
Indeed, we need to truncate thr names at 15 characters: see this answer in SO. |
4bcce89
to
04ae2b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @luis4a0!
This works as expected now and the code LGTM. 👍
Ah, right, there is a private side, will merge manually. |
This PR fixes issue 3 of this review.