-
Notifications
You must be signed in to change notification settings - Fork 118
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.
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
Conversation
| nodeName := node.GetName() | ||
| nodePathSuffix := path.Join(nodes) | ||
| nodepath, err := getMustGatherFullPathsWithFilter(mustGatherDirPath, nodePathSuffix, clusterScopedResources) | ||
| nodepath, err := getMustGatherFullPathsWithFilter(mustGatherDirPath, nodePathSuffix, clusterScopedResources, namespaces) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
|
@shajmakh: This pull request references Jira Issue OCPBUGS-60218, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
ffromani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC PPC doesn't care about the content in namespaces at all, and if so this change should be good. I want to verify that before approving. With this assumption, codewise LGTM
|
/jira refresh |
|
@shajmakh: This pull request references Jira Issue OCPBUGS-60218, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (liqcui@redhat.com), skipping review request. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
The failing lanes are not related to the PPC |
|
/retest |
ffromani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove from the change .idea/workspace.xml
ffromani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
LGTM once the unnecessary file is removed
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ffromani, shajmakh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold |
Thanks for the review. The file is removed. |
The consts are not used anywhere else so make them private. Signed-off-by: Shereen Haj <shajmakh@redhat.com>
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 <shajmakh@redhat.com>
ffromani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
verified manually by me using the updated binary. Still this needs a QE confirmation post-merge: |
|
@shajmakh: This PR has been marked to be verified later by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/cherry-pick release-4.20 |
|
@shajmakh: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@shajmakh: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@shajmakh: Jira Issue OCPBUGS-60218: All pull requests linked via external trackers have merged: This pull request has the DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@shajmakh: new pull request created: #1421 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Fix included in accepted release 4.21.0-0.nightly-2025-11-03-191704 |
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 towork when such namespaces exist.