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

csi: add log rotation for csi rbd pod containers #14305

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

parth-gr
Copy link
Member

@parth-gr parth-gr commented Jun 4, 2024

  1. Make the csi rbd container logs persisted in a file
    (csi plugin, csi provisioner, csi addons sidecar)

  2. Use the cephcluster api specs to configure the log rotate

  3. Add log rotation to rotate the log file and
    Add a sidecar log collector container

part-of: #12809

Updates:

  • --logtostderr=false
  • --alsologtostderr=true
  • --log_file=/var/lib/cephcsi/log/rbdprov.log
sh-4.4# cat csi
/dev/*.log {
  rotate 3
  daily
  size 1M
  copytruncate
  start 1
}

When you mount a volume to a directory in a container, it effectively overrides the contents of that directory with the contents of the volume. This can cause issues if there are existing configurations or files in that directory that you want to preserve while adding new files or configurations.

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 minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@parth-gr parth-gr changed the title csi: add log rotation for csi pod containers [WIP] csi: add log rotation for csi pod containers Jun 4, 2024
@parth-gr parth-gr requested a review from yati1998 June 4, 2024 15:59
@parth-gr parth-gr force-pushed the csi-logrotate branch 4 times, most recently from a44aabc to 5d60a52 Compare June 10, 2024 15:12
@parth-gr parth-gr requested review from travisn and Madhu-1 June 10, 2024 15:12
@parth-gr parth-gr changed the title [WIP] csi: add log rotation for csi pod containers csi: add log rotation for csi pod containers Jun 10, 2024
@parth-gr parth-gr force-pushed the csi-logrotate branch 3 times, most recently from 144aee9 to 5ac055f Compare June 12, 2024 15:08
@parth-gr parth-gr requested a review from BlaineEXE June 12, 2024 15:08
@parth-gr parth-gr force-pushed the csi-logrotate branch 3 times, most recently from 08f58ca to 5d9fb68 Compare June 12, 2024 15:24
Copy link
Member

@BlaineEXE BlaineEXE left a comment

Choose a reason for hiding this comment

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

I'd like to get @Madhu-1 's thought's on this as well, but it seems to me that adding these changes to all 4 major CSI deployment templates will be a lot of boilerplate to maintain over time. It seems like it might be in the interest of future devs to define this as a common Golang template that gets applied after template loading.

Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

Just a couple nits, plus as discussed let's see if we can implement purely as a sidecar and no init container.

pkg/operator/ceph/csi/controller.go Outdated Show resolved Hide resolved
pkg/operator/ceph/csi/controller.go Outdated Show resolved Hide resolved
@parth-gr
Copy link
Member Author

parth-gr commented Jun 13, 2024

I'd like to get @Madhu-1 's thought's on this as well, but it seems to me that adding these changes to all 4 major CSI deployment templates will be a lot of boilerplate to maintain over time. It seems like it might be in the interest of future devs to define this as a common Golang template that gets applied after template loading.

can we have a common shell script?

@BlaineEXE or I think if one sidecar running the logrotate per node is fine, we can simply add the sidecar with the csi daemon set.

As we run the logrotate for a csi config file, which points to a same csi/log directory.
As all the logs will be present in same folder we can just make use of side car

With CSI as we do not re-load the containers we can use this method

parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 1, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

Closes: rook#12809

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 2, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 2, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 3, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 3, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 5, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809

Signed-off-by: parth-gr <partharora1010@gmail.com>
Signed-off-by: parth-gr <paarora@redhat.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 9, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 9, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 9, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 10, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 10, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 10, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 10, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 10, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 10, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 10, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 10, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 10, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 10, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 11, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 11, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 11, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <paarora@redhat.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 11, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 11, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 11, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 11, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 11, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
parth-gr added a commit to parth-gr/rook that referenced this pull request Jul 12, 2024
Use the same logrotate flow of rbd, as merged in rook#14305

1) Make the csi cephfs and nfs container logs persisted in a file

2) Use the cephcluster api specs to configure the log rotate

3) Add log rotation to rotate the log file and
Add a sidecar log collector container

And some fixes on the implementation:

1) Add a operator namespace in the log file path

2) Only add volume to the container if logrotate is enabled

Closes: rook#12809, rook#14429

Signed-off-by: parth-gr <partharora1010@gmail.com>
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.

None yet

4 participants