-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-33012][BUILD][K8S] Upgrade fabric8 to 4.10.3 #29888
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
[SPARK-33012][BUILD][K8S] Upgrade fabric8 to 4.10.3 #29888
Conversation
| </dependency> | ||
| <!-- End of shaded deps. --> | ||
|
|
||
| <dependency> |
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.
Why does this become necessary, out of curiosity? looks like a downgrade. I'm just wondering if it affects anything else that depends on okhttp.
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.
I agree with @srowen . This looks suspicious.
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 was my mistake - reverted.
| kubernetes-model-admissionregistration/4.10.3//kubernetes-model-admissionregistration-4.10.3.jar | ||
| kubernetes-model-apiextensions/4.10.3//kubernetes-model-apiextensions-4.10.3.jar | ||
| kubernetes-model-apps/4.10.3//kubernetes-model-apps-4.10.3.jar | ||
| kubernetes-model-autoscaling/4.10.3//kubernetes-model-autoscaling-4.10.3.jar |
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.
Are all of these additional deps needed - if they really aren't we might exclude them for tidiness.
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.
I don't think we have a choice here, it looks like kubernetes-client split their jar into smaller dependencies:
fabric8io/kubernetes-client#2108, and since we depend on the top level kubernetes-client, we must pull these in.
|
ok to test |
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.
Thank you for your first contribution, @laflechejonathan .
- Please don't remove the
<!-- Note: Please update the kubernetes client version in kubernetes/integration-tests/pom.xml -->. - Which K8s server version are you using with this fabric8 library?
- Although the matrix shows like that, Apache Spark
masterbranch seems to work with K8s v1.18.8 without this upgrade, doesn't it?
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #129191 has finished for PR 29888 at commit
|
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.
The K8s IT failure looks relevant a little.
KubernetesSuite:
...
- Run in client mode. *** FAILED ***
io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: POST at: https://192.168.39.171:8443/api/v1/namespaces/2899456ab5484196859800707ad72210/pods. Message: No API token found for service account "default", retry after the token is automatically created and added to the service account. Received status: Status(apiVersion=v1, code=500, details=StatusDetails(causes=[], group=null, kind=serviceaccounts, name=create pod, retryAfterSeconds=1, uid=null, additionalProperties={}), kind=Status, message=No API token found for service account "default", retry after the token is automatically created and added to the service account, metadata=ListMeta(_continue=null, remainingItemCount=null, resourceVersion=null, selfLink=null, additionalProperties={}), reason=ServerTimeout, status=Failure, additionalProperties={}).
at io.fabric8.kubernetes.client.dsl.base.OperationSupport.requestFailure(OperationSupport.java:589)
at io.fabric8.kubernetes.client.dsl.base.OperationSupport.assertResponseCode(OperationSupport.java:528)
at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleResponse(OperationSupport.java:492)
at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleResponse(OperationSupport.java:451)
at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleCreate(OperationSupport.java:252)
at io.fabric8.kubernetes.client.dsl.base.BaseOperation.handleCreate(BaseOperation.java:844)
at io.fabric8.kubernetes.client.dsl.base.BaseOperation.create(BaseOperation.java:352)
at io.fabric8.kubernetes.client.dsl.base.BaseOperation.lambda$createNew$0(BaseOperation.java:365)
at io.fabric8.kubernetes.api.model.DoneablePod.done(DoneablePod.java:26)
at
|
I believe I've addressed your comments, thank you for the quick review! @dongjoon-hyun, to answer your questions:
On the integration test failure, I'm fairly certain it's a flake. We used to run into this error when launching spark applications in production, and had to add functionality to block spark-submit until the service account token is created for a namespace. Would be good to add similar functionality to solve this flake, but should be out of scope of this PR. |
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #129269 has finished for PR 29888 at commit
|
dongjoon-hyun
left a comment
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, LGTM. Thank you, @laflechejonathan and @srowen .
Merged to master for Apache Spark 3.1.0 on December 2020.
|
Thank you again for your first contribution, @laflechejonathan . |
This PR aims to upgrade `kubernetes-client` library to track fabric8's declared compatibility for k8s 1.18.0: https://github.com/fabric8io/kubernetes-client#compatibility-matrix According to fabric8, 4.9.2 is incompatible with k8s 1.18.0. No. Not tested yet. Closes apache#29888 from laflechejonathan/jlf/fabric8Ugprade. Authored-by: jlafleche <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
* [SPARK-33012][BUILD][K8S] Upgrade fabric8 to 4.10.3 This PR aims to upgrade `kubernetes-client` library to track fabric8's declared compatibility for k8s 1.18.0: https://github.com/fabric8io/kubernetes-client#compatibility-matrix According to fabric8, 4.9.2 is incompatible with k8s 1.18.0. No. Not tested yet. Closes apache#29888 from laflechejonathan/jlf/fabric8Ugprade. Authored-by: jlafleche <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> * Fix kubernetes tests Co-authored-by: jlafleche <[email protected]> Co-authored-by: Willi Raschkowski <[email protected]>
* [SPARK-33012][BUILD][K8S] Upgrade fabric8 to 4.10.3 This PR aims to upgrade `kubernetes-client` library to track fabric8's declared compatibility for k8s 1.18.0: https://github.com/fabric8io/kubernetes-client#compatibility-matrix According to fabric8, 4.9.2 is incompatible with k8s 1.18.0. No. Not tested yet. Closes apache#29888 from laflechejonathan/jlf/fabric8Ugprade. Authored-by: jlafleche <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> * Fix kubernetes tests Co-authored-by: jlafleche <[email protected]> Co-authored-by: Willi Raschkowski <[email protected]>
* [SPARK-33012][BUILD][K8S] Upgrade fabric8 to 4.10.3 This PR aims to upgrade `kubernetes-client` library to track fabric8's declared compatibility for k8s 1.18.0: https://github.com/fabric8io/kubernetes-client#compatibility-matrix According to fabric8, 4.9.2 is incompatible with k8s 1.18.0. No. Not tested yet. Closes apache#29888 from laflechejonathan/jlf/fabric8Ugprade. Authored-by: jlafleche <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> * Fix kubernetes tests Co-authored-by: jlafleche <[email protected]> Co-authored-by: Willi Raschkowski <[email protected]>
What changes were proposed in this pull request?
This PR aims to upgrade
kubernetes-clientlibrary to track fabric8's declared compatibility for k8s 1.18.0:https://github.com/fabric8io/kubernetes-client#compatibility-matrix
Why are the changes needed?
According to fabric8, 4.9.2 is incompatible with k8s 1.18.0.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Not tested yet.