You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/05/16 06:09:28 UTC

[GitHub] [flink-kubernetes-operator] morhidi opened a new pull request, #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

morhidi opened a new pull request, #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217

   This PR is meant to improve the ability to identify malicious Flink versions (CVE affected, deprecated, etc.) in managed environments. The operator propagates exact versioning information in status:
   ```
   status:
       clusterInfo:
           flink-revision: 3a4c113 @ 2022-04-20T19:50:32+02:00
           flink-version: 1.15.0
   ```
   
   
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#discussion_r873817316


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/deployment/AbstractDeploymentObserver.java:
##########
@@ -112,6 +125,7 @@ protected void observeJmDeployment(
         if (JobManagerDeploymentStatus.DEPLOYED_NOT_READY == previousJmStatus) {
             logger.info("JobManager deployment is ready");
             deploymentStatus.setJobManagerDeploymentStatus(JobManagerDeploymentStatus.READY);
+            observeClusterInfo(flinkApp, effectiveConfig);

Review Comment:
   Thanks for the suggestion. Updated the logic again, @wangyang0918. PTAL



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#issuecomment-1127883324

   @wangyang0918  Do you have any idea why the CI test `CI / e2e_ci (Default configuration, default, flink:1.13, v1_13) (pull_request) ` is braking? Made a simple test locally to see if it has anything to do with parsing the response body, without luck so far:
   ```
      @Test
       public void testClusterInfoRestCompatibility() throws JsonProcessingException {
           ObjectMapper objectMapper = new ObjectMapper();
   
           String flink13Response = "{\"refresh-interval\":3000,\"timezone-name\":\"Coordinated Universal Time\",\"timezone-offset\":0,\"flink-version\":\"1.13.6\",\"flink-revision\":\"b2ca390 @ 2022-02-03T14:54:22+01:00\",\"features\":{\"web-submit\":false}}";
           String flink14Response = "{\"refresh-interval\":3000,\"timezone-name\":\"Coordinated Universal Time\",\"timezone-offset\":0,\"flink-version\":\"1.14.4\",\"flink-revision\":\"895c609 @ 2022-02-25T11:57:14+01:00\",\"features\":{\"web-submit\":false,\"web-cancel\":false}}";
           String flink15Response = "{\"refresh-interval\":3000,\"timezone-name\":\"Coordinated Universal Time\",\"timezone-offset\":0,\"flink-version\":\"1.15.0\",\"flink-revision\":\"3a4c113 @ 2022-04-20T19:50:32+02:00\",\"features\":{\"web-submit\":false,\"web-cancel\":false}}";
   
           var dashboardConfiguration = objectMapper.readValue(flink13Response, DashboardConfiguration.class);
           dashboardConfiguration = objectMapper.readValue(flink14Response, DashboardConfiguration.class);
           dashboardConfiguration = objectMapper.readValue(flink15Response, DashboardConfiguration.class);
       }
   ```


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#issuecomment-1127259011

   cc @wangyang0918 @Aitozi 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#discussion_r873853713


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/deployment/AbstractDeploymentObserver.java:
##########
@@ -112,6 +125,7 @@ protected void observeJmDeployment(
         if (JobManagerDeploymentStatus.DEPLOYED_NOT_READY == previousJmStatus) {
             logger.info("JobManager deployment is ready");
             deploymentStatus.setJobManagerDeploymentStatus(JobManagerDeploymentStatus.READY);
+            observeClusterInfo(flinkApp, effectiveConfig);

Review Comment:
   v1.14, v1.15 worked fine locally, seemingly 1.13 is broken, I couldn't verify it locally due to missing arm64 based image. I hoped for a pass through..argh...



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#discussion_r873833638


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/deployment/AbstractDeploymentObserver.java:
##########
@@ -112,6 +125,7 @@ protected void observeJmDeployment(
         if (JobManagerDeploymentStatus.DEPLOYED_NOT_READY == previousJmStatus) {
             logger.info("JobManager deployment is ready");
             deploymentStatus.setJobManagerDeploymentStatus(JobManagerDeploymentStatus.READY);
+            observeClusterInfo(flinkApp, effectiveConfig);

Review Comment:
   Just noticed the broken e2e tests, looking...



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] wangyang0918 commented on pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#issuecomment-1128328901

   @morhidi You could find the following exception in the e2e tests with default namespace. We do not print the operator logs when `watchNamespaces` enabled. This also need to be fixed. https://github.com/apache/flink-kubernetes-operator/runs/6454327460?check_suite_focus=true
   
   ```
   Error: m2022-05-16 14:47:56,840 o.a.f.r.r.RestClient           [ERROR] Received response was neither of the expected type ([simple type, class org.apache.flink.runtime.rest.messages.DashboardConfiguration]) nor an error. Response=JsonResponse{json={"refresh-interval":3000,"timezone-name":"Coordinated Universal Time","timezone-offset":0,"flink-version":"1.13.6","flink-revision":"b2ca390 @ 2022-02-03T14:54:22+01:00","features":{"web-submit":false}}, httpResponseStatus=200 OK}
   org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException: Unrecognized field "refresh-interval" (class org.apache.flink.runtime.rest.messages.ErrorResponseBody), not marked as ignorable (one known property: "errors"])
    at [Source: UNKNOWN; line: -1, column: -1] (through reference chain: org.apache.flink.runtime.rest.messages.ErrorResponseBody["refresh-interval"])
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException.from(UnrecognizedPropertyException.java:61)
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.DeserializationContext.handleUnknownProperty(DeserializationContext.java:987)
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.std.StdDeserializer.handleUnknownProperty(StdDeserializer.java:1974)
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.BeanDeserializerBase.handleUnknownProperty(BeanDeserializerBase.java:1701)
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.BeanDeserializerBase.handleUnknownProperties(BeanDeserializerBase.java:1650)
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:541)
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1405)
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:362)
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:195)
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:322)
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.ObjectMapper._readValue(ObjectMapper.java:4569)
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2798)
   	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.ObjectMapper.treeToValue(ObjectMapper.java:3261)
   	at org.apache.flink.runtime.rest.RestClient.parseResponse(RestClient.java:529)
   	at org.apache.flink.runtime.rest.RestClient.lambda$submitRequest$3(RestClient.java:512)
   	at java.base/java.util.concurrent.CompletableFuture$UniCompose.tryFire(Unknown Source)
   	at java.base/java.util.concurrent.CompletableFuture$Completion.run(Unknown Source)
   	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
   	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
   	at java.base/java.lang.Thread.run(Unknown Source)
   ```


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] gyfora merged pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
gyfora merged PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] wangyang0918 commented on a diff in pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on code in PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#discussion_r873752101


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/deployment/AbstractDeploymentObserver.java:
##########
@@ -112,6 +125,7 @@ protected void observeJmDeployment(
         if (JobManagerDeploymentStatus.DEPLOYED_NOT_READY == previousJmStatus) {
             logger.info("JobManager deployment is ready");
             deploymentStatus.setJobManagerDeploymentStatus(JobManagerDeploymentStatus.READY);
+            observeClusterInfo(flinkApp, effectiveConfig);

Review Comment:
   `JobManagerDeploymentStatus.READY` does not mean the JobManager Rest API is ready for serving. So we might failed to get the cluster info.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#issuecomment-1128388724

   > @morhidi You could find the following exception in the e2e tests with default namespace. We do not print the operator logs when `watchNamespaces` enabled. This also need to be fixed. https://github.com/apache/flink-kubernetes-operator/runs/6454327460?check_suite_focus=true
   > 
   > ```
   > Error: m2022-05-16 14:47:56,840 o.a.f.r.r.RestClient           [ERROR] Received response was neither of the expected type ([simple type, class org.apache.flink.runtime.rest.messages.DashboardConfiguration]) nor an error. Response=JsonResponse{json={"refresh-interval":3000,"timezone-name":"Coordinated Universal Time","timezone-offset":0,"flink-version":"1.13.6","flink-revision":"b2ca390 @ 2022-02-03T14:54:22+01:00","features":{"web-submit":false}}, httpResponseStatus=200 OK}
   > org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException: Unrecognized field "refresh-interval" (class org.apache.flink.runtime.rest.messages.ErrorResponseBody), not marked as ignorable (one known property: "errors"])
   >  at [Source: UNKNOWN; line: -1, column: -1] (through reference chain: org.apache.flink.runtime.rest.messages.ErrorResponseBody["refresh-interval"])
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException.from(UnrecognizedPropertyException.java:61)
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.DeserializationContext.handleUnknownProperty(DeserializationContext.java:987)
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.std.StdDeserializer.handleUnknownProperty(StdDeserializer.java:1974)
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.BeanDeserializerBase.handleUnknownProperty(BeanDeserializerBase.java:1701)
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.BeanDeserializerBase.handleUnknownProperties(BeanDeserializerBase.java:1650)
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:541)
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1405)
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:362)
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:195)
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:322)
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.ObjectMapper._readValue(ObjectMapper.java:4569)
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2798)
   > 	at org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.ObjectMapper.treeToValue(ObjectMapper.java:3261)
   > 	at org.apache.flink.runtime.rest.RestClient.parseResponse(RestClient.java:529)
   > 	at org.apache.flink.runtime.rest.RestClient.lambda$submitRequest$3(RestClient.java:512)
   > 	at java.base/java.util.concurrent.CompletableFuture$UniCompose.tryFire(Unknown Source)
   > 	at java.base/java.util.concurrent.CompletableFuture$Completion.run(Unknown Source)
   > 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
   > 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
   > 	at java.base/java.lang.Thread.run(Unknown Source)
   > ```
   
   This exception is not the real root cause, unfortunately, and it's misleading. I noticed when debugging, when the rest client is handling the original exception, the real response arrives, and it couldn't parse that response into a ErrorResponseBody. We should address this in core Flink. Regardless I'm planning to use a custom response class with the only information we need here. Overall I agree with the approach of having our own request/response classes in the operator that are more resilient for API changes. The current ones are far from ideal. We're going to hit these issues all the time.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] wangyang0918 commented on a diff in pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on code in PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#discussion_r873497824


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/deployment/AbstractDeploymentObserver.java:
##########
@@ -85,16 +86,29 @@ public void observe(FlinkDeployment flinkApp, Context context) {
         if (!isJmDeploymentReady(flinkApp)) {
             observeJmDeployment(flinkApp, context, observeConfig);
         }
+
         if (isJmDeploymentReady(flinkApp)) {
+            observeClusterInfo(flinkApp, observeConfig);

Review Comment:
   Since the cluster info does not change except for upgrading/rollback, we do not need to fetch it for every observation. The rest HTTP call is a little heavy :)



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] gyfora commented on pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
gyfora commented on PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#issuecomment-1128084664

   @morhidi very confusing error message indeed. We can easily fix this tomorrow by the copy trick we did in other incompatible cases :)


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] wangyang0918 commented on pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#issuecomment-1128330687

   >  We can easily fix this tomorrow by the copy trick we did in other incompatible cases :)
   
   @gyfora Flink is now providing an experimental REST API specification following the [OpenAPI](https://www.openapis.org/) standard. Maybe we could manage to use the pure HTTP request/response for posting/getting the cluster-info, savepoint, checkpoint, etc. TBH, I do not like to copy the classes from Flink and make some tiny changes.
   
   I do not mean we need to do this in this PR. It indeed deserves more discussion after release-1.0.0.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#discussion_r873581362


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/deployment/AbstractDeploymentObserver.java:
##########
@@ -85,16 +86,29 @@ public void observe(FlinkDeployment flinkApp, Context context) {
         if (!isJmDeploymentReady(flinkApp)) {
             observeJmDeployment(flinkApp, context, observeConfig);
         }
+
         if (isJmDeploymentReady(flinkApp)) {
+            observeClusterInfo(flinkApp, observeConfig);

Review Comment:
   Fair point :) Moved it to (hopefully) the right spot. PTAL



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] wangyang0918 commented on a diff in pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on code in PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#discussion_r873826015


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/deployment/AbstractDeploymentObserver.java:
##########
@@ -112,6 +125,7 @@ protected void observeJmDeployment(
         if (JobManagerDeploymentStatus.DEPLOYED_NOT_READY == previousJmStatus) {
             logger.info("JobManager deployment is ready");
             deploymentStatus.setJobManagerDeploymentStatus(JobManagerDeploymentStatus.READY);
+            observeClusterInfo(flinkApp, effectiveConfig);

Review Comment:
   Hmm. Do you have verified this PR could work correctly when upgrade with new image or rollback to old image?



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on pull request #217: [FLINK-27609] Tracking flink-version and flink-revision in FlinkDeploymentStatus

Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #217:
URL: https://github.com/apache/flink-kubernetes-operator/pull/217#issuecomment-1128077507

   > @wangyang0918 Do you have any idea why the CI test `CI / e2e_ci (Default configuration, default, flink:1.13, v1_13) (pull_request) ` is braking? Made a simple test locally to see if it has anything to do with parsing the response body, without luck so far:
   > 
   > ```
   >    @Test
   >     public void testClusterInfoRestCompatibility() throws JsonProcessingException {
   >         ObjectMapper objectMapper = new ObjectMapper();
   > 
   >         String flink13Response = "{\"refresh-interval\":3000,\"timezone-name\":\"Coordinated Universal Time\",\"timezone-offset\":0,\"flink-version\":\"1.13.6\",\"flink-revision\":\"b2ca390 @ 2022-02-03T14:54:22+01:00\",\"features\":{\"web-submit\":false}}";
   >         String flink14Response = "{\"refresh-interval\":3000,\"timezone-name\":\"Coordinated Universal Time\",\"timezone-offset\":0,\"flink-version\":\"1.14.4\",\"flink-revision\":\"895c609 @ 2022-02-25T11:57:14+01:00\",\"features\":{\"web-submit\":false,\"web-cancel\":false}}";
   >         String flink15Response = "{\"refresh-interval\":3000,\"timezone-name\":\"Coordinated Universal Time\",\"timezone-offset\":0,\"flink-version\":\"1.15.0\",\"flink-revision\":\"3a4c113 @ 2022-04-20T19:50:32+02:00\",\"features\":{\"web-submit\":false,\"web-cancel\":false}}";
   > 
   >         var dashboardConfiguration = objectMapper.readValue(flink13Response, DashboardConfiguration.class);
   >         dashboardConfiguration = objectMapper.readValue(flink14Response, DashboardConfiguration.class);
   >         dashboardConfiguration = objectMapper.readValue(flink15Response, DashboardConfiguration.class);
   >     }
   > ```
   
   I managed to debug this issue locally finally on an older mac (there's no Flink v1.13 for silicon :/). It is indeed an 501 on the first try, then a JSON parsing issue on the retries. (The error handling in the rest client is a mess, the client side errors are swallowed completely) I'll try to come up with something tomorrow:
   
   ```
   org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot map `null` into type `boolean` (set DeserializationConfig.DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES to 'false' to allow)
    at [Source: UNKNOWN; line: -1, column: -1] (through reference chain: org.apache.flink.runtime.rest.messages.DashboardConfiguration["features"]->org.apache.flink.runtime.rest.messages.DashboardConfiguration$Features["web-cancel"])
   ``` 
   
   This makes more sense ^^^ than the ones I saw in the CI logs.
   .


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org