Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Enable multiple non-IP interface to be connected via tc redirect #836

Merged
merged 28 commits into from
Jun 25, 2021
Merged

Enable multiple non-IP interface to be connected via tc redirect #836

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
0d7046e
First crack at multinet implementation
networkop May 10, 2021
0b3dfdb
fix the dhcp server bug
networkop May 10, 2021
9acef4f
Merge branch 'weaveworks:main' into multinet
networkop May 10, 2021
56ca546
Added wait for X number of interfaces to be connected
networkop May 10, 2021
52776a6
Merge branch 'weaveworks:main' into multinet
networkop May 10, 2021
e3baa91
Merge branch 'multinet' of github.com:networkop/ignite into multinet
networkop May 10, 2021
17765be
Fixed multiple bugs
networkop May 11, 2021
6e19436
Ensure maxIntfs is at least 1
networkop May 11, 2021
39d395e
Fixed formatting
networkop May 11, 2021
9a31fb9
Forgot to assign envVars
networkop May 11, 2021
20b4aa8
Updated docs
networkop May 11, 2021
35dc891
More updated docs
networkop May 11, 2021
e8bd47c
More docs updates
networkop May 11, 2021
b8ef5d3
Not using env vars anymore
networkop May 22, 2021
47c9140
Re-introducing env var support
networkop May 23, 2021
6f1f14a
Reverting the order to error checks
networkop May 24, 2021
90eaa58
Added multinet tests
networkop Jun 9, 2021
1020f90
Fixing dox
networkop Jun 9, 2021
053def5
Refactored StartVM with async spawn channel
networkop Jun 10, 2021
eb8997f
Refactored ignite-spawn's network setup
networkop Jun 10, 2021
051ad2b
Fixed the CNI plugin bug
networkop Jun 11, 2021
1c70e78
Tested with containerlab
networkop Jun 12, 2021
9957b70
Merging suggested changes
networkop Jun 21, 2021
3161ef4
Alternative e2e test
networkop Jun 22, 2021
f8d361d
Removed --wait flag and fixed up multinet tests
networkop Jun 22, 2021
668b5fe
Removing waitForSSH from exec
networkop Jun 24, 2021
71c801c
Removing leftover comment
networkop Jun 24, 2021
abddda2
Adding waitforSSH back
networkop Jun 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions cmd/ignite-spawn/ignite-spawn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"os"
"path"
"strings"

log "github.com/sirupsen/logrus"
api "github.com/weaveworks/ignite/pkg/apis/ignite"
Expand Down Expand Up @@ -39,8 +40,11 @@ func decodeVM(vmID string) (*api.VM, error) {
}

func StartVM(vm *api.VM) (err error) {

envVars := parseEnvVars(os.Environ())

// Setup networking inside of the container, return the available interfaces
dhcpIfaces, err := container.SetupContainerNetworking()
fcIfaces, dhcpIfaces, err := container.SetupContainerNetworking(envVars)
if err != nil {
return fmt.Errorf("network setup failed: %v", err)
}
Expand All @@ -66,7 +70,7 @@ func StartVM(vm *api.VM) (err error) {
defer util.DeferErr(&err, func() error { return os.Remove(metricsSocket) })

// Execute Firecracker
if err = container.ExecuteFirecracker(vm, dhcpIfaces); err != nil {
if err = container.ExecuteFirecracker(vm, fcIfaces); err != nil {
return fmt.Errorf("runtime error for VM %q: %v", vm.GetUID(), err)
}

Expand Down Expand Up @@ -96,3 +100,18 @@ func patchStopped(vm *api.VM) error {
patch := []byte(`{"status":{"running":false,"network":null,"runtime":null,"startTime":null}}`)
return patchutil.NewPatcher(scheme.Serializer).ApplyOnFile(constants.IGNITE_SPAWN_VM_FILE_PATH, patch, vm.GroupVersionKind())
}

func parseEnvVars(vars []string) map[string]string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is used just once, above, maybe os.Environ() should be called in this function itself for now. Keeping true to the meaning of the function name.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO using map of config key=values without mutating the ignite-spawn Environment is better, but this is conflating a lot of config ideas. (flags producing ENV vs. flags overriding ENV)

In general though, is this the proper strategy for config?

Since we already have the information in the VM object, ignite-spawn has access to it and it can read the value from it directly.

That will get of all of the flag-passing and parsing code within ignite & ignite-spawn.

nit:
The names of the variables/annotations in this code should not reference any idea of ENV.
We should call it what it is with a proper domain.
There should be an explicit key for each thing you want.

metadata:
  annotations:
    ignite.weave.works/interface-count: '3'
    # ignite.weave.works/interface-mode/eth0: 'dhcp-bridge'
    # ignite.weave.works/interface-mode/eth2: 'tc-redirect'
    # ignite.weave.works/interface-mode/macvtap-uvnrt: 'macvtap'

result := make(map[string]string)

for _, arg := range vars {
parts := strings.Split(arg, "=")
if len(parts) != 2 {
continue
}

result[parts[0]] = parts[1]
}

return result
}
4 changes: 4 additions & 0 deletions cmd/ignite/cmd/cmdutil/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,7 @@ func AddSSHFlags(fs *pflag.FlagSet, identityFile *string, timeout *uint32) {
fs.StringVarP(identityFile, "identity", "i", "", "Override the vm's default identity file")
fs.Uint32Var(timeout, "timeout", constants.SSH_DEFAULT_TIMEOUT_SECONDS, "Timeout waiting for connection in seconds")
}

func AddEnvVarsFlag(fs *pflag.FlagSet, vars *string) {
fs.StringVar(vars, "sandbox-env-vars", *vars, "A list of comma-separated key=value pairs to pass as sandbox env vars")
}
1 change: 1 addition & 0 deletions cmd/ignite/cmd/vmcmd/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func addCreateFlags(fs *pflag.FlagSet, cf *run.CreateFlags) {
// Register common flags
cmdutil.AddNameFlag(fs, &cf.VM.ObjectMeta.Name)
cmdutil.AddConfigFlag(fs, &cf.ConfigFile)
cmdutil.AddEnvVarsFlag(fs, &cf.EnvVars)

// Register flags bound to temporary holder values
fs.StringSliceVarP(&cf.PortMappings, "ports", "p", cf.PortMappings, "Map host ports to VM ports")
Expand Down
5 changes: 5 additions & 0 deletions cmd/ignite/run/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/weaveworks/ignite/pkg/apis/ignite/validation"
meta "github.com/weaveworks/ignite/pkg/apis/meta/v1alpha1"
"github.com/weaveworks/ignite/pkg/config"
"github.com/weaveworks/ignite/pkg/constants"
"github.com/weaveworks/ignite/pkg/dmlegacy"
"github.com/weaveworks/ignite/pkg/metadata"
"github.com/weaveworks/ignite/pkg/operations"
Expand Down Expand Up @@ -39,6 +40,7 @@ type CreateFlags struct {
ConfigFile string
VM *api.VM
Labels []string
EnvVars string
RequireName bool
}

Expand Down Expand Up @@ -95,6 +97,9 @@ func (cf *CreateFlags) NewCreateOptions(args []string, fs *flag.FlagSet) (*Creat
return nil, err
}

// Save env variables
baseVM.SetAnnotation(constants.IGNITE_ENV_VAR_ANNOTATION, cf.EnvVars)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to provide a way to set this cf.EnvVars via the ignite configuration (global) as well, like other similar VM configurations in

ignite/pkg/apis/ignite/types.go

Lines 261 to 266 in 999ad7c

type ConfigurationSpec struct {
Runtime igniteRuntime.Name `json:"runtime,omitempty"`
NetworkPlugin igniteNetwork.PluginName `json:"networkPlugin,omitempty"`
VMDefaults VMSpec `json:"vmDefaults,omitempty"`
IDPrefix string `json:"idPrefix,omitempty"`
}
to allow setting default for all the VMs and allow overriding that through cf flag and individual VM configuration.
Since this is set as annotation, the baseVM should have the annotation inherited from the ignite configuration. Annotation in VM configuration should override/append the baseVM configuration. The VM create flag value should append/override everything before.


// If --require-name is true, VM name must be provided.
if cf.RequireName && len(baseVM.Name) == 0 {
return nil, fmt.Errorf("must set VM name, flag --require-name set")
Expand Down
2 changes: 1 addition & 1 deletion cmd/ignite/run/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func Start(so *StartOptions, fs *flag.FlagSet) error {
return err
}

if err := operations.StartVM(so.vm, so.Debug); err != nil {
if err := operations.StartVM(so.vm, so.Debug, true); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe before setting true here, the VM configuration/annotation needs to be checked?

return err
}

Expand Down
1 change: 1 addition & 0 deletions docs/cli/ignite/ignite_create.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ ignite create <OCI image> [flags]
-p, --ports strings Map host ports to VM ports
--require-name Require VM name to be passed, no name generation
--runtime runtime Container runtime to use. Available options are: [docker containerd] (default containerd)
--sandbox-env-vars string A list of comma-separated key=value pairs to pass as sandbox env vars
--sandbox-image oci-image Specify an OCI image for the VM sandbox (default weaveworks/ignite:dev)
-s, --size size VM filesystem size, for example 5GB or 2048MB (default 4.0 GB)
--ssh[=<path>] Enable SSH for the VM. If <path> is given, it will be imported as the public key. If just '--ssh' is specified, a new keypair will be generated. (default is unset, which disables SSH access to the VM)
Expand Down
1 change: 1 addition & 0 deletions docs/cli/ignite/ignite_run.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ ignite run <OCI image> [flags]
-p, --ports strings Map host ports to VM ports
--require-name Require VM name to be passed, no name generation
--runtime runtime Container runtime to use. Available options are: [docker containerd] (default containerd)
--sandbox-env-vars string A list of comma-separated key=value pairs to pass as sandbox env vars
--sandbox-image oci-image Specify an OCI image for the VM sandbox (default weaveworks/ignite:dev)
-s, --size size VM filesystem size, for example 5GB or 2048MB (default 4.0 GB)
--ssh[=<path>] Enable SSH for the VM. If <path> is given, it will be imported as the public key. If just '--ssh' is specified, a new keypair will be generated. (default is unset, which disables SSH access to the VM)
Expand Down
1 change: 1 addition & 0 deletions docs/cli/ignite/ignite_vm_create.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ ignite vm create <OCI image> [flags]
-p, --ports strings Map host ports to VM ports
--require-name Require VM name to be passed, no name generation
--runtime runtime Container runtime to use. Available options are: [docker containerd] (default containerd)
--sandbox-env-vars string A list of comma-separated key=value pairs to pass as sandbox env vars
--sandbox-image oci-image Specify an OCI image for the VM sandbox (default weaveworks/ignite:dev)
-s, --size size VM filesystem size, for example 5GB or 2048MB (default 4.0 GB)
--ssh[=<path>] Enable SSH for the VM. If <path> is given, it will be imported as the public key. If just '--ssh' is specified, a new keypair will be generated. (default is unset, which disables SSH access to the VM)
Expand Down
1 change: 1 addition & 0 deletions docs/cli/ignite/ignite_vm_run.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ ignite vm run <OCI image> [flags]
-p, --ports strings Map host ports to VM ports
--require-name Require VM name to be passed, no name generation
--runtime runtime Container runtime to use. Available options are: [docker containerd] (default containerd)
--sandbox-env-vars string A list of comma-separated key=value pairs to pass as sandbox env vars
--sandbox-image oci-image Specify an OCI image for the VM sandbox (default weaveworks/ignite:dev)
-s, --size size VM filesystem size, for example 5GB or 2048MB (default 4.0 GB)
--ssh[=<path>] Enable SSH for the VM. If <path> is given, it will be imported as the public key. If just '--ssh' is specified, a new keypair will be generated. (default is unset, which disables SSH access to the VM)
Expand Down
3 changes: 3 additions & 0 deletions pkg/constants/vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,7 @@ const (
// DEFAULT_SANDBOX_IMAGE_NAME is the name of the default sandbox container
// image to be used.
DEFAULT_SANDBOX_IMAGE_TAG = "dev"

// Env variable annotation key
IGNITE_ENV_VAR_ANNOTATION = "ignite.env.vars"
)
9 changes: 0 additions & 9 deletions pkg/container/dhcp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,13 @@ import (
log "github.com/sirupsen/logrus"
api "github.com/weaveworks/ignite/pkg/apis/ignite"
"github.com/weaveworks/ignite/pkg/constants"
"github.com/weaveworks/ignite/pkg/util"
)

var leaseDuration, _ = time.ParseDuration(constants.DHCP_INFINITE_LEASE) // Infinite lease time

// StartDHCPServers starts multiple DHCP servers for the VM, one per interface
// It returns the IP addresses that the API object may post in .status, and a potential error
func StartDHCPServers(vm *api.VM, dhcpIfaces []DHCPInterface) error {
// Generate the MAC addresses for the VM's adapters
macAddresses := make([]string, 0, len(dhcpIfaces))
if err := util.NewMAC(&macAddresses); err != nil {
return fmt.Errorf("failed to generate MAC addresses: %v", err)
}

// Fetch the DNS servers given to the container
clientConfig, err := dns.ClientConfigFromFile("/etc/resolv.conf")
Expand All @@ -36,9 +30,6 @@ func StartDHCPServers(vm *api.VM, dhcpIfaces []DHCPInterface) error {
// Set the VM hostname to the VM ID
dhcpIface.Hostname = vm.GetUID().String()

// Set the MAC address filter for the DHCP server
dhcpIface.MACFilter = macAddresses[i]

// Add the DNS servers from the container
dhcpIface.SetDNSServers(clientConfig.Servers)

Expand Down
10 changes: 5 additions & 5 deletions pkg/container/firecracker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ import (
)

// ExecuteFirecracker executes the firecracker process using the Go SDK
func ExecuteFirecracker(vm *api.VM, dhcpIfaces []DHCPInterface) (err error) {
func ExecuteFirecracker(vm *api.VM, fcIfaces []FCInterface) (err error) {
drivePath := vm.SnapshotDev()

networkInterfaces := make([]firecracker.NetworkInterface, 0, len(dhcpIfaces))
for _, dhcpIface := range dhcpIfaces {
networkInterfaces := make([]firecracker.NetworkInterface, 0, len(fcIfaces))
for _, intf := range fcIfaces {
networkInterfaces = append(networkInterfaces, firecracker.NetworkInterface{
StaticConfiguration: &firecracker.StaticNetworkConfiguration{
MacAddress: dhcpIface.MACFilter,
HostDevName: dhcpIface.VMTAP,
MacAddress: intf.MACFilter,
HostDevName: intf.VMTAP,
},
})
}
Expand Down
Loading