-
Notifications
You must be signed in to change notification settings - Fork 350
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
Add support for Kubernetes tolerations #2848
Conversation
Thanks for making a pull request to Elyra! To try out this branch on binder, follow this link: |
I think the |
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.
This looks great! I added some taints to one of the KFP RTCs (since untainted) and it's working well for me. Same with all the validation stuff.
I think the chosen key=value
format makes sense for the current implementation, especially for overriding pipeline default values. It also seems in line with how taints are specified via kubectl
. Plus I'm not sure one way would be all that better than any other until RJSF support is in. Approving now, but should this format be changed before merge, I'll re-review
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.
Changes LGTM.
Maybe sometime in the future if we continue to add and expose even more k8s api parameters, we can put them into a "advanced properties" section with an arrow toggle dropdown to make things cleaner ..
e.g. >, v
Re-confirmed* that after the merge the following scenarios yield the expected results
(*) by inspecting
|
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.
New changes to the handling of <4-part values looks good!
This PR adds support for Kubernetes tolerations for Kubeflow Pipelines and Apache Airflow runtime environments.
Deviations from the specification:
tolerationSeconds
is not supported (as requested in allow setting Kubernetes tolerations in KFP pipelines #2823) - but support can be added in the futureTolerations are supported for generic components and custom components and can be defined as pipeline defaults or for individual nodes.
Pipeline default settings in VPE
Generic node setting in VPE
![image](https://user-images.githubusercontent.com/13068832/182774476-31ac8cbc-6d34-4bb5-919a-cbca23b9c765.png)
Custom node setting in VPE
Applied tolerations (KFP)
Confirmation that the tolerations are applied on the pod
kubectl describe pod lambda-...
Notes:
Status (if checked, functionality is implemented and was verified):
[Requires discussion] The current UI support for list-based properties requires input in the form
SOME_KEY=SOME_VALUE
. The current implementation assumes input as<T_ID>=<key>:<op>:<value>:<effect>
, where<T_ID>
is an arbitrary identifier.Proper UI support requires Add initial support for rjsf in pipeline properties #2780
Closes Accommodate for tainted nodes to match kubeflow pipeline pods #2681
Closes allow setting Kubernetes tolerations in KFP pipelines #2823
What changes were proposed in this pull request?
How was this pull request tested?
Developer's Certificate of Origin 1.1