-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix(K8sPipelineClient): tolerate pod/job deserialization errors from newer Kubernetes (fixes HTTP 400 on ingestionPipelines/status) #29791
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
base: main
Are you sure you want to change the base?
Changes from all commits
2df05fa
7cb3bae
a73705f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -930,6 +930,14 @@ public List<PipelineStatus> getQueuedPipelineStatusInternal(IngestionPipeline in | |
|
|
||
| } catch (ApiException e) { | ||
| LOG.error("Failed to check queued pipeline status: {}", e.getResponseBody()); | ||
| } catch (IllegalArgumentException e) { | ||
| // client-java could not deserialize a job returned by a newer Kubernetes | ||
| // than its models (see getServiceStatusInternal). Treat as "no queued jobs | ||
| // visible" rather than propagating the error to the caller. | ||
| LOG.warn( | ||
| "Could not deserialize queued job status (client-java lags the cluster's " | ||
| + "Kubernetes version): {}", | ||
| e.getMessage()); | ||
| } | ||
|
|
||
| return queuedStatuses; | ||
|
|
@@ -1002,6 +1010,25 @@ protected PipelineServiceClientResponse getServiceStatusInternal() { | |
| e.getResponseBody()); | ||
| LOG.error(error); | ||
| return buildUnhealthyStatus(error); | ||
| } catch (IllegalArgumentException e) { | ||
| // The list calls above reached the API server successfully (any real | ||
| // access/RBAC failure would be an ApiException, handled above). An | ||
| // IllegalArgumentException here is thrown by the bundled Kubernetes client | ||
| // (client-java) when it cannot DESERIALIZE a pod/job in the response — | ||
| // typically because the | ||
| // cluster runs a newer Kubernetes than the client models and the response | ||
| // carries an unmodeled status field, e.g.: | ||
| // IllegalArgumentException: The field `allocatedResources` in the JSON | ||
| // string is not defined in the `V1PodStatus` properties | ||
| // Cluster access is fine, so report healthy instead of failing the whole | ||
| // /services/ingestionPipelines/status endpoint (which blanks the UI). | ||
| LOG.warn( | ||
| "Kubernetes namespace {} is reachable but a pod/job could not be deserialized " | ||
| + "(client-java models likely lag the cluster's Kubernetes version): {}", | ||
| namespace, | ||
| e.getMessage()); | ||
| return buildHealthyStatus(getKubernetesVersion()) | ||
| .withReason(String.format(K8S_AVAILABLE_FORMAT, namespace, serviceAccount)); | ||
|
Comment on lines
+1013
to
+1031
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| } | ||
|
|
||
|
|
@@ -1128,6 +1155,16 @@ public Map<String, String> getLastIngestionLogs( | |
| } catch (ApiException e) { | ||
| LOG.error("Failed to get logs for pipeline {}: {}", pipelineName, e.getResponseBody()); | ||
| return Map.of("logs", FAILED_LOGS_MESSAGE + e.getMessage()); | ||
| } catch (IllegalArgumentException e) { | ||
| // client-java could not deserialize a pod returned by a newer Kubernetes | ||
| // than its models (see getServiceStatusInternal). Return a graceful message | ||
| // instead of propagating the error. | ||
| LOG.warn( | ||
| "Could not deserialize pod while fetching logs for pipeline {} (client-java lags " | ||
| + "the cluster's Kubernetes version): {}", | ||
| pipelineName, | ||
| e.getMessage()); | ||
| return Map.of("logs", FAILED_LOGS_MESSAGE + e.getMessage()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+1158
to
+1167
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
|
|
||
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.