Skip to content

Commit

Permalink
networking: review logs (#1782)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jmontesi
jmontesi authored Jan 4, 2024
1 parent 6f4041b commit 2aeaa8c
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 115 deletions.
50 changes: 32 additions & 18 deletions cnf-certification-test/networking/icmp/icmp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,42 +50,44 @@ func (results PingResults) String() string {
func BuildNetTestContext(pods []*provider.Pod, aIPVersion netcommons.IPVersion, aType netcommons.IFType, logger *log.Logger) (netsUnderTest map[string]netcommons.NetTestContext) {
netsUnderTest = make(map[string]netcommons.NetTestContext)
for _, put := range pods {
logger.Info("Testing Pod %q", put)
if put.SkipNetTests {
logger.Info("Skipping %s because it is excluded from all connectivity tests", put)
logger.Info("Skipping %q because it is excluded from all connectivity tests", put)
continue
}

if aType == netcommons.MULTUS {
if put.SkipMultusNetTests {
logger.Info("Skipping pod %s because it is excluded from %s connectivity tests only", put.Name, aType)
logger.Info("Skipping pod %q because it is excluded from %q connectivity tests only", put.Name, aType)
continue
}
for netKey, multusIPAddress := range put.MultusIPs {
// The first container is used to get the network namespace
ProcessContainerIpsPerNet(put.Containers[0], netKey, multusIPAddress, netsUnderTest, aIPVersion)
processContainerIpsPerNet(put.Containers[0], netKey, multusIPAddress, netsUnderTest, aIPVersion, logger)
}
continue
}

const defaultNetKey = "default"
defaultIPAddress := put.Status.PodIPs
// The first container is used to get the network namespace
ProcessContainerIpsPerNet(put.Containers[0], defaultNetKey, netcommons.PodIPsToStringList(defaultIPAddress), netsUnderTest, aIPVersion)
processContainerIpsPerNet(put.Containers[0], defaultNetKey, netcommons.PodIPsToStringList(defaultIPAddress), netsUnderTest, aIPVersion, logger)
}
return netsUnderTest
}

// processContainerIpsPerNet takes a container ip addresses for a given network attachment's and uses it as a test target.
// The first container in the loop is selected as the test initiator. the Oc context of the container is used to initiate the pings
func ProcessContainerIpsPerNet(containerID *provider.Container,
func processContainerIpsPerNet(containerID *provider.Container,
netKey string,
ipAddresses []string,
netsUnderTest map[string]netcommons.NetTestContext,
aIPVersion netcommons.IPVersion) {
aIPVersion netcommons.IPVersion,
logger *log.Logger) {
ipAddressesFiltered := netcommons.FilterIPListByIPVersion(ipAddresses, aIPVersion)
if len(ipAddressesFiltered) == 0 {
// if no multus addresses found, skip this container
log.Debug("Skipping %s, Network %s because no multus IPs are present", containerID, netKey)
logger.Debug("Skipping %q, Network %q because no multus IPs are present", containerID, netKey)
return
}
// Create an entry at "key" if it is not present
Expand All @@ -97,7 +99,7 @@ func ProcessContainerIpsPerNet(containerID *provider.Container,
// Then modify the copy
firstIPIndex := 0
if entry.TesterSource.ContainerIdentifier == nil {
log.Debug("%s selected to initiate ping tests", containerID)
logger.Debug("%q selected to initiate ping tests", containerID)
entry.TesterSource.ContainerIdentifier = containerID
// if multiple interfaces are present for this network on this container/pod, pick the first one as the tester source ip
entry.TesterSource.IP = ipAddressesFiltered[firstIPIndex]
Expand Down Expand Up @@ -126,7 +128,7 @@ func RunNetworkingTests( //nolint:funlen
logger.Debug("%s", netcommons.PrintNetTestContextMap(netsUnderTest))
skip = false
if len(netsUnderTest) == 0 {
logger.Debug("There are no %s networks to test, skipping test", aIPVersion)
logger.Debug("There are no %q networks to test, skipping test", aIPVersion)
skip = true
return report, skip
}
Expand All @@ -139,27 +141,32 @@ func RunNetworkingTests( //nolint:funlen
compliantNets[netName] = 0
nonCompliantNets[netName] = 0
if len(netUnderTest.DestTargets) == 0 {
logger.Debug("There are no containers to ping for %s network %s. A minimum of 2 containers is needed to run a ping test (a source and a destination) Skipping test", aIPVersion, netName)
logger.Debug("There are no containers to ping for %q network %q. A minimum of 2 containers is needed to run a ping test (a source and a destination) Skipping test", aIPVersion, netName)
continue
}
atLeastOneNetworkTested = true
logger.Debug("%s Ping tests on network %s. Number of target IPs: %d", aIPVersion, netName, len(netUnderTest.DestTargets))
logger.Debug("%q Ping tests on network %q. Number of target IPs: %d", aIPVersion, netName, len(netUnderTest.DestTargets))

for _, aDestIP := range netUnderTest.DestTargets {
logger.Debug("%s ping test on network %s from ( %s srcip: %s ) to ( %s dstip: %s )",
logger.Debug("%q ping test on network %q from ( %q srcip: %q ) to ( %q dstip: %q )",
aIPVersion, netName,
netUnderTest.TesterSource.ContainerIdentifier, netUnderTest.TesterSource.IP,
aDestIP.ContainerIdentifier, aDestIP.IP)
result, err := TestPing(netUnderTest.TesterSource.ContainerIdentifier, aDestIP, count)
logger.Debug("Ping results: %s", result.String())
logger.Info("%s ping test on network %s from ( %s srcip: %s ) to ( %s dstip: %s ) result: %s",
logger.Debug("Ping results: %q", result)
logger.Info("%q ping test on network %q from ( %q srcip: %q ) to ( %q dstip: %q ) result: %q",
aIPVersion, netName,
netUnderTest.TesterSource.ContainerIdentifier, netUnderTest.TesterSource.IP,
aDestIP.ContainerIdentifier, aDestIP.IP, result.String())
aDestIP.ContainerIdentifier, aDestIP.IP, result)
if err != nil {
logger.Debug("Ping failed with err:%s", err)
logger.Debug("Ping failed, err=%v", err)
}
if result.outcome != testhelper.SUCCESS {
logger.Error("Ping from %q (srcip: %q) to %q (dstip: %q) failed",
netUnderTest.TesterSource.ContainerIdentifier,
netUnderTest.TesterSource.IP,
aDestIP.ContainerIdentifier,
aDestIP.IP)
nonCompliantNets[netName]++
nonCompliantObject := testhelper.NewContainerReportObject(netUnderTest.TesterSource.ContainerIdentifier.Namespace,
netUnderTest.TesterSource.ContainerIdentifier.Podname,
Expand All @@ -173,6 +180,11 @@ func RunNetworkingTests( //nolint:funlen
AddField(testhelper.DestinationIP, aDestIP.IP)
report.NonCompliantObjectsOut = append(report.NonCompliantObjectsOut, nonCompliantObject)
} else {
logger.Info("Ping from %q (srcip: %q) to %q (dstip: %q) succeeded",
netUnderTest.TesterSource.ContainerIdentifier,
netUnderTest.TesterSource.IP,
aDestIP.ContainerIdentifier,
aDestIP.IP)
compliantNets[netName]++
CompliantObject := testhelper.NewContainerReportObject(netUnderTest.TesterSource.ContainerIdentifier.Namespace,
netUnderTest.TesterSource.ContainerIdentifier.Podname,
Expand All @@ -188,16 +200,18 @@ func RunNetworkingTests( //nolint:funlen
}
}
if nonCompliantNets[netName] != 0 {
logger.Error("ICMP tests failed for %d IP source/destination in this network", nonCompliantNets[netName])
report.NonCompliantObjectsOut = append(report.NonCompliantObjectsOut, testhelper.NewReportObject(fmt.Sprintf("ICMP tests failed for %d IP source/destination in this network", nonCompliantNets[netName]), testhelper.NetworkType, false).
AddField(testhelper.NetworkName, netName))
}
if compliantNets[netName] != 0 {
report.CompliantObjectsOut = append(report.CompliantObjectsOut, testhelper.NewReportObject(fmt.Sprintf("ICMP tests were successful for all %d IP source/destination in this network", compliantNets[netName]), testhelper.NetworkType, true).
logger.Info("ICMP tests were successful for all %d IP source/destination in this network", compliantNets[netName])
report.CompliantObjectsOut = append(report.CompliantObjectsOut, testhelper.NewReportObject(fmt.Sprintf("ICMP tests were successful for all %d IP source/destination in this network", compliantNets[netName]), testhelper.NetworkType, true).
AddField(testhelper.NetworkName, netName))
}
}
if !atLeastOneNetworkTested {
logger.Debug("There are no %s networks to test, skipping test", aIPVersion)
logger.Debug("There are no %q networks to test, skipping test", aIPVersion)
skip = true
}

Expand Down
59 changes: 6 additions & 53 deletions cnf-certification-test/networking/icmp/icmp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,14 +298,17 @@ func TestProcessContainerIpsPerNet(t *testing.T) {
},
},
}
var logArchive strings.Builder
log.SetupLogger(&logArchive, "INFO")
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ProcessContainerIpsPerNet(
processContainerIpsPerNet(
tt.args.containerID,
tt.args.netKey,
tt.args.ipAddresses,
tt.args.netsUnderTest,
tt.args.aIPVersion,
log.GetLogger(),
)
if !reflect.DeepEqual(tt.args.netsUnderTest, tt.args.wantNetsUnderTest) {
t.Errorf(
Expand All @@ -328,7 +331,6 @@ func TestBuildNetTestContext(t *testing.T) {
name string
args args
wantNetsUnderTest map[string]netcommons.NetTestContext
wantLogArchive []string
}{
{
name: "ipv4ok",
Expand Down Expand Up @@ -394,10 +396,6 @@ func TestBuildNetTestContext(t *testing.T) {
DestTargets: nil,
},
},

wantLogArchive: []string{
"Skipping pod: pod2 ns: ns1 because it is excluded from all connectivity tests",
},
},
{
name: "ipv4ok multus",
Expand Down Expand Up @@ -511,10 +509,6 @@ func TestBuildNetTestContext(t *testing.T) {
DestTargets: nil,
},
},

wantLogArchive: []string{
"Skipping pod pod2 because it is excluded from Multus connectivity tests only",
},
},
}
var logArchive strings.Builder
Expand Down Expand Up @@ -548,34 +542,10 @@ func TestBuildNetTestContext(t *testing.T) {
tt.wantNetsUnderTest,
)
}

logArchiveMsgs := getLogArchiveMsgs(logArchive)
if !reflect.DeepEqual(logArchiveMsgs, tt.wantLogArchive) {
t.Errorf(
"BuildNetTestContext() gotClaimsLog = %v, want %v",
logArchiveMsgs,
tt.wantLogArchive,
)
}
logArchive.Reset()
})
}
}

func getLogArchiveMsgs(logArchive strings.Builder) []string {
var logArchiveMsgs []string
logLines := strings.Split(logArchive.String(), "\n")

for _, logLine := range logLines {
logMsgs := strings.Split(logLine, "]")
if len(logMsgs) > 2 {
logArchiveMsgs = append(logArchiveMsgs, strings.TrimSpace(logMsgs[2]))
}
}

return logArchiveMsgs
}

var (
pod1 = provider.Pod{ //nolint:dupl
Pod: &corev1.Pod{
Expand Down Expand Up @@ -741,7 +711,6 @@ func TestRunNetworkingTests(t *testing.T) {
name string
args args
wantReport testhelper.FailureReasonOut
wantLogArchive []string
testPingSuccess bool
}{
{name: "ok",
Expand Down Expand Up @@ -810,16 +779,13 @@ func TestRunNetworkingTests(t *testing.T) {
ObjectType: "Network",
ObjectFieldsKeys: []string{testhelper.ReasonForCompliance, testhelper.NetworkName},
ObjectFieldsValues: []string{
"ICMP tests were successful for all 1 IP source/destination in this network",
"ICMP tests were successful for all 1 IP source/destination in this network",
"default",
},
},
},
NonCompliantObjectsOut: []*testhelper.ReportObject{},
},
wantLogArchive: []string{
"IPv4 ping test on network default from ( container: test1 pod: test-0 ns: tnf srcip: 10.244.195.231 ) to ( container: test2 pod: test-1 ns: tnf dstip: 10.244.195.232 ) result: outcome: SUCCESS transmitted: 10 received: 10 errors: 0",
},
testPingSuccess: true,
},
{name: "noNetToTest",
Expand Down Expand Up @@ -965,10 +931,6 @@ func TestRunNetworkingTests(t *testing.T) {
},
},
},
wantLogArchive: []string{
"IPv4 ping test on network default from ( container: test1 pod: test-0 ns: tnf srcip: 10.244.195.231 ) to ( container: test2 pod: test-1 ns: tnf dstip: 10.244.195.232 ) result: outcome: FAILURE transmitted: 10 received: 5 errors: 5", //nolint:lll
"IPv4 ping test on network default from ( container: test1 pod: test-0 ns: tnf srcip: 10.244.195.231 ) to ( container: test3 pod: test-1 ns: tnf dstip: 10.244.195.233 ) result: outcome: FAILURE transmitted: 10 received: 5 errors: 5",
},
testPingSuccess: false,
},
}
Expand All @@ -990,20 +952,11 @@ func TestRunNetworkingTests(t *testing.T) {
)
if !gotReport.Equal(tt.wantReport) {
t.Errorf(
"RunNetworkingTests() gotReport = %s, want %s",
"RunNetworkingTests() gotReport = %q, want %q",
testhelper.FailureReasonOutTestString(gotReport),
testhelper.FailureReasonOutTestString(tt.wantReport),
)
}
logArchiveMsgs := getLogArchiveMsgs(logArchive)
if !reflect.DeepEqual(logArchiveMsgs, tt.wantLogArchive) {
t.Errorf(
"RunNetworkingTests() gotReport = %+v, want %+v",
logArchiveMsgs,
tt.wantLogArchive,
)
}
logArchive.Reset()
})
}
}
Expand Down
22 changes: 13 additions & 9 deletions cnf-certification-test/networking/netcommons/netcommons.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,18 +149,20 @@ func FilterIPListByIPVersion(ipList []string, aIPVersion IPVersion) []string {
return filteredIPList
}

func FindRogueContainersDeclaringPorts(containers []*provider.Container, portsToTest map[int32]bool, portsOrigin string) (compliantObjects, nonCompliantObjects []*testhelper.ReportObject) {
func findRogueContainersDeclaringPorts(containers []*provider.Container, portsToTest map[int32]bool, portsOrigin string, logger *log.Logger) (compliantObjects, nonCompliantObjects []*testhelper.ReportObject) {
for _, cut := range containers {
logger.Info("Testing Container %q", cut)
for _, port := range cut.Ports {
if portsToTest[port.ContainerPort] {
log.Debug("%s has declared a port (%d) that has been reserved", cut, port.ContainerPort)
logger.Error("%q declares %s reserved port %d (%s)", cut, portsOrigin, port.ContainerPort, port.Protocol)
nonCompliantObjects = append(nonCompliantObjects,
testhelper.NewContainerReportObject(cut.Namespace, cut.Podname, cut.Name,
fmt.Sprintf("Container declares %s reserved port in %v", portsOrigin, portsToTest), false).
SetType(testhelper.DeclaredPortType).
AddField(testhelper.PortNumber, strconv.Itoa(int(port.ContainerPort))).
AddField(testhelper.PortProtocol, string(port.Protocol)))
} else {
logger.Info("%q does not declare any %s reserved port", cut, portsOrigin)
compliantObjects = append(compliantObjects,
testhelper.NewContainerReportObject(cut.Namespace, cut.Podname, cut.Name,
fmt.Sprintf("Container does not declare %s reserved port in %v", portsOrigin, portsToTest), true).
Expand All @@ -187,16 +189,17 @@ var ReservedIstioPorts = map[int32]bool{
15000: true, // Envoy admin port (commands/diagnostics)
}

func FindRoguePodsListeningToPorts(pods []*provider.Pod, portsToTest map[int32]bool, portsOrigin string) (compliantObjects, nonCompliantObjects []*testhelper.ReportObject) {
func findRoguePodsListeningToPorts(pods []*provider.Pod, portsToTest map[int32]bool, portsOrigin string, logger *log.Logger) (compliantObjects, nonCompliantObjects []*testhelper.ReportObject) {
for _, put := range pods {
compliantObjectsEntries, nonCompliantObjectsEntries := FindRogueContainersDeclaringPorts(put.Containers, portsToTest, portsOrigin)
logger.Info("Testing Pod %q", put)
compliantObjectsEntries, nonCompliantObjectsEntries := findRogueContainersDeclaringPorts(put.Containers, portsToTest, portsOrigin, logger)
nonCompliantPortFound := len(nonCompliantObjectsEntries) > 0
compliantObjects = append(compliantObjects, compliantObjectsEntries...)
nonCompliantObjects = append(nonCompliantObjects, nonCompliantObjectsEntries...)
cut := put.Containers[0]
listeningPorts, err := netutil.GetListeningPorts(cut)
if err != nil {
log.Debug("Failed to get the listening ports on %s, err: %v", cut, err)
logger.Error("Failed to get the listening ports on %q, err: %v", cut, err)
nonCompliantObjects = append(nonCompliantObjects,
testhelper.NewPodReportObject(cut.Namespace, put.Name,
fmt.Sprintf("Failed to get the listening ports on pod, err: %v", err), false))
Expand All @@ -207,10 +210,10 @@ func FindRoguePodsListeningToPorts(pods []*provider.Pod, portsToTest map[int32]b
// If pod contains an "istio-proxy" container, we need to make sure that the ports returned
// overlap with the known istio ports
if put.ContainsIstioProxy() && ReservedIstioPorts[int32(port.PortNumber)] {
log.Debug("%s was found to be listening to port %d due to istio-proxy being present. Ignoring.", put, port.PortNumber)
logger.Info("%q was found to be listening to port %d due to istio-proxy being present. Ignoring.", put, port.PortNumber)
continue
}
log.Debug("%s has one container (%s) listening on port %d that has been reserved", put, cut.Name, port.PortNumber)
logger.Error("%q has one container (%q) listening on port %d (%s) that has been reserved", put, cut.Name, port.PortNumber, port.Protocol)
nonCompliantObjects = append(nonCompliantObjects,
testhelper.NewPodReportObject(cut.Namespace, put.Name,
fmt.Sprintf("Pod Listens to %s reserved port in %v", portsOrigin, portsToTest), false).
Expand All @@ -219,6 +222,7 @@ func FindRoguePodsListeningToPorts(pods []*provider.Pod, portsToTest map[int32]b
AddField(testhelper.PortProtocol, port.Protocol))
nonCompliantPortFound = true
} else {
logger.Info("%q listens in %s unreserved port %d (%s)", put, portsOrigin, port.PortNumber, port.Protocol)
compliantObjects = append(compliantObjects,
testhelper.NewPodReportObject(cut.Namespace, put.Name,
fmt.Sprintf("Pod Listens to port not in %s reserved port %v", portsOrigin, portsToTest), true).
Expand All @@ -240,8 +244,8 @@ func FindRoguePodsListeningToPorts(pods []*provider.Pod, portsToTest map[int32]b
return compliantObjects, nonCompliantObjects
}

func TestReservedPortsUsage(env *provider.TestEnvironment, reservedPorts map[int32]bool, portsOrigin string) (compliantObjects, nonCompliantObjects []*testhelper.ReportObject) {
compliantObjectsEntries, nonCompliantObjectsEntries := FindRoguePodsListeningToPorts(env.Pods, reservedPorts, portsOrigin)
func TestReservedPortsUsage(env *provider.TestEnvironment, reservedPorts map[int32]bool, portsOrigin string, logger *log.Logger) (compliantObjects, nonCompliantObjects []*testhelper.ReportObject) {
compliantObjectsEntries, nonCompliantObjectsEntries := findRoguePodsListeningToPorts(env.Pods, reservedPorts, portsOrigin, logger)
compliantObjects = append(compliantObjects, compliantObjectsEntries...)
nonCompliantObjects = append(nonCompliantObjects, nonCompliantObjectsEntries...)

Expand Down
Loading

0 comments on commit 2aeaa8c

Please sign in to comment.