gnodet-bot commented on code in PR #29919:
URL: https://github.com/apache/camel/pull/29919#discussion_r4573211563
##########
dsl/camel-jbang/camel-jbang-plugin-kubernetes/src/main/java/org/apache/camel/dsl/jbang/core/commands/kubernetes/KubernetesPodLogs.java:
##########
@@ -111,52 +111,83 @@ private boolean watchLogs() {
podFound = true;
var terminated = isPodTerminated(podRes);
- if (!terminated) {
-
- LogWatch logs;
- if (tail < 0) {
- if (!ObjectHelper.isEmpty(container)) {
- logs = podRes.inContainer(container).watchLog();
- } else {
- logs = podRes.watchLog();
- }
+ if (terminated) {
+ // a pod that has finished, such as the pod of a CronJob: its log
once, as kubectl logs does
+ printLog(podRes);
+ return false;
+ }
+
+ LogWatch logs;
+ if (tail < 0) {
+ if (!ObjectHelper.isEmpty(container)) {
+ logs = podRes.inContainer(container).watchLog();
} else {
- if (!ObjectHelper.isEmpty(container)) {
- logs =
podRes.inContainer(container).tailingLines(tail).watchLog();
- } else {
- logs = podRes.tailingLines(tail).watchLog();
- }
+ logs = podRes.watchLog();
}
+ } else {
+ if (!ObjectHelper.isEmpty(container)) {
+ logs =
podRes.inContainer(container).tailingLines(tail).watchLog();
+ } else {
+ logs = podRes.tailingLines(tail).watchLog();
+ }
+ }
- try (logs; BufferedReader reader = new BufferedReader(new
InputStreamReader(logs.getOutput()))) {
- String line;
- while ((line = reader.readLine()) != null) {
- printer().println(line);
- if (maxMessageCount > 0 && ++messageCount >
maxMessageCount) {
- return false;
- }
- retryCount = 0;
+ try (logs; BufferedReader reader = new BufferedReader(new
InputStreamReader(logs.getOutput()))) {
+ String line;
+ while ((line = reader.readLine()) != null) {
+ printer().println(line);
+ if (maxMessageCount > 0 && ++messageCount > maxMessageCount) {
+ return false;
}
- readFailed = false;
- } catch (IOException e) {
- printer().printErr("Failed to read pod logs - " +
e.getMessage());
- readFailed = true;
+ retryCount = 0;
}
-
- terminated = isPodTerminated(podRes);
+ readFailed = false;
+ } catch (IOException e) {
+ printer().printErr("Failed to read pod logs - " + e.getMessage());
+ readFailed = true;
}
+ terminated = isPodTerminated(podRes);
+
return !terminated;
}
+ private void printLog(PodResource podRes) {
+ String log;
+ try {
+ if (!ObjectHelper.isEmpty(container)) {
+ log = tail < 0
+ ? podRes.inContainer(container).getLog() :
podRes.inContainer(container).tailingLines(tail).getLog();
+ } else {
+ log = tail < 0 ? podRes.getLog() :
podRes.tailingLines(tail).getLog();
+ }
+ } catch (RuntimeException e) {
+ printer().printErr("Failed to read pod logs - " + e.getMessage());
+ readFailed = true;
Review Comment:
💡 `printLog()` sets `readFailed = true` on exception but never resets it on
success. If a prior iteration threw an `IOException` and the pod subsequently
terminates cleanly, this method succeeds but the stale `readFailed = true`
causes a spurious `FAILURE` exit code.
The `watchLog` path in `watchLogs()` correctly sets `readFailed = false` on
success (line 196 of this diff). Add the same here — after the `if (log !=
null)` block, before the closing `}` of `printLog()`:
```java
readFailed = false;
}
```
##########
dsl/camel-jbang/camel-jbang-plugin-kubernetes/src/main/java/org/apache/camel/dsl/jbang/core/commands/kubernetes/KubernetesPodLogs.java:
##########
@@ -111,52 +111,83 @@ private boolean watchLogs() {
podFound = true;
var terminated = isPodTerminated(podRes);
- if (!terminated) {
-
- LogWatch logs;
- if (tail < 0) {
- if (!ObjectHelper.isEmpty(container)) {
- logs = podRes.inContainer(container).watchLog();
- } else {
- logs = podRes.watchLog();
- }
+ if (terminated) {
+ // a pod that has finished, such as the pod of a CronJob: its log
once, as kubectl logs does
+ printLog(podRes);
+ return false;
+ }
+
+ LogWatch logs;
+ if (tail < 0) {
+ if (!ObjectHelper.isEmpty(container)) {
+ logs = podRes.inContainer(container).watchLog();
} else {
- if (!ObjectHelper.isEmpty(container)) {
- logs =
podRes.inContainer(container).tailingLines(tail).watchLog();
- } else {
- logs = podRes.tailingLines(tail).watchLog();
- }
+ logs = podRes.watchLog();
}
+ } else {
+ if (!ObjectHelper.isEmpty(container)) {
+ logs =
podRes.inContainer(container).tailingLines(tail).watchLog();
+ } else {
+ logs = podRes.tailingLines(tail).watchLog();
+ }
+ }
- try (logs; BufferedReader reader = new BufferedReader(new
InputStreamReader(logs.getOutput()))) {
- String line;
- while ((line = reader.readLine()) != null) {
- printer().println(line);
- if (maxMessageCount > 0 && ++messageCount >
maxMessageCount) {
- return false;
- }
- retryCount = 0;
+ try (logs; BufferedReader reader = new BufferedReader(new
InputStreamReader(logs.getOutput()))) {
+ String line;
+ while ((line = reader.readLine()) != null) {
+ printer().println(line);
+ if (maxMessageCount > 0 && ++messageCount > maxMessageCount) {
+ return false;
}
- readFailed = false;
- } catch (IOException e) {
- printer().printErr("Failed to read pod logs - " +
e.getMessage());
- readFailed = true;
+ retryCount = 0;
}
-
- terminated = isPodTerminated(podRes);
+ readFailed = false;
+ } catch (IOException e) {
+ printer().printErr("Failed to read pod logs - " + e.getMessage());
+ readFailed = true;
}
+ terminated = isPodTerminated(podRes);
+
return !terminated;
}
+ private void printLog(PodResource podRes) {
+ String log;
+ try {
+ if (!ObjectHelper.isEmpty(container)) {
+ log = tail < 0
+ ? podRes.inContainer(container).getLog() :
podRes.inContainer(container).tailingLines(tail).getLog();
+ } else {
+ log = tail < 0 ? podRes.getLog() :
podRes.tailingLines(tail).getLog();
+ }
+ } catch (RuntimeException e) {
+ printer().printErr("Failed to read pod logs - " + e.getMessage());
+ readFailed = true;
+ return;
+ }
+ if (log != null) {
+ for (String line : log.split("\\R")) {
+ printer().println(line);
+ if (maxMessageCount > 0 && ++messageCount > maxMessageCount) {
+ return;
+ }
+ }
+ }
+ }
+
+ /**
+ * Whether the pod has finished: the phase Succeeded or Failed
(CAMEL-26934: it checked for Terminated, which is no
+ * pod phase, so the log of a finished pod was printed again and again).
+ */
Review Comment:
Nit: the `CAMEL-26934:` note reads as an internal tracker reference rather
than API docs. Consider keeping the Javadoc focused on the contract and moving
the fix note to a code comment or commit message.
```suggestion
* Whether the pod has finished: the phase {@code Succeeded} or {@code
Failed}.
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
