-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
[FLINK-24212][kerbernets] fix the problem that kerberos krb5.conf file is mounted as empty directory, not a expected file #17198
Conversation
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 0cf21dd (Wed Sep 08 12:28:35 UTC 2021) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
Thanks for the contribution @lzshlzsh . Your fix looks good. Have you verified it in your local env? Also, it would be good to add a test for it. |
Yes, I have verified it in my company production env. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update, @lzshlzsh . It LGTM except for some minor comments. Also, please squash your two commits and add the prefix "[FLINK-24212][k8s]" to your commit message.
.../src/main/java/org/apache/flink/kubernetes/kubeclient/decorators/KerberosMountDecorator.java
Outdated
Show resolved
Hide resolved
.../test/java/org/apache/flink/kubernetes/kubeclient/decorators/KerberosMountDecoratorTest.java
Outdated
Show resolved
Hide resolved
.../test/java/org/apache/flink/kubernetes/kubeclient/decorators/KerberosMountDecoratorTest.java
Outdated
Show resolved
Hide resolved
39d9b35
to
fe3037b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to merge it once CI is green. @xintongsong Would you like to take another look and help to merge it?
.../test/java/org/apache/flink/kubernetes/kubeclient/decorators/KerberosMountDecoratorTest.java
Outdated
Show resolved
Hide resolved
fe3037b
to
d2f8b4a
Compare
…nted as empty directory, not the expected file
d2f8b4a
to
7140554
Compare
…nted as empty directory, not the expected file This closes #17198
…nted as empty directory, not the expected file This closes apache#17198
…nted as empty directory, not the expected file This closes apache#17198
What is the purpose of the change
If the user-provided krb5 conf file is not named krb5.conf (e.g named mykrb5.conf),the mount path /etc/krb5.conf in pod will be an empty directory, not a file that we expect.
Brief change log
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation