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-15639][k8s] Support to set tolerations for jobmanager and taskmanger pod #11606
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 f95d4be (Wed Apr 01 15:15:00 UTC 2020) 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:
|
@tisonkun @zhengcanbin Do you mind to take a look at your convenience? |
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, @wangyang0918! The PR generally looks good to me. I have left two comments.
...tes/src/main/java/org/apache/flink/kubernetes/kubeclient/resources/KubernetesToleration.java
Show resolved
Hide resolved
...es/src/main/java/org/apache/flink/kubernetes/kubeclient/parameters/KubernetesParameters.java
Outdated
Show resolved
Hide resolved
f95d4be
to
491f1ef
Compare
@zhengcanbin I have integrated your suggestions and rebased the latest master. Please take another look at your convenience. |
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, @wangyang0918! The PR looks good to me now.
491f1ef
to
80f0b20
Compare
@tisonkun Do you mind to also take a look and help with merging? I think the changes in this PR is quite straightforward. |
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 contribution @wangyang0918 ! One minor comment: the form of list of map config "should be" in the strict form.
.mapType() | ||
.asList() | ||
.noDefaultValue() | ||
.withDescription("The user-specified tolerations to be set to the JobManager pod. The value could be " + |
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.
.withDescription("The user-specified tolerations to be set to the JobManager pod. The value could be " + | |
.withDescription("The user-specified tolerations to be set to the JobManager pod. The value should be " + |
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.
Make sense. I will fix it.
.mapType() | ||
.asList() | ||
.noDefaultValue() | ||
.withDescription("The user-specified tolerations to be set to the TaskManager pod. The value could be " + |
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.
.withDescription("The user-specified tolerations to be set to the TaskManager pod. The value could be " + | |
.withDescription("The user-specified tolerations to be set to the TaskManager pod. The value should be " + |
…manger pod Taints and tolerations work together to ensure that pods are not scheduled onto inappropriate nodes. One or more taints are applied to a node; this marks that the node should not accept any pods that do not tolerate the taints. Tolerations are applied to pods, and allow (but do not require) the pods to schedule onto nodes with matching taints.
80f0b20
to
f84d471
Compare
@tisonkun Thanks for your review. I have integrated your comments and force pushed the PR. Please have another look. |
What is the purpose of the change
The toleration is used to separate the K8s cluster into several individual partitions. Usually it it related with business group. So i treat is as a first class feature. Since at least in our production environment, every Flink job need to specify the toleration so that it could be scheduled to the corresponding resource pool.
Moreover, it is sth very like YARN partition.
Brief change log
Verifying this change
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation