Skip to content
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

[#3432] improvement(test): Add kerberos authentication IT for HDFS cluster #3435

Merged
merged 27 commits into from
May 23, 2024

Conversation

yuqi1129
Copy link
Contributor

@yuqi1129 yuqi1129 commented May 17, 2024

What changes were proposed in this pull request?

Add test cases to test Kerberos authentication for the HDFS cluster.

Why are the changes needed?

To make code more robust.

Fix: #3432

Does this PR introduce any user-facing change?

N/A.

How was this patch tested?

test cases.

@yuqi1129
Copy link
Contributor Author

This PR depends on #3431

@yuqi1129 yuqi1129 marked this pull request as draft May 17, 2024 09:19
@yuqi1129 yuqi1129 self-assigned this May 17, 2024
@yuqi1129 yuqi1129 changed the title [#3432] improvement(tes): Add kerberos authentication IT [#3432] improvement(tes): Add kerberos authentication IT for HDFS cluster May 17, 2024
@yuqi1129 yuqi1129 marked this pull request as ready for review May 18, 2024 00:31
@yuqi1129 yuqi1129 added the upload log Always upload container log label May 18, 2024
@yuqi1129 yuqi1129 changed the title [#3432] improvement(tes): Add kerberos authentication IT for HDFS cluster [#3432] improvement(test): Add kerberos authentication IT for HDFS cluster May 20, 2024
@yuqi1129
Copy link
Contributor Author

@jerryshao @qqqttt123
This PR is ready for review, please help to take a look

@jerryshao
Copy link
Contributor

@yuqi1129 is this PR required, do you need to update this PR?

@yuqi1129
Copy link
Contributor Author

@yuqi1129 is this PR required, do you need to update this PR?

I think so, this PR is just to test whether we can access a kerberized HDFS cluster successfully locally to verify the docker image, a complete e2e test is ongoing.

@yuqi1129
Copy link
Contributor Author

@jerryshao
All resolved.

() -> {
try {
FileSystem fs = FileSystem.get(conf);
Path path = new Path("/");
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to create a file and check the file owner to see if it is expected.

If kerberos is enabled, the user should be "cli" from my understanding.

.withHostName("gravitino-ci-kerberos-hive")
.withEnvVars(
ImmutableMap.<String, String>builder()
.put("HADOOP_USER_NAME", "datastrato")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is needed, you can remove and take a try.

jerryshao
jerryshao previously approved these changes May 22, 2024
@jerryshao
Copy link
Contributor

Please fix the style issues.

@yuqi1129
Copy link
Contributor Author

Please fix the style issues.

done

@yuqi1129 yuqi1129 closed this May 22, 2024
@yuqi1129 yuqi1129 reopened this May 22, 2024
@yuqi1129 yuqi1129 closed this May 23, 2024
@yuqi1129 yuqi1129 reopened this May 23, 2024
@jerryshao jerryshao merged commit 097e560 into apache:main May 23, 2024
22 checks passed
github-actions bot pushed a commit that referenced this pull request May 23, 2024
…uster (#3435)

### What changes were proposed in this pull request?

Add test cases to test Kerberos authentication for the HDFS cluster.

### Why are the changes needed?

To make code more robust. 

Fix: #3432


### Does this PR introduce _any_ user-facing change?

N/A.

### How was this patch tested?

test cases.
diqiu50 pushed a commit to diqiu50/gravitino that referenced this pull request Jun 13, 2024
…DFS cluster (apache#3435)

### What changes were proposed in this pull request?

Add test cases to test Kerberos authentication for the HDFS cluster.

### Why are the changes needed?

To make code more robust. 

Fix: apache#3432


### Does this PR introduce _any_ user-facing change?

N/A.

### How was this patch tested?

test cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-0.5 upload log Always upload container log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improvement] Add IT to test kerberosized HDFS
2 participants