You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "HuangZhenQiu (via GitHub)" <gi...@apache.org> on 2025/02/03 21:20:10 UTC
[PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
HuangZhenQiu opened a new pull request, #26101:
URL: https://github.com/apache/flink/pull/26101
## What is the purpose of the change
In the Flink yarn deployment mode, It is possible that the deployment service acquire the delegation tokens in advance for yarn clusters to download jars and configs. In this case, the delegation tokens needs to be added into Yarn deployment context, and further be loaded in UGI. Thus, we need to add tokens acquired by current user for token refresh.
## Brief change log
- Add credentials of current user into the token refresh and interval calculation
- Add test case of token refresh for current user who already have token created.
## Verifying this change
This change added tests and can be verified as follows:
- *Add test case of token refresh for current user who already have token created.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (no)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
- The serializers: (no)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
- The S3 file system connector: (no)
## Documentation
- Does this pull request introduce a new feature? (no)
- If yes, how is the feature documented? (not applicable)
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "gaborgsomogyi (via GitHub)" <gi...@apache.org>.
gaborgsomogyi commented on PR #26101:
URL: https://github.com/apache/flink/pull/26101#issuecomment-2634681108
Correct me if I'm wrong but if I understand correctly then you guys inject external tokens into the UGI which then you would expect to be maintained by Flink, right? If that's true then:
* We can take them over in `YarnClusterDescriptor.setTokensFor`
* but don't expect that the responsibility of them will be taken over by the internal framework
If you need something like that then I would create a new provider/receiver pair for the custom need where the externally generated tokens are externally updated, the new receiver can detect it, triggers a manager cycle and then then the provider updates it.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "HuangZhenQiu (via GitHub)" <gi...@apache.org>.
HuangZhenQiu commented on PR #26101:
URL: https://github.com/apache/flink/pull/26101#issuecomment-2634923756
> Correct me if I'm wrong but if I understand correctly then you guys inject external tokens into the UGI which then you would expect to be maintained by Flink, right? If that's true then:
>
> * We can take them over in `YarnClusterDescriptor.setTokensFor`
> * but don't expect that the responsibility of them will be taken over by the internal framework
>
> If you need something like that then I would create a new provider/receiver pair for the custom need where the externally generated tokens are externally updated, the new receiver can detect it, triggers a manager cycle and then then the provider updates it.
From the Flink error log, the token needed is for checkpoint. It means the token is mainly used for HDFS. That is why I want to use HadoopFSDelegationTokenProvider to refresh it within the internal framework.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "HuangZhenQiu (via GitHub)" <gi...@apache.org>.
HuangZhenQiu commented on code in PR #26101:
URL: https://github.com/apache/flink/pull/26101#discussion_r1941868325
##########
flink-runtime/src/main/java/org/apache/flink/runtime/security/token/hadoop/HadoopFSDelegationTokenProvider.java:
##########
@@ -110,7 +110,10 @@ public ObtainedDelegationTokens obtainDelegationTokens() throws Exception {
return freshUGI.doAs(
(PrivilegedExceptionAction<ObtainedDelegationTokens>)
() -> {
- Credentials credentials = new Credentials();
+ LOG.debug("Loading delegation tokens available to UGI current user");
+ Credentials credentials =
+ new Credentials(
+ UserGroupInformation.getCurrentUser().getCredentials());
Review Comment:
Understand. How about define a new CurrentUserDelegationTokenProvider mainly for refresh the tokens that are added into container launch context? They are useful as these tokens are required for setup execution context for containers. When a TM failure or JM recover from HA mode, these tokens need to be still valid.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "gaborgsomogyi (via GitHub)" <gi...@apache.org>.
gaborgsomogyi commented on PR #26101:
URL: https://github.com/apache/flink/pull/26101#issuecomment-2633222072
Not necessarily understand what the new code would modify. The YARN related code is here and should do exactly the same, right?
https://github.com/apache/flink/blob/b1eb1406504449849c234046786dce5ca1375ddf/flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java#L1320-L1344
* Gets tokens from current user
* Puts it into credentials
* Obtains new tokens which may overwrite tokens from current user
* Set it on lunch context
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "gaborgsomogyi (via GitHub)" <gi...@apache.org>.
gaborgsomogyi commented on PR #26101:
URL: https://github.com/apache/flink/pull/26101#issuecomment-2633227610
Is it possible that `secretKeysMap` needs to be copied over? Please check `Credentials.addAll`.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #26101: [FLINK-37246] add tokens acquired by current user for token refresh
URL: https://github.com/apache/flink/pull/26101
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "HuangZhenQiu (via GitHub)" <gi...@apache.org>.
HuangZhenQiu commented on code in PR #26101:
URL: https://github.com/apache/flink/pull/26101#discussion_r1941868325
##########
flink-runtime/src/main/java/org/apache/flink/runtime/security/token/hadoop/HadoopFSDelegationTokenProvider.java:
##########
@@ -110,7 +110,10 @@ public ObtainedDelegationTokens obtainDelegationTokens() throws Exception {
return freshUGI.doAs(
(PrivilegedExceptionAction<ObtainedDelegationTokens>)
() -> {
- Credentials credentials = new Credentials();
+ LOG.debug("Loading delegation tokens available to UGI current user");
+ Credentials credentials =
+ new Credentials(
+ UserGroupInformation.getCurrentUser().getCredentials());
Review Comment:
Understand. How about define a new CurrentUserDelegationTokenProvider mainly for refresh the tokens added into container launch context? They are useful as these tokens are returned for setup execution context for containers. When a TM failure or JM recover from HA mode, these tokens need to be still valid.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "gaborgsomogyi (via GitHub)" <gi...@apache.org>.
gaborgsomogyi commented on PR #26101:
URL: https://github.com/apache/flink/pull/26101#issuecomment-2634966208
> From the Flink error log, the token needed is for checkpoint. It means the token is mainly used for HDFS. That is why I want to use HadoopFSDelegationTokenProvider to refresh it within the internal framework.
Good, then the only thing is needed to add the FS as default FS, or to `security.kerberos.access.hadoopFileSystems` to YARN staging dir and it will be handled automatically, right?
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #26101:
URL: https://github.com/apache/flink/pull/26101#issuecomment-2632127786
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "fe70df160f5698d8a942279c5e2158f3d18b63d2",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "fe70df160f5698d8a942279c5e2158f3d18b63d2",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* fe70df160f5698d8a942279c5e2158f3d18b63d2 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "HuangZhenQiu (via GitHub)" <gi...@apache.org>.
HuangZhenQiu commented on PR #26101:
URL: https://github.com/apache/flink/pull/26101#issuecomment-2634521238
@gaborgsomogyi Thanks for the review. I did a change as you suggested. Would you please review before I do a manual testing in our testing yarn cluster?
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "HuangZhenQiu (via GitHub)" <gi...@apache.org>.
HuangZhenQiu commented on code in PR #26101:
URL: https://github.com/apache/flink/pull/26101#discussion_r1941868325
##########
flink-runtime/src/main/java/org/apache/flink/runtime/security/token/hadoop/HadoopFSDelegationTokenProvider.java:
##########
@@ -110,7 +110,10 @@ public ObtainedDelegationTokens obtainDelegationTokens() throws Exception {
return freshUGI.doAs(
(PrivilegedExceptionAction<ObtainedDelegationTokens>)
() -> {
- Credentials credentials = new Credentials();
+ LOG.debug("Loading delegation tokens available to UGI current user");
+ Credentials credentials =
+ new Credentials(
+ UserGroupInformation.getCurrentUser().getCredentials());
Review Comment:
Understand. How about define a new CurrentUserDelegationTokenProvider mainly for refresh the tokens that are added into container launch context? They are useful as these tokens are returned for setup execution context for containers. When a TM failure or JM recover from HA mode, these tokens need to be still valid.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26101:
URL: https://github.com/apache/flink/pull/26101#issuecomment-2948233446
This PR has been closed since it has not had any activity in 120 days.
If you feel like this was a mistake, or you would like to continue working on it,
please feel free to re-open the PR and ask for a 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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "gaborgsomogyi (via GitHub)" <gi...@apache.org>.
gaborgsomogyi commented on code in PR #26101:
URL: https://github.com/apache/flink/pull/26101#discussion_r1941660983
##########
flink-runtime/src/main/java/org/apache/flink/runtime/security/token/hadoop/HadoopFSDelegationTokenProvider.java:
##########
@@ -110,7 +110,10 @@ public ObtainedDelegationTokens obtainDelegationTokens() throws Exception {
return freshUGI.doAs(
(PrivilegedExceptionAction<ObtainedDelegationTokens>)
() -> {
- Credentials credentials = new Credentials();
+ LOG.debug("Loading delegation tokens available to UGI current user");
+ Credentials credentials =
+ new Credentials(
+ UserGroupInformation.getCurrentUser().getCredentials());
Review Comment:
Adding current user here would change the behaviour so that Hadoop FS provider would send out Kudu or any other tokens to providers which Hadoop supports. That's not something what I'm comfortable with based on my current knowledge.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26101:
URL: https://github.com/apache/flink/pull/26101#issuecomment-2853418841
This PR is being marked as stale since it has not had any activity in the last 90 days.
If you would like to keep this PR alive, please leave a comment asking for a review.
If the PR has merge conflicts, update it with the latest from the base branch.
If you are having difficulty finding a reviewer, please reach out to the
community, contact details can be found here: https://flink.apache.org/what-is-flink/community/
If this PR is no longer valid or desired, please feel free to close it.
If no activity occurs in the next 30 days, it will be automatically closed.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-37246] add tokens acquired by current user for token refresh [flink]
Posted by "Aireed (via GitHub)" <gi...@apache.org>.
Aireed commented on PR #26101:
URL: https://github.com/apache/flink/pull/26101#issuecomment-6117314915
@HuangZhenQiu hi, anything update??
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org