From dcc17ffc39a9acb1161631f8bc38b42359217560 Mon Sep 17 00:00:00 2001 From: Shereen Haj Date: Thu, 2 Oct 2025 15:50:19 +0300 Subject: [PATCH 1/2] PPC: pkg: public consts to private The consts are not used anywhere else so make them private. Signed-off-by: Shereen Haj --- .../profilecreator/ghwhandler.go | 8 ++--- .../profilecreator/mustgather.go | 34 +++++++++---------- .../profilecreator/profilecreator.go | 2 +- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/pkg/performanceprofile/profilecreator/ghwhandler.go b/pkg/performanceprofile/profilecreator/ghwhandler.go index 07ea9feb8f..01e2dd0b9e 100644 --- a/pkg/performanceprofile/profilecreator/ghwhandler.go +++ b/pkg/performanceprofile/profilecreator/ghwhandler.go @@ -33,17 +33,17 @@ import ( // NewGHWHandler is a handler to use ghw options corresponding to a node func NewGHWHandler(mustGatherDirPath string, node *v1.Node) (*GHWHandler, error) { nodeName := node.GetName() - nodePathSuffix := path.Join(Nodes) - nodepath, err := getMustGatherFullPathsWithFilter(mustGatherDirPath, nodePathSuffix, ClusterScopedResources) + nodePathSuffix := path.Join(nodes) + nodepath, err := getMustGatherFullPathsWithFilter(mustGatherDirPath, nodePathSuffix, clusterScopedResources) if err != nil { return nil, fmt.Errorf("can't obtain the node path %s: %v", nodeName, err) } - _, err = os.Stat(path.Join(nodepath, nodeName, SysInfoFileName)) + _, err = os.Stat(path.Join(nodepath, nodeName, sysInfoFileName)) if err != nil { return nil, fmt.Errorf("can't obtain the path: %s for node %s: %v", nodeName, nodepath, err) } options := ghw.WithSnapshot(ghw.SnapshotOptions{ - Path: path.Join(nodepath, nodeName, SysInfoFileName), + Path: path.Join(nodepath, nodeName, sysInfoFileName), }) ghwHandler := &GHWHandler{snapShotOptions: options, Node: node} return ghwHandler, nil diff --git a/pkg/performanceprofile/profilecreator/mustgather.go b/pkg/performanceprofile/profilecreator/mustgather.go index b99ed60ee7..1c737884af 100644 --- a/pkg/performanceprofile/profilecreator/mustgather.go +++ b/pkg/performanceprofile/profilecreator/mustgather.go @@ -29,22 +29,22 @@ import ( ) const ( - // ClusterScopedResources defines the subpath, relative to the top-level must-gather directory. + // clusterScopedResources defines the subpath, relative to the top-level must-gather directory. // A top-level must-gather directory is of the following format: // must-gather-dir/quay-io-openshift-kni-performance-addon-operator-must-gather-sha256- // Here we find the cluster-scoped definitions saved by must-gather - ClusterScopedResources = "cluster-scoped-resources" - // CoreNodes defines the subpath, relative to ClusterScopedResources, on which we find node-specific data - CoreNodes = "core/nodes" - // MCPools defines the subpath, relative to ClusterScopedResources, on which we find the machine config pool definitions - MCPools = "machineconfiguration.openshift.io/machineconfigpools" - // YAMLSuffix is the extension of the yaml files saved by must-gather - YAMLSuffix = ".yaml" - // Nodes defines the subpath, relative to top-level must-gather directory, on which we find node-specific data - Nodes = "nodes" - // SysInfoFileName defines the name of the file where ghw snapshot is stored - SysInfoFileName = "sysinfo.tgz" - // defines the sub path relative to ClusterScopedResources, on which OCP infrastructure object is + clusterScopedResources = "cluster-scoped-resources" + // coreNodes defines the subpath, relative to ClusterScopedResources, on which we find node-specific data + coreNodes = "core/nodes" + // mcpools defines the subpath, relative to ClusterScopedResources, on which we find the machine config pool definitions + mcpools = "machineconfiguration.openshift.io/machineconfigpools" + // yamlSuffix is the extension of the yaml files saved by must-gather + yamlSuffix = ".yaml" + // nodes defines the subpath, relative to top-level must-gather directory, on which we find node-specific data + nodes = "nodes" + // sysInfoFileName defines the name of the file where ghw snapshot is stored + sysInfoFileName = "sysinfo.tgz" + // configOCPInfra defines the sub path relative to ClusterScopedResources, on which OCP infrastructure object is configOCPInfra = "config.openshift.io/infrastructures" ) @@ -80,7 +80,7 @@ func getMustGatherFullPaths(mustGatherPath string, suffix string) (string, error func getNode(mustGatherDirPath, nodeName string) (*v1.Node, error) { var node v1.Node - nodePathSuffix := path.Join(ClusterScopedResources, CoreNodes, nodeName) + nodePathSuffix := path.Join(clusterScopedResources, coreNodes, nodeName) path, err := getMustGatherFullPaths(mustGatherDirPath, nodePathSuffix) if err != nil { return nil, fmt.Errorf("failed to get MachineConfigPool for %s: %v", nodeName, err) @@ -103,7 +103,7 @@ func getNode(mustGatherDirPath, nodeName string) (*v1.Node, error) { func GetNodeList(mustGatherDirPath string) ([]*v1.Node, error) { machines := make([]*v1.Node, 0) - nodePathSuffix := path.Join(ClusterScopedResources, CoreNodes) + nodePathSuffix := path.Join(clusterScopedResources, coreNodes) nodePath, err := getMustGatherFullPaths(mustGatherDirPath, nodePathSuffix) if err != nil { return nil, fmt.Errorf("failed to get Nodes from must gather directory: %v", err) @@ -131,7 +131,7 @@ func GetNodeList(mustGatherDirPath string) ([]*v1.Node, error) { func GetMCPList(mustGatherDirPath string) ([]*machineconfigv1.MachineConfigPool, error) { pools := make([]*machineconfigv1.MachineConfigPool, 0) - mcpPathSuffix := path.Join(ClusterScopedResources, MCPools) + mcpPathSuffix := path.Join(clusterScopedResources, mcpools) mcpPath, err := getMustGatherFullPaths(mustGatherDirPath, mcpPathSuffix) if err != nil { return nil, fmt.Errorf("failed to get MCPs: %v", err) @@ -161,7 +161,7 @@ func GetMCPList(mustGatherDirPath string) ([]*machineconfigv1.MachineConfigPool, func GetMCP(mustGatherDirPath, mcpName string) (*machineconfigv1.MachineConfigPool, error) { var mcp machineconfigv1.MachineConfigPool - mcpPathSuffix := path.Join(ClusterScopedResources, MCPools, mcpName+YAMLSuffix) + mcpPathSuffix := path.Join(clusterScopedResources, mcpools, mcpName+yamlSuffix) mcpPath, err := getMustGatherFullPaths(mustGatherDirPath, mcpPathSuffix) if err != nil { return nil, fmt.Errorf("failed to obtain MachineConfigPool %s: %v", mcpName, err) diff --git a/pkg/performanceprofile/profilecreator/profilecreator.go b/pkg/performanceprofile/profilecreator/profilecreator.go index a2fea1a1b4..4c7566e24b 100644 --- a/pkg/performanceprofile/profilecreator/profilecreator.go +++ b/pkg/performanceprofile/profilecreator/profilecreator.go @@ -570,7 +570,7 @@ func IsLogicalProcessorUsed(extCPUInfo *extendedCPUInfo, logicalProcessor int) b // IsExternalControlPlaneCluster return whether the control plane is running on externally outside the cluster func IsExternalControlPlaneCluster(mustGatherDirPath string) (bool, error) { - infraPath := path.Join(ClusterScopedResources, configOCPInfra, "cluster.yaml") + infraPath := path.Join(clusterScopedResources, configOCPInfra, "cluster.yaml") fullInfraPath, err := getMustGatherFullPaths(mustGatherDirPath, infraPath) if fullInfraPath == "" || err != nil { return false, fmt.Errorf("failed to get Infrastructure object from must gather directory path: %s; %w", mustGatherDirPath, err) From 35c4645d0afe3424e697c0185c4af384bb06aa6c Mon Sep 17 00:00:00 2001 From: Shereen Haj Date: Thu, 2 Oct 2025 17:51:16 +0300 Subject: [PATCH 2/2] PPC: ghw: filter out `namespaces` dir The previous code fails to run PPC when there is a namespace that has a suffix `nodes`. ensure filtering the namespace directory to allow PPC to work when such namespaces exist. Signed-off-by: Shereen Haj --- .../profilecreator/ghwhandler.go | 2 +- .../profilecreator/mustgather.go | 17 +++++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/pkg/performanceprofile/profilecreator/ghwhandler.go b/pkg/performanceprofile/profilecreator/ghwhandler.go index 01e2dd0b9e..8488912f55 100644 --- a/pkg/performanceprofile/profilecreator/ghwhandler.go +++ b/pkg/performanceprofile/profilecreator/ghwhandler.go @@ -34,7 +34,7 @@ import ( func NewGHWHandler(mustGatherDirPath string, node *v1.Node) (*GHWHandler, error) { nodeName := node.GetName() nodePathSuffix := path.Join(nodes) - nodepath, err := getMustGatherFullPathsWithFilter(mustGatherDirPath, nodePathSuffix, clusterScopedResources) + nodepath, err := getMustGatherFullPathsWithFilter(mustGatherDirPath, nodePathSuffix, clusterScopedResources, namespaces) if err != nil { return nil, fmt.Errorf("can't obtain the node path %s: %v", nodeName, err) } diff --git a/pkg/performanceprofile/profilecreator/mustgather.go b/pkg/performanceprofile/profilecreator/mustgather.go index 1c737884af..25b8c2a9cc 100644 --- a/pkg/performanceprofile/profilecreator/mustgather.go +++ b/pkg/performanceprofile/profilecreator/mustgather.go @@ -42,21 +42,26 @@ const ( yamlSuffix = ".yaml" // nodes defines the subpath, relative to top-level must-gather directory, on which we find node-specific data nodes = "nodes" + // namespaces defines the subpath, relative to top-level must-gather directory, on which we find openshift namespace-specific data + namespaces = "namespaces" // sysInfoFileName defines the name of the file where ghw snapshot is stored sysInfoFileName = "sysinfo.tgz" // configOCPInfra defines the sub path relative to ClusterScopedResources, on which OCP infrastructure object is configOCPInfra = "config.openshift.io/infrastructures" ) -func getMustGatherFullPathsWithFilter(mustGatherPath string, suffix string, filter string) (string, error) { +func getMustGatherFullPathsWithFilter(mustGatherPath string, suffix string, filters ...string) (string, error) { var paths []string - // don't assume directory names, only look for the suffix, filter out files having "filter" in their names + // don't assume directory names, only look for the suffix, filter out files having "filters" in their names err := filepath.Walk(mustGatherPath, func(path string, info os.FileInfo, err error) error { if strings.HasSuffix(path, suffix) { - if len(filter) == 0 || !strings.Contains(path, filter) { - paths = append(paths, path) + for _, f := range filters { + if strings.Contains(path, f) { + return nil + } } + paths = append(paths, path) } return nil }) @@ -68,14 +73,14 @@ func getMustGatherFullPathsWithFilter(mustGatherPath string, suffix string, filt } if len(paths) > 1 { 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) + 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 return paths[0], err } func getMustGatherFullPaths(mustGatherPath string, suffix string) (string, error) { - return getMustGatherFullPathsWithFilter(mustGatherPath, suffix, "") + return getMustGatherFullPathsWithFilter(mustGatherPath, suffix) } func getNode(mustGatherDirPath, nodeName string) (*v1.Node, error) {