-
Notifications
You must be signed in to change notification settings - Fork 518
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
HDDS-11194. OM missing audit log for upgrade prepare, cancel and finalize #6958
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sumitagrawl for working on this patch. Changes looks good. Just a few minor comments.
@@ -149,6 +157,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn | |||
} | |||
} | |||
|
|||
auditLog(auditLogger, buildAuditMessage(OMAction.UPGRADE_PREPARE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add transactionLogIndex
and OMNodeId
in auditMap as an additional info for audit message ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TermIndex will be handled as part of "HDDS-10658. Add Object ID and Update ID to OM audit log messages" when this is merged implicitly.
OMNodeId is not required as audit-log is per node itself. There is no need for same.
response = new OMFinalizeUpgradeResponse( | ||
createErrorOMResponse(responseBuilder, e), -1); | ||
} | ||
|
||
auditLog(auditLogger, buildAuditMessage(OMAction.UPGRADE_FINALIZE, | ||
new HashMap<>(), exception, userInfo)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add termIndex
in auditMap as an additional info for audit message ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TermIndex will be handled as part of "HDDS-10658. Add Object ID and Update ID to OM audit log messages" when this is merged.
LOG.error("Cancel Prepare Request apply failed in {}. ", | ||
ozoneManager.getOMNodeId(), e); | ||
response = new OMPrepareResponse( | ||
createErrorOMResponse(responseBuilder, e)); | ||
} | ||
|
||
auditLog(auditLogger, buildAuditMessage(OMAction.UPGRADE_CANCEL, | ||
new HashMap<>(), exception, userInfo)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add OMNodeId
and termIndex
in auditMap
as an additional info for audit message ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TermIndex will be handled as part of "HDDS-10658. Add Object ID and Update ID to OM audit log messages" when this is merged.
OMNodeId is not required as audit-log is per node itself. There is no need for same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sumitagrawl for the clarification. LGTM +1
Thanks @sumitagrawl for the patch. |
…-delete * HDDS-10239-container-reconciliation: (184 commits) HDDS-10373. Implement framework for capturing Merkle Tree Metrics. (apache#6864) HDDS-11188. Initial setup for new UI layout and enable users to switch to new UI (apache#6953) HDDS-11120. Rich rebalancing status info (apache#6911) HDDS-11187. Fix Event Handling in Recon OMDBUpdatesHandler to Prevent ClassCastException. (apache#6950) HDDS-11213. Bump commons-daemon to 1.4.0 (apache#6971) HDDS-11212. Bump commons-net to 3.11.1 (apache#6973) HDDS-11211. Bump assertj-core to 3.26.3 (apache#6972) HDDS-11210. Bump log4j2 to 2.23.1 (apache#6970) HDDS-11150. Recon Overview page crashes due to failed API Calls (apache#6944) HDDS-11183. Keys from DeletedTable and DeletedDirTable of AOS should be deleted on batch operation while creating a snapshot (apache#6946) HDDS-11198. Fix Typescript configs for Recon (apache#6961) HDDS-11180. Simplify HttpServer2#inferMimeType return statement (apache#6963) HDDS-11194. OM missing audit log for upgrade (apache#6958) HDDS-10389. Implement a search feature for users to locate open keys within the Open Keys Insights section. (apache#6231) HDDS-10561. Dashboard for delete key metrics (apache#6948) HDDS-11192. Increase SPNEGO URL test coverage (apache#6956) HDDS-11179. DBConfigFromFile#readFromFile result of toIOException not thrown (apache#6957) HDDS-11186. First container log missing from bundle (apache#6952) HDDS-10844. Clarify snapshot create error message. (apache#6955) HDDS-11166. Switch to Rocky Linux-based ozone-runner (apache#6942) ...
What changes were proposed in this pull request?
Audit log is added for upgrade prepare, cancel and finalize as this was missing. Since this is user request, this also needs logged.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11194
How was this patch tested?