log: remove loghelper package (#1721)

The functions in the loghelper package are not necessary anymore with
the new log package as the check multilogger is able to log into the log
file as well as into each check's log archive, which can be later used to
fill the log section in the claim file.

Helper functions for the checks receive now the logger instance of the
check, so that logs can be written inside those functions but tagged with
the check's name and recorded in that check's log archive. For this, a
new Logger type has been defined, as well as a set of methods to be used
by the logger instance.

With the help of the new Logger type a small refactor of the log package
is included in this change. Now the "log/slog" library is only used
inside this package, isolating the rest of the code from any other
logging libray than our internal log package.

cnf-certification-test/accesscontrol/namespace/namespace.go

@@ -22,7 +22,6 @@ import (
 
 	"github.com/test-network-function/cnf-certification-test/internal/clientsholder"
 	"github.com/test-network-function/cnf-certification-test/internal/log"
-	"github.com/test-network-function/cnf-certification-test/pkg/loghelper"
 	"github.com/test-network-function/cnf-certification-test/pkg/stringhelper"
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -92,14 +91,15 @@ func getCrsPerNamespaces(aCrd *apiextv1.CustomResourceDefinition) (crdNamespaces
 }
 
 // GetInvalidCRDsNum returns the number of invalid CRs in the map
-func GetInvalidCRsNum(invalidCrs map[string]map[string][]string) (invalidCrsNum int, claimsLog loghelper.CuratedLogLines) {
+func GetInvalidCRsNum(invalidCrs map[string]map[string][]string, logger *log.Logger) int {
+	var invalidCrsNum int
 	for crdName, namespaces := range invalidCrs {
 		for namespace, crNames := range namespaces {
 			for _, crName := range crNames {
-				claimsLog.AddLogLine("crName=%s namespace=%s is invalid (crd=%s)", crName, namespace, crdName)
+				logger.Error("crName=%s namespace=%s is invalid (crd=%s)", crName, namespace, crdName)
 				invalidCrsNum++
 			}
 		}
 	}
-	return invalidCrsNum, claimsLog
+	return invalidCrsNum
 }

cnf-certification-test/accesscontrol/namespace/namespace_test.go

@@ -17,9 +17,11 @@
 package namespace
 
 import (
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/test-network-function/cnf-certification-test/internal/log"
 )
 
 func TestGetInvalidCRsNum(t *testing.T) {
@@ -44,7 +46,8 @@ func TestGetInvalidCRsNum(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		result, _ := GetInvalidCRsNum(tc.invalidCrs)
+		log.SetupLogger(os.Stdout, "INFO")
+		result := GetInvalidCRsNum(tc.invalidCrs, log.GetLogger())
 		assert.Equal(t, tc.expectedInvalidCRs, result)
 	}
 }

cnf-certification-test/accesscontrol/suite.go

@@ -509,9 +509,8 @@ func testNamespace(check *checksdb.Check, env *provider.TestEnvironment) {
 		return
 	}
 
-	invalidCrsNum, claimsLog := namespace.GetInvalidCRsNum(invalidCrs)
-	if invalidCrsNum > 0 && len(claimsLog.GetLogLines()) > 0 {
-		check.LogDebug("%s", claimsLog.GetLogLines())
+	invalidCrsNum := namespace.GetInvalidCRsNum(invalidCrs, check.GetLoggger())
+	if invalidCrsNum > 0 {
 		nonCompliantObjects = append(nonCompliantObjects, testhelper.NewReportObject("CRs are not in the configured namespaces", testhelper.Namespace, false))
 	} else {
 		compliantObjects = append(compliantObjects, testhelper.NewReportObject("CRs are in the configured namespaces", testhelper.Namespace, true))

cnf-certification-test/lifecycle/podsets/podsets.go

@@ -22,7 +22,6 @@ import (
 
 	"github.com/test-network-function/cnf-certification-test/internal/clientsholder"
 	"github.com/test-network-function/cnf-certification-test/internal/log"
-	"github.com/test-network-function/cnf-certification-test/pkg/loghelper"
 	"github.com/test-network-function/cnf-certification-test/pkg/provider"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
@@ -181,25 +180,24 @@ func getNotReadyStatefulSets(statefulSets []*provider.StatefulSet) []*provider.S
 	return notReadyStatefulSets
 }
 
-func WaitForAllPodSetsReady(env *provider.TestEnvironment, timeout time.Duration) (
-	claimsLog loghelper.CuratedLogLines,
+func WaitForAllPodSetsReady(env *provider.TestEnvironment, timeout time.Duration, logger *log.Logger) (
 	notReadyDeployments []*provider.Deployment,
 	notReadyStatefulSets []*provider.StatefulSet) {
 	const queryInterval = 15 * time.Second
 
 	deploymentsToCheck := env.Deployments
 	statefulSetsToCheck := env.StatefulSets
 
-	log.Info("Waiting %s for %d podsets to be ready.", timeout, len(deploymentsToCheck)+len(statefulSetsToCheck))
+	logger.Info("Waiting %s for %d podsets to be ready.", timeout, len(deploymentsToCheck)+len(statefulSetsToCheck))
 	for startTime := time.Now(); time.Since(startTime) < timeout; {
-		log.Info("Checking Deployments readiness of Deployments %v", getDeploymentsInfo(deploymentsToCheck))
+		logger.Info("Checking Deployments readiness of Deployments %v", getDeploymentsInfo(deploymentsToCheck))
 		notReadyDeployments = getNotReadyDeployments(deploymentsToCheck)
 
-		log.Info("Checking StatefulSets readiness of StatefulSets %v", getStatefulSetsInfo(statefulSetsToCheck))
+		logger.Info("Checking StatefulSets readiness of StatefulSets %v", getStatefulSetsInfo(statefulSetsToCheck))
 		notReadyStatefulSets = getNotReadyStatefulSets(statefulSetsToCheck)
 
-		log.Info("Not ready Deployments: %v", getDeploymentsInfo(notReadyDeployments))
-		log.Info("Not ready StatefulSets: %v", getStatefulSetsInfo(notReadyStatefulSets))
+		logger.Info("Not ready Deployments: %v", getDeploymentsInfo(notReadyDeployments))
+		logger.Info("Not ready StatefulSets: %v", getStatefulSetsInfo(notReadyStatefulSets))
 
 		deploymentsToCheck = notReadyDeployments
 		statefulSetsToCheck = notReadyStatefulSets
@@ -213,10 +211,10 @@ func WaitForAllPodSetsReady(env *provider.TestEnvironment, timeout time.Duration
 	}
 
 	// Here, either we reached the timeout or there's no more not-ready deployments or statefulsets.
-	claimsLog.AddLogLine("Not ready Deployments: %v", getDeploymentsInfo(deploymentsToCheck))
-	claimsLog.AddLogLine("Not ready StatefulSets: %v", getStatefulSetsInfo(statefulSetsToCheck))
+	logger.Error("Not ready Deployments: %v", getDeploymentsInfo(deploymentsToCheck))
+	logger.Error("Not ready StatefulSets: %v", getStatefulSetsInfo(statefulSetsToCheck))
 
-	return claimsLog, deploymentsToCheck, statefulSetsToCheck
+	return deploymentsToCheck, statefulSetsToCheck
 }
 
 func GetAllNodesForAllPodSets(pods []*provider.Pod) (nodes map[string]bool) {

cnf-certification-test/lifecycle/suite.go

@@ -599,9 +599,8 @@ func testPodsRecreation(check *checksdb.Check, env *provider.TestEnvironment) {
 	// Before draining any node, wait until all podsets are ready. The timeout depends on the number of podsets to check.
 	// timeout = k-mins + (1min * (num-deployments + num-statefulsets))
 	allPodsetsReadyTimeout := timeoutPodSetReady + time.Minute*time.Duration(len(env.Deployments)+len(env.StatefulSets))
-	claimsLog, notReadyDeployments, notReadyStatefulSets := podsets.WaitForAllPodSetsReady(env, allPodsetsReadyTimeout)
+	notReadyDeployments, notReadyStatefulSets := podsets.WaitForAllPodSetsReady(env, allPodsetsReadyTimeout, check.GetLoggger())
 	if len(notReadyDeployments) > 0 || len(notReadyStatefulSets) > 0 {
-		check.LogDebug("%s", claimsLog.GetLogLines())
 		for _, dep := range notReadyDeployments {
 			nonCompliantObjects = append(nonCompliantObjects, testhelper.NewDeploymentReportObject(dep.Namespace, dep.Name, "Deployment was not ready before draining any node.", false))
 		}
@@ -655,9 +654,8 @@ func testPodsRecreation(check *checksdb.Check, env *provider.TestEnvironment) {
 			return
 		}
 
-		claimsLog, notReadyDeployments, notReadyStatefulSets := podsets.WaitForAllPodSetsReady(env, nodeTimeout)
+		notReadyDeployments, notReadyStatefulSets := podsets.WaitForAllPodSetsReady(env, nodeTimeout, check.GetLoggger())
 		if len(notReadyDeployments) > 0 || len(notReadyStatefulSets) > 0 {
-			check.LogDebug("%s", claimsLog.GetLogLines())
 			for _, dep := range notReadyDeployments {
 				nonCompliantObjects = append(nonCompliantObjects, testhelper.NewDeploymentReportObject(dep.Namespace, dep.Name, "Deployment not ready after draining node "+nodeName, false))
 			}

cnf-certification-test/networking/icmp/icmp.go

@@ -24,7 +24,6 @@ import (
 	"github.com/test-network-function/cnf-certification-test/cnf-certification-test/networking/netcommons"
 	"github.com/test-network-function/cnf-certification-test/internal/crclient"
 	"github.com/test-network-function/cnf-certification-test/internal/log"
-	"github.com/test-network-function/cnf-certification-test/pkg/loghelper"
 	"github.com/test-network-function/cnf-certification-test/pkg/provider"
 	"github.com/test-network-function/cnf-certification-test/pkg/testhelper"
 )
@@ -48,17 +47,17 @@ func (results PingResults) String() string {
 	return fmt.Sprintf("outcome: %s transmitted: %d received: %d errors: %d", testhelper.ResultToString(results.outcome), results.transmitted, results.received, results.errors)
 }
 
-func BuildNetTestContext(pods []*provider.Pod, aIPVersion netcommons.IPVersion, aType netcommons.IFType) (netsUnderTest map[string]netcommons.NetTestContext, claimsLog loghelper.CuratedLogLines) {
+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 {
 		if put.SkipNetTests {
-			claimsLog.AddLogLine("Skipping %s because it is excluded from all connectivity tests", put)
+			logger.Info("Skipping %s because it is excluded from all connectivity tests", put)
 			continue
 		}
 
 		if aType == netcommons.MULTUS {
 			if put.SkipMultusNetTests {
-				claimsLog.AddLogLine("Skipping pod %s because it is excluded from %s connectivity tests only", put.Name, aType)
+				logger.Info("Skipping pod %s because it is excluded from %s connectivity tests only", put.Name, aType)
 				continue
 			}
 			for netKey, multusIPAddress := range put.MultusIPs {
@@ -73,7 +72,7 @@ func BuildNetTestContext(pods []*provider.Pod, aIPVersion netcommons.IPVersion,
 		// The first container is used to get the network namespace
 		ProcessContainerIpsPerNet(put.Containers[0], defaultNetKey, netcommons.PodIPsToStringList(defaultIPAddress), netsUnderTest, aIPVersion)
 	}
-	return netsUnderTest, claimsLog
+	return netsUnderTest
 }
 
 // processContainerIpsPerNet takes a container ip addresses for a given network attachment's and uses it as a test target.
@@ -122,13 +121,14 @@ func ProcessContainerIpsPerNet(containerID *provider.Container,
 func RunNetworkingTests( //nolint:funlen
 	netsUnderTest map[string]netcommons.NetTestContext,
 	count int,
-	aIPVersion netcommons.IPVersion) (report testhelper.FailureReasonOut, claimsLog loghelper.CuratedLogLines, skip bool) {
-	log.Debug("%s", netcommons.PrintNetTestContextMap(netsUnderTest))
+	aIPVersion netcommons.IPVersion,
+	logger *log.Logger) (report testhelper.FailureReasonOut, skip bool) {
+	logger.Debug("%s", netcommons.PrintNetTestContextMap(netsUnderTest))
 	skip = false
 	if len(netsUnderTest) == 0 {
-		log.Debug("There are no %s networks to test, skipping test", aIPVersion)
+		logger.Debug("There are no %s networks to test, skipping test", aIPVersion)
 		skip = true
-		return report, claimsLog, skip
+		return report, skip
 	}
 	// if no network can be tested, then we need to skip the test entirely.
 	// If at least one network can be tested (e.g. > 2 IPs/ interfaces present), then we do not skip the test
@@ -139,25 +139,25 @@ func RunNetworkingTests( //nolint:funlen
 		compliantNets[netName] = 0
 		nonCompliantNets[netName] = 0
 		if len(netUnderTest.DestTargets) == 0 {
-			log.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 %s network %s. A minimum of 2 containers is needed to run a ping test (a source and a destination) Skipping test", aIPVersion, netName)
 			continue
 		}
 		atLeastOneNetworkTested = true
-		log.Debug("%s Ping tests on network %s. Number of target IPs: %d", aIPVersion, netName, len(netUnderTest.DestTargets))
+		logger.Debug("%s Ping tests on network %s. Number of target IPs: %d", aIPVersion, netName, len(netUnderTest.DestTargets))
 
 		for _, aDestIP := range netUnderTest.DestTargets {
-			log.Debug("%s ping test on network %s from ( %s  srcip: %s ) to ( %s dstip: %s )",
+			logger.Debug("%s ping test on network %s from ( %s  srcip: %s ) to ( %s dstip: %s )",
 				aIPVersion, netName,
 				netUnderTest.TesterSource.ContainerIdentifier, netUnderTest.TesterSource.IP,
 				aDestIP.ContainerIdentifier, aDestIP.IP)
 			result, err := TestPing(netUnderTest.TesterSource.ContainerIdentifier, aDestIP, count)
-			log.Debug("Ping results: %s", result.String())
-			claimsLog.AddLogLine("%s ping test on network %s from ( %s  srcip: %s ) to ( %s dstip: %s ) result: %s",
+			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",
 				aIPVersion, netName,
 				netUnderTest.TesterSource.ContainerIdentifier, netUnderTest.TesterSource.IP,
 				aDestIP.ContainerIdentifier, aDestIP.IP, result.String())
 			if err != nil {
-				log.Debug("Ping failed with err:%s", err)
+				logger.Debug("Ping failed with err:%s", err)
 			}
 			if result.outcome != testhelper.SUCCESS {
 				nonCompliantNets[netName]++
@@ -197,11 +197,11 @@ func RunNetworkingTests( //nolint:funlen
 		}
 	}
 	if !atLeastOneNetworkTested {
-		log.Debug("There are no %s networks to test, skipping test", aIPVersion)
+		logger.Debug("There are no %s networks to test, skipping test", aIPVersion)
 		skip = true
 	}
 
-	return report, claimsLog, skip
+	return report, skip
 }
 
 // TestPing Initiates a ping test between a source container and network (1 ip) and a destination container and network (1 ip)