You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@heron.apache.org by GitBox <gi...@apache.org> on 2019/12/28 19:24:23 UTC
[GitHub] [incubator-heron] nicknezis opened a new pull request #3426: WIP:
Updated to support Kubernetes 1.16
nicknezis opened a new pull request #3426: WIP: Updated to support Kubernetes 1.16
URL: https://github.com/apache/incubator-heron/pull/3426
Updating to the latest Kubernetes Java client and API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 commented on a change in pull
request #3426: WIP: Updated to support Kubernetes 1.16
Posted by GitBox <gi...@apache.org>.
joshfischer1108 commented on a change in pull request #3426: WIP: Updated to support Kubernetes 1.16
URL: https://github.com/apache/incubator-heron/pull/3426#discussion_r361853285
##########
File path: heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/AppsV1Controller.java
##########
@@ -97,20 +95,13 @@ boolean submit(PackingPlan packingPlan) {
for (PackingPlan.ContainerPlan containerPlan : packingPlan.getContainers()) {
numberOfInstances = Math.max(numberOfInstances, containerPlan.getInstances().size());
}
- final V1beta1StatefulSet statefulSet = createStatefulSet(containerResource, numberOfInstances);
+ final V1StatefulSet statefulSet = createStatefulSet(containerResource, numberOfInstances);
try {
- final Response response =
- client.createNamespacedStatefulSetCall(getNamespace(), statefulSet, null,
- null, null).execute();
- if (!response.isSuccessful()) {
- LOG.log(Level.SEVERE, "Error creating topology message: " + response.message());
- KubernetesUtils.logResponseBodyIfPresent(LOG, response);
- // construct a message based on the k8s API server response
- throw new TopologySubmissionException(
- KubernetesUtils.errorMessageFromResponse(response));
- }
- } catch (IOException | ApiException e) {
+ final V1StatefulSet response =
+ client.createNamespacedStatefulSet(getNamespace(), statefulSet, null,
Review comment:
Should we check if the response is successful here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 commented on issue #3426: WIP:
Updated to support Kubernetes 1.16
Posted by GitBox <gi...@apache.org>.
joshfischer1108 commented on issue #3426: WIP: Updated to support Kubernetes 1.16
URL: https://github.com/apache/incubator-heron/pull/3426#issuecomment-572552735
@nicknezis I think you were doing some additional work on this PR. Is this ready to review again or should we wait?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 commented on a change in pull
request #3426: WIP: Updated to support Kubernetes 1.16
Posted by GitBox <gi...@apache.org>.
joshfischer1108 commented on a change in pull request #3426: WIP: Updated to support Kubernetes 1.16
URL: https://github.com/apache/incubator-heron/pull/3426#discussion_r362137719
##########
File path: heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/AppsV1Controller.java
##########
@@ -97,20 +95,13 @@ boolean submit(PackingPlan packingPlan) {
for (PackingPlan.ContainerPlan containerPlan : packingPlan.getContainers()) {
numberOfInstances = Math.max(numberOfInstances, containerPlan.getInstances().size());
}
- final V1beta1StatefulSet statefulSet = createStatefulSet(containerResource, numberOfInstances);
+ final V1StatefulSet statefulSet = createStatefulSet(containerResource, numberOfInstances);
try {
- final Response response =
- client.createNamespacedStatefulSetCall(getNamespace(), statefulSet, null,
- null, null).execute();
- if (!response.isSuccessful()) {
- LOG.log(Level.SEVERE, "Error creating topology message: " + response.message());
- KubernetesUtils.logResponseBodyIfPresent(LOG, response);
- // construct a message based on the k8s API server response
- throw new TopologySubmissionException(
- KubernetesUtils.errorMessageFromResponse(response));
- }
- } catch (IOException | ApiException e) {
+ final V1StatefulSet response =
+ client.createNamespacedStatefulSet(getNamespace(), statefulSet, null,
Review comment:
Nothing off the top of my head that would make it any better. It looks like the client handles printing out enough information. We can always come back and add more logging if we run into issues or someone requests it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] nicknezis commented on a change in pull request
#3426: WIP: Updated to support Kubernetes 1.16
Posted by GitBox <gi...@apache.org>.
nicknezis commented on a change in pull request #3426: WIP: Updated to support Kubernetes 1.16
URL: https://github.com/apache/incubator-heron/pull/3426#discussion_r361897294
##########
File path: heron/schedulers/src/java/org/apache/heron/scheduler/kubernetes/AppsV1Controller.java
##########
@@ -97,20 +95,13 @@ boolean submit(PackingPlan packingPlan) {
for (PackingPlan.ContainerPlan containerPlan : packingPlan.getContainers()) {
numberOfInstances = Math.max(numberOfInstances, containerPlan.getInstances().size());
}
- final V1beta1StatefulSet statefulSet = createStatefulSet(containerResource, numberOfInstances);
+ final V1StatefulSet statefulSet = createStatefulSet(containerResource, numberOfInstances);
try {
- final Response response =
- client.createNamespacedStatefulSetCall(getNamespace(), statefulSet, null,
- null, null).execute();
- if (!response.isSuccessful()) {
- LOG.log(Level.SEVERE, "Error creating topology message: " + response.message());
- KubernetesUtils.logResponseBodyIfPresent(LOG, response);
- // construct a message based on the k8s API server response
- throw new TopologySubmissionException(
- KubernetesUtils.errorMessageFromResponse(response));
- }
- } catch (IOException | ApiException e) {
+ final V1StatefulSet response =
+ client.createNamespacedStatefulSet(getNamespace(), statefulSet, null,
Review comment:
I believe the `ApiException` will have the status code because the `ApiClient` class checks the status code and throws the exception if there is an error. Is there any better logging you would suggest in the catch block when we deal with the `ApiException`?
Here is a link to the [`ApiClient.handleResponse()`](https://github.com/kubernetes-client/java/blob/d94b271a7dc9636d1b5d930b047b94273b58aa83/kubernetes/src/main/java/io/kubernetes/client/openapi/ApiClient.java#L954) logic.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] nicknezis commented on issue #3426: WIP: Updated
to support Kubernetes 1.16
Posted by GitBox <gi...@apache.org>.
nicknezis commented on issue #3426: WIP: Updated to support Kubernetes 1.16
URL: https://github.com/apache/incubator-heron/pull/3426#issuecomment-572603983
I think it's ready to review. I'm working on making the K8s client the internal cluster config, but that can be submitted as a separate pull request.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 merged pull request #3426:
Updated to support Kubernetes 1.16
Posted by GitBox <gi...@apache.org>.
joshfischer1108 merged pull request #3426: Updated to support Kubernetes 1.16
URL: https://github.com/apache/incubator-heron/pull/3426
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] nicknezis commented on issue #3426: WIP: Updated
to support Kubernetes 1.16
Posted by GitBox <gi...@apache.org>.
nicknezis commented on issue #3426: WIP: Updated to support Kubernetes 1.16
URL: https://github.com/apache/incubator-heron/pull/3426#issuecomment-569576823
I just realized there is now a 7.0.0 release of the Kubernetes Java client. I'm upgrading to use that version. Will also update okhttp dependency from 2.7.5 to 3.4.14. This is the version referenced in the k8s client pom.xml.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services