Skip to content
This repository has been archived by the owner on Apr 17, 2024. It is now read-only.

code: update to give warning if not collecting logs #9

Merged
merged 4 commits into from
May 11, 2023

Conversation

ideepika
Copy link
Contributor

No description provided.

@linear
Copy link

linear bot commented May 10, 2023

Copy link
Member

@galexrt galexrt left a comment

Choose a reason for hiding this comment

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

Commit lint is complaining

galexrt
galexrt previously approved these changes May 11, 2023
Copy link
Member

@galexrt galexrt left a comment

Choose a reason for hiding this comment

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

good to merge, have you tested the script with these changes on a cluster?

Deepika Upadhyay added 3 commits May 11, 2023 17:28
Signed-off-by: Deepika Upadhyay <deepika@koor.tech>
Signed-off-by: Deepika Upadhyay <deepika@koor.tech>
Signed-off-by: Deepika Upadhyay <deepika@koor.tech>
@ideepika
Copy link
Contributor Author

good to merge, have you tested the script with these changes on a cluster?

yes, it should be work fine when log file is empty or when using ceph commands:

++ echo 'ceph osd numa-status'
++ sed 's/ /_/g'
+ command=ceph_osd_numa-status
++ echo ceph_osd_numa-status
++ sed s/-/_/g
+ command=ceph_osd_numa_status
+ kubectl -n rook-ceph exec -it deploy/rook-ceph-tools -- ceph osd numa-status

@galexrt
Copy link
Member

galexrt commented May 11, 2023

@ideepika shellcheck is now complaining. If you are using VSCode, I would recommend you install the recommended extensions to have shellcheck run locally as well.

earlier the format looks like also has gibberish from %s :
➜  ~ mktemp -d -t gather-logs-$(date +"%s-%Y-%m-%d")-XXXXXXXXXX
/tmp/gather-logs-1683807557-2023-05-11-PV4QT8Xr7F

now:
➜  ~ mktemp -d -t gather-logs-$(date +"%Y-%m-%d")-XXXXXXXXXX
/tmp/gather-logs-2023-05-11-CgtfU7q6Tc

Signed-off-by: Deepika Upadhyay <deepika@koor.tech>
@ideepika ideepika merged commit c8120a3 into main May 11, 2023
@zalsader zalsader deleted the feature/ksd-206 branch May 12, 2023 15:47
}

gatherCrashInfo() {
for crash in $(runCommandInToolsPod ceph crash ls-new); do

Choose a reason for hiding this comment

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

@ideepika Are you sure this works? The output is redirected to a file, so there will be no output

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point @zalsader , since there wasn't any crash in my env, I simulated a bit different condition, let me fix that tomorrow first thing!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants