Skip to content
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

Bug 2308101: logrotate: add logrotate functionality for csi operator #190

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

parth-gr
Copy link
Member

currently client operator has started using csi operator so collecting the csi logs from a new host path
hat csi-operator supports

Copy link
Contributor

openshift-ci bot commented Aug 27, 2024

@parth-gr: This pull request references Bugzilla bug 2308101, which is valid.

No validations were run on this bug

Requesting review from QA contact:
/cc @PrasadDesala

In response to this:

Bug 2308101: logrotate: add logrotate functionality for csi operator

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.

Copy link
Contributor

openshift-ci bot commented Aug 27, 2024

@openshift-ci[bot]: GitHub didn't allow me to request PR reviews from the following users: PrasadDesala.

Note that only red-hat-storage members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

@parth-gr: This pull request references Bugzilla bug 2308101, which is valid.

No validations were run on this bug

Requesting review from QA contact:
/cc @PrasadDesala

In response to this:

Bug 2308101: logrotate: add logrotate functionality for csi operator

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.

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.

@parth-gr
Copy link
Member Author

/cherry-pick 4.17

@openshift-cherrypick-robot

@parth-gr: once the present PR merges, I will cherry-pick it on top of 4.17 in a new PR and assign it to you.

In response to this:

/cherry-pick 4.17

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.

@parth-gr
Copy link
Member Author

/assign @nb-ohad @Madhu-1 @yati1998

Copy link
Contributor

openshift-ci bot commented Aug 27, 2024

@parth-gr: GitHub didn't allow me to assign the following users: Madhu-1.

Note that only red-hat-storage members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @nb-ohad @Madhu-1 @yati1998

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.

dbglog "collecting csi logs from node ${node}"
oc rsync -n "${ns}" "$(oc get pods -n "${ns}" --no-headers | grep "${node//./}-debug" | awk '{print $1}')":/host/var/lib/cephcsi/"${ns}".rbd.csi.ceph.com-ctrlplugin "${CSI_LOG_OUTPUT_DIR}"
oc rsync -n "${ns}" "$(oc get pods -n "${ns}" --no-headers | grep "${node//./}-debug" | awk '{print $1}')":/host/var/lib/cephcsi/"${ns}".rbd.csi.ceph.com-nodeplugin "${CSI_LOG_OUTPUT_DIR}"
oc rsync -n "${ns}" "$(oc get pods -n "${ns}" --no-headers | grep "${node//./}-debug" | awk '{print $1}')":/host/var/lib/cephcsi/"${ns}".cephfs.csi.ceph.com-ctrlplugin "${CSI_LOG_OUTPUT_DIR}"
Copy link
Member Author

Choose a reason for hiding this comment

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

@yati1998 if the required host path is not present var/lib/cephcsi/"${ns}".cephfs.csi.ceph.com-ctrlplugin , the other must gather collection wont fail, right?

Copy link
Member

Choose a reason for hiding this comment

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

no it wont. Did you test the changes?

Copy link
Member

Choose a reason for hiding this comment

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

Can we add a check below before calling the function, to make sure we collect it incase of csi-operator only?

Copy link
Member Author

Choose a reason for hiding this comment

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

Did you test the changes?

Can you help me doing it, i am not sure on the steps,

Can we add a check below before calling the function, to make sure we collect it incase of csi-operator only?
Actually even if its csi operator there might be case that nfs driver is disabled then dont collect nfs,

So if the current case is working as expected there is no need for seprate checks

Copy link
Member

Choose a reason for hiding this comment

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

sure, we can discuss this offline

@parth-gr parth-gr requested a review from yati1998 August 27, 2024 12:26
currently client operator has started using csi operator
so collecting the csi logs from a new host path
hat csi-operator supports

Signed-off-by: parth-gr <[email protected]>
@openshift-ci openshift-ci bot added the lgtm label Aug 28, 2024
Copy link
Contributor

openshift-ci bot commented Aug 28, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: parth-gr, yati1998

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:

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

@openshift-merge-bot openshift-merge-bot bot merged commit 491aef5 into red-hat-storage:main Aug 28, 2024
5 checks passed
@openshift-cherrypick-robot

@parth-gr: cannot checkout 4.17: error checking out "4.17": exit status 1 error: pathspec '4.17' did not match any file(s) known to git

In response to this:

/cherry-pick 4.17

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.

@black-dragon74
Copy link
Collaborator

black-dragon74 commented Aug 28, 2024

@yati1998 Where is this function csi_operator_log_collection called?

The changes only define the function but never call it.

@parth-gr
Copy link
Member Author

@yati1998 Where is this function csi_operator_log_collection called?

The changes only define the function but never call it.

Ohh , will add a commit, Thanks for catching

@yati1998
Copy link
Member

@yati1998 Where is this function csi_operator_log_collection called?

The changes only define the function but never call it.

it is called in earlier PR made by him right?

@yati1998
Copy link
Member

okay, it's just csi_log_collection, okay my bad!!

@parth-gr
Copy link
Member Author

/cherry-pick release-4.17

@openshift-cherrypick-robot

@parth-gr: new pull request created: #192

In response to this:

/cherry-pick release-4.17

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.

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

Successfully merging this pull request may close these issues.

5 participants