OCPBUGS-69436: Fix for must-gather scripting #1843
Conversation
…ate namespace based on the csv object installed. - Modified the way of detecting the namespace in case that the operator is running in several namespaces.
openshift-ci
bot
added
the
size/M
openshift-ci
bot
added
the
do-not-merge/work-in-progress
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1843 +/- ##
==========================================
- Coverage 48.63% 48.23% -0.41%
==========================================
Files 52 52
Lines 3709 3709
==========================================
- Hits 1804 1789 -15
- Misses 1754 1768 +14
- Partials 151 152 +1 🚀 New features to boost your workflow:
|
- Pulled latest changes
jeff-roche
changed the title
|
@deik0: No Jira issue with key OCPEDGE-69436 exists in the tracker at https://issues.redhat.com/. 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. |
jeff-roche
changed the title
openshift-ci-robot
added
jira/severity-important
|
@deik0: This pull request references Jira Issue OCPBUGS-69436, 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. |
|
/assign @suleymanakbas91 @qJkee @jeff-roche |
|
@deik0 running the precommit-hooks should resolve your end of line issue in the precommit-check, otherwise lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deik0 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/retest |
| for resource in "${resources[@]}"; do | ||
| echo "collecting dump of ${resource}" | tee -a "${BASE_COLLECTION_PATH}/gather-debug.log" | ||
| { oc adm --dest-dir="${BASE_COLLECTION_PATH}/namespaces/all/" inspect "${resource}" --all-namespaces ${log_collection_args}; } >>"${BASE_COLLECTION_PATH}/gather-debug.log" 2>&1 | ||
| { oc adm --dest-dir="${BASE_COLLECTION_PATH}/namespaces/${NAMESPACE}/" inspect "${resource}" --all-namespaces ${log_collection_args}; } >>"${BASE_COLLECTION_PATH}/gather-debug.log" 2>&1 |
There was a problem hiding this comment.
This runs on all namespaces to fetch all the LVMCluster objects in the cluster. Is there a reason to change this to namespace-only?
There was a problem hiding this comment.
I am not sure that is working as expected.
Based on this:
# Resource List
resources=()
# collect lvmcluster resources
#resources+=(lvmclusters)
It looks like it's never initialized properly, hence when executing that part it will not collecting anything.
I've tested this recently:
$ cat lvm-test.logs | grep "collecting dump of"
[must-gather-6pxtp] POD 2025-12-25T10:31:45.252310962Z collecting dump of namespace
[must-gather-6pxtp] POD 2025-12-25T10:31:46.730431061Z collecting dump of clusterresourceversion
[must-gather-6pxtp] POD 2025-12-25T10:31:52.458643433Z collecting dump of oc get pvc all namespaces
[must-gather-6pxtp] POD 2025-12-25T10:31:53.030236440Z collecting dump of snapshot information for all namespaces
$ oc get lvmcluster -n openshift-storage
NAME STATUS
my-lvmcluster Failed
And, unless I'm missing something, it's not getting triggered as the resource list is empty.
Regardless of the above, with this change, the script should be able to collect lvmcluster objects in every namespace that they are available(considering that a CSV object exist in them,plus any other resource that might have been configured).
There was a problem hiding this comment.
Oh, I see. Would you be so kind to remove it all together then?
| echo "collecting dump of namespace" | tee -a "${BASE_COLLECTION_PATH}"/gather-debug.log | ||
| oc adm --dest-dir="${BASE_COLLECTION_PATH}" inspect ns/"${INSTALL_NAMESPACE}" ${log_collection_args} >>"${BASE_COLLECTION_PATH}"/gather-debug.log 2>&1 | ||
|
|
||
| for NAMESPACE in "${INSTALL_NAMESPACE[@]}"; do |
There was a problem hiding this comment.
Instead of having many for loops for namespaces throughout the file, we can have the one here which ends right before the collection of cluster-wide objects.
There was a problem hiding this comment.
Yeah, I totally agree that single run for each project would be a better approach, we can change it, I was trying to avoid a major change while maintaining current structure when the data is collected.
There was a problem hiding this comment.
It'll just make the code shorter but it's not a big deal. I'm ok for both approaches.
There was a problem hiding this comment.
i'd like to see that we are checking only one namespace to avoid confuses in future
Also, because of this, we have to have for loop everywhere
There was a problem hiding this comment.
i'd like to see that we are checking only one namespace to avoid confuses in future
Can you elaborate a bit more on this?
Fixing typo Co-authored-by: Suleyman Akbas <[email protected]>
|
@deik0: The following test failed, say
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. |
6 participants
Changed the must-gather namespaced data collection with the following:
Previously only data was collected from "openshift-lvm-storage", now it will collect from the namespace where the csv object for the operator is installed, this should cover old installations for "openshift-storage" and future installation where it might be in a different(custom) namespace.
In the case that the operator is installed with a csv in any other namespace it will also collect data from that namespace.
This should solve: https://issues.redhat.com/browse/OCPBUGS-69401 plus also considering scenarios where the operator is installed in more than one namespace.