Skip to content

Commit

Permalink
refactor, rename structs and functions to use consistent DHCP term
Browse files Browse the repository at this point in the history 
This commit only changes the structs and functions to use DHCP terminology
(instead of Dhcp/dhcp) in the following instances:
- DHCPConfig
- generateDHCPConfig
- decorateDHCPConfigRoutes
- fakeDHCPConfigCacheStore
- CacheDHCPConfigForPid
- DHCPConfigStore
- newDHCPConfigCacheStore
- createDummyDHCPConfig
- getDHCPStartedFilePath
- EnsureDHCPServerStarted

The change does not introduce any logic changes.

Signed-off-by: Ram Lavi <[email protected]>
  • Loading branch information
@RamLavi
RamLavi committed Jun 15, 2021
1 parent 09be138 commit c933095
Show file tree
Hide file tree
Showing 10 changed files with 72 additions and 72 deletions.
18 changes: 9 additions & 9 deletions pkg/network/cache/fake/infocache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ func NewFakeInMemoryNetworkCacheFactory() cache.InterfaceCacheFactory {
return &fakeInterfaceCacheFactory{
vmiCacheStores: map[types.UID]*fakePodInterfaceCacheStore{},
domainCacheStores: map[string]*fakeDomainInterfaceStore{},
dhcpConfigStores: map[string]*fakeDhcpConfigCacheStore{},
dhcpConfigStores: map[string]*fakeDHCPConfigCacheStore{},
lock: &sync.Mutex{},
}
}

type fakeInterfaceCacheFactory struct {
vmiCacheStores map[types.UID]*fakePodInterfaceCacheStore
domainCacheStores map[string]*fakeDomainInterfaceStore
dhcpConfigStores map[string]*fakeDhcpConfigCacheStore
dhcpConfigStores map[string]*fakeDHCPConfigCacheStore
lock *sync.Mutex
}

Expand Down Expand Up @@ -53,14 +53,14 @@ func (f *fakeInterfaceCacheFactory) CacheDomainInterfaceForPID(pid string) cache
return f.domainCacheStores[pid]
}

func (f *fakeInterfaceCacheFactory) CacheDhcpConfigForPid(pid string) cache.DhcpConfigStore {
func (f *fakeInterfaceCacheFactory) CacheDHCPConfigForPid(pid string) cache.DHCPConfigStore {
f.lock.Lock()
defer f.lock.Unlock()
if store, exists := f.dhcpConfigStores[pid]; exists {
return store
}
f.dhcpConfigStores[pid] = &fakeDhcpConfigCacheStore{
store: map[string]*cache.DhcpConfig{},
f.dhcpConfigStores[pid] = &fakeDHCPConfigCacheStore{
store: map[string]*cache.DHCPConfig{},
lock: &sync.Mutex{},
}
return f.dhcpConfigStores[pid]
Expand Down Expand Up @@ -115,12 +115,12 @@ func (f *fakeDomainInterfaceStore) Write(ifaceName string, cacheInterface *api.I
return nil
}

type fakeDhcpConfigCacheStore struct {
type fakeDHCPConfigCacheStore struct {
lock *sync.Mutex
store map[string]*cache.DhcpConfig
store map[string]*cache.DHCPConfig
}

func (f *fakeDhcpConfigCacheStore) Read(ifaceName string) (*cache.DhcpConfig, error) {
func (f *fakeDHCPConfigCacheStore) Read(ifaceName string) (*cache.DHCPConfig, error) {
f.lock.Lock()
defer f.lock.Unlock()
if val, exists := f.store[ifaceName]; exists {
Expand All @@ -129,7 +129,7 @@ func (f *fakeDhcpConfigCacheStore) Read(ifaceName string) (*cache.DhcpConfig, er
return nil, os.ErrNotExist
}

func (f *fakeDhcpConfigCacheStore) Write(ifaceName string, vifToCache *cache.DhcpConfig) error {
func (f *fakeDHCPConfigCacheStore) Write(ifaceName string, vifToCache *cache.DHCPConfig) error {
f.lock.Lock()
defer f.lock.Unlock()
f.store[ifaceName] = vifToCache
Expand Down
20 changes: 10 additions & 10 deletions pkg/network/cache/infocache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ var dhcpConfigCachedPattern = "/proc/%s/root/var/run/kubevirt-private/vif-cache-
type InterfaceCacheFactory interface {
CacheForVMI(vmi *v1.VirtualMachineInstance) PodInterfaceCacheStore
CacheDomainInterfaceForPID(pid string) DomainInterfaceStore
CacheDhcpConfigForPid(pid string) DhcpConfigStore
CacheDHCPConfigForPid(pid string) DHCPConfigStore
}

func NewInterfaceCacheFactory() *interfaceCacheFactory {
Expand All @@ -59,8 +59,8 @@ func (i *interfaceCacheFactory) CacheDomainInterfaceForPID(pid string) DomainInt
return newDomainInterfaceStore(pid, i.baseDir, virtLauncherCachedPattern)
}

func (i *interfaceCacheFactory) CacheDhcpConfigForPid(pid string) DhcpConfigStore {
return newDhcpConfigCacheStore(pid, i.baseDir, dhcpConfigCachedPattern)
func (i *interfaceCacheFactory) CacheDHCPConfigForPid(pid string) DHCPConfigStore {
return newDHCPConfigCacheStore(pid, i.baseDir, dhcpConfigCachedPattern)
}

type DomainInterfaceStore interface {
Expand All @@ -74,9 +74,9 @@ type PodInterfaceCacheStore interface {
Remove() error
}

type DhcpConfigStore interface {
Read(ifaceName string) (*DhcpConfig, error)
Write(ifaceName string, cacheInterface *DhcpConfig) error
type DHCPConfigStore interface {
Read(ifaceName string) (*DHCPConfig, error)
Write(ifaceName string, cacheInterface *DHCPConfig) error
}

type domainInterfaceStore struct {
Expand Down Expand Up @@ -131,21 +131,21 @@ type dhcpConfigCacheStore struct {
baseDir string
}

func (d dhcpConfigCacheStore) Read(ifaceName string) (*DhcpConfig, error) {
cachedIface := &DhcpConfig{}
func (d dhcpConfigCacheStore) Read(ifaceName string) (*DHCPConfig, error) {
cachedIface := &DHCPConfig{}
err := readFromCachedFile(cachedIface, d.getInterfaceCacheFile(ifaceName))
return cachedIface, err
}

func (d dhcpConfigCacheStore) Write(ifaceName string, ifaceToCache *DhcpConfig) error {
func (d dhcpConfigCacheStore) Write(ifaceName string, ifaceToCache *DHCPConfig) error {
return writeToCachedFile(ifaceToCache, d.getInterfaceCacheFile(ifaceName))
}

func (d dhcpConfigCacheStore) getInterfaceCacheFile(ifaceName string) string {
return getInterfaceCacheFile(d.baseDir, d.pattern, d.pid, ifaceName)
}

func newDhcpConfigCacheStore(pid string, baseDir, pattern string) dhcpConfigCacheStore {
func newDHCPConfigCacheStore(pid string, baseDir, pattern string) dhcpConfigCacheStore {
return dhcpConfigCacheStore{pid: pid, baseDir: baseDir, pattern: pattern}
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/network/cache/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type PodCacheInterface struct {
PodIPs []string `json:"podIPs,omitempty"`
}

type DhcpConfig struct {
type DHCPConfig struct {
Name string
IP netlink.Addr
IPv6 netlink.Addr
Expand All @@ -28,9 +28,9 @@ type DhcpConfig struct {
Gateway net.IP
}

func (d DhcpConfig) String() string {
func (d DHCPConfig) String() string {
return fmt.Sprintf(
"DhcpConfig: { Name: %s, IPv4: %s, IPv6: %s, MAC: %s, AdvertisingIPAddr: %s, MTU: %d, Gateway: %s, IPAMDisabled: %t}",
"DHCPConfig: { Name: %s, IPv4: %s, IPv6: %s, MAC: %s, AdvertisingIPAddr: %s, MTU: %d, Gateway: %s, IPAMDisabled: %t}",
d.Name,
d.IP,
d.IPv6,
Expand Down
18 changes: 9 additions & 9 deletions pkg/network/cache/types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/vishvananda/netlink"
)

var _ = Describe("DhcpConfig", func() {
var _ = Describe("DHCPConfig", func() {
const ipv4Cidr = "10.0.0.200/24"
const ipv6Cidr = "fd10:0:2::2/120"
const mac = "de:ad:00:00:be:ef"
Expand All @@ -20,32 +20,32 @@ var _ = Describe("DhcpConfig", func() {

Context("String", func() {
It("returns correct string representation", func() {
dhcpConfig := createDummyDhcpConfig(vifName, ipv4Cidr, ipv4Gateway, "", mac, mtu)
Expect(dhcpConfig.String()).To(Equal(fmt.Sprintf("DhcpConfig: { Name: %s, IPv4: %s, IPv6: <nil>, MAC: %s, AdvertisingIPAddr: %s, MTU: %d, Gateway: %s, IPAMDisabled: false}", vifName, ipv4Cidr, mac, ipv4Gateway, mtu, ipv4Gateway)))
dhcpConfig := createDummyDHCPConfig(vifName, ipv4Cidr, ipv4Gateway, "", mac, mtu)
Expect(dhcpConfig.String()).To(Equal(fmt.Sprintf("DHCPConfig: { Name: %s, IPv4: %s, IPv6: <nil>, MAC: %s, AdvertisingIPAddr: %s, MTU: %d, Gateway: %s, IPAMDisabled: false}", vifName, ipv4Cidr, mac, ipv4Gateway, mtu, ipv4Gateway)))
})
It("returns correct string representation with ipv6", func() {
dhcpConfig := createDummyDhcpConfig(vifName, ipv4Cidr, ipv4Gateway, ipv6Cidr, mac, mtu)
Expect(dhcpConfig.String()).To(Equal(fmt.Sprintf("DhcpConfig: { Name: %s, IPv4: %s, IPv6: %s, MAC: %s, AdvertisingIPAddr: %s, MTU: %d, Gateway: %s, IPAMDisabled: false}", vifName, ipv4Cidr, ipv6Cidr, mac, ipv4Gateway, mtu, ipv4Gateway)))
dhcpConfig := createDummyDHCPConfig(vifName, ipv4Cidr, ipv4Gateway, ipv6Cidr, mac, mtu)
Expect(dhcpConfig.String()).To(Equal(fmt.Sprintf("DHCPConfig: { Name: %s, IPv4: %s, IPv6: %s, MAC: %s, AdvertisingIPAddr: %s, MTU: %d, Gateway: %s, IPAMDisabled: false}", vifName, ipv4Cidr, ipv6Cidr, mac, ipv4Gateway, mtu, ipv4Gateway)))
})
It("returns correct string representation when an IP is not defined", func() {
gw := net.ParseIP(ipv4Gateway)
macAddr, _ := net.ParseMAC(mac)
dhcpConfig := DhcpConfig{
dhcpConfig := DHCPConfig{
Name: vifName,
MAC: macAddr,
AdvertisingIPAddr: gw,
Mtu: mtu,
Gateway: gw,
}
Expect(dhcpConfig.String()).To(Equal(fmt.Sprintf("DhcpConfig: { Name: %s, IPv4: <nil>, IPv6: <nil>, MAC: %s, AdvertisingIPAddr: %s, MTU: %d, Gateway: %s, IPAMDisabled: false}", vifName, mac, ipv4Gateway, mtu, ipv4Gateway)))
Expect(dhcpConfig.String()).To(Equal(fmt.Sprintf("DHCPConfig: { Name: %s, IPv4: <nil>, IPv6: <nil>, MAC: %s, AdvertisingIPAddr: %s, MTU: %d, Gateway: %s, IPAMDisabled: false}", vifName, mac, ipv4Gateway, mtu, ipv4Gateway)))
})
})
})

func createDummyDhcpConfig(vifName, ipv4cidr, ipv4gateway, ipv6cidr, macStr string, mtu uint16) *DhcpConfig {
func createDummyDHCPConfig(vifName, ipv4cidr, ipv4gateway, ipv6cidr, macStr string, mtu uint16) *DHCPConfig {
mac, _ := net.ParseMAC(macStr)
gw := net.ParseIP(ipv4gateway)
dhcpConfig := &DhcpConfig{
dhcpConfig := &DHCPConfig{
Name: vifName,
MAC: mac,
AdvertisingIPAddr: gw,
Expand Down
16 changes: 8 additions & 8 deletions pkg/network/dhcp/configurator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type Configurator struct {
}

// NewConfiguratorWithClientFilter should be used when the DHCP server is
// expected to only reply to the MAC specified in the `cache.DhcpConfig` struct
// expected to only reply to the MAC specified in the `cache.DHCPConfig` struct
func NewConfiguratorWithClientFilter(cacheFactory cache.InterfaceCacheFactory, launcherPID string, advertisingIfaceName string, handler netdriver.NetworkHandler) *Configurator {
return &Configurator{
advertisingIfaceName: advertisingIfaceName,
Expand All @@ -46,8 +46,8 @@ func NewConfigurator(cacheFactory cache.InterfaceCacheFactory, launcherPID strin
}
}

func (d Configurator) ImportConfiguration(ifaceName string) (*cache.DhcpConfig, error) {
dhcpConfig, err := d.cacheFactory.CacheDhcpConfigForPid(d.launcherPID).Read(ifaceName)
func (d Configurator) ImportConfiguration(ifaceName string) (*cache.DHCPConfig, error) {
dhcpConfig, err := d.cacheFactory.CacheDHCPConfigForPid(d.launcherPID).Read(ifaceName)
if err != nil {
return nil, err
}
Expand All @@ -57,15 +57,15 @@ func (d Configurator) ImportConfiguration(ifaceName string) (*cache.DhcpConfig,
return dhcpConfig, nil
}

func (d Configurator) ExportConfiguration(config cache.DhcpConfig) error {
return d.cacheFactory.CacheDhcpConfigForPid(d.launcherPID).Write(config.Name, &config)
func (d Configurator) ExportConfiguration(config cache.DHCPConfig) error {
return d.cacheFactory.CacheDHCPConfigForPid(d.launcherPID).Write(config.Name, &config)
}

func (d Configurator) EnsureDhcpServerStarted(podInterfaceName string, dhcpConfig cache.DhcpConfig, dhcpOptions *v1.DHCPOptions) error {
func (d Configurator) EnsureDHCPServerStarted(podInterfaceName string, dhcpConfig cache.DHCPConfig, dhcpOptions *v1.DHCPOptions) error {
if dhcpConfig.IPAMDisabled {
return nil
}
dhcpStartedFile := d.getDhcpStartedFilePath(podInterfaceName)
dhcpStartedFile := d.getDHCPStartedFilePath(podInterfaceName)
_, err := os.Stat(dhcpStartedFile)
if os.IsNotExist(err) {
if err := d.handler.StartDHCP(&dhcpConfig, d.advertisingIfaceName, dhcpOptions, d.filterByMac); err != nil {
Expand All @@ -80,6 +80,6 @@ func (d Configurator) EnsureDhcpServerStarted(podInterfaceName string, dhcpConfi
return nil
}

func (d Configurator) getDhcpStartedFilePath(podInterfaceName string) string {
func (d Configurator) getDHCPStartedFilePath(podInterfaceName string) string {
return fmt.Sprintf("%s/dhcp_started-%s", d.dhcpStartedDirectory, podInterfaceName)
}
20 changes: 10 additions & 10 deletions pkg/network/dhcp/configurator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ var _ = Describe("DHCP configurator", func() {
)

table.DescribeTable("should succeed when cache file present", func(configurator *Configurator) {
dhcpConfig := cache.DhcpConfig{
dhcpConfig := cache.DHCPConfig{
Name: ifaceName,
IP: netlink.Addr{},
Mtu: 1400,
Expand All @@ -77,7 +77,7 @@ var _ = Describe("DHCP configurator", func() {

Context("start DHCP function", func() {
var advertisingAddr netlink.Addr
var dhcpConfig cache.DhcpConfig
var dhcpConfig cache.DHCPConfig
var dhcpOptions *v1.DHCPOptions

BeforeEach(func() {
Expand All @@ -87,7 +87,7 @@ var _ = Describe("DHCP configurator", func() {
})

BeforeEach(func() {
dhcpConfig = cache.DhcpConfig{
dhcpConfig = cache.DHCPConfig{
Name: ifaceName,
IP: netlink.Addr{},
AdvertisingIPAddr: advertisingAddr.IP,
Expand All @@ -99,7 +99,7 @@ var _ = Describe("DHCP configurator", func() {
table.DescribeTable("should succeed when DHCP server started", func(configurator *Configurator) {
configurator.handler.(*netdriver.MockNetworkHandler).EXPECT().StartDHCP(&dhcpConfig, bridgeName, nil, configurator.filterByMac).Return(nil)

Expect(configurator.EnsureDhcpServerStarted(ifaceName, dhcpConfig, dhcpOptions)).To(Succeed())
Expect(configurator.EnsureDHCPServerStarted(ifaceName, dhcpConfig, dhcpOptions)).To(Succeed())
},
table.Entry("with active client filtering", newConfiguratorWithClientFilter(launcherPID, bridgeName)),
table.Entry("without client filtering", newConfigurator(launcherPID, bridgeName)),
Expand All @@ -108,8 +108,8 @@ var _ = Describe("DHCP configurator", func() {
table.DescribeTable("should succeed when DHCP server is started multiple times", func(configurator *Configurator) {
configurator.handler.(*netdriver.MockNetworkHandler).EXPECT().StartDHCP(&dhcpConfig, bridgeName, nil, configurator.filterByMac).Return(nil)

Expect(configurator.EnsureDhcpServerStarted(ifaceName, dhcpConfig, dhcpOptions)).To(Succeed())
Expect(configurator.EnsureDhcpServerStarted(ifaceName, dhcpConfig, dhcpOptions)).To(Succeed())
Expect(configurator.EnsureDHCPServerStarted(ifaceName, dhcpConfig, dhcpOptions)).To(Succeed())
Expect(configurator.EnsureDHCPServerStarted(ifaceName, dhcpConfig, dhcpOptions)).To(Succeed())
},
table.Entry("with active client filtering", newConfiguratorWithClientFilter(launcherPID, bridgeName)),
table.Entry("without client filtering", newConfigurator(launcherPID, bridgeName)),
Expand All @@ -118,15 +118,15 @@ var _ = Describe("DHCP configurator", func() {
table.DescribeTable("should fail when DHCP server failed", func(configurator *Configurator) {
configurator.handler.(*netdriver.MockNetworkHandler).EXPECT().StartDHCP(&dhcpConfig, bridgeName, nil, configurator.filterByMac).Return(fmt.Errorf("failed to start DHCP server"))

Expect(configurator.EnsureDhcpServerStarted(ifaceName, dhcpConfig, dhcpOptions)).To(HaveOccurred())
Expect(configurator.EnsureDHCPServerStarted(ifaceName, dhcpConfig, dhcpOptions)).To(HaveOccurred())
},
table.Entry("with active client filtering", newConfiguratorWithClientFilter(launcherPID, bridgeName)),
table.Entry("without client filtering", newConfigurator(launcherPID, bridgeName)),
)

When("IPAM is disabled on the DhcpConfig", func() {
When("IPAM is disabled on the DHCPConfig", func() {
BeforeEach(func() {
dhcpConfig = cache.DhcpConfig{
dhcpConfig = cache.DHCPConfig{
Name: ifaceName,
IP: netlink.Addr{},
AdvertisingIPAddr: advertisingAddr.IP,
Expand All @@ -138,7 +138,7 @@ var _ = Describe("DHCP configurator", func() {
table.DescribeTable("should fail when DHCP server failed", func(configurator *Configurator) {
configurator.handler.(*netdriver.MockNetworkHandler).EXPECT().StartDHCP(&dhcpConfig, bridgeName, nil, configurator.filterByMac).Return(nil).Times(0)

Expect(configurator.EnsureDhcpServerStarted(ifaceName, dhcpConfig, dhcpOptions)).To(Succeed())
Expect(configurator.EnsureDHCPServerStarted(ifaceName, dhcpConfig, dhcpOptions)).To(Succeed())
},
table.Entry("with active client filtering", newConfiguratorWithClientFilter(launcherPID, bridgeName)),
table.Entry("without client filtering", newConfigurator(launcherPID, bridgeName)),
Expand Down
6 changes: 3 additions & 3 deletions pkg/network/driver/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ type NetworkHandler interface {
SetRandomMac(iface string) (net.HardwareAddr, error)
GetMacDetails(iface string) (net.HardwareAddr, error)
LinkSetMaster(link netlink.Link, master *netlink.Bridge) error
StartDHCP(nic *cache.DhcpConfig, bridgeInterfaceName string, dhcpOptions *v1.DHCPOptions, filterByMAC bool) error
StartDHCP(nic *cache.DHCPConfig, bridgeInterfaceName string, dhcpOptions *v1.DHCPOptions, filterByMAC bool) error
HasNatIptables(proto iptables.Protocol) bool
IsIpv6Enabled(interfaceName string) (bool, error)
IsIpv4Primary() (bool, error)
Expand Down Expand Up @@ -365,7 +365,7 @@ func (h *NetworkUtilsHandler) SetRandomMac(iface string) (net.HardwareAddr, erro
return currentMac, nil
}

func (h *NetworkUtilsHandler) StartDHCP(nic *cache.DhcpConfig, bridgeInterfaceName string, dhcpOptions *v1.DHCPOptions, filterByMAC bool) error {
func (h *NetworkUtilsHandler) StartDHCP(nic *cache.DHCPConfig, bridgeInterfaceName string, dhcpOptions *v1.DHCPOptions, filterByMAC bool) error {
log.Log.V(4).Infof("StartDHCP network Nic: %+v", nic)
nameservers, searchDomains, err := converter.GetResolvConfDetailsFromPod()
if err != nil {
Expand Down Expand Up @@ -475,7 +475,7 @@ var DHCPServer = dhcp.SingleClientDHCPServer
var DHCPv6Server = dhcpv6.SingleClientDHCPv6Server

// filter out irrelevant routes
func FilterPodNetworkRoutes(routes []netlink.Route, nic *cache.DhcpConfig) (filteredRoutes []netlink.Route) {
func FilterPodNetworkRoutes(routes []netlink.Route, nic *cache.DHCPConfig) (filteredRoutes []netlink.Route) {
for _, route := range routes {
log.Log.V(5).Infof("route: %s", route.String())
// don't create empty static routes
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/driver/generated_mock_common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func (_mr *_MockNetworkHandlerRecorder) LinkSetMaster(arg0, arg1 interface{}) *g
return _mr.mock.ctrl.RecordCall(_mr.mock, "LinkSetMaster", arg0, arg1)
}

func (_m *MockNetworkHandler) StartDHCP(nic *cache.DhcpConfig, bridgeInterfaceName string, dhcpOptions *v1.DHCPOptions, filterByMAC bool) error {
func (_m *MockNetworkHandler) StartDHCP(nic *cache.DHCPConfig, bridgeInterfaceName string, dhcpOptions *v1.DHCPOptions, filterByMAC bool) error {
ret := _m.ctrl.Call(_m, "StartDHCP", nic, bridgeInterfaceName, dhcpOptions, filterByMAC)
ret0, _ := ret[0].(error)
return ret0
Expand Down
Loading

0 comments on commit c933095

Please sign in to comment.