Skip to content

Commit

Permalink
address openstack network duplication when interface may have both ip…
Browse files Browse the repository at this point in the history 
…v6 and ipv4 addresses

add preflight check to dedup source networks
  • Loading branch information
@ibrokethecloud
ibrokethecloud committed Sep 11, 2024
1 parent 409740f commit 8e59514
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 46 deletions.
10 changes: 10 additions & 0 deletions pkg/controllers/migration/virtualmachine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,16 @@ func (h *virtualMachineHandler) preFlightChecks(vm *migration.VirtualMachineImpo
}
}

// dedup source network names as the same source network name cannot appear twice
sourceNetworkMap := make(map[string]bool)
for _, network := range vm.Spec.Mapping {
if _, ok := sourceNetworkMap[network.SourceNetwork]; ok {
return fmt.Errorf("source network %s appears multiple times in vm spec", network.SourceNetwork)
} else {
sourceNetworkMap[network.SourceNetwork] = true
}
}

return nil
}

Expand Down
73 changes: 53 additions & 20 deletions pkg/source/openstack/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@ type ExtendedVolume struct {
VolumeImageMetadata map[string]string `json:"volume_image_metadata,omitempty"`
}

// ExtendedServer The original `Server` structure does not contain the `Description` field.
// References:
// - https://github.com/gophercloud/gophercloud/pull/1505
// - https://docs.openstack.org/api-ref/compute/?expanded=list-all-metadata-detail%2Ccreate-server-detail#show-server-details
type ExtendedServer struct {
servers.Server
Description string `json:"description,omitempty"`
}

// NewClient will generate a GopherCloud client
func NewClient(ctx context.Context, endpoint string, region string, secret *corev1.Secret) (*Client, error) {
username, ok := secret.Data["username"]
Expand Down Expand Up @@ -332,32 +341,23 @@ func (c *Client) GenerateVirtualMachine(vm *migration.VirtualMachineImport) (*ku
return nil, fmt.Errorf("error looking up flavor: %v", err)
}

uefi, tpm, secureboot, err := c.ImageFirmwareSettings(vmObj)
uefi, tpm, secureboot, err := c.ImageFirmwareSettings(&vmObj.Server)
if err != nil {
return nil, fmt.Errorf("error getting firware settings: %v", err)
}

var networks []networkInfo
for network, values := range vmObj.Addresses {
valArr, ok := values.([]interface{})
if !ok {
return nil, fmt.Errorf("error asserting interface []interface")
}
for _, v := range valArr {
valMap, ok := v.(map[string]interface{})
if !ok {
return nil, fmt.Errorf("error asserting network array element into map[string]string")
}
networks = append(networks, networkInfo{
NetworkName: network,
MAC: valMap["OS-EXT-IPS-MAC:mac_addr"].(string),
})
}
networks, err := generateNetworkInfo(vmObj.Addresses)
if err != nil {
return nil, err
}

newVM := &kubevirt.VirtualMachine{
ObjectMeta: metav1.ObjectMeta{
Name: vm.Spec.VirtualMachineName,
Namespace: vm.Namespace,
Annotations: map[string]string{
"field.cattle.io/description": vmObj.Description,
},
},
}

Expand Down Expand Up @@ -574,13 +574,15 @@ func (c *Client) checkOrGetUUID(input string) (string, error) {
return filteredServers[0].ID, nil
}

func (c *Client) findVM(name string) (*servers.Server, error) {
func (c *Client) findVM(name string) (*ExtendedServer, error) {
parsedUUID, err := c.checkOrGetUUID(name)
if err != nil {
return nil, err
}
return servers.Get(c.computeClient, parsedUUID).Extract()

sr := servers.Get(c.computeClient, parsedUUID)
var s ExtendedServer
err = sr.ExtractInto(&s)
return &s, err
}

type networkInfo struct {
Expand Down Expand Up @@ -640,3 +642,34 @@ func (c *Client) ImageFirmwareSettings(instance *servers.Server) (bool, bool, bo
}
return uefiType, tpmEnabled, secureBoot, nil
}

func generateNetworkInfo(info map[string]interface{}) ([]networkInfo, error) {
var networks, uniqueNetworks []networkInfo
for network, values := range info {
valArr, ok := values.([]interface{})
if !ok {
return nil, fmt.Errorf("error asserting interface []interface")
}
for _, v := range valArr {
valMap, ok := v.(map[string]interface{})
if !ok {
return nil, fmt.Errorf("error asserting network array element into map[string]string")
}
networks = append(networks, networkInfo{
NetworkName: network,
MAC: valMap["OS-EXT-IPS-MAC:mac_addr"].(string),
})
}
}
// in case of interfaces with ipv6 and ipv4 addresses they are reported twice, so we need to dedup them
// based on a mac address
networksMap := make(map[string]networkInfo)
for _, v := range networks {
networksMap[v.MAC] = v
}

for _, v := range networksMap {
uniqueNetworks = append(uniqueNetworks, v)
}
return uniqueNetworks, nil
}
83 changes: 57 additions & 26 deletions pkg/source/openstack/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package openstack

import (
"context"
"encoding/json"
"os"
"testing"

Expand All @@ -13,53 +14,58 @@ import (
)

var (
c *Client
c *Client
skipTests bool
)

func TestMain(t *testing.M) {
var err error

// skip tests, needed for current builds
_, ok := os.LookupEnv("USE_EXISTING_CLUSTER")
if !ok {
logrus.Warn("skipping tests")
return
}
if ok {
s, err := SetupOpenstackSecretFromEnv("devstack")
if err != nil {
logrus.Fatal(err)
}
endpoint, region, err := SetupOpenstackSourceFromEnv()
if err != nil {
logrus.Fatal(err)
}

s, err := SetupOpenstackSecretFromEnv("devstack")
if err != nil {
logrus.Fatal(err)
}
endpoint, region, err := SetupOpenstackSourceFromEnv()
if err != nil {
logrus.Fatal(err)
}
c, err = NewClient(context.TODO(), endpoint, region, s)
if err != nil {
logrus.Fatal(err)
}

c, err = NewClient(context.TODO(), endpoint, region, s)
if err != nil {
logrus.Fatal(err)
}
go func() {
if err = server.NewServer(context.TODO()); err != nil {
logrus.Fatalf("error creating server: %v", err)
}
}()

go func() {
if err = server.NewServer(context.TODO()); err != nil {
logrus.Fatalf("error creating server: %v", err)
if err != nil {
logrus.Fatal(err)
}
}()

if err != nil {
logrus.Fatal(err)
} else {
skipTests = true
}

code := t.Run()
os.Exit(code)
}

func Test_NewClient(t *testing.T) {
if skipTests {
t.Skip("skipping Test_NewClient")
}
assert := require.New(t)
err := c.Verify()
assert.NoError(err, "expect no error during verify of client")
}

func Test_checkOrGetUUID(t *testing.T) {
if skipTests {
t.Skip("skipping Test_checkOrGetUUID")
}
assert := require.New(t)
vmName, ok := os.LookupEnv("OS_VM_NAME")
assert.True(ok, "expected env variable VM_NAME to be set")
Expand All @@ -68,6 +74,9 @@ func Test_checkOrGetUUID(t *testing.T) {
}

func Test_IsPoweredOff(t *testing.T) {
if skipTests {
t.Skip("skipping Test_IsPoweredOff")
}
assert := require.New(t)
vmName, ok := os.LookupEnv("OS_VM_NAME")
assert.True(ok, "expected env variable VM_NAME to be set")
Expand All @@ -81,6 +90,9 @@ func Test_IsPoweredOff(t *testing.T) {
}

func Test_PowerOffVirtualMachine(t *testing.T) {
if skipTests {
t.Skip("skipping Test_PowerOffVirtualMachine")
}
assert := require.New(t)
vmName, ok := os.LookupEnv("OS_VM_NAME")
assert.True(ok, "expected env variable VM_NAME to be set")
Expand All @@ -94,6 +106,9 @@ func Test_PowerOffVirtualMachine(t *testing.T) {
}

func Test_ExportVirtualMachine(t *testing.T) {
if skipTests {
t.Skip("skipping Test_ExportVirtualMachine")
}
assert := require.New(t)
vmName, ok := os.LookupEnv("OS_VM_NAME")
assert.True(ok, "expected env variable VM_NAME to be set")
Expand All @@ -109,6 +124,9 @@ func Test_ExportVirtualMachine(t *testing.T) {
}

func Test_GenerateVirtualMachine(t *testing.T) {
if skipTests {
t.Skip("skipping Test_GenerateVirtualMachine")
}
assert := require.New(t)
vmName := os.Getenv("OS_VM_NAME")
assert.NotEmpty(vmName, "expected env variable VM_NAME to be set")
Expand All @@ -124,3 +142,16 @@ func Test_GenerateVirtualMachine(t *testing.T) {
assert.NotEmpty(newVM.Spec.Template.Spec.Networks, "expected to find atleast 1 network as pod network should have been applied")
assert.NotEmpty(newVM.Spec.Template.Spec.Domain.Devices.Interfaces, "expected to find atleast 1 interface for pod-network")
}

func Test_generateNetworkInfo(t *testing.T) {
networkInfoByte := []byte(`{"private":[{"OS-EXT-IPS-MAC:mac_addr":"fa:16:3e:92:5f:45","OS-EXT-IPS:type":"fixed","addr":"fd5b:731d:94e1:0:f816:3eff:fe92:5f45","version":6},{"OS-EXT-IPS-MAC:mac_addr":"fa:16:3e:92:5f:45","OS-EXT-IPS:type":"fixed","addr":"10.0.0.38","version":4}],"shared":[{"OS-EXT-IPS-MAC:mac_addr":"fa:16:3e:ec:49:11","OS-EXT-IPS:type":"fixed","addr":"192.168.233.233","version":4}]}`)
var networkInfoMap map[string]interface{}
assert := require.New(t)
err := json.Unmarshal(networkInfoByte, &networkInfoMap)
assert.NoError(err, "expected no error while unmarshalling network info")

vmInterfaceDetails, err := generateNetworkInfo(networkInfoMap)
assert.NoError(err, "expected no error while generating network info")
assert.Len(vmInterfaceDetails, 2, "expected to find 2 interfaces only")

}

0 comments on commit 8e59514

Please sign in to comment.