Skip to content

Conversation

@adoroszlai
Copy link
Contributor

@adoroszlai adoroszlai commented Dec 17, 2023

What changes were proposed in this pull request?

Ozone's TokenRenewer implementations do not close OzoneClient after use.

Both O3FS and OFS define their own TokenRenewer implementation.

Hadoop finds implementations via ServiceLoader, and uses the first one that handles the kind of token being used:

https://github.com/apache/hadoop/blob/7db9895000860605a66dd6403005b0c61a6ed744/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/Token.java#L454-L470

The two implementations are the same (not FileSystem-specific), kind is the same for both, so we can remove the duplicate one.

https://issues.apache.org/jira/browse/HDDS-9943

How was this patch tested?

Ran ozonesecure-mr/test.sh, checked OM audit log for delegation token operations:

...  | OMAudit | user=hadoop/[email protected] | ip=172.31.0.11 | op=GET_DELEGATION_TOKEN {kind=OzoneToken, service=172.31.0.9:9862, renewer=rm} | ret=SUCCESS |  
...  | OMAudit | user=rm/[email protected] | ip=172.31.0.11 | op=RENEW_DELEGATION_TOKEN {kind=OzoneToken, service=172.31.0.9:9862, renewer=rm} | ret=SUCCESS |  
...  | OMAudit | user=hadoop/[email protected] | ip=172.31.0.11 | op=GET_DELEGATION_TOKEN {kind=OzoneToken, service=172.31.0.9:9862, renewer=rm} | ret=SUCCESS |  
...  | OMAudit | user=rm/[email protected] | ip=172.31.0.11 | op=RENEW_DELEGATION_TOKEN {kind=OzoneToken, service=172.31.0.9:9862, renewer=rm} | ret=SUCCESS |  
...  | OMAudit | user=rm/[email protected] | ip=172.31.0.11 | op=CANCEL_DELEGATION_TOKEN {kind=OzoneToken, service=172.31.0.9:9862} | ret=SUCCESS |  
...  | OMAudit | user=rm/[email protected] | ip=172.31.0.11 | op=CANCEL_DELEGATION_TOKEN {kind=OzoneToken, service=172.31.0.9:9862} | ret=SUCCESS |  

CI:
https://github.com/adoroszlai/ozone/actions/runs/7237484751

@adoroszlai adoroszlai self-assigned this Dec 17, 2023
@adoroszlai adoroszlai requested a review from smengcl December 17, 2023 20:01
Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

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

Good catch, thanks!

@adoroszlai adoroszlai merged commit 0112a71 into apache:master Dec 18, 2023
@adoroszlai adoroszlai deleted the HDDS-9943 branch December 18, 2023 20:06
@adoroszlai
Copy link
Contributor Author

Thanks @jojochuang for the review.

adoroszlai added a commit to adoroszlai/ozone that referenced this pull request Jan 25, 2024
ptlrs pushed a commit to ptlrs/ozone that referenced this pull request Mar 8, 2025
apache#5809) (apache#90)

Change-Id: Ieb7f6d67ca255270636580d97fb78d960b40b326

Co-authored-by: Doroszlai, Attila <[email protected]>
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.

2 participants