Skip to content

OCPBUGS-60218: PPC: ghw: filter out namespaces dir #1403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

OCPBUGS-60218: PPC: ghw: filter out namespaces dir #1403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dcc17ff
PPC: pkg: public consts to private
shajmakh Oct 2, 2025
35c4645
PPC: ghw: filter out `namespaces` dir
shajmakh Oct 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pkg/performanceprofile/profilecreator/ghwhandler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, namespaces)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the core idea is to filter out clusterScopedResources and namespaces right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct. namespaces is affected by the user configuration so we should filter it out anyway

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense but need to review again why it causes issues (and why now)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, regarding "why now" it has always been the case but it is less common to have namespaces with "nodes" suffix. The bug has additional info.

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
Expand Down
51 changes: 28 additions & 23 deletions pkg/performanceprofile/profilecreator/mustgather.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,34 +29,39 @@ 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-<Image SHA>
// 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"
// 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
})
Expand All @@ -68,19 +73,19 @@ 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) {
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)
Expand All @@ -103,7 +108,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)
Expand Down Expand Up @@ -131,7 +136,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)
Expand Down Expand Up @@ -161,7 +166,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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/performanceprofile/profilecreator/profilecreator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down