fix: tap interface attach to bridge #479
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
richardcase
changed the title
Callisto13
previously approved these changes
Jul 12, 2022
core/models/network.go
Outdated
| @@ -23,7 +23,9 @@ type NetworkInterface struct { | |||
| Type IfaceType `json:"type" validate:"oneof=tap macvtap unsupported"` | |||
| // StaticAddress is an optional static IP address to assign to this interface. | |||
| // If not supplied then DHCP will be used. | |||
| StaticAddress *StaticAddress `json:"staticAddress,omitempty"` | |||
| StaticAddress *StaticAddress `json:"staticAddrss,omitempty"` | |||
| // BranchName is the name of the Linux bridge to attach the TAP device to. | |||
There was a problem hiding this comment.
Suggested change
| // BranchName is the name of the Linux bridge to attach the TAP device to. | |
| // BridgeName is the name of the Linux bridge to attach the TAP device to. |
There was a problem hiding this comment.
guessing that will need a regenerate
There was a problem hiding this comment.
Bugger, i had a real mental block between bridge name and branch name. Changing....
internal/config/validation.go
Outdated
| @@ -5,6 +5,11 @@ import ( | |||
| "os" | |||
| ) | |||
|
|
|||
| // Validate will validate the Config. | |||
| func (c *Config) Validate() error { | |||
There was a problem hiding this comment.
Removed, i did start to go down a rabbit hole with validation and then came to my senses. This got accidentally left behind.
There was a problem hiding this comment.
we can probs have a ticket for that
When requesting that a tap device is added to a microvm then we should attach (i.e. set the master) to a bridge. The bridge will be supplied via a new bridge-name flag. And the attach should not be done for the mmds network interface. The grpc api has been updated to allow the consumer to specify a bridge-name that is different as part of the create microvm call. Signed-off-by: Richard Case <[email protected]>
Callisto13
approved these changes
Jul 12, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When requestinf that a tap device is added to a microvm then we should
attach (i.e. set the master) to a bridge.
What this PR does / why we need it:
The bridge will be supplied via the parent-iface flag. And the attach should not be done for the mmds network interface.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #478
Special notes for your reviewer:
Checklist: