You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "chungen0126 (via GitHub)" <gi...@apache.org> on 2024/12/18 15:59:24 UTC
[PR] fix TestOzoneDelegationTokenSecretManager [ozone]
chungen0126 opened a new pull request, #7594:
URL: https://github.com/apache/ozone/pull/7594
## What changes were proposed in this pull request?
Fix tests to renew/cancel TestOzoneDelegationTokenSecretManager with OM Ratis enabled.
In ratis, `createToken` is seperated to `createToken` and `updateToken`, and it is run until `validateAndUpdateCache`.
Add `updateToken` to the tests.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11869
## How was this patch tested?
CI:
https://github.com/chungen0126/ozone/actions/runs/12396367209
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11869. Remove non-Ratis OM support in OzoneDelegationTokenSecretManager [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #7594:
URL: https://github.com/apache/ozone/pull/7594#issuecomment-2552877813
@adoroszlai I agree. I'll revert those changes in the production code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11869. Enable OM Ratis in TestOzoneDelegationTokenSecretManager [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #7594:
URL: https://github.com/apache/ozone/pull/7594
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11869. Remove non-Ratis OM support in OzoneDelegationTokenSecretManager [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7594:
URL: https://github.com/apache/ozone/pull/7594#issuecomment-2552872351
Sorry for nitpicking, but I think we should restrict this change to the test. If we merge it as is, `OzoneDelegationTokenSecretManager` will ignore the config, while other parts of the code continue to use it. `OzoneDelegationTokenSecretManager` will stop working with OM Ratis disabled.
I think we should drop support in the following steps:
- ignore config, use hard-coded value (HDDS-11864)
- remove production code (HDDS-11866)
- remove tests (HDDS-11865 + couple of tickets for specific tests, like this)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11869. Enable OM Ratis in TestOzoneDelegationTokenSecretManager [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7594:
URL: https://github.com/apache/ozone/pull/7594#issuecomment-2554166158
Thanks @chungen0126 for the patch, @ChenSammi, @jojochuang for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11869. Enable OM Ratis in TestOzoneDelegationTokenSecretManager [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7594:
URL: https://github.com/apache/ozone/pull/7594#issuecomment-2552624829
> The OzoneDelegationTokenSecretManager.renewToken() has this code snippet:
>
> I believe this snippet should be removed as part of this PR.
>
> There are other places in this class where non-HA code is expected to be removed too.
Originally prod code was planned to be removed in HDDS-11866.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11869. Remove non-Ratis OM support in OzoneDelegationTokenSecretManager [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #7594:
URL: https://github.com/apache/ozone/pull/7594#discussion_r1891155770
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java:
##########
@@ -377,24 +353,12 @@ public OzoneTokenIdentifier cancelToken(Token<OzoneTokenIdentifier> token,
// For HA ratis will take care of removal.
Review Comment:
Please remove these two line comments too. The rest looks good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11869. Enable OM Ratis in TestOzoneDelegationTokenSecretManager [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #7594:
URL: https://github.com/apache/ozone/pull/7594#discussion_r1891155770
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java:
##########
@@ -377,24 +353,12 @@ public OzoneTokenIdentifier cancelToken(Token<OzoneTokenIdentifier> token,
// For HA ratis will take care of removal.
Review Comment:
Please remove these two line comments too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org