From 1d943a61a4959c5a8262bab5f51a41f6d7784b79 Mon Sep 17 00:00:00 2001 From: George Hicken Date: Wed, 12 Jul 2023 18:31:58 -0700 Subject: [PATCH] Support containerIP as mgmtIP and tweaks for podman This adds some tweaks for deltas in how podman returns values - specifically podman uses well formed json for network ls. Known issues: * podman volume ls filters act as OR instead of AND which results in all volumes being deleted any time a single host is removed --- simulator/container.go | 84 ++++++++++++++++--------- simulator/container_host_system.go | 12 ++-- simulator/container_host_system_test.go | 27 ++++++-- simulator/host_system.go | 75 +++++++++++++--------- 4 files changed, 128 insertions(+), 70 deletions(-) diff --git a/simulator/container.go b/simulator/container.go index e79df1db9..681011111 100644 --- a/simulator/container.go +++ b/simulator/container.go @@ -199,6 +199,10 @@ func createVolume(volumeName string, labels []string, files []tarEntry) (string, return "", err } uid = strings.TrimSpace(string(out)) + + if name == "" { + name = uid + } } run := []string{"run", "--rm", "-i"} @@ -255,23 +259,18 @@ func createVolume(volumeName string, labels []string, files []tarEntry) (string, return uid, err } -// createBridge creates a bridge network if one does not already exist -// returns: -// -// uid - string -// err - error or nil -func createBridge(bridgeName string, labels ...string) (string, error) { - +func getBridge(bridgeName string) (string, error) { // {"CreatedAt":"2023-07-11 19:22:25.45027052 +0000 UTC","Driver":"bridge","ID":"fe52c7502c5d","IPv6":"false","Internal":"false","Labels":"goodbye=,hello=","Name":"testnet","Scope":"local"} + // podman has distinctly different fields at v4.4.1 so commented out fields that don't match. We only actually care about ID type bridgeNet struct { - CreatedAt string - Driver string - ID string - IPv6 string - Internal string - Labels string - Name string - Scope string + // CreatedAt string + Driver string + ID string + // IPv6 string + // Internal string + // Labels string + Name string + // Scope string } // if the underlay bridge already exists, return that @@ -281,17 +280,41 @@ func createBridge(bridgeName string, labels ...string) (string, error) { out, err := cmd.Output() if err != nil { log.Printf("vcsim %s: %s", cmd.Args, err) + return "", err } - // unfortunately docker returns an empty string not an empty json doc - if len(out) != 0 { - err = json.Unmarshal(out, &bridge) - if err != nil { - log.Printf("vcsim %s: %s", cmd.Args, err) - return "", err - } + // unfortunately docker returns an empty string not an empty json doc and podman returns '[]' + // podman also returns an array of matches even when there's only one, so we normalize. + str := strings.TrimSpace(string(out)) + str = strings.TrimPrefix(str, "[") + str = strings.TrimSuffix(str, "]") + if len(str) == 0 { + return "", nil + } - return bridge.ID, nil + err = json.Unmarshal([]byte(str), &bridge) + if err != nil { + log.Printf("vcsim %s: %s", cmd.Args, err) + return "", err + } + + return bridge.ID, nil +} + +// createBridge creates a bridge network if one does not already exist +// returns: +// +// uid - string +// err - error or nil +func createBridge(bridgeName string, labels ...string) (string, error) { + + id, err := getBridge(bridgeName) + if err != nil { + return "", err + } + + if id != "" { + return id, nil } run := []string{"network", "create", "--label", createdByVcsim} @@ -300,16 +323,19 @@ func createBridge(bridgeName string, labels ...string) (string, error) { } run = append(run, bridgeName) - cmd = exec.Command("docker", run...) - out, err = cmd.Output() + cmd := exec.Command("docker", run...) + out, err := cmd.Output() if err != nil { - log.Printf("vcsim %s: %s", cmd.Args, err) + log.Printf("vcsim %s: %s: %s", cmd.Args, out, err) return "", err } - // the ID returned in network ls is only 12 characters, so normalize to that - id := string(out[0:12]) - log.Printf("vcsim %s: id=%s", cmd.Args, id) + // docker returns the ID regardless of whether you supply a name when creating the network, however + // podman returns the pretty name, so we have to normalize + id, err = getBridge(bridgeName) + if err != nil { + return "", err + } return id, nil } diff --git a/simulator/container_host_system.go b/simulator/container_host_system.go index 1c061a01b..59f477b35 100644 --- a/simulator/container_host_system.go +++ b/simulator/container_host_system.go @@ -259,10 +259,14 @@ func createSimulationHost(ctx *Context, host *HostSystem) (*simHost, error) { pnic.Mac = settings.MacAddress } - // TODO iterate over the following to update the IPs and MACs: - // 1. host.Config.Network.Pnic - // 2. host.Config.Network.Vnic - // 3. host.Config.VirtualNicManagerInfo.NetConfig + // update the active "management" nicType with the container IP for vmnic0 + netconfig, err := host.getNetConfigInterface(ctx, "management") + if err != nil { + return nil, err + } + netconfig.vmk.Spec.Ip.IpAddress = netconfig.uplink.Spec.Ip.IpAddress + netconfig.vmk.Spec.Ip.SubnetMask = netconfig.uplink.Spec.Ip.SubnetMask + netconfig.vmk.Spec.Mac = netconfig.uplink.Mac return sh, nil } diff --git a/simulator/container_host_system_test.go b/simulator/container_host_system_test.go index e69e95710..0ac82fb51 100644 --- a/simulator/container_host_system_test.go +++ b/simulator/container_host_system_test.go @@ -174,9 +174,14 @@ func TestHostContainerBacking(t *testing.T) { hs := NewHostSystem(esx.HostSystem) hs.configureContainerBacking(ctx, "alpine", defaultSimVolumes, "vcsim-mgmt-underlay") + details, err := hs.getNetConfigInterface(ctx, "management") + assert.NoError(t, err, "Expected no error from management netconfig check") + assert.Equal(t, "0.0.0.0", details.vmk.Spec.Ip.IpAddress, "Expected IP to be empty prior to container creation") + hs.configure(ctx, types.HostConnectSpec{}, true) - //TODO: assert there's a container representing the host (consider a separate test for matching datastores and networks) + assert.NoError(t, err, "Expected no error from management netconfig check") + assert.NotEqual(t, "0.0.0.0", details.vmk.Spec.Ip.IpAddress, "Expected management IP to set after container creation") hs.sh.remove(ctx) } @@ -194,17 +199,27 @@ func TestMultipleSimHost(t *testing.T) { hs := NewHostSystem(esx.HostSystem) hs.configureContainerBacking(ctx, "alpine", defaultSimVolumes) - hs.configure(ctx, types.HostConnectSpec{}, true) - // TODO: assert container present - hs2 := NewHostSystem(esx.HostSystem) hs2.configureContainerBacking(ctx, "alpine", defaultSimVolumes) + details, err := hs.getNetConfigInterface(ctx, "management") + assert.NoError(t, err, "Expected no error from management netconfig check") + assert.Equal(t, "0.0.0.0", details.vmk.Spec.Ip.IpAddress, "Expected IP to be empty prior to container creation") + + hs.configure(ctx, types.HostConnectSpec{}, true) + + details2, err := hs2.getNetConfigInterface(ctx, "management") + assert.NoError(t, err, "Expected no error from management netconfig check") + assert.Equal(t, "0.0.0.0", details2.vmk.Spec.Ip.IpAddress, "Expected IP to be empty prior to container creation") + hs2.configure(ctx, types.HostConnectSpec{}, true) - // TODO: assert 2nd container present + + assert.NotEqual(t, details.vmk.Spec.Ip.IpAddress, details2.vmk.Spec.Ip.IpAddress, "Expected hosts to get different IPs") hs.sh.remove(ctx) - // TODO: assert one container plus volumes left + // TODO: assert one container plus volumes left - need to wait for + // https://github.com/containers/podman/issues/19219 to be fixed for podman to work - otherwise all volumes get removed + // with the first host removed hs2.sh.remove(ctx) } diff --git a/simulator/host_system.go b/simulator/host_system.go index 69a9518a4..eb07b6eb4 100644 --- a/simulator/host_system.go +++ b/simulator/host_system.go @@ -20,6 +20,7 @@ import ( "fmt" "net" "os" + "sync" "time" "github.com/vmware/govmomi/simulator/esx" @@ -32,6 +33,8 @@ import ( var ( hostPortUnique = os.Getenv("VCSIM_HOST_PORT_UNIQUE") == "true" + globalLock sync.Mutex + // globalHostCount is used to construct unique hostnames. Should be consumed under globalLock. globalHostCount = 0 ) @@ -47,10 +50,6 @@ func asHostSystemMO(obj mo.Reference) (*mo.HostSystem, bool) { } func NewHostSystem(host mo.HostSystem) *HostSystem { - // lets us construct non-conflicting hostname automatically if omitted - // does not use the unique port to avoid constraints on port, such as >1024 - globalHostCount++ - if hostPortUnique { // configure unique port for each host port := &esx.HostSystem.Summary.Config.Port *port++ @@ -116,8 +115,16 @@ func (h *HostSystem) configure(ctx *Context, spec types.HostConnectSpec, connect h.Runtime.ConnectionState = types.HostSystemConnectionStateConnected } + // lets us construct non-conflicting hostname automatically if omitted + // does not use the unique port instead to avoid constraints on port, such as >1024 + + globalLock.Lock() + instanceID := globalHostCount + globalHostCount++ + globalLock.Unlock() + if spec.HostName == "" { - spec.HostName = fmt.Sprintf("esx-%d", globalHostCount) + spec.HostName = fmt.Sprintf("esx-%d", instanceID) } else if net.ParseIP(spec.HostName) != nil { h.Config.Network.Vnic[0].Spec.Ip.IpAddress = spec.HostName } @@ -146,6 +153,11 @@ func (h *HostSystem) configure(ctx *Context, spec types.HostConnectSpec, connect // * no mock of VLAN connectivity // * only a single vmknic, used for "the management IP" // * pNIC connectivity does not directly impact VMs/vmks using it as uplink +// +// The pnics will be named using standard pattern, ie. vmnic0, vmnic1, ... +// This will sanity check the NetConfig for "management" nicType to ensure that it maps through PortGroup->vSwitch->pNIC to vmnic0. +// +// TODO: figure out which other HostVirtualNicSpec's need to be updated with IP, eg. Config.vMotion, Config.Network.vNIC func (h *HostSystem) configureContainerBacking(ctx *Context, image string, mounts []types.HostFileSystemMountInfo, networks ...string) error { option := &types.OptionValue{ Key: advOptContainerBackingImage, @@ -249,15 +261,16 @@ func (h *HostSystem) configureContainerBacking(ctx *Context, image string, mount } } - // TODO: construct management bindings - should we do this using public APIs now that host has pNICs? Or do we need - // to have a baseline set of entries first? - // sanity check that everything's hung together sufficiently well - _, err := getNetConfigInterface(ctx, h, "management") + details, err := h.getNetConfigInterface(ctx, "management") if err != nil { return err } + if details.uplink == nil || details.uplink.Device != "vmnic0" { + return fmt.Errorf("Config provided for host %s does not result in a consistent 'management' NetConfig that's bound to 'vmnic0'", h.Name) + } + return nil } @@ -276,14 +289,14 @@ type netConfigDetails struct { // This method is provided because the Config structure held by HostSystem is heavily interconnected but serialized and not cross-linked with pointers. // As such there's a _lot_ of cross-referencing that needs to be done to navigate. // The pNIC returned is the uplink associated with the vSwitch for the netconfig -func getNetConfigInterface(ctx *Context, host *HostSystem, nicType string) (*netConfigDetails, error) { +func (h *HostSystem) getNetConfigInterface(ctx *Context, nicType string) (*netConfigDetails, error) { details := &netConfigDetails{ nicType: nicType, } - for i := range host.Config.VirtualNicManagerInfo.NetConfig { - if host.Config.VirtualNicManagerInfo.NetConfig[i].NicType == nicType { - details.netconfig = &host.Config.VirtualNicManagerInfo.NetConfig[i] + for i := range h.Config.VirtualNicManagerInfo.NetConfig { + if h.Config.VirtualNicManagerInfo.NetConfig[i].NicType == nicType { + details.netconfig = &h.Config.VirtualNicManagerInfo.NetConfig[i] break } } @@ -303,58 +316,58 @@ func getNetConfigInterface(ctx *Context, host *HostSystem, nicType string) (*net } } if details.vmk == nil { - panic(fmt.Sprintf("NetConfig for host %s references non-existant vNIC key %s for %s nicType", host.Name, vnicKey, nicType)) + panic(fmt.Sprintf("NetConfig for host %s references non-existant vNIC key %s for %s nicType", h.Name, vnicKey, nicType)) } portgroupName := details.vmk.Portgroup netstackKey := details.vmk.Spec.NetStackInstanceKey - for i := range host.Config.Network.NetStackInstance { - if host.Config.Network.NetStackInstance[i].Key == netstackKey { - details.netstack = &host.Config.Network.NetStackInstance[i] + for i := range h.Config.Network.NetStackInstance { + if h.Config.Network.NetStackInstance[i].Key == netstackKey { + details.netstack = &h.Config.Network.NetStackInstance[i] break } } if details.netstack == nil { - panic(fmt.Sprintf("NetConfig for host %s references non-existant NetStack key %s for %s nicType", host.Name, netstackKey, nicType)) + panic(fmt.Sprintf("NetConfig for host %s references non-existant NetStack key %s for %s nicType", h.Name, netstackKey, nicType)) } - for i := range host.Config.Network.Portgroup { + for i := range h.Config.Network.Portgroup { // TODO: confirm correctness of this - seems weird it references the Spec.Name instead of the key like everything else. - if host.Config.Network.Portgroup[i].Spec.Name == portgroupName { - details.portgroup = &host.Config.Network.Portgroup[i] + if h.Config.Network.Portgroup[i].Spec.Name == portgroupName { + details.portgroup = &h.Config.Network.Portgroup[i] break } } if details.portgroup == nil { - panic(fmt.Sprintf("NetConfig for host %s references non-existant PortGroup name %s for %s nicType", host.Name, portgroupName, nicType)) + panic(fmt.Sprintf("NetConfig for host %s references non-existant PortGroup name %s for %s nicType", h.Name, portgroupName, nicType)) } vswitchKey := details.portgroup.Vswitch - for i := range host.Config.Network.Vswitch { - if host.Config.Network.Vswitch[i].Key == vswitchKey { - details.vswitch = &host.Config.Network.Vswitch[i] + for i := range h.Config.Network.Vswitch { + if h.Config.Network.Vswitch[i].Key == vswitchKey { + details.vswitch = &h.Config.Network.Vswitch[i] break } } if details.vswitch == nil { - panic(fmt.Sprintf("NetConfig for host %s references non-existant vSwitch key %s for %s nicType", host.Name, vswitchKey, nicType)) + panic(fmt.Sprintf("NetConfig for host %s references non-existant vSwitch key %s for %s nicType", h.Name, vswitchKey, nicType)) } if len(details.vswitch.Pnic) != 1 { // to change this, look at the Active NIC in the NicTeamingPolicy, but for now not worth it - panic(fmt.Sprintf("vSwitch %s for host %s has multiple pNICs associated which is not supported.", vswitchKey, host.Name)) + panic(fmt.Sprintf("vSwitch %s for host %s has multiple pNICs associated which is not supported.", vswitchKey, h.Name)) } pnicKey := details.vswitch.Pnic[0] - for i := range host.Config.Network.Pnic { - if host.Config.Network.Pnic[i].Key == pnicKey { - details.uplink = &host.Config.Network.Pnic[i] + for i := range h.Config.Network.Pnic { + if h.Config.Network.Pnic[i].Key == pnicKey { + details.uplink = &h.Config.Network.Pnic[i] break } } if details.uplink == nil { - panic(fmt.Sprintf("NetConfig for host %s references non-existant pNIC key %s for %s nicType", host.Name, pnicKey, nicType)) + panic(fmt.Sprintf("NetConfig for host %s references non-existant pNIC key %s for %s nicType", h.Name, pnicKey, nicType)) } return details, nil