The fix was two fold:

1. Update the build.xml to use the core jar instead of the classes, so that the service lookup works correctly.

2. Fix the deprecated DistributedFileSystem.getDelegationToken and DFSClient.getDelegationToken to correctly set the service.

I think you need to also handle the case in DFSClient where the ctor is passed a null nameNodeAddr.

What is the reason for moving setting of service from DistributedFileSystem (DFS) to DFSClient? One flip side I see is that DFSClient now needs to store the nnAddress, but DFS can get it from URI.

Token#setKind
Please add a comment why renewer is being set to null in setKind.

I believe the null nameNodeAddr is only done in some of the unit tests that aren't creating delegation tokens. I haven't checked extensively.

Yeah, I just went through the 4 callers of DFSClient that pass in non-null Client protocols and they are all testing append or retry in the client. There is no service to set in those cases and we are better off getting a runtime error in that case.

Jitendra,
The previous code would not set the service if the user called DFSClient.getDelegationToken, which is broken. It was also only setting it for the non-deprecated DistributedFileSystem.getDelegationToken.

Added the requested comment about clearing the renewer cache in Token.setKind.

The comment hasn't clarified the reasoning (for me) behind blanking out the renewer when the kind is changed. Twiddling the kind is only used to trick hftp into renewing hdfs tokens, so it doesn't seem like the renewer field should be changed? The comment alludes to the renewer being looked up again. How does this happen?

I don't think there is currently a case where a token will have a non-null renewer when setKind is called, but it could.

When isManaged, renew or cancel are called on a token, it first calls getRenewer. getRenewer returns the renewer field, if it is set or otherwise it finds the right kind of renewer based on the kind. When you change the kind, generally the discovered renewer will be different. Therefore, to avoid accidentally using the wrong renewer, it is safer to clear it when we set the kind.

+1. Looks good to me.

If the renewer field is defined, it appears to mean that a renew or cancel has already been performed on the token. Will it get dicey to change the kind on a token that it already being manipulated by something else?

+1 if what I mentioned before is not a problem

I've committed this to 205 and 20-s. It will be committed to trunk with MAPREDUCE-2764.

After commit of this jira HADOOP-7649 has been failing.

After the commit of this jira, two tests have been failing. See HADOOP-7649.

Closed upon release of 0.20.205.0

Hey Owen,

How does using the core jar (instead of the test classes dir) cause the ServiceLoader work? Wouldn't it be better to specify the renewer classes explicitly in build.xml for the tests using the <service>/<provider> tags?

Thanks,
Eli

Ah, looks like from HADOOP-7644 it doesn't, a services file is still needed.