Skip to content

feat: Add K8sClientPool Support #17127

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

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

PengJingzhao
Copy link

@PengJingzhao PengJingzhao commented Apr 13, 2025

Purpose of the pull request

This project aims to add K8s connection pool support to DolphinScheduler to enhance the utilization of K8s connections in the worker module. It also enables the worker to perform operations on a single K8s cluster using a shared K8s connection.
This pr is related to this issue #17128

Brief change log

Currently, two classes have been developed. Among them, K8sClientPool acts as a connection pool, maintaining a Map<String, KubernetesClient> where the cluster’s masterUrl is used to identify each cluster. Each cluster shares a single K8sClient. The other class, K8sExecutor, corresponds to the previous K8sUtils. However, instead of creating and maintaining a separate connection for each task as in the past, connections are now obtained from the connection pool.

As a connection pool, K8sClientPool implements connection creation, retrieval, graceful shutdown, and health checks for the pool. In the future, more features will be added based on the requirements of the production environment.

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

Copy link

boring-cyborg bot commented Apr 13, 2025

Thanks for opening this pull request! Please check out our contributing guidelines. (https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md)

Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

Please follow the pull request and create an issue to describe it first. @PengJingzhao

@PengJingzhao
Copy link
Author

ok

@nielifeng nielifeng requested a review from Copilot April 17, 2025 02:01
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/k8s/pool/K8sExecutor.java:67

  • [nitpick] The error message in the jobExists method contains a trailing colon unlike the consistent messaging in other methods. Consider removing the colon to maintain consistency.
throw new TaskException("fail to check job: ", e);

Comment on lines +40 to +44
client.batch()
.v1()
.jobs()
.inNamespace(namespace)
.create(job);

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note

Invoking
ItemWritableOperation.create
should be avoided because it has been deprecated.
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
10.8% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants