OCPBUGS-43352: calculate outgoing interface for arping

.gitignore

@@ -6,3 +6,4 @@ subsystem/logs
 .project
 subsystem/data/*.leases*
 *.log
+**/junit_unit_test.xml

Dockerfile.assisted_installer_agent-build

@@ -6,7 +6,7 @@ RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/i
         go install golang.org/x/tools/cmd/[email protected] && \
         go install github.com/onsi/ginkgo/[email protected] && \
         go install github.com/golang/mock/[email protected] && \
-        go install github.com/vektra/mockery/v2@v2.9.6 && \
+        go install github.com/vektra/mockery/v2@v2.39.2 && \
         go install gotest.tools/[email protected] && \
         go install github.com/axw/gocov/[email protected] && \
         go install github.com/AlekSi/[email protected]

src/connectivity_check/connectivity_check_test.go

@@ -218,7 +218,7 @@ var _ = Describe("check host parallel validation", func() {
 		It(t.name, func() {
 			d := setupDispather(t.simulateL2IPConflict, t.success, t.hosts.Nics)
 			ret, err := d.Run(models.ConnectivityCheckParams{t.hosts}, funk.Map(t.nics, func(s string) OutgoingNic {
-				return OutgoingNic{Name: s, HasIpv4Addresses: true, HasIpv6Addresses: true}
+				return OutgoingNic{Name: s, HasIpv4Addresses: true, HasIpv6Addresses: true, Addresses: getAddr("192.168.1.133", 24)}
 			}).([]OutgoingNic))
 			Expect(err).ToNot(HaveOccurred())
 			Expect(ret.RemoteHosts).To(HaveLen(1))
@@ -346,3 +346,13 @@ func TestConnectivityCheck(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "connectivity check tests")
 }
+
+func getAddr(addr string, mask int) []net.Addr {
+	localIP := net.ParseIP(addr)
+	localMask := net.CIDRMask(mask, 32)
+
+	return []net.Addr{&net.IPNet{
+		IP:   localIP,
+		Mask: localMask,
+	}}
+}

src/connectivity_check/ipv4_arping_checker.go

@@ -1,10 +1,11 @@
 package connectivity_check
 
 import (
-	"os/exec"
+	"net"
 	"regexp"
 	"sort"
 	"strings"
+	"time"
 
 	"github.com/openshift/assisted-installer-agent/src/util"
 	"github.com/openshift/assisted-service/models"
@@ -24,61 +25,81 @@ func (p *arpingChecker) Check(attributes Attributes) ResultReporter {
 		return nil
 	}
 
-	result, err := p.executer.Execute("arping", "-c", "10", "-w", "5", "-I", attributes.OutgoingNIC.Name, attributes.RemoteIPAddress)
-	if err != nil {
-		// Ignore exit code of 1; only 2 or -1 are actual errors
-		if exitErr, ok := err.(*exec.ExitError); !ok || exitErr.ExitCode() != 1 {
-			log.WithError(err).Error("Error while processing 'arping' command")
+	for _, addr := range attributes.OutgoingNIC.Addresses {
+		_, nicNetwork, err := net.ParseCIDR(addr.String())
+		if err != nil {
+			log.WithError(err).Errorf("Cannot Parse CIDR: %s", addr.String())
 			return nil
 		}
-	}
-	lines := strings.Split(result, "\n")
-	if len(lines) == 0 {
-		log.Warn("Missing output for arping")
-		return nil
-	}
 
-	hRegex := regexp.MustCompile("^ARPING ([^ ]+) from ([^ ]+) ([^ ]+)$")
-	parts := hRegex.FindStringSubmatch(lines[0])
-	if len(parts) != 4 {
-		log.Warnf("Wrong format for header line: %s", lines[0])
-		return nil
-	}
+		remoteIP := net.ParseIP(attributes.RemoteIPAddress)
+		if !nicNetwork.Contains(remoteIP) {
+			continue
+		}
 
-	outgoingIpAddress := parts[2]
-	rRegexp := regexp.MustCompile(`^Unicast reply from ([^ ]+) \[([^]]+)\]  [^ ]+$`)
+		log.Infof("Waiting for host to be available , with IP %s", attributes.RemoteIPAddress)
+		for i := 0; i < 10; i++ {
+			_, err = p.executer.Execute("ping", "-c", "2", attributes.RemoteIPAddress)
+			if err != nil {
+				time.Sleep(3 * time.Second)
+			}
+		}
 
-	// We use a map to de-duplicate arping responses from the same mac. They are redundant
-	// because the reason we're reporting more than one line of arping in the first place is to allow
-	// the service to also detect devices in the network that have IP conflict with cluster hosts
-	returnValues := make(map[string]*models.L2Connectivity)
-	for _, line := range lines[1:] {
-		parts = rRegexp.FindStringSubmatch(line)
-		if len(parts) != 3 {
-			continue
+		result, err := p.executer.Execute("arping", "-c", "10", "-w", "5", "-I", attributes.OutgoingNIC.Name, attributes.RemoteIPAddress)
+		if err != nil {
+			log.WithError(err).Errorf("Error while processing 'arping' command. remote address: %v, Outgioing NIC: %v", attributes.RemoteIPAddress, attributes.OutgoingNIC.Name)
+			return nil
 		}
-		remoteMAC := strings.ToLower(parts[2])
-		successful := macInDstMacs(remoteMAC, attributes.RemoteMACAddresses)
-		if !successful {
-			log.Warnf("Unexpected mac address for arping %s on nic %s: %s", attributes.RemoteIPAddress, attributes.OutgoingNIC.Name, remoteMAC)
+		lines := strings.Split(result, "\n")
+		if len(lines) == 0 {
+			log.Warn("Missing output for arping")
+			return nil
 		}
-		if strings.ToLower(attributes.RemoteMACAddress) != remoteMAC {
-			log.Infof("Received remote mac %s different then expected mac %s", remoteMAC, attributes.RemoteMACAddress)
+
+		hRegex := regexp.MustCompile("^ARPING ([^ ]+) from ([^ ]+) ([^ ]+)$")
+		parts := hRegex.FindStringSubmatch(lines[0])
+		if len(parts) != 4 {
+			log.Warnf("Wrong format for header line: %s", lines[0])
+			return nil
 		}
 
-		returnValues[remoteMAC] = &models.L2Connectivity{
-			OutgoingIPAddress: outgoingIpAddress,
-			OutgoingNic:       attributes.OutgoingNIC.Name,
-			RemoteIPAddress:   attributes.RemoteIPAddress,
-			RemoteMac:         remoteMAC,
-			Successful:        successful,
+		outgoingIpAddress := parts[2]
+		rRegexp := regexp.MustCompile(`^Unicast reply from ([^ ]+) \[([^]]+)\]  [^ ]+$`)
+
+		// We use a map to de-duplicate arping responses from the same mac. They are redundant
+		// because the reason we're reporting more than one line of arping in the first place is to allow
+		// the service to also detect devices in the network that have IP conflict with cluster hosts
+		returnValues := make(map[string]*models.L2Connectivity)
+		for _, line := range lines[1:] {
+			parts = rRegexp.FindStringSubmatch(line)
+			if len(parts) != 3 {
+				continue
+			}
+			remoteMAC := strings.ToLower(parts[2])
+			successful := macInDstMacs(remoteMAC, attributes.RemoteMACAddresses)
+			if !successful {
+				log.Warnf("Unexpected mac address for arping %s on nic %s: %s", attributes.RemoteIPAddress, attributes.OutgoingNIC.Name, remoteMAC)
+			}
+			if strings.ToLower(attributes.RemoteMACAddress) != remoteMAC {
+				log.Infof("Received remote mac %s different then expected mac %s", remoteMAC, attributes.RemoteMACAddress)
+			}
+
+			returnValues[remoteMAC] = &models.L2Connectivity{
+				OutgoingIPAddress: outgoingIpAddress,
+				OutgoingNic:       attributes.OutgoingNIC.Name,
+				RemoteIPAddress:   attributes.RemoteIPAddress,
+				RemoteMac:         remoteMAC,
+				Successful:        successful,
+			}
 		}
+		var l2Connectivity []*models.L2Connectivity
+		for _, v := range returnValues {
+			l2Connectivity = append(l2Connectivity, v)
+		}
+		return newArpingResultReporter(l2Connectivity)
 	}
-	var l2Connectivity []*models.L2Connectivity
-	for _, v := range returnValues {
-		l2Connectivity = append(l2Connectivity, v)
-	}
-	return newArpingResultReporter(l2Connectivity)
+
+	return nil
 }
 
 func (p *arpingChecker) Finalize(resultingHost *models.ConnectivityRemoteHost) {

src/connectivity_check/ipv4_arping_checker_test.go

@@ -55,7 +55,7 @@ Received 0 response(s)
 		attributes := Attributes{
 			RemoteIPAddress:    remoteIPAddress,
 			RemoteMACAddress:   remoteMACAddress,
-			OutgoingNIC:        OutgoingNic{Name: outgoingNIC, HasIpv4Addresses: true},
+			OutgoingNIC:        OutgoingNic{Name: outgoingNIC, HasIpv4Addresses: true, Addresses: getAddr(outgoingIPAddress, 24)},
 			RemoteMACAddresses: remoteMACAddresses,
 		}
 		mockFullReply(remoteIPAddress, remoteMACAddress)
@@ -84,7 +84,7 @@ Received 0 response(s)
 		attributes := Attributes{
 			RemoteIPAddress:    remoteIPAddress,
 			RemoteMACAddress:   remoteMACAddress,
-			OutgoingNIC:        OutgoingNic{Name: outgoingNIC, HasIpv4Addresses: true},
+			OutgoingNIC:        OutgoingNic{Name: outgoingNIC, HasIpv4Addresses: true, Addresses: getAddr(outgoingIPAddress, 24)},
 			RemoteMACAddresses: remoteMACAddresses,
 		}
 		mockFullReply(remoteIPAddress, secondaryMACAddress)
@@ -103,7 +103,7 @@ Received 0 response(s)
 		attributes := Attributes{
 			RemoteIPAddress:    remoteIPAddress,
 			RemoteMACAddress:   remoteMACAddress,
-			OutgoingNIC:        OutgoingNic{Name: outgoingNIC, HasIpv4Addresses: true},
+			OutgoingNIC:        OutgoingNic{Name: outgoingNIC, HasIpv4Addresses: true, Addresses: getAddr(outgoingIPAddress, 24)},
 			RemoteMACAddresses: remoteMACAddresses,
 		}
 		mockFullReply(remoteIPAddress, additionalMACAddress)
@@ -122,7 +122,7 @@ Received 0 response(s)
 		attributes := Attributes{
 			RemoteIPAddress:    remoteIPAddress,
 			RemoteMACAddress:   remoteMACAddress,
-			OutgoingNIC:        OutgoingNic{Name: outgoingNIC, HasIpv4Addresses: true},
+			OutgoingNIC:        OutgoingNic{Name: outgoingNIC, HasIpv4Addresses: true, Addresses: getAddr(outgoingIPAddress, 24)},
 			RemoteMACAddresses: remoteMACAddresses,
 		}
 		mockEmptyReply(remoteIPAddress)