Add StorageCluster.spec.nfs.LogLevel in storagecluster CR #2953
Conversation
OdedViner
force-pushed
the
logging_level_nfs
branch
7 times, most recently
from
1a2335a to
bc4b05b
Compare
There was a problem hiding this comment.
I'm assuming the ocs-operator team will be able to double check functionality, but everything looks good to me.
The only suggestions I have are to help document, which I think/hope will be helpful for support engineering. These suggestions are also optional if ocs-op maintainers have differing opinions.
api/v1/storagecluster_types.go
Outdated
| // LogLevel set logging level | ||
| // +optional | ||
| LogLevel string `json:"logLevel,omitempty"` |
There was a problem hiding this comment.
Ganesha has some strange and poorly documented log levels. I'd recommend listing them here to help users and support engineers.
At the same time, I would recommend not specifying these as an enum on the CRD spec. I believe there may be more log levels accepted by ganesha than this list, and they could be useful. Just the comment will be best IMO.
| // LogLevel set logging level | |
| // +optional | |
| LogLevel string `json:"logLevel,omitempty"` | |
| // LogLevel set logging level | |
| // Log levels: NIV_NULL | NIV_FATAL | NIV_MAJ | NIV_CRIT | NIV_WARN | NIV_EVENT | NIV_INFO | NIV_DEBUG | NIV_MID_DEBUG | NIV_FULL_DEBUG | NB_LOG_LEVEL | |
| // +optional | |
| LogLevel string `json:"logLevel,omitempty"` |
There was a problem hiding this comment.
+1 to having these documented.
It will act as reference for someone trying to set the value in the CR.
There was a problem hiding this comment.
Please split the commits into multiple commits where
1st commit would be the code change.
2nd commit would be generated changes.
Also, please make sure to add proper commit,PR description. It's always a good practice to have these added wherever you are contributing.
api/v1/storagecluster_types.go
Outdated
| // LogLevel set logging level | ||
| // +optional | ||
| LogLevel string `json:"logLevel,omitempty"` |
There was a problem hiding this comment.
+1 to having these documented.
It will act as reference for someone trying to set the value in the CR.
|
/cherry-pick release-4.18 |
|
@agarwal-mudit: once the present PR merges, I will cherry-pick it on top of In response to this:
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. |
OdedViner
force-pushed
the
logging_level_nfs
branch
7 times, most recently
from
912192e to
df333b2
Compare
OdedViner
force-pushed
the
logging_level_nfs
branch
5 times, most recently
from
dcae01f to
240cdd4
Compare
Signed-off-by: Oded Viner <[email protected]>
Signed-off-by: Oded Viner <[email protected]>
OdedViner
force-pushed
the
logging_level_nfs
branch
from
240cdd4 to
f497695
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: iamniting, Nikhil-Ladha, OdedViner 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 |
openshift-ci
bot
added
the
approved
openshift-merge-bot
bot
merged commit 941e9cf
into
red-hat-storage:main
|
@agarwal-mudit: new pull request created: #2962 In response to this:
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. |
This PR introduces a new parameter, spec.nfs.LogLevel, to the StorageCluster CR.
The LogLevel parameter allows users to configure the log level for NFS, providing greater flexibility and control over logging behavior.
This enhancement aims to improve the user experience by enabling more precise logging settings for debugging and monitoring purposes.
Additionally, a unit test has been added to verify that the StorageCluster CR exports the Rook NFS CR, ensuring proper integration between the OCS operator CR and the Rook operator CR.