Skip to content

Commit

Permalink
Build infra configurator at constructor
Browse files Browse the repository at this point in the history
When testing podNIC we need to mock the infraconfigurators, to allow
this a new field "infraConfigurator" is added to the struct and the
constructor has being split into two "newPodNIC" and
"newPodNICWithoutInfraConfigurator".

Call newPodNICWithoutInfraConfigurator at phase2

Signed-off-by: Quique Llorente <[email protected]>
  • Loading branch information
qinqon committed Jul 6, 2021
1 parent 2577e37 commit b120380
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 62 deletions.
1 change: 0 additions & 1 deletion pkg/virt-launcher/virtwrap/network/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,5 @@ go_test(
"//vendor/github.com/onsi/ginkgo:go_default_library",
"//vendor/github.com/onsi/gomega:go_default_library",
"//vendor/github.com/vishvananda/netlink:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
],
)
15 changes: 6 additions & 9 deletions pkg/virt-launcher/virtwrap/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,15 @@ func NewVMNetworkConfigurator(vmi *v1.VirtualMachineInstance, cacheFactory cache
return newVMNetworkConfiguratorWithHandlerAndCache(vmi, &netdriver.NetworkUtilsHandler{}, cacheFactory)
}

func (v VMNetworkConfigurator) getNICs() ([]podNIC, error) {
return v.getNICsWithLauncherPID(nil)

}

func (v VMNetworkConfigurator) getNICsWithLauncherPID(launcherPID *int) ([]podNIC, error) {
func (v VMNetworkConfigurator) getNICs(podNICFactory func(*v1.VirtualMachineInstance, *v1.Network, netdriver.NetworkHandler, cache.InterfaceCacheFactory, *int) (*podNIC, error), launcherPID *int) ([]podNIC, error) {
nics := []podNIC{}

if len(v.vmi.Spec.Domain.Devices.Interfaces) == 0 {
return nics, nil
}

for i, _ := range v.vmi.Spec.Networks {
nic, err := newPodNIC(v.vmi, &v.vmi.Spec.Networks[i], v.handler, v.cacheFactory, launcherPID)
nic, err := podNICFactory(v.vmi, &v.vmi.Spec.Networks[i], v.handler, v.cacheFactory, launcherPID)
if err != nil {
return nil, err
}
Expand All @@ -72,7 +67,8 @@ func (v VMNetworkConfigurator) getNICsWithLauncherPID(launcherPID *int) ([]podNI
}

func (n *VMNetworkConfigurator) SetupPodNetworkPhase1(pid int) error {
nics, err := n.getNICsWithLauncherPID(&pid)
launcherPID := &pid
nics, err := n.getNICs(newPodNIC, launcherPID)
if err != nil {
return err
}
Expand All @@ -85,7 +81,8 @@ func (n *VMNetworkConfigurator) SetupPodNetworkPhase1(pid int) error {
}

func (n *VMNetworkConfigurator) SetupPodNetworkPhase2(domain *api.Domain) error {
nics, err := n.getNICs()
var launcherPID *int
nics, err := n.getNICs(newPodNICWithoutInfraConfigurator, launcherPID)
if err != nil {
return err
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/virt-launcher/virtwrap/network/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ var _ = Describe("VMNetworkConfigurator", func() {
vmNetworkConfigurator := NewVMNetworkConfigurator(vm, fake.NewFakeInMemoryNetworkCacheFactory())
iface := v1.DefaultBridgeNetworkInterface()
defaultNet := v1.DefaultPodNetwork()
nics, err := vmNetworkConfigurator.getNICs()
nics, err := vmNetworkConfigurator.getNICs(newPodNICWithoutInfraConfigurator, nil)
Expect(err).ToNot(HaveOccurred())
Expect(nics).To(ConsistOf([]podNIC{{
vmi: vm,
Expand All @@ -83,7 +83,7 @@ var _ = Describe("VMNetworkConfigurator", func() {
It("should accept empty network list", func() {
vmi := newVMI("testnamespace", "testVmName")
vmNetworkConfigurator := NewVMNetworkConfigurator(vmi, fake.NewFakeInMemoryNetworkCacheFactory())
nics, err := vmNetworkConfigurator.getNICs()
nics, err := vmNetworkConfigurator.getNICs(newPodNICWithoutInfraConfigurator, nil)
Expect(err).ToNot(HaveOccurred())
Expect(nics).To(BeEmpty())
})
Expand All @@ -99,7 +99,7 @@ var _ = Describe("VMNetworkConfigurator", func() {
}
vmi.Spec.Networks = []v1.Network{*cniNet}
vmNetworkConfigurator := NewVMNetworkConfigurator(vmi, fake.NewFakeInMemoryNetworkCacheFactory())
nics, err := vmNetworkConfigurator.getNICs()
nics, err := vmNetworkConfigurator.getNICs(newPodNICWithoutInfraConfigurator, nil)
Expect(err).ToNot(HaveOccurred())
Expect(nics).To(ConsistOf([]podNIC{{
vmi: vmi,
Expand Down Expand Up @@ -163,7 +163,7 @@ var _ = Describe("VMNetworkConfigurator", func() {
vm.Spec.Networks = []v1.Network{*additionalCNINet1, *cniNet, *additionalCNINet2}

vmNetworkConfigurator := NewVMNetworkConfigurator(vm, fake.NewFakeInMemoryNetworkCacheFactory())
nics, err := vmNetworkConfigurator.getNICs()
nics, err := vmNetworkConfigurator.getNICs(newPodNICWithoutInfraConfigurator, nil)
Expect(err).ToNot(HaveOccurred())
Expect(nics).To(ContainElements([]podNIC{
{
Expand Down
96 changes: 48 additions & 48 deletions pkg/virt-launcher/virtwrap/network/podnic.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,52 @@ import (
)

type podNIC struct {
vmi *v1.VirtualMachineInstance
podInterfaceName string
launcherPID *int
vmiSpecIface *v1.Interface
vmiSpecNetwork *v1.Network
handler netdriver.NetworkHandler
cacheFactory cache.InterfaceCacheFactory
dhcpConfigurator *dhcpconfigurator.Configurator
vmi *v1.VirtualMachineInstance
podInterfaceName string
launcherPID *int
vmiSpecIface *v1.Interface
vmiSpecNetwork *v1.Network
handler netdriver.NetworkHandler
cacheFactory cache.InterfaceCacheFactory
dhcpConfigurator *dhcpconfigurator.Configurator
infraConfigurator infraconfigurators.PodNetworkInfraConfigurator
}

func newPodNIC(vmi *v1.VirtualMachineInstance, network *v1.Network, handler netdriver.NetworkHandler, cacheFactory cache.InterfaceCacheFactory, launcherPID *int) (*podNIC, error) {
podnic, err := newPodNICWithoutInfraConfigurator(vmi, network, handler, cacheFactory, launcherPID)
if err != nil {
return nil, err
}

if launcherPID == nil {
return nil, fmt.Errorf("missing launcher PID to construct infra configurators")
}

if podnic.vmiSpecIface.Bridge != nil {
podnic.infraConfigurator = infraconfigurators.NewBridgePodNetworkConfigurator(
podnic.vmi,
podnic.vmiSpecIface,
generateInPodBridgeInterfaceName(podnic.podInterfaceName),
*podnic.launcherPID,
handler)
} else if podnic.vmiSpecIface.Masquerade != nil {
podnic.infraConfigurator = infraconfigurators.NewMasqueradePodNetworkConfigurator(
podnic.vmi,
podnic.vmiSpecIface,
generateInPodBridgeInterfaceName(podnic.podInterfaceName),
podnic.vmiSpecNetwork.Pod.VMNetworkCIDR,
podnic.vmiSpecNetwork.Pod.VMIPv6NetworkCIDR,
*podnic.launcherPID,
podnic.handler)
} else if podnic.vmiSpecIface.Macvtap != nil {
podnic.infraConfigurator = infraconfigurators.NewMacvtapPodNetworkConfigurator(
podnic.podInterfaceName,
podnic.vmiSpecIface,
podnic.handler)
}
return podnic, nil
}
func newPodNICWithoutInfraConfigurator(vmi *v1.VirtualMachineInstance, network *v1.Network, handler netdriver.NetworkHandler, cacheFactory cache.InterfaceCacheFactory, launcherPID *int) (*podNIC, error) {
if network.Pod == nil && network.Multus == nil {
return nil, fmt.Errorf("Network not implemented")
}
Expand Down Expand Up @@ -74,6 +109,7 @@ func newPodNIC(vmi *v1.VirtualMachineInstance, network *v1.Network, handler netd
generateInPodBridgeInterfaceName(podInterfaceName),
handler)
}

return &podNIC{
cacheFactory: cacheFactory,
handler: handler,
Expand Down Expand Up @@ -159,29 +195,24 @@ func (l *podNIC) PlugPhase1() error {
}

if !doesExist {
podNetworkingConfigurator, err := l.newPodNetworkConfigurator()
if err != nil {
return err
}

if err := podNetworkingConfigurator.DiscoverPodNetworkInterface(l.podInterfaceName); err != nil {
if err := l.infraConfigurator.DiscoverPodNetworkInterface(l.podInterfaceName); err != nil {
return err
}

if l.dhcpConfigurator != nil {
dhcpConfig := podNetworkingConfigurator.GenerateDHCPConfig()
dhcpConfig := l.infraConfigurator.GenerateDHCPConfig()
log.Log.V(4).Infof("The generated dhcpConfig: %s", dhcpConfig.String())
if err := l.dhcpConfigurator.ExportConfiguration(*dhcpConfig); err != nil {
log.Log.Reason(err).Error("failed to save dhcpConfig configuration")
return errors.CreateCriticalNetworkError(err)
}
}

domainIface := podNetworkingConfigurator.GenerateDomainIfaceSpec()
domainIface := l.infraConfigurator.GenerateDomainIfaceSpec()
// preparePodNetworkInterface must be called *after* the generate
// methods since it mutates the pod interface from which those
// generator methods get their info from.
if err := podNetworkingConfigurator.PreparePodNetworkInterface(); err != nil {
if err := l.infraConfigurator.PreparePodNetworkInterface(); err != nil {
log.Log.Reason(err).Error("failed to prepare pod networking")
return errors.CreateCriticalNetworkError(err)
}
Expand Down Expand Up @@ -261,37 +292,6 @@ func (l *podNIC) newLibvirtSpecGenerator(domain *api.Domain) (LibvirtSpecGenerat
return nil, fmt.Errorf("Not implemented")
}

func (l *podNIC) newPodNetworkConfigurator() (infraconfigurators.PodNetworkInfraConfigurator, error) {
if l.vmiSpecIface.Bridge != nil {
return infraconfigurators.NewBridgePodNetworkConfigurator(
l.vmi,
l.vmiSpecIface,
generateInPodBridgeInterfaceName(l.podInterfaceName),
*l.launcherPID,
l.handler), nil
}
if l.vmiSpecIface.Masquerade != nil {
return infraconfigurators.NewMasqueradePodNetworkConfigurator(
l.vmi,
l.vmiSpecIface,
generateInPodBridgeInterfaceName(l.podInterfaceName),
l.vmiSpecNetwork.Pod.VMNetworkCIDR,
l.vmiSpecNetwork.Pod.VMIPv6NetworkCIDR,
*l.launcherPID,
l.handler), nil
}
if l.vmiSpecIface.Slirp != nil {
return nil, nil
}
if l.vmiSpecIface.Macvtap != nil {
return infraconfigurators.NewMacvtapPodNetworkConfigurator(
l.podInterfaceName,
l.vmiSpecIface,
l.handler), nil
}
return nil, fmt.Errorf("Not implemented")
}

func (l *podNIC) cachedDomainInterface() (*api.Interface, error) {
ifaceConfig, err := l.cacheFactory.CacheDomainInterfaceForPID(getPIDString(l.launcherPID)).Read(l.vmiSpecIface.Name)

Expand Down

0 comments on commit b120380

Please sign in to comment.