From 12e700ca6df4e9013a6307a83df0abb881e4f8ba Mon Sep 17 00:00:00 2001 From: Turo Soisenniemi Date: Sat, 23 Jun 2018 13:53:51 +0300 Subject: [PATCH 1/2] Initial implementation for backwards compatible multinic in openstack Signed-off-by: Turo Soisenniemi --- .gitlab-ci.yml | 1 - drivers/openstack/client.go | 42 +++++++++---- drivers/openstack/openstack.go | 27 ++++----- drivers/openstack/openstack_test.go | 92 +++++++++++++++++++++++++++++ 4 files changed, 136 insertions(+), 26 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 0fa77afb25..bc93f4279d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -29,7 +29,6 @@ before_script: expire_in: 1 week tags: - docker - - privileged .build_validate: &build_validate <<: *build_base diff --git a/drivers/openstack/client.go b/drivers/openstack/client.go index 8b47b86e49..b413d65bc3 100644 --- a/drivers/openstack/client.go +++ b/drivers/openstack/client.go @@ -6,6 +6,7 @@ import ( "fmt" "io/ioutil" "net/http" + "strings" "time" "github.com/docker/machine/libmachine/log" @@ -72,11 +73,12 @@ func (c *GenericClient) CreateInstance(d *Driver) (string, error) { Metadata: d.GetMetadata(), } if d.NetworkId != "" { - serverOpts.Networks = []servers.Network{ - { - UUID: d.NetworkId, - }, + networkIDs := strings.Split(d.NetworkId, ",") + networks := make([]servers.Network, len(networkIDs)) + for i, id := range networkIDs { + networks[i] = servers.Network{UUID: id} } + serverOpts.Networks = networks } log.Info("Creating machine...") @@ -212,10 +214,21 @@ func (c *GenericClient) GetFloatingIPPoolID(d *Driver) (string, error) { return c.getNetworkID(d, d.FloatingIpPool) } +func Contains(slice []string, element string) (bool, int) { + for i, e := range slice { + if e == element { + return true, i + } + } + return false, -1 +} + func (c *GenericClient) getNetworkID(d *Driver, networkName string) (string, error) { opts := networks.ListOpts{Name: networkName} pager := networks.List(c.Network, opts) - networkID := "" + networkNames := strings.Split(networkName, ",") + remainingNetworks := len(networkNames) + networkID := make([]string, remainingNetworks) err := pager.EachPage(func(page pagination.Page) (bool, error) { networkList, err := networks.ExtractNetworks(page) @@ -224,16 +237,25 @@ func (c *GenericClient) getNetworkID(d *Driver, networkName string) (string, err } for _, n := range networkList { - if n.Name == networkName { - networkID = n.ID - return false, nil + match, index := Contains(networkNames, n.Name) + if match { + networkID[index] = n.ID + remainingNetworks-- + + if remainingNetworks == 0 { + return false, nil + } } } return true, nil }) - return networkID, err + if remainingNetworks != 0 { + return "", err + } + + return strings.Join(networkID, ","), err } func (c *GenericClient) GetFlavorID(d *Driver) (string, error) { @@ -454,7 +476,7 @@ func (c *GenericClient) getNeutronNetworkFloatingIPs(d *Driver) ([]FloatingIP, e func (c *GenericClient) GetInstancePortID(d *Driver) (string, error) { pager := ports.List(c.Network, ports.ListOpts{ DeviceID: d.MachineId, - NetworkID: d.NetworkId, + NetworkID: strings.Split(d.NetworkId, ",")[0], }) var portID string diff --git a/drivers/openstack/openstack.go b/drivers/openstack/openstack.go index 7e305d840a..4be92455d8 100644 --- a/drivers/openstack/openstack.go +++ b/drivers/openstack/openstack.go @@ -164,11 +164,11 @@ func (d *Driver) GetCreateFlags() []mcnflag.Flag { Usage: "OpenStack keypair to use to SSH to the instance", Value: "", }, - mcnflag.StringFlag{ + mcnflag.StringSliceFlag{ EnvVar: "OS_NETWORK_ID", Name: "openstack-net-id", Usage: "OpenStack network id the machine will be connected on", - Value: "", + Value: []string{}, }, mcnflag.StringFlag{ EnvVar: "OS_PRIVATE_KEY_FILE", @@ -182,11 +182,11 @@ func (d *Driver) GetCreateFlags() []mcnflag.Flag { Usage: "File containing an openstack userdata script", Value: "", }, - mcnflag.StringFlag{ + mcnflag.StringSliceFlag{ EnvVar: "OS_NETWORK_NAME", Name: "openstack-net-name", Usage: "OpenStack network name the machine will be connected on", - Value: "", + Value: []string{}, }, mcnflag.StringFlag{ EnvVar: "OS_SECURITY_GROUPS", @@ -202,7 +202,7 @@ func (d *Driver) GetCreateFlags() []mcnflag.Flag { mcnflag.StringFlag{ EnvVar: "OS_FLOATINGIP_POOL", Name: "openstack-floatingip-pool", - Usage: "OpenStack floating IP pool to get an IP from to assign to the instance", + Usage: "OpenStack floating IP pool to get an IP from to assign to the instance (first network only)", Value: "", }, mcnflag.IntFlag{ @@ -291,8 +291,8 @@ func (d *Driver) SetConfigFromFlags(flags drivers.DriverOptions) error { d.FlavorName = flags.String("openstack-flavor-name") d.ImageId = flags.String("openstack-image-id") d.ImageName = flags.String("openstack-image-name") - d.NetworkId = flags.String("openstack-net-id") - d.NetworkName = flags.String("openstack-net-name") + d.NetworkId = strings.Join(flags.StringSlice("openstack-net-id"), ",") + d.NetworkName = strings.Join(flags.StringSlice("openstack-net-name"), ",") d.metadata = flags.String("openstack-metadata") if flags.String("openstack-sec-groups") != "" { d.SecurityGroups = strings.Split(flags.String("openstack-sec-groups"), ",") @@ -553,7 +553,7 @@ func (d *Driver) checkConfig() error { } if d.NetworkName != "" && d.NetworkId != "" { - return fmt.Errorf(errorExclusiveOptions, "Network name", "Network id") + return fmt.Errorf(errorExclusiveOptions, "Network names", "Network ids") } if d.EndpointType != "" && (d.EndpointType != "publicURL" && d.EndpointType != "adminURL" && d.EndpointType != "internalURL") { return fmt.Errorf(errorWrongEndpointType) @@ -570,21 +570,18 @@ func (d *Driver) resolveIds() error { return err } - networkID, err := d.client.GetNetworkID(d) + networkIDs, err := d.client.GetNetworkID(d) if err != nil { return err } - if networkID == "" { + if networkIDs == "" { return fmt.Errorf(errorUnknownNetworkName, d.NetworkName) } - d.NetworkId = networkID - log.Debug("Found network id using its name", map[string]string{ - "Name": d.NetworkName, - "ID": d.NetworkId, - }) + d.NetworkId = networkIDs + //TODO: log found networks? } if d.FlavorName != "" { diff --git a/drivers/openstack/openstack_test.go b/drivers/openstack/openstack_test.go index 49134c0855..cd31b42eb7 100644 --- a/drivers/openstack/openstack_test.go +++ b/drivers/openstack/openstack_test.go @@ -27,3 +27,95 @@ func TestSetConfigFromFlags(t *testing.T) { assert.NoError(t, err) assert.Empty(t, checkFlags.InvalidFlags) } + +func TestSetSingleNetworkId(t *testing.T) { + driver := NewDriver("default", "path") + + checkFlags := &drivers.CheckDriverOptions{ + FlagsValues: map[string]interface{}{ + "openstack-auth-url": "http://url", + "openstack-username": "user", + "openstack-password": "pwd", + "openstack-tenant-id": "ID", + "openstack-flavor-id": "ID", + "openstack-image-id": "ID", + "openstack-net-id": "ID", + }, + CreateFlags: driver.GetCreateFlags(), + } + + err := driver.SetConfigFromFlags(checkFlags) + + assert.NoError(t, err) + assert.Empty(t, checkFlags.InvalidFlags) +} + +func TestSetSingleNetworkName(t *testing.T) { + driver := NewDriver("default", "path") + + checkFlags := &drivers.CheckDriverOptions{ + FlagsValues: map[string]interface{}{ + "openstack-auth-url": "http://url", + "openstack-username": "user", + "openstack-password": "pwd", + "openstack-tenant-id": "ID", + "openstack-flavor-id": "ID", + "openstack-image-id": "ID", + "openstack-net-name": "ID", + }, + CreateFlags: driver.GetCreateFlags(), + } + + err := driver.SetConfigFromFlags(checkFlags) + + assert.NoError(t, err) + assert.Empty(t, checkFlags.InvalidFlags) +} + +func TestSetMultipleNetworkIds(t *testing.T) { + driver := NewDriver("default", "path") + + checkFlags := &drivers.CheckDriverOptions{ + FlagsValues: map[string]interface{}{ + "openstack-auth-url": "http://url", + "openstack-username": "user", + "openstack-password": "pwd", + "openstack-tenant-id": "ID", + "openstack-flavor-id": "ID", + "openstack-image-id": "ID", + //TODO: multivalue test + //"openstack-net-id": "ID", + "openstack-net-id": "ID2", + }, + CreateFlags: driver.GetCreateFlags(), + } + + err := driver.SetConfigFromFlags(checkFlags) + + assert.NoError(t, err) + assert.Empty(t, checkFlags.InvalidFlags) +} + +func TestSetMultipleNetworkNames(t *testing.T) { + driver := NewDriver("default", "path") + + checkFlags := &drivers.CheckDriverOptions{ + FlagsValues: map[string]interface{}{ + "openstack-auth-url": "http://url", + "openstack-username": "user", + "openstack-password": "pwd", + "openstack-tenant-id": "ID", + "openstack-flavor-id": "ID", + "openstack-image-id": "ID", + "openstack-net-name": "ID", + //TODO: multivalue test + //"openstack-net-name": "ID2", + }, + CreateFlags: driver.GetCreateFlags(), + } + + err := driver.SetConfigFromFlags(checkFlags) + + assert.NoError(t, err) + assert.Empty(t, checkFlags.InvalidFlags) +} From 47ee663bdf7948545c5917708e6f602e241199c8 Mon Sep 17 00:00:00 2001 From: Turo Soisenniemi Date: Tue, 26 Jun 2018 14:25:24 +0300 Subject: [PATCH 2/2] Fixed openstack-net-name after last commit and added support for defining floating ip target network Signed-off-by: Turo Soisenniemi --- drivers/openstack/client.go | 49 ++++++++---------------- drivers/openstack/openstack.go | 58 ++++++++++++++++++++++++++--- drivers/openstack/openstack_test.go | 34 ++++++++--------- 3 files changed, 84 insertions(+), 57 deletions(-) diff --git a/drivers/openstack/client.go b/drivers/openstack/client.go index b413d65bc3..a9057abd36 100644 --- a/drivers/openstack/client.go +++ b/drivers/openstack/client.go @@ -44,12 +44,11 @@ type Client interface { GetPublicKey(keyPairName string) ([]byte, error) CreateKeyPair(d *Driver, name string, publicKey string) error DeleteKeyPair(d *Driver, name string) error - GetNetworkID(d *Driver) (string, error) + GetNetworkIDs(networkNames []string) (string, error) GetFlavorID(d *Driver) (string, error) GetImageID(d *Driver) (string, error) AssignFloatingIP(d *Driver, floatingIP *FloatingIP) error GetFloatingIPs(d *Driver) ([]FloatingIP, error) - GetFloatingIPPoolID(d *Driver) (string, error) GetInstancePortID(d *Driver) (string, error) GetTenantID(d *Driver) (string, error) } @@ -206,29 +205,22 @@ func (c *GenericClient) GetInstanceIPAddresses(d *Driver) ([]IPAddress, error) { return addresses, nil } -func (c *GenericClient) GetNetworkID(d *Driver) (string, error) { - return c.getNetworkID(d, d.NetworkName) -} - -func (c *GenericClient) GetFloatingIPPoolID(d *Driver) (string, error) { - return c.getNetworkID(d, d.FloatingIpPool) -} - -func Contains(slice []string, element string) (bool, int) { - for i, e := range slice { - if e == element { - return true, i +func (c *GenericClient) GetNetworkIDs(networkNames []string) (string, error) { + ips := []string{} + for _, name := range networkNames { + id, err := c.getNetworkID(name) + if err != nil { + return "", err } + ips = append(ips, id) } - return false, -1 + return strings.Join(ips, ","), nil } -func (c *GenericClient) getNetworkID(d *Driver, networkName string) (string, error) { +func (c *GenericClient) getNetworkID(networkName string) (string, error) { opts := networks.ListOpts{Name: networkName} pager := networks.List(c.Network, opts) - networkNames := strings.Split(networkName, ",") - remainingNetworks := len(networkNames) - networkID := make([]string, remainingNetworks) + networkID := "" err := pager.EachPage(func(page pagination.Page) (bool, error) { networkList, err := networks.ExtractNetworks(page) @@ -237,25 +229,16 @@ func (c *GenericClient) getNetworkID(d *Driver, networkName string) (string, err } for _, n := range networkList { - match, index := Contains(networkNames, n.Name) - if match { - networkID[index] = n.ID - remainingNetworks-- - - if remainingNetworks == 0 { - return false, nil - } + if n.Name == networkName { + networkID = n.ID + return false, nil } } return true, nil }) - if remainingNetworks != 0 { - return "", err - } - - return strings.Join(networkID, ","), err + return networkID, err } func (c *GenericClient) GetFlavorID(d *Driver) (string, error) { @@ -476,7 +459,7 @@ func (c *GenericClient) getNeutronNetworkFloatingIPs(d *Driver) ([]FloatingIP, e func (c *GenericClient) GetInstancePortID(d *Driver) (string, error) { pager := ports.List(c.Network, ports.ListOpts{ DeviceID: d.MachineId, - NetworkID: strings.Split(d.NetworkId, ",")[0], + NetworkID: d.FloatingIpNetworkId, }) var portID string diff --git a/drivers/openstack/openstack.go b/drivers/openstack/openstack.go index 4be92455d8..0f5a29dbc1 100644 --- a/drivers/openstack/openstack.go +++ b/drivers/openstack/openstack.go @@ -53,6 +53,9 @@ type Driver struct { client Client // ExistingKey keeps track of whether the key was created by us or we used an existing one. If an existing one was used, we shouldn't delete it when the machine is deleted. ExistingKey bool + // To get right port id, if multiple networks are configured and first one is not the one for floating ip. + FloatingIpNetworkName string + FloatingIpNetworkId string } const ( @@ -202,7 +205,19 @@ func (d *Driver) GetCreateFlags() []mcnflag.Flag { mcnflag.StringFlag{ EnvVar: "OS_FLOATINGIP_POOL", Name: "openstack-floatingip-pool", - Usage: "OpenStack floating IP pool to get an IP from to assign to the instance (first network only)", + Usage: "OpenStack floating IP pool to get an IP from to assign to the instance.", + Value: "", + }, + mcnflag.StringFlag{ + EnvVar: "OS_FLOATINGIP_NETWORK_NAME", + Name: "openstack-floatingip-net-name", + Usage: "OpenStack floating IP target network name to get right port for assingin IP.", + Value: "", + }, + mcnflag.StringFlag{ + EnvVar: "OS_FLOATINGIP_NETWORK_ID", + Name: "openstack-floatingip-net-id", + Usage: "OpenStack floating IP target network id to get right port for assingin IP.", Value: "", }, mcnflag.IntFlag{ @@ -298,6 +313,8 @@ func (d *Driver) SetConfigFromFlags(flags drivers.DriverOptions) error { d.SecurityGroups = strings.Split(flags.String("openstack-sec-groups"), ",") } d.FloatingIpPool = flags.String("openstack-floatingip-pool") + d.FloatingIpNetworkName = flags.String("openstack-floatingip-net-name") + d.FloatingIpNetworkId = flags.String("openstack-floatingip-net-id") d.IpVersion = flags.Int("openstack-ip-version") d.ComputeNetwork = flags.Bool("openstack-nova-network") d.SSHUser = flags.String("openstack-ssh-user") @@ -561,6 +578,9 @@ func (d *Driver) checkConfig() error { if (d.KeyPairName != "" && d.PrivateKeyFile == "") || (d.KeyPairName == "" && d.PrivateKeyFile != "") { return fmt.Errorf(errorBothOptions, "KeyPairName", "PrivateKeyFile") } + if d.FloatingIpNetworkName != "" && d.FloatingIpNetworkId != "" { + return fmt.Errorf(errorBothOptions, "FloatingIpNetworkName", "FloatingIpNetworkId") + } return nil } @@ -570,7 +590,7 @@ func (d *Driver) resolveIds() error { return err } - networkIDs, err := d.client.GetNetworkID(d) + networkIDs, err := d.client.GetNetworkIDs(strings.Split(d.NetworkName, ",")) if err != nil { return err @@ -581,7 +601,10 @@ func (d *Driver) resolveIds() error { } d.NetworkId = networkIDs - //TODO: log found networks? + log.Debug("Found networks using their name", map[string]string{ + "Names": d.NetworkName, + "IDs": d.NetworkId, + }) } if d.FlavorName != "" { @@ -630,23 +653,46 @@ func (d *Driver) resolveIds() error { if err := d.initNetwork(); err != nil { return err } - f, err := d.client.GetFloatingIPPoolID(d) + + networkID, err := d.client.GetNetworkIDs([]string{d.FloatingIpPool}) if err != nil { return err } - if f == "" { + if networkID == "" { return fmt.Errorf(errorUnknownNetworkName, d.FloatingIpPool) } - d.FloatingIpPoolId = f + d.FloatingIpPoolId = networkID log.Debug("Found floating IP pool id using its name", map[string]string{ "Name": d.FloatingIpPool, "ID": d.FloatingIpPoolId, }) } + if d.FloatingIpNetworkId == "" && !d.ComputeNetwork { + d.FloatingIpNetworkId = strings.Split(d.NetworkId, ",")[0] + log.Debug("Using first network as floating ip network") + } else if d.FloatingIpNetworkName != "" && !d.ComputeNetwork { + if err := d.initNetwork(); err != nil { + return err + } + + networkID, err := d.client.GetNetworkIDs([]string{d.FloatingIpNetworkName}) + + if err != nil { + return err + } + + d.FloatingIpNetworkId = networkID + + log.Debug("Found floating network for floating ip using its name", map[string]string{ + "Name": d.FloatingIpNetworkName, + "ID": d.FloatingIpNetworkId, + }) + } + if d.TenantName != "" && d.TenantId == "" { if err := d.initIdentity(); err != nil { return err diff --git a/drivers/openstack/openstack_test.go b/drivers/openstack/openstack_test.go index cd31b42eb7..fc0b0ec7d1 100644 --- a/drivers/openstack/openstack_test.go +++ b/drivers/openstack/openstack_test.go @@ -77,15 +77,14 @@ func TestSetMultipleNetworkIds(t *testing.T) { checkFlags := &drivers.CheckDriverOptions{ FlagsValues: map[string]interface{}{ - "openstack-auth-url": "http://url", - "openstack-username": "user", - "openstack-password": "pwd", - "openstack-tenant-id": "ID", - "openstack-flavor-id": "ID", - "openstack-image-id": "ID", - //TODO: multivalue test - //"openstack-net-id": "ID", - "openstack-net-id": "ID2", + "openstack-auth-url": "http://url", + "openstack-username": "user", + "openstack-password": "pwd", + "openstack-tenant-id": "ID", + "openstack-flavor-id": "ID", + "openstack-image-id": "ID", + "openstack-net-id": []string{"ID", "ID2"}, + "openstack-floatingip-net-id": "ID2", }, CreateFlags: driver.GetCreateFlags(), } @@ -101,15 +100,14 @@ func TestSetMultipleNetworkNames(t *testing.T) { checkFlags := &drivers.CheckDriverOptions{ FlagsValues: map[string]interface{}{ - "openstack-auth-url": "http://url", - "openstack-username": "user", - "openstack-password": "pwd", - "openstack-tenant-id": "ID", - "openstack-flavor-id": "ID", - "openstack-image-id": "ID", - "openstack-net-name": "ID", - //TODO: multivalue test - //"openstack-net-name": "ID2", + "openstack-auth-url": "http://url", + "openstack-username": "user", + "openstack-password": "pwd", + "openstack-tenant-id": "ID", + "openstack-flavor-id": "ID", + "openstack-image-id": "ID", + "openstack-net-name": []string{"ID", "ID2"}, + "openstack-floatingip-net-name": "ID2", }, CreateFlags: driver.GetCreateFlags(), }