Add new flag to collect minimal resources #206

Sheetalpamecha · 2024-11-06T08:57:22Z

Creating a flag to limit data collection to only major CRDs and CSVs, reducing time taken and log clutter.
It collects minimum required data and have kept the alignment similar to other flags for streamlined parsing.

openshift-ci · 2024-11-06T08:57:33Z

Hi @Sheetalpamecha. Thanks for your PR.

I'm waiting for a red-hat-storage member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

Sheetalpamecha · 2024-11-06T08:58:01Z

@yati1998 and @black-dragon74 Please take a look.

yati1998 · 2024-11-06T13:16:24Z

collection-scripts/gather

@@ -93,6 +99,7 @@ Options:
  -cl, --ceph-logs          Collect ceph daemon, kernel, journal logs and crash reports
  -ns, --namespaced         Collect namespaced resources
  -cs, --clusterscoped      Collect clusterscoped resources
+  --minimal-crds            Collect only relevant CRDs and CSVs


how about adding short form for it too?

Sure, How about -mr, --minimal-resources?

MG always collects CRDs or resources. Let's omit it and call it -m, minimal?

yati1998 · 2024-11-06T13:24:28Z

@Sheetalpamecha can you link the issue which pointed to raise this PR? I didn't get why do we want only these specific crds to be collected? I just see this as a code duplication and I don't think user would be happy just with these resources to debug any case?

Sheetalpamecha · 2024-11-06T13:48:53Z

Hi Yati, True there will be redundancy, As we want to highlight few configurations only and we don't want the logs and other information collected.
Here is the epic link - https://issues.redhat.com/browse/RHSTOR-5936

black-dragon74 · 2024-11-06T16:00:11Z

collection-scripts/gather

@@ -197,6 +204,12 @@ if [ "$clusterscoped" == true ] && [ "$odf" == false ]; then
    pids+=($!)
 fi

+if [ "$minimal_crds" == true ] && [ "$odf" == false ]; then


Suggested change

if [ "$minimal_crds" == true ] && [ "$odf" == false ]; then

if [ "$minimal_crds" == true ]; then

We do not need to check for odf == false here as the minimal script is not part of the odf collection. The reason we check for odf == false is so we do not collect resources that are collected with --odf mode twice.

Umm.. I think you forgot to commit the changes..

P.S: We should not mark conversations as resolved by ourselves as it makes it harder to keep track of changes for reviewers. Ideally, the person who left the comment resolves it upon confirming the changes :)

Noted, will be careful with future PRs.
Pushed the correct changes

black-dragon74 · 2024-11-06T16:06:23Z

/ok-to-test

black-dragon74 · 2024-11-08T10:17:17Z

collection-scripts/gather

@@ -93,6 +99,7 @@ Options:
  -cl, --ceph-logs          Collect ceph daemon, kernel, journal logs and crash reports
  -ns, --namespaced         Collect namespaced resources
  -cs, --clusterscoped      Collect clusterscoped resources
+  -m,  --minimal            Collect only relevant CRDs and CSVs


A more specific description would be nice. "only relevant" sounds a bit vague?

black-dragon74 · 2024-11-08T10:17:45Z

collection-scripts/gather

@@ -197,6 +204,12 @@ if [ "$clusterscoped" == true ] && [ "$odf" == false ]; then
    pids+=($!)
 fi

+if [ "$minimal" == true ]; then
+    echo "Collect minimal resource files..."
+    gather_minimal_resources ${BASE_COLLECTION_PATH} &


Suggested change

gather_minimal_resources ${BASE_COLLECTION_PATH} &

gather_minimal_resources &

Hi Niraj, We can keep it as it is, as this is to keep the same structure for all options as there will be a python script running over this. I have tested with both cmds but we should call with argument only for future use also.

It’s fine I guess. But if you look at actual scripts, none of them read this arg. They read the exported BASE_COLLECTION_PATH.

Anyways, let’s keep it.

yati1998 · 2024-11-11T04:31:06Z

@Sheetalpamecha I hope you have tested this change and verified that you are able to collect all the required crds using this flag?

Creating a flag to limit data collection to only major CRDs and CSVs, reducing time taken and log clutter. Signed-off-by: Sheetal Pamecha <[email protected]>

Sheetalpamecha · 2024-11-11T05:22:08Z

Hi @yati1998 , Yes I have tested and this option is able to collect required crds.

black-dragon74 · 2024-11-11T05:59:46Z

/lgtm
/approve

openshift-ci · 2024-11-11T05:59:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: black-dragon74, Sheetalpamecha

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [black-dragon74]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot added the needs-ok-to-test label Nov 6, 2024

Sheetalpamecha force-pushed the min_flag branch 2 times, most recently from 96520a7 to 5e4dafd Compare November 6, 2024 12:03

yati1998 reviewed Nov 6, 2024

View reviewed changes

black-dragon74 reviewed Nov 6, 2024

View reviewed changes

openshift-ci bot added ok-to-test and removed needs-ok-to-test labels Nov 6, 2024

Sheetalpamecha force-pushed the min_flag branch 2 times, most recently from daca790 to f8e1372 Compare November 7, 2024 16:05

black-dragon74 reviewed Nov 8, 2024

View reviewed changes

Sheetalpamecha force-pushed the min_flag branch from f8e1372 to 1b03be0 Compare November 11, 2024 03:29

Add new flag to collect minimal resources

30c8ec6

Creating a flag to limit data collection to only major CRDs and CSVs, reducing time taken and log clutter. Signed-off-by: Sheetal Pamecha <[email protected]>

Sheetalpamecha force-pushed the min_flag branch from 1b03be0 to 30c8ec6 Compare November 11, 2024 05:21

yati1998 requested a review from black-dragon74 November 11, 2024 05:56

openshift-ci bot assigned black-dragon74 Nov 11, 2024

openshift-ci bot added the lgtm label Nov 11, 2024

openshift-ci bot added the approved label Nov 11, 2024

openshift-merge-bot bot merged commit 7922423 into red-hat-storage:main Nov 11, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new flag to collect minimal resources #206

Add new flag to collect minimal resources #206

Sheetalpamecha commented Nov 6, 2024

openshift-ci bot commented Nov 6, 2024

Sheetalpamecha commented Nov 6, 2024

yati1998 Nov 6, 2024

Sheetalpamecha Nov 6, 2024

black-dragon74 Nov 6, 2024

Sheetalpamecha Nov 6, 2024

yati1998 commented Nov 6, 2024

Sheetalpamecha commented Nov 6, 2024

black-dragon74 Nov 6, 2024

Sheetalpamecha Nov 6, 2024

black-dragon74 Nov 7, 2024

Sheetalpamecha Nov 7, 2024

black-dragon74 commented Nov 6, 2024

black-dragon74 Nov 8, 2024

Sheetalpamecha Nov 11, 2024

black-dragon74 Nov 8, 2024

Sheetalpamecha Nov 11, 2024 •

edited

Loading

black-dragon74 Nov 11, 2024

yati1998 commented Nov 11, 2024

Sheetalpamecha commented Nov 11, 2024

black-dragon74 commented Nov 11, 2024

openshift-ci bot commented Nov 11, 2024

	if [ "$minimal_crds" == true ] && [ "$odf" == false ]; then
	if [ "$minimal_crds" == true ]; then

	gather_minimal_resources ${BASE_COLLECTION_PATH} &
	gather_minimal_resources &

Add new flag to collect minimal resources #206

Add new flag to collect minimal resources #206

Conversation

Sheetalpamecha commented Nov 6, 2024

openshift-ci bot commented Nov 6, 2024

Sheetalpamecha commented Nov 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yati1998 commented Nov 6, 2024

Sheetalpamecha commented Nov 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

black-dragon74 commented Nov 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sheetalpamecha Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yati1998 commented Nov 11, 2024

Sheetalpamecha commented Nov 11, 2024

black-dragon74 commented Nov 11, 2024

openshift-ci bot commented Nov 11, 2024

Sheetalpamecha Nov 11, 2024 •

edited

Loading