Skip to content

Commit

Permalink
fix additional issues found
Browse files Browse the repository at this point in the history
  • Loading branch information
prezha committed Dec 7, 2022
1 parent 796a4b8 commit bd7efa0
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 44 deletions.
1 change: 1 addition & 0 deletions pkg/drivers/kic/oci/network_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ func tryCreateDockerNetwork(ociBin string, subnet *network.Parameters, mtu int,

rr, err := runCmd(exec.Command(ociBin, args...))
if err != nil {
klog.Errorf("failed to create %s network %s %s with gateway %s and MTU of %d: %v", ociBin, name, subnet.CIDR, subnet.Gateway, mtu, err)
// Pool overlaps with other one on this address space
if strings.Contains(rr.Output(), "Pool overlaps") {
return nil, ErrNetworkSubnetTaken
Expand Down
27 changes: 14 additions & 13 deletions pkg/drivers/kvm/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,19 +477,20 @@ func ifListFromAPI(conn *libvirt.Connect, domain string) ([]libvirt.DomainInterf
}
defer func() { _ = dom.Free() }()

ifs, err := dom.ListAllInterfaceAddresses(libvirt.DOMAIN_INTERFACE_ADDRESSES_SRC_ARP)
if ifs == nil {
if err != nil {
log.Debugf("failed listing network interface addresses of domain %s(source=arp): %w", domain, err)
} else {
log.Debugf("No network interface addresses found for domain %s(source=arp)", domain)
}
log.Debugf("trying to list again with source=lease")

ifs, err = dom.ListAllInterfaceAddresses(libvirt.DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE)
if err != nil {
return nil, fmt.Errorf("failed listing network interface addresses of domain %s(source=lease): %w", domain, err)
}
// check lease first, then arp
// ref: https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainInterfaceAddresses
ifs, err := dom.ListAllInterfaceAddresses(libvirt.DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE)
if err == nil && len(ifs) > 0 {
return ifs, nil
}
if err != nil {
log.Debugf("failed listing network interface addresses of domain %s using lease (will try arp): %v", domain, err)
} else if len(ifs) == 0 {
log.Debugf("no network interface addresses found for domain %s using lease (will try arp)", domain)
}
ifs, err = dom.ListAllInterfaceAddresses(libvirt.DOMAIN_INTERFACE_ADDRESSES_SRC_ARP)
if err != nil {
return nil, fmt.Errorf("failed listing network interface addresses of domain %s using arp: %w", domain, err)
}

return ifs, nil
Expand Down
3 changes: 2 additions & 1 deletion pkg/minikube/cni/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ import (
)

// bridge is what minikube defaulted to when `--enable-default-cni=true`
// /~https://github.com/containernetworking/plugins/blob/master/plugins/main/bridge/README.md
// ref: https://www.cni.dev/plugins/current/main/bridge/
// ref: https://www.cni.dev/plugins/current/meta/portmap/

var bridgeConf = template.Must(template.New("bridge").Parse(`
{
Expand Down
35 changes: 20 additions & 15 deletions pkg/minikube/cni/cni.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,9 @@ func applyManifest(cc config.ClusterConfig, r Runner, f assets.CopyableFile) err
}

if driver.IsKIC(cc.Driver) {
klog.Info("disabling CRIO bridge for Docker driver ...")
klog.Infof("disabling cri-o bridge for %s driver ... ", cc.Driver)
if err := disableCrioBridge(r); err != nil {
klog.Warningf("disabling CRIO bridge for Docker driver: %v", err)
klog.Warningf("unable to disable cri-o bridge for %s driver: %v", cc.Driver, err)
}
}

Expand Down Expand Up @@ -244,8 +244,6 @@ func configureCNI(cc *config.ClusterConfig, cnm Manager) error {
// respect user-specified custom CNI Config Directory
ConfDir = cc.KubernetesConfig.ExtraOptions.Get("cni-conf-dir", "kubelet")
}
} else {
ConfDir = CustomConfDir
}
}
return nil
Expand All @@ -257,25 +255,32 @@ func configureCNI(cc *config.ClusterConfig, cnm Manager) error {
// Failed to create pod sandbox: rpc error: code = Unknown desc = [failed to set up sandbox container "..." network for pod "...": networkPlugin cni failed to set up pod "..." network: missing network name:,
// failed to clean up sandbox container "..." network for pod "...": networkPlugin cni failed to teardown pod "..." network: missing network name]
func NameLoopback(r Runner) error {
loopback := "/etc/cni/net.d/*loopback.conf" // usually: 200-loopback.conf
loopback := "/etc/cni/net.d/*loopback.conf*" // usually: 200-loopback.conf
// turn { "cniVersion": "0.3.1", "type": "loopback" }
// into { "cniVersion": "0.3.1", "name": "loopback", "type": "loopback" }
if _, err := r.RunCmd(exec.Command("stat", loopback)); err != nil {
klog.Warningf("%q not found, skipping patching loopback config step: %v", loopback, err)
} else if _, err := r.RunCmd(exec.Command(
"sudo", "find", filepath.Dir(loopback), "-name", filepath.Base(loopback), "-type", "f", "-exec", "sh", "-c",
`grep -q loopback {} && ( grep -q name {} || sed -i '/"type": "loopback"/i \ \ \ \ "name": "loopback",' {} )`, ";")); err != nil {
if _, err := r.RunCmd(exec.Command("sh", "-c", fmt.Sprintf("stat %s", loopback))); err != nil {
klog.Warningf("%q not found, skipping patching loopback config step", loopback)
return nil
}
if _, err := r.RunCmd(exec.Command(
"sudo", "find", filepath.Dir(loopback), "-maxdepth", "1", "-type", "f", "-name", filepath.Base(loopback), "-exec", "sh", "-c",
`grep -q loopback {} && ( grep -q name {} || sudo sed -i '/"type": "loopback"/i \ \ \ \ "name": "loopback",' {} )`, ";")); err != nil {
return fmt.Errorf("unable to patch loopback config %q: %v", loopback, err)
}
return nil
}

// disableCrioBridge disables cri-o bridge by prefixing its config file in /etc/cni/net.d with "DISABLED" (effectively deprioritising its lexicographical order placement)
// disableCrioBridge disables cri-o bridge config file in /etc/cni/net.d by changing extension to "mk_disabled"
// ref: /~https://github.com/containernetworking/cni/blob/6e5ac36b42c0b582358e67ac581d0316507967cb/libcni/conf.go#L176-L184
func disableCrioBridge(r Runner) error {
conflict := "/etc/cni/net.d/100-crio-bridge.conf"
if _, err := r.RunCmd(exec.Command("stat", conflict)); err != nil {
klog.Warningf("%q not found, skipping disabling cri-o bridge config step: %v", conflict, err)
} else if _, err = r.RunCmd(exec.Command("sudo", "mv", conflict, filepath.Join(filepath.Dir(conflict), "DISABLED-"+filepath.Base(conflict)))); err != nil {
conflict := "/etc/cni/net.d/*crio-bridge.conf" // usually: 100-crio-bridge.conf
if _, err := r.RunCmd(exec.Command("sh", "-c", fmt.Sprintf("stat %s", conflict))); err != nil {
klog.Warningf("%q not found, skipping disabling cri-o bridge config step", conflict)
return nil
}
if _, err := r.RunCmd(exec.Command(
"sudo", "find", filepath.Dir(conflict), "-maxdepth", "1", "-type", "f", "-name", filepath.Base(conflict), "-exec", "sh", "-c",
`sudo mv {} {}.mk_disabled`, ";")); err != nil {
return fmt.Errorf("unable to disable cri-o bridge config %q: %v", conflict, err)
}
return nil
Expand Down
9 changes: 6 additions & 3 deletions pkg/minikube/cruntime/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -682,9 +682,12 @@ func dockerConfigureNetworkPlugin(r Docker, cr CommandRunner, networkPlugin stri
klog.Warningf("unable to name loopback interface in dockerConfigureNetworkPlugin: %v", err)
}

args := " --cni-bin-dir=" + CNIBinDir
args += " --cni-cache-dir=" + CNICacheDir
args += " --cni-conf-dir=" + cni.ConfDir
args := ""
if r.KubernetesVersion.LT(semver.MustParse("1.24.0-alpha.2")) {
args += " --cni-bin-dir=" + CNIBinDir
args += " --cni-cache-dir=" + CNICacheDir
args += " --cni-conf-dir=" + cni.ConfDir
}
args += " --hairpin-mode=promiscuous-bridge"

opts := struct {
Expand Down
14 changes: 6 additions & 8 deletions pkg/network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,20 +262,18 @@ func FreeSubnet(startSubnet string, step, tries int) (*Parameters, error) {
}

// reserveSubnet returns if subnet was successfully reserved for given period:
// - false, if it already has unexpired reservation
// - true, if new reservation was created or expired one renewed
//
// - false, if it already has unexpired reservation
// - true, if new reservation was created or expired one renewed
// uses sync.Map to manage reservations thread-safe
var reserveSubnet = func(subnet string, period time.Duration) bool {
// put 'zero' reservation{} Map value for subnet Map key
// put nil reservation{} Map value for subnet Map key
// to block other processes from concurrently changing this subnet
zero := reservation{}
r, loaded := reservedSubnets.LoadOrStore(subnet, zero)
r, loaded := reservedSubnets.LoadOrStore(subnet, nil)
// check if there was previously issued reservation
if loaded {
// back off if previous reservation was already set to 'zero'
// back off if previous reservation was already set to 'nil'
// as then other process is already managing this subnet concurrently
if r == zero {
if r == nil {
klog.Infof("backing off reserving subnet %s (other process is managing it!): %+v", subnet, &reservedSubnets)
return false
}
Expand Down
6 changes: 4 additions & 2 deletions test/integration/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,10 +263,12 @@ func PostMortemLogs(t *testing.T, profile string, multinode ...bool) {
t.Logf("%s: %v", rr.Command(), rerr)
return
}
notRunning := strings.Split(rr.Stdout.String(), " ")
if len(notRunning) == 0 {
// strings.Split("", " ") results in [""] slice of len 1 !
out := strings.TrimSpace(rr.Stdout.String())
if len(out) == 0 {
continue
}
notRunning := strings.Split(out, " ")
t.Logf("non-running pods: %s", strings.Join(notRunning, " "))

t.Logf("======> post-mortem[%s]: describe non-running pods <======", t.Name())
Expand Down
5 changes: 4 additions & 1 deletion test/integration/net_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ import (
// Options tested: kubenet, bridge, flannel, kindnet, calico, cilium
// Flags tested: enable-default-cni (legacy), false (CNI off), auto-detection
func TestNetworkPlugins(t *testing.T) {
// generate reasonably unique profile name suffix to be used for all tests
suffix := UniqueProfileName("")

MaybeParallel(t)
if NoneDriver() {
t.Skip("skipping since test for none driver")
Expand Down Expand Up @@ -71,7 +74,7 @@ func TestNetworkPlugins(t *testing.T) {
tc := tc

t.Run(tc.name, func(t *testing.T) {
profile := UniqueProfileName(tc.name)
profile := tc.name + suffix

ctx, cancel := context.WithTimeout(context.Background(), Minutes(40))
defer CleanupWithLogs(t, profile, cancel)
Expand Down
2 changes: 1 addition & 1 deletion test/integration/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func UniqueProfileName(prefix string) string {
return "minikube"
}
// example: prefix-162239
return fmt.Sprintf("%s-%s", prefix, time.Now().Format("150405"))
return fmt.Sprintf("%s-%s", prefix, fmt.Sprintf("%06d", time.Now().UnixNano()%1000000))
}

// auditContains checks if the provided string is contained within the logs.
Expand Down

0 comments on commit bd7efa0

Please sign in to comment.