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

logrotate: add logrotate functionality for csi #50

Merged
merged 2 commits into from
Aug 19, 2024

Conversation

parth-gr
Copy link
Contributor

Describe what this PR does

  1. Make main container and csi addons container
    log to a file(dependency on klog)

  2. Add a log-rotate sidecar container,
    so it can rotate the logs

  3. Added other volume and volumemounts as needed

  4. Added the privileged option for controllerplugin

Provide some context for the reviewer

Is there anything that requires special attention

Do you have any questions?

Is the change backward compatible?

Are there concerns around backward compatibility?

Provide any external context for the change, if any.

For example:

  • Kubernetes links that explain why the change is required
  • Ceph-CSI spec related changes/catch-up that necessitates this patch
  • golang related practices that necessitates this change

Related issues

Mention any github issues relevant to this PR. Adding below line
will help to auto close the issue once the PR is merged.

Fixes: #issue_number

Future concerns

List items that are not part of the PR and do not impact it's
functionality, but are work items that can be taken up subsequently.

Checklist:

  • Commit Message Formatting: Commit titles and messages follow
    guidelines in the developer
    guide
    .
  • Reviewed the developer guide on Submitting a Pull
    Request
  • Pending release
    notes

    updated with breaking and/or notable changes for the next major release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@parth-gr
Copy link
Contributor Author

@nb-ohad @Madhu-1 ^^

@parth-gr parth-gr force-pushed the logrotate-feature branch 2 times, most recently from 8fa90b4 to 10e384a Compare July 23, 2024 14:53
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
internal/utils/csi.go Outdated Show resolved Hide resolved
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
@parth-gr parth-gr force-pushed the logrotate-feature branch 7 times, most recently from 20a6a92 to c5892cb Compare July 24, 2024 15:12
@parth-gr parth-gr requested a review from nb-ohad July 24, 2024 15:12
@parth-gr parth-gr force-pushed the logrotate-feature branch 3 times, most recently from 221e980 to ca82c65 Compare July 25, 2024 16:38
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
internal/utils/csi.go Outdated Show resolved Hide resolved
@parth-gr parth-gr force-pushed the logrotate-feature branch 7 times, most recently from 1c5b866 to 2c9dea5 Compare August 7, 2024 11:29
@parth-gr parth-gr requested review from Madhu-1 and BlaineEXE August 7, 2024 11:29
@parth-gr parth-gr force-pushed the logrotate-feature branch 3 times, most recently from 1eb909f to 0d3a5b1 Compare August 7, 2024 18:13
@parth-gr parth-gr force-pushed the logrotate-feature branch 2 times, most recently from dae6541 to c67e20f Compare August 16, 2024 09:01
@Madhu-1
Copy link
Collaborator

Madhu-1 commented Aug 19, 2024

@parth-gr may i know the recent change you pushed after i approved the PR?

internal/controller/driver_controller.go Outdated Show resolved Hide resolved
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
@parth-gr parth-gr requested a review from nb-ohad August 19, 2024 08:03
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
@parth-gr parth-gr requested a review from nb-ohad August 19, 2024 08:33
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
internal/controller/driver_controller.go Outdated Show resolved Hide resolved
@parth-gr parth-gr force-pushed the logrotate-feature branch 2 times, most recently from 0bb7091 to 937b089 Compare August 19, 2024 08:56
@parth-gr parth-gr requested review from nb-ohad and Madhu-1 August 19, 2024 08:56
@parth-gr parth-gr requested a review from Madhu-1 August 19, 2024 10:37
Madhu-1
Madhu-1 previously approved these changes Aug 19, 2024
currently security context was just set by looking logrotate
is enabled or not, but from this commit we will also check
if the cntrlplugin has prviliged true

Signed-off-by: parth-gr <partharora1010@gmail.com>
@nb-ohad
Copy link
Collaborator

nb-ohad commented Aug 19, 2024

LGTM
@Madhu-1 Are you ok with the state of the PR?

@Madhu-1 Madhu-1 merged commit 88e6db2 into ceph:main Aug 19, 2024
8 checks passed
iamniting pushed a commit to iamniting/ceph-csi-operator that referenced this pull request Nov 12, 2024
Syncing latest changes from upstream main for ceph-csi-operator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants