Skip to content

improve network lookup for vmware source imports #70

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,5 @@ const (
VirtualMachineImageFailed condition.Cond = "VirtualMachineImageFailed"
VirtualMachineExportFailed condition.Cond = "VMExportFailed"
VirtualMachineMigrationFailed ImportStatus = "VMMigrationFailed"
SkipPreflightChecks string = "harvesterhci.io/skip-preflight-checks"
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain why you prefer using an annotation over adding a new field to VirtualMachineImportSpec?

)
152 changes: 134 additions & 18 deletions pkg/source/vmware/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/vmware/govmomi/nfc"
"github.com/vmware/govmomi/object"
"github.com/vmware/govmomi/session"
"github.com/vmware/govmomi/view"
"github.com/vmware/govmomi/vim25"
"github.com/vmware/govmomi/vim25/mo"
"github.com/vmware/govmomi/vim25/soap"
Expand All @@ -31,8 +32,9 @@ import (
type Client struct {
ctx context.Context
*govmomi.Client
tmpCerts string
dc string
tmpCerts string
dc string
networkMapping map[string]string
}

func NewClient(ctx context.Context, endpoint string, dc string, secret *corev1.Secret) (*Client, error) {
Expand Down Expand Up @@ -91,6 +93,11 @@ func NewClient(ctx context.Context, endpoint string, dc string, secret *corev1.S
vmwareClient.ctx = ctx
vmwareClient.Client = c
vmwareClient.dc = dc
networkMap, err := GenerateNetworkMapByRef(ctx, c.Client)
if err != nil {
return nil, fmt.Errorf("error generating network map during client initialisation: %v", err)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return nil, fmt.Errorf("error generating network map during client initialisation: %v", err)
return nil, fmt.Errorf("error generating network map during client initialisation: %w", err)

}
vmwareClient.networkMapping = networkMap
return vmwareClient, nil

}
Expand Down Expand Up @@ -125,10 +132,15 @@ func (c *Client) Verify() error {

func (c *Client) PreFlightChecks(vm *migration.VirtualMachineImport) (err error) {
// Check the source network mappings.
f := find.NewFinder(c.Client.Client, true)
dc := c.dc
if !strings.HasPrefix(c.dc, "/") {
dc = fmt.Sprintf("/%s", c.dc)
if vm.Annotations != nil {
if _, ok := vm.Annotations[migration.SkipPreflightChecks]; ok {
return nil
}
}

networkMap, err := GenerateNetworkMapByName(c.ctx, c.Client.Client)
if err != nil {
return fmt.Errorf("error generating network map: %v", err)
}
for _, nm := range vm.Spec.Mapping {
logrus.WithFields(logrus.Fields{
Expand All @@ -137,11 +149,9 @@ func (c *Client) PreFlightChecks(vm *migration.VirtualMachineImport) (err error)
"sourceNetwork": nm.SourceNetwork,
}).Info("Checking the source network as part of the preflight checks")

// The path looks like `/<datacenter>/network/<network-name>`.
path := filepath.Join(dc, "/network", nm.SourceNetwork)
_, err := f.Network(c.ctx, path)
if err != nil {
return fmt.Errorf("error getting source network '%s': %v", nm.SourceNetwork, err)
elements := strings.Split(nm.SourceNetwork, "/")
if _, ok := networkMap[elements[len(elements)-1]]; ok {
return nil
}
}

Expand Down Expand Up @@ -316,7 +326,7 @@ func (c *Client) GenerateVirtualMachine(vm *migration.VirtualMachineImport) (*ku
}

// Need CPU, Socket, Memory, VirtualNIC information to perform the mapping
networkInfo := identifyNetworkCards(o.Config.Hardware.Device)
networkInfo := identifyNetworkCards(c.networkMapping, o.Config.Hardware.Device)

vmSpec := kubevirt.VirtualMachineSpec{
RunStrategy: &[]kubevirt.VirtualMachineRunStrategy{kubevirt.RunStrategyRerunOnFailure}[0],
Expand Down Expand Up @@ -438,38 +448,58 @@ type networkInfo struct {
MappedNetwork string
}

func identifyNetworkCards(devices []types.BaseVirtualDevice) []networkInfo {
func identifyNetworkCards(networkMap map[string]string, devices []types.BaseVirtualDevice) []networkInfo {
var resp []networkInfo
for _, d := range devices {
switch d := d.(type) {
case *types.VirtualVmxnet:
obj := d
summary := identifyNetworkName(networkMap, *obj.GetVirtualDevice())
if summary == "" {
summary = obj.DeviceInfo.GetDescription().Summary
}
resp = append(resp, networkInfo{
NetworkName: obj.DeviceInfo.GetDescription().Summary,
NetworkName: summary,
MAC: obj.MacAddress,
})
case *types.VirtualE1000e:
obj := d
summary := identifyNetworkName(networkMap, *obj.GetVirtualDevice())
if summary == "" {
summary = obj.DeviceInfo.GetDescription().Summary
}
resp = append(resp, networkInfo{
NetworkName: obj.DeviceInfo.GetDescription().Summary,
NetworkName: summary,
MAC: obj.MacAddress,
})
case *types.VirtualE1000:
obj := d
summary := identifyNetworkName(networkMap, *obj.GetVirtualDevice())
if summary == "" {
summary = obj.DeviceInfo.GetDescription().Summary
}
resp = append(resp, networkInfo{
NetworkName: obj.DeviceInfo.GetDescription().Summary,
NetworkName: summary,
MAC: obj.MacAddress,
})
case *types.VirtualVmxnet3:
obj := d
summary := identifyNetworkName(networkMap, *obj.GetVirtualDevice())
if summary == "" {
summary = obj.DeviceInfo.GetDescription().Summary
}
resp = append(resp, networkInfo{
NetworkName: obj.DeviceInfo.GetDescription().Summary,
NetworkName: summary,
MAC: obj.MacAddress,
})
case *types.VirtualVmxnet2:
obj := d
summary := identifyNetworkName(networkMap, *obj.GetVirtualDevice())
if summary == "" {
summary = obj.DeviceInfo.GetDescription().Summary
}
resp = append(resp, networkInfo{
NetworkName: obj.DeviceInfo.GetDescription().Summary,
NetworkName: summary,
MAC: obj.MacAddress,
})
}
Expand Down Expand Up @@ -510,3 +540,89 @@ func (c *Client) SanitizeVirtualMachineImport(vm *migration.VirtualMachineImport

return nil
}

// GenerateNetworkMayByRef lists all networks defined in the DC and converts them to
// network id: network name
// this subsequently used to map a network id to network name if needed based on the type
// of backing device for a nic
func GenerateNetworkMapByRef(ctx context.Context, c *vim25.Client) (map[string]string, error) {
networks, err := generateNetworkList(ctx, c)
if err != nil {
return nil, err
}
returnMap := make(map[string]string, len(networks))
for _, v := range networks {
returnMap[v.Reference().Value] = v.Name
}
logrus.Infof("generated networkMapByRef: %v", returnMap)
return returnMap, nil
}

func GenerateNetworkMapByName(ctx context.Context, c *vim25.Client) (map[string]string, error) {
networks, err := generateNetworkList(ctx, c)
if err != nil {
return nil, err
}
returnMap := make(map[string]string, len(networks))
for _, v := range networks {
returnMap[v.Name] = v.Reference().Value
}
logrus.Infof("generated networkMapByName: %v", returnMap)
return returnMap, nil
}

func generateNetworkList(ctx context.Context, c *vim25.Client) ([]mo.Network, error) {
var networks []mo.Network
manager := view.NewManager(c)
networkView, err := manager.CreateContainerView(ctx, c.ServiceContent.RootFolder, []string{"Network"}, true)
if err != nil {
return networks, fmt.Errorf("error generating network container view: %v", err)
}
defer networkView.Destroy(ctx)
if err := networkView.Retrieve(ctx, []string{"Network"}, nil, &networks); err != nil {
return networks, fmt.Errorf("error retreiving networks: %v", err)
}
return networks, nil
}

// identifyNetworkName uses the backing device for a nic to identify network name correctly
// in case of a nic using a Distributed VSwitch the summary returned from device is of the form
// DVSwitch : HEX NUMBER which breaks network lookup. As a result we need to identify the network name
// from the PortGroupKey
func identifyNetworkName(networkMap map[string]string, device types.VirtualDevice) string {
var summary string
backing := device.Backing
switch b := backing.(type) {
case *types.VirtualEthernetCardDistributedVirtualPortBackingInfo:
obj := b
logrus.Infof("looking up portgroupkey: %v", obj.Port.PortgroupKey)
summary = networkMap[obj.Port.PortgroupKey]
case *types.VirtualEthernetCardNetworkBackingInfo:
obj := b
logrus.Infof("using devicename: %v", obj.DeviceName)
summary = obj.DeviceName
}
Copy link
Member

Choose a reason for hiding this comment

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

The return variable is already set to empty, but to simplify reading the code I would add a default case to the switch statement. In this case it is totally clear what happens.

Suggested change
}
default:
summary = ""
}

return summary
}

func (c *Client) ListNetworks() error {
mgr := view.NewManager(c.Client.Client)

v, err := mgr.CreateContainerView(c.ctx, c.ServiceContent.RootFolder, []string{"Network"}, true)
if err != nil {
return fmt.Errorf("error creating view %v", err)
}

defer v.Destroy(c.ctx)
var networks []mo.Network
err = v.Retrieve(c.ctx, []string{"Network"}, nil, &networks)
if err != nil {
return fmt.Errorf("error fetching networks: %v", err)
}

for _, net := range networks {
fmt.Printf("%s: %s\n", net.Name, net.Reference())
}

return nil
}
27 changes: 25 additions & 2 deletions pkg/source/vmware/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,16 @@ func Test_identifyNetworkCards(t *testing.T) {
err = vmObj.Properties(c.ctx, vmObj.Reference(), []string{}, &o)
assert.NoError(err, "expected no error looking up vmObj properties")

networkInfo := identifyNetworkCards(o.Config.Hardware.Device)
networkInfo := identifyNetworkCards(c.networkMapping, o.Config.Hardware.Device)
assert.Len(networkInfo, 1, "expected to find only 1 item in the networkInfo")
t.Log(networkInfo)
networkMapping := []migration.NetworkMapping{
{
SourceNetwork: "dummyNetwork",
DestinationNetwork: "harvester1",
},
{
SourceNetwork: "DVSwitch: fea97929-4b2d-5972-b146-930c6d0b4014",
SourceNetwork: "DC0_DVPG0",
DestinationNetwork: "pod-network",
},
}
Expand All @@ -304,3 +305,25 @@ func Test_identifyNetworkCards(t *testing.T) {
noMappedInfo := mapNetworkCards(networkInfo, noNetworkMapping)
assert.Len(noMappedInfo, 0, "expected to find no item in the mapped networkinfo")
}

func Test_ListNetworks(t *testing.T) {
ctx := context.TODO()
endpoint := fmt.Sprintf("https://thinkpad.local:%s/sdk", "8989")
dc := "DC0"
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "default",
},
Data: map[string][]byte{
"username": []byte("user"),
"password": []byte("pass"),
},
}

c, err := NewClient(ctx, endpoint, dc, secret)
assert := require.New(t)
assert.NoError(err)
err = c.ListNetworks()
assert.NoError(err)
}
Loading