profilecreator: add and use Alert function

We used the `logrus` package internally, but minimally
and pretty much only to notify processing status to the user.
This code was thus the only remaining consumer of that package,
which we don't really need.

Add a neutral (nor log, nor warning) minimal abstraction `Alert`,
similar in spirit to `ghw`, to be used when we need to notify the
user about processing status.

Make the backend easy to replace to make integration of this code
easier. We don't see this happening, but the extra cost is negligible.

Remove the direct dependency to `logrus`.
Note we still have quite many indirect dependencies pulling it in.

Signed-off-by: Francesco Romani <[email protected]>

Author: ffromani

go.mod

@@ -26,7 +26,6 @@ require (
 	github.com/openshift/library-go v0.0.0-20240419113445-f1541d628746
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.21.1
-	github.com/sirupsen/logrus v1.9.3
 	github.com/spf13/cobra v1.9.1
 	github.com/spf13/pflag v1.0.6
 	gopkg.in/fsnotify.v1 v1.4.7

go.sum

@@ -434,8 +434,6 @@ github.com/rwcarlsen/goexif v0.0.0-20190401172101-9e8deecbddbd/go.mod h1:hPqNNc0
 github.com/sirupsen/logrus v1.4.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo=
 github.com/sirupsen/logrus v1.4.1/go.mod h1:ni0Sbl8bgC9z8RoU9G6nDWqqs/fq4eDPysMBDgk/93Q=
 github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
-github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
-github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
 github.com/spf13/cobra v0.0.3/go.mod h1:1l0Ry5zgKvJasoi3XT1TypsSe7PqH0Sj9dhYf7v3XqQ=
 github.com/spf13/cobra v1.9.1 h1:CXSaggrXdbHK9CF+8ywj8Amf7PBRmPCOJugH954Nnlo=
 github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wxYW0=
@@ -686,7 +684,6 @@ golang.org/x/sys v0.0.0-20220310020820-b874c991c1a5/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20220319134239-a9b59b0215f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220422013727-9388b58f7150/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

pkg/performanceprofile/profilecreator/alert.go

@@ -0,0 +1,30 @@
+package profilecreator
+
+import (
+	"log"
+	"os"
+)
+
+var alertSink *log.Logger
+
+func init() {
+	// intentionally no flag for minimal output
+	alertSink = log.New(os.Stderr, "PPC: ", 0)
+}
+
+// Alert raise status messages. `profilecreator` is meant to run interactively,
+// so the default behavior is to bubble up the messages on the user console.
+// It is however possible to reroute the messages to any log.Logger compliant backend
+// see the function SetAlertSink. All Alerts are considered Info messages.
+func Alert(format string, values ...any) {
+	alertSink.Printf(format, values...)
+}
+
+// Call this function before any other else in the `profilecreator` package
+func SetAlertSink(lh *log.Logger) {
+	alertSink = lh
+}
+
+func GetAlertSink() *log.Logger {
+	return alertSink
+}

pkg/performanceprofile/profilecreator/cmd/info.go

@@ -7,7 +7,6 @@ import (
 	"sort"
 	"strings"
 
-	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
@@ -95,13 +94,13 @@ func makeClusterInfoFromClusterData(cluster ClusterData) ClusterInfo {
 		for _, handle := range nodeHandlers {
 			topology, err := handle.SortedTopology()
 			if err != nil {
-				log.Infof("%s(Topology discovery error: %v)", handle.Node.GetName(), err)
+				Alert("%s(Topology discovery error: %v)", handle.Node.GetName(), err)
 				continue
 			}
 
 			htEnabled, err := handle.IsHyperthreadingEnabled()
 			if err != nil {
-				log.Infof("%s(HT discovery error: %v)", handle.Node.GetName(), err)
+				Alert("%s(HT discovery error: %v)", handle.Node.GetName(), err)
 			}
 
 			nInfo := NodeInfo{
@@ -136,7 +135,7 @@ func showClusterInfo(cInfo ClusterInfo, infoOpts *infoOptions) error {
 		fmt.Print(textInfo)
 		return nil
 	}
-	log.Infof("Cluster info:\n%s", textInfo)
+	Alert("Cluster info:\n%s", textInfo)
 	return nil
 }
 

pkg/performanceprofile/profilecreator/cmd/root.go

@@ -22,7 +22,6 @@ import (
 	"os"
 	"strings"
 
-	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
@@ -105,11 +104,10 @@ type ProfileData struct {
 // ClusterData collects the cluster wide information, each mcp points to a list of ghw node handlers
 type ClusterData map[string][]*profilecreator.GHWHandler
 
+// shortcut
+var Alert = profilecreator.Alert
+
 func init() {
-	log.SetOutput(os.Stderr)
-	log.SetFormatter(&log.TextFormatter{
-		DisableTimestamp: true,
-	})
 	utilruntime.Must(performancev2.AddToScheme(scheme))
 }
 
@@ -200,7 +198,7 @@ func makeNodesHandlers(mustGatherDirPath, poolName string, nodes []*corev1.Node)
 		sb.WriteString(node.Name + " ")
 	}
 	// NodePoolName is alias of MCPName
-	log.Infof("Nodes names targeted by %s pool are: %s", poolName, sb.String())
+	Alert("Nodes names targeted by %s pool are: %s", poolName, sb.String())
 	return handlers, nil
 }
 
@@ -310,8 +308,8 @@ func makeProfileDataFrom(nodeHandler *profilecreator.GHWHandler, args *ProfileCr
 	if err != nil {
 		return nil, fmt.Errorf("failed to compute the reserved and isolated CPUs: %v", err)
 	}
-	log.Infof("%d reserved CPUs allocated: %v ", reservedCPUs.Size(), reservedCPUs.String())
-	log.Infof("%d isolated CPUs allocated: %v", isolatedCPUs.Size(), isolatedCPUs.String())
+	Alert("%d reserved CPUs allocated: %v ", reservedCPUs.Size(), reservedCPUs.String())
+	Alert("%d isolated CPUs allocated: %v", isolatedCPUs.Size(), isolatedCPUs.String())
 	kernelArgs := profilecreator.GetAdditionalKernelArgs(args.DisableHT)
 	profileData := &ProfileData{
 		reservedCPUs:              reservedCPUs.String(),

pkg/performanceprofile/profilecreator/mcp.go

@@ -4,8 +4,6 @@ import (
 	"fmt"
 	"strings"
 
-	log "github.com/sirupsen/logrus"
-
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -77,7 +75,7 @@ func GetNodesForPool(pool *mcfgv1.MachineConfigPool, clusterPools []*mcfgv1.Mach
 	for _, n := range clusterNodes {
 		p, err := getPrimaryPoolForNode(n, clusterPools)
 		if err != nil {
-			log.Warningf("can't get pool for node %q: %v", n.Name, err)
+			Alert("can't get pool for node %q: %v", n.Name, err)
 			continue
 		}
 		if p == nil {
@@ -121,7 +119,7 @@ func getPoolsForNode(node *corev1.Node, clusterPools []*mcfgv1.MachineConfigPool
 	if isWindows(node) {
 		// This is not an error, is this a Windows Node and it won't be managed by MCO. We're explicitly logging
 		// here at a high level to disambiguate this from other pools = nil  scenario
-		log.Infof("Node %v is a windows node so won't be managed by MCO", node.Name)
+		Alert("Node %v is a windows node so won't be managed by MCO", node.Name)
 		return nil, nil
 	}
 

pkg/performanceprofile/profilecreator/mustgather.go

@@ -24,7 +24,6 @@ import (
 	"strings"
 
 	machineconfigv1 "github.com/openshift/api/machineconfiguration/v1"
-	log "github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
 	k8syaml "k8s.io/apimachinery/pkg/util/yaml"
 )
@@ -68,7 +67,7 @@ func getMustGatherFullPathsWithFilter(mustGatherPath string, suffix string, filt
 		return "", fmt.Errorf("no match for the specified must gather directory path: %s and suffix: %s", mustGatherPath, suffix)
 	}
 	if len(paths) > 1 {
-		log.Warnf("Multiple matches for the specified must gather directory path: %s and suffix: %s", mustGatherPath, suffix)
+		Alert("Multiple matches for the specified must gather directory path: %s and suffix: %s", mustGatherPath, suffix)
 		return "", fmt.Errorf("Multiple matches for the specified must gather directory path: %s and suffix: %s.\n Expected only one performance-addon-operator-must-gather* directory, please check the must-gather tarball", mustGatherPath, suffix)
 	}
 	// returning one possible path

pkg/performanceprofile/profilecreator/profilecreator.go

@@ -25,7 +25,6 @@ import (
 
 	"github.com/jaypipes/ghw/pkg/cpu"
 	"github.com/jaypipes/ghw/pkg/topology"
-	log "github.com/sirupsen/logrus"
 
 	configv1 "github.com/openshift/api/config/v1"
 	k8syaml "k8s.io/apimachinery/pkg/util/yaml"
@@ -239,7 +238,7 @@ func getOfflinedCPUs(extCpuInfo *extendedCPUInfo, offlinedCPUCount int, disableH
 	}
 
 	if lpOfflined < offlinedCPUCount {
-		log.Warnf("could not offline enough logical processors (required:%d, offlined:%d)", offlinedCPUCount, lpOfflined)
+		Alert("could not offline enough logical processors (required:%d, offlined:%d)", offlinedCPUCount, lpOfflined)
 	}
 	return offlined.Result(), nil
 }
@@ -248,29 +247,29 @@ func updateTopologyInfo(topoInfo *topology.Info, disableHTFlag bool, htEnabled b
 	//currently HT is enabled on the system and the user wants to disable HT
 
 	if htEnabled && disableHTFlag {
-		log.Infof("Updating Topology info because currently hyperthreading is enabled and the performance profile will disable it")
+		Alert("Updating Topology info because currently hyperthreading is enabled and the performance profile will disable it")
 		return topologyHTDisabled(topoInfo), nil
 	}
 	return topoInfo, nil
 }
 
 func getReservedCPUs(topologyInfo *topology.Info, reservedCPUCount int, splitReservedCPUsAcrossNUMA bool, disableHTFlag bool, htEnabled bool) (cpuset.CPUSet, error) {
 	if htEnabled && disableHTFlag {
-		log.Infof("Currently hyperthreading is enabled and the performance profile will disable it")
+		Alert("Currently hyperthreading is enabled and the performance profile will disable it")
 		htEnabled = false
 	}
-	log.Infof("NUMA cell(s): %d", len(topologyInfo.Nodes))
+	Alert("NUMA cell(s): %d", len(topologyInfo.Nodes))
 	totalCPUs := 0
 	for id, node := range topologyInfo.Nodes {
 		coreList := []int{}
 		for _, core := range node.Cores {
 			coreList = append(coreList, core.LogicalProcessors...)
 		}
-		log.Infof("NUMA cell %d : %v", id, coreList)
+		Alert("NUMA cell %d : %v", id, coreList)
 		totalCPUs += len(coreList)
 	}
 
-	log.Infof("CPU(s): %d", totalCPUs)
+	Alert("CPU(s): %d", totalCPUs)
 
 	if splitReservedCPUsAcrossNUMA {
 		res, err := getCPUsSplitAcrossNUMA(reservedCPUCount, htEnabled, topologyInfo.Nodes)
@@ -351,7 +350,7 @@ func getCPUsSplitAcrossNUMA(reservedCPUCount int, htEnabled bool, topologyInfoNo
 	reservedPerNuma := reservedCPUCount / numaNodeNum
 	remainder := reservedCPUCount % numaNodeNum
 	if remainder != 0 {
-		log.Warnf("The reserved CPUs cannot be split equally across NUMA Nodes")
+		Alert("The reserved CPUs cannot be split equally across NUMA Nodes")
 	}
 	for numaID, node := range topologyInfoNodes {
 		if remainder != 0 {
@@ -467,7 +466,7 @@ func ensureSameTopology(topology1, topology2 *topology.Info, tols toleration.Set
 				// as the NUMA cells have same logical processors' count and IDs and same threads' number,
 				// core ID equality is treated as best effort. That is because when scheduling workloads,
 				// we care about the logical processors ids and their location on the NUMAs.
-				log.Warnf("the CPU core ids in NUMA node %d differ: %d vs %d", node1.ID, core1.ID, cores2[j].ID)
+				Alert("the CPU core ids in NUMA node %d differ: %d vs %d", node1.ID, core1.ID, cores2[j].ID)
 				tols[toleration.DifferentCoreIDs] = true
 			}
 			if core1.NumThreads != cores2[j].NumThreads {
@@ -488,7 +487,7 @@ func GetAdditionalKernelArgs(disableHT bool) []string {
 		kernelArgs = append(kernelArgs, noSMTKernelArg)
 	}
 	sort.Strings(kernelArgs)
-	log.Infof("Additional Kernel Args based on configuration: %v", kernelArgs)
+	Alert("Additional Kernel Args based on configuration: %v", kernelArgs)
 	return kernelArgs
 }