NO-JIRA: performance profile: internal cleanup

cmd/performance-profile-creator/main.go

@@ -19,7 +19,7 @@ package main
 import (
 	"os"
 
-	"github.com/openshift/cluster-node-tuning-operator/cmd/performance-profile-creator/cmd"
+	"github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/profilecreator/cmd"
 )
 
 func main() {

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
+}

cmd/performance-profile-creator/cmd/info.go → pkg/performanceprofile/profilecreator/cmd/info.go

@@ -5,8 +5,8 @@ import (
 	"fmt"
 	"os"
 	"sort"
+	"strings"
 
-	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
@@ -17,6 +17,7 @@ const (
 
 type infoOptions struct {
 	jsonOutput bool
+	textOutput bool
 }
 
 // NewInfoCommand return info command, which provides a list of the cluster nodes and their hardware topology.
@@ -31,6 +32,7 @@ func NewInfoCommand(pcArgs *ProfileCreatorArgs) *cobra.Command {
 		},
 	}
 	info.Flags().BoolVar(&opts.jsonOutput, "json", false, "output as JSON")
+	info.Flags().BoolVar(&opts.textOutput, "text", false, "output as plain text")
 	return info
 }
 
@@ -40,12 +42,8 @@ func executeInfoMode(mustGatherDirPath string, createForHypershift bool, infoOpt
 		return fmt.Errorf("failed to parse the cluster data: %w", err)
 	}
 	clusterInfo := makeClusterInfoFromClusterData(clusterData)
-	if infoOpts.jsonOutput {
-		if err := showClusterInfoJSON(clusterInfo); err != nil {
-			return fmt.Errorf("unable to show cluster info %w", err)
-		}
-	} else {
-		showClusterInfoLog(clusterInfo)
+	if err := showClusterInfo(clusterInfo, infoOpts); err != nil {
+		return fmt.Errorf("unable to show cluster info %w", err)
 	}
 	return nil
 }
@@ -96,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{
@@ -128,21 +126,36 @@ func makeClusterInfoFromClusterData(cluster ClusterData) ClusterInfo {
 	return cInfo.Sort()
 }
 
-func showClusterInfoJSON(cInfo ClusterInfo) error {
-	return json.NewEncoder(os.Stdout).Encode(cInfo)
+func showClusterInfo(cInfo ClusterInfo, infoOpts *infoOptions) error {
+	if infoOpts.jsonOutput {
+		return json.NewEncoder(os.Stdout).Encode(cInfo)
+	}
+	textInfo := dumpClusterInfo(cInfo)
+	if infoOpts.textOutput {
+		fmt.Print(textInfo)
+		return nil
+	}
+	Alert("Cluster info:\n%s", textInfo)
+	return nil
 }
 
-func showClusterInfoLog(cInfo ClusterInfo) {
-	log.Infof("Cluster info:")
+func dumpClusterInfo(cInfo ClusterInfo) string {
+	var sb strings.Builder
 	for _, mcpInfo := range cInfo {
-		log.Infof("MCP '%s' nodes:", mcpInfo.Name)
+		fmt.Fprintf(&sb, "MCP '%s' nodes:", mcpInfo.Name)
+		sb.WriteString("\n")
 		for _, nInfo := range mcpInfo.Nodes {
-			log.Infof("Node: %s (NUMA cells: %d, HT: %v)", nInfo.Name, len(nInfo.NUMACells), nInfo.HTEnabled)
+			fmt.Fprintf(&sb, "Node: %s (NUMA cells: %d, HT: %v)", nInfo.Name, len(nInfo.NUMACells), nInfo.HTEnabled)
+			sb.WriteString("\n")
 			for _, cInfo := range nInfo.NUMACells {
-				log.Infof("NUMA cell %d : %v", cInfo.ID, cInfo.CoreList)
+				fmt.Fprintf(&sb, "NUMA cell %d : %v", cInfo.ID, cInfo.CoreList)
+				sb.WriteString("\n")
 			}
-			log.Infof("CPU(s): %d", nInfo.CPUsCount)
+			fmt.Fprintf(&sb, "CPU(s): %d", nInfo.CPUsCount)
+			sb.WriteString("\n")
 		}
-		log.Infof("---")
+		fmt.Fprintf(&sb, "---")
+		sb.WriteString("\n")
 	}
+	return sb.String()
 }

cmd/performance-profile-creator/cmd/root.go → pkg/performanceprofile/profilecreator/cmd/root.go

@@ -18,31 +18,28 @@ package cmd
 
 import (
 	"bytes"
-	"encoding/json"
 	"fmt"
-	"io"
 	"os"
 	"strings"
 
-	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	serializer "k8s.io/apimachinery/pkg/runtime/serializer/json"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	kubeletconfig "k8s.io/kubelet/config/v1beta1"
 	"k8s.io/utils/ptr"
-	"sigs.k8s.io/yaml"
 
 	machineconfigv1 "github.com/openshift/api/machineconfiguration/v1"
-	"github.com/openshift/cluster-node-tuning-operator/cmd/performance-profile-creator/cmd/pkg/hypershift"
 	performancev2 "github.com/openshift/cluster-node-tuning-operator/pkg/apis/performanceprofile/v2"
 	"github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/profilecreator"
+	"github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/profilecreator/cmd/hypershift"
+	"github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/profilecreator/serialize"
+	"github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/profilecreator/toleration"
 )
 
 const (
@@ -107,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))
 }
 
@@ -128,7 +124,7 @@ func NewRootCommand() *cobra.Command {
 		"must-gather-dir-path",
 	}
 
-	tolerations := profilecreator.TolerationSet{}
+	tols := toleration.Set{}
 
 	root := &cobra.Command{
 		Use:   "performance-profile-creator",
@@ -164,7 +160,7 @@ func NewRootCommand() *cobra.Command {
 				return err
 			}
 
-			err = profilecreator.EnsureNodesHaveTheSameHardware(nodesHandlers, tolerations)
+			err = profilecreator.EnsureNodesHaveTheSameHardware(nodesHandlers, tols)
 			if err != nil {
 				return fmt.Errorf("targeted nodes differ: %w", err)
 			}
@@ -175,12 +171,17 @@ func NewRootCommand() *cobra.Command {
 			if err != nil {
 				return fmt.Errorf("failed to make profile data from node handler: %w", err)
 			}
-			tolerations[profilecreator.EnableHardwareTuning] = profileData.enableHardwareTuning
+			tols[toleration.EnableHardwareTuning] = profileData.enableHardwareTuning
 			profile, err := makePerformanceProfileFrom(*profileData)
 			if err != nil {
 				return err
 			}
-			return writeProfile(profile, tolerations)
+			profData, err := serialize.Profile(profile, tols)
+			if err != nil {
+				return err
+			}
+			_, err = fmt.Println(profData)
+			return err
 		},
 	}
 	pcArgs.AddFlags(root.PersistentFlags())
@@ -201,7 +202,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
 }
 
@@ -311,8 +312,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(),
@@ -521,93 +522,6 @@ func makePerformanceProfileFrom(profileData ProfileData) (runtime.Object, error)
 	return profile, nil
 }
 
-func writeProfile(obj runtime.Object, tolerations profilecreator.TolerationSet) error {
-	// write CSV to out dir
-	writer := strings.Builder{}
-	if err := MarshallObject(obj, &writer); err != nil {
-		return err
-	}
-
-	if tolerations[profilecreator.EnableHardwareTuning] {
-		if _, err := writer.Write([]byte(profilecreator.HardwareTuningMessage)); err != nil {
-			return err
-		}
-	}
-
-	if tolerations[profilecreator.DifferentCoreIDs] {
-		if _, err := writer.Write([]byte(profilecreator.DifferentCoreIDsMessage)); err != nil {
-			return err
-		}
-	}
-
-	fmt.Printf("%s", writer.String())
-	return nil
-}
-
-// MarshallObject mashals an object, usually a CSV into YAML
-func MarshallObject(obj interface{}, writer io.Writer) error {
-	jsonBytes, err := json.Marshal(obj)
-	if err != nil {
-		return err
-	}
-
-	var r unstructured.Unstructured
-	if err := json.Unmarshal(jsonBytes, &r.Object); err != nil {
-		return err
-	}
-
-	// remove status and metadata.creationTimestamp
-	unstructured.RemoveNestedField(r.Object, "metadata", "creationTimestamp")
-	unstructured.RemoveNestedField(r.Object, "template", "metadata", "creationTimestamp")
-	unstructured.RemoveNestedField(r.Object, "spec", "template", "metadata", "creationTimestamp")
-	unstructured.RemoveNestedField(r.Object, "status")
-
-	deployments, exists, err := unstructured.NestedSlice(r.Object, "spec", "install", "spec", "deployments")
-	if err != nil {
-		return err
-	}
-	if exists {
-		for _, obj := range deployments {
-			deployment := obj.(map[string]interface{})
-			unstructured.RemoveNestedField(deployment, "metadata", "creationTimestamp")
-			unstructured.RemoveNestedField(deployment, "spec", "template", "metadata", "creationTimestamp")
-			unstructured.RemoveNestedField(deployment, "status")
-		}
-		if err := unstructured.SetNestedSlice(r.Object, deployments, "spec", "install", "spec", "deployments"); err != nil {
-			return err
-		}
-	}
-
-	jsonBytes, err = json.Marshal(r.Object)
-	if err != nil {
-		return err
-	}
-
-	yamlBytes, err := yaml.JSONToYAML(jsonBytes)
-	if err != nil {
-		return err
-	}
-
-	// fix double quoted strings by removing unneeded single quotes...
-	s := string(yamlBytes)
-	s = strings.Replace(s, " '\"", " \"", -1)
-	s = strings.Replace(s, "\"'\n", "\"\n", -1)
-
-	yamlBytes = []byte(s)
-
-	_, err = writer.Write([]byte("---\n"))
-	if err != nil {
-		return err
-	}
-
-	_, err = writer.Write(yamlBytes)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 func listNodesForPool(args *ProfileCreatorArgs) ([]*corev1.Node, error) {
 	nodes, err := profilecreator.GetNodeList(args.MustGatherDirPath)
 	if err != nil {