logrotate: add design and api for logrotate

[parth-gr]

Describe what this PR does

api for the csi pods logrotate

Provide some context for the reviewer

Check the API flow

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]

[rider@localhost ceph-csi-operator]$ make generate 
Downloading sigs.k8s.io/controller-tools/cmd/[email protected]
# runtime/cgo
In file included from /usr/include/features.h:489,
                 from /usr/include/bits/libc-header-start.h:33,
                 from /usr/include/stdlib.h:25,
                 from _cgo_export.c:3:
/usr/include/gnu/stubs.h:7:11: fatal error: gnu/stubs-32.h: No such file or directory
    7 | # include <gnu/stubs-32.h>
      |           ^~~~~~~~~~~~~~~~
compilation terminated.
make: *** [Makefile:180: /home/rider/go/src/github.com/ceph-csi-operator/bin/controller-gen-v0.14.0] Error 1

Not able to build the crds

[nb-ohad]

@parth-gr The way this API change is written assumes log rotation is always enabled if you want to change the log level which I think is a mistake. We need to make a decision:

• either we have log rotation always which means we need good defaults
• or we separate the rotation params to LogRotationSpec which is an optional field under LogSpec (via a pointer)

[parth-gr]

Collaborator

@parth-gr The way this API change is written assumes log rotation is always enabled if you want to change the log level which I think is a mistake. We need to make a decision:

• either we have log rotation always which means we need good defaults
• or we separate the rotation params to LogRotationSpec which is an optional field under LogSpec (via a pointer)

@Madhu-1 what do you think? should we have enabled disable??
Or do good defaults work??

And downstream it is only enabled when

`CephCSIOperatorConfig CRD`:
```yaml
spec:
    provisioner:
        privileged: true

[Madhu-1]

Collaborator

@parth-gr The way this API change is written assumes log rotation is always enabled if you want to change the log level which I think is a mistake. We need to make a decision:

• either we have log rotation always which means we need good defaults
• or we separate the rotation params to LogRotationSpec which is an optional field under LogSpec (via a pointer)

@Madhu-1 what do you think? should we have enabled disable?? Or do good defaults work??

And downstream it is only enabled when

`CephCSIOperatorConfig CRD`:
```yaml
spec:
    provisioner:
        privileged: true

both are independent, both need to be set for it to work properly for openshift. why do we need to enable it by default, we are setting the values that user expected us to set, user should know what is being done.

[parth-gr]

@Madhu-1 @nb-ohad good for final round of review, once this has the green flag, I can work on the follow up subtasks(Prs)

[Madhu-1]

@parth-gr can you please rebase this PR?

[Madhu-1]

LGTM

[nb-ohad]

@parth-gr I already asked in a prev comment to call the field Rotation and not LogRotation because we do not need the reputation:
Driver.Spec.Log.LogRotation -> Driver.Spec.Log.Rotation

The struct name itself should still be called LogRotationSepc because that is what it describes but the field name should change

As this one was already merged by @Madhu-1. Please issue another PR for the API fix

[parth-gr]

@nb-ohad I did updated it before, but with your recent suggestions I did change it back #37 (comment)

I can open a follow PR to address this ;)

[nb-ohad]

You are correct!!!

When I wrote the comment you are pointing to, I was thinking about the field in isolation, not as part of the entire spec. The point I was trying to make was that we need Spec in the struct name. calling the field LogRotation instead of Rotation, on that comment, was just a simple oversight.

My bad. And yes let's have a follow up PR to fix this.