Skip to content

fix(report,cluster): ensure the process continues when previous logs are unavailable#8992

Merged
gbartolini merged 1 commit intomainfrom
dev/8985
Oct 30, 2025
Merged

fix(report,cluster): ensure the process continues when previous logs are unavailable#8992
gbartolini merged 1 commit intomainfrom
dev/8985

Conversation

@armru
Copy link
Member

@armru armru commented Oct 29, 2025

Fixed two bugs in the pod logs writer that caused log collection to fail:

  1. When Previous option is enabled, sendLogsToWriter was attempting to fetch logs with Previous=true twice - once for previous logs and again for current logs. This caused failures when containers had never been restarted (no previous terminated container exists). Now it correctly fetches previous logs first (with error handling), then fetches current logs with Previous=false.

  2. The Multiple method was incorrectly passing opts instead of containerOpts to sendLogsToWriter, causing the container name to not be set properly.

These fixes ensure that:

  • Missing previous logs are handled gracefully without stopping execution
  • Current logs are always collected even if previous logs are unavailable
  • Each container's logs are fetched with the correct container name

Fixes #8985

@armru armru requested a review from a team as a code owner October 29, 2025 10:30
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Oct 29, 2025
@cnpg-bot cnpg-bot added backport-requested ◀️ This pull request should be backported to all supported releases release-1.25 release-1.26 release-1.27 labels Oct 29, 2025
@github-actions
Copy link
Contributor

❗ By default, the pull request is configured to backport to all release branches.

  • To stop backporting this pr, remove the label: backport-requested ◀️ or add the label 'do not backport'
  • To stop backporting this pr to a certain release branch, remove the specific branch label: release-x.y

@dosubot dosubot bot added bug 🐛 Something isn't working ok to merge 👌 This PR can be merged labels Oct 29, 2025
@armru armru changed the title fix(plugin): kubectl cnpg report cluster --logs fails when previous logs unavailable fix(report,cluster): ensure the process continues when previous logs are unavailable Oct 29, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Oct 29, 2025
…ogs unavailable

Fixed two bugs in the pod logs writer that caused log collection to fail:

1. When Previous option is enabled, sendLogsToWriter was attempting to
   fetch logs with Previous=true twice - once for previous logs and
   again for current logs. This caused failures when containers had
   never been restarted (no previous terminated container exists).
   Now it correctly fetches previous logs first (with error handling),
   then fetches current logs with Previous=false.

2. The Multiple method was incorrectly passing opts instead of
   containerOpts to sendLogsToWriter, causing the container name to
   not be set properly.

These fixes ensure that:
- Missing previous logs are handled gracefully without stopping execution
- Current logs are always collected even if previous logs are unavailable
- Each container's logs are fetched with the correct container name

Fixes #8985

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
@gbartolini gbartolini merged commit eef478d into main Oct 30, 2025
34 checks passed
@gbartolini gbartolini deleted the dev/8985 branch October 30, 2025 18:12
cnpg-bot pushed a commit that referenced this pull request Oct 30, 2025
…unavailable (#8992)

Fixes two issues in the pod logs writer that caused log collection to fail:

1. Incorrect handling of previous logs: when the `Previous` option was enabled,
   `sendLogsToWriter` attempted to fetch logs with `Previous=true` twice — once
   for the previous logs and again for the current logs.  This failed when
   containers had never been restarted (no previous terminated container).
   The logic now correctly fetches previous logs first (with proper error
   handling) and then retrieves current logs with `Previous=false`.

2. Incorrect container options: the `Multiple` method was passing `opts`
   instead of `containerOpts` to `sendLogsToWriter`, resulting in missing
   container names in the log collection process.

With these fixes:

- Missing previous logs no longer stop the process.
- Current logs are always collected, even if previous logs are unavailable.
- Each container’s logs are fetched with the correct container name.

Fixes #8985

Signed-off-by: Armando Ruocco armando.ruocco@enterprisedb.com
(cherry picked from commit eef478d)
cnpg-bot pushed a commit that referenced this pull request Oct 30, 2025
…unavailable (#8992)

Fixes two issues in the pod logs writer that caused log collection to fail:

1. Incorrect handling of previous logs: when the `Previous` option was enabled,
   `sendLogsToWriter` attempted to fetch logs with `Previous=true` twice — once
   for the previous logs and again for the current logs.  This failed when
   containers had never been restarted (no previous terminated container).
   The logic now correctly fetches previous logs first (with proper error
   handling) and then retrieves current logs with `Previous=false`.

2. Incorrect container options: the `Multiple` method was passing `opts`
   instead of `containerOpts` to `sendLogsToWriter`, resulting in missing
   container names in the log collection process.

With these fixes:

- Missing previous logs no longer stop the process.
- Current logs are always collected, even if previous logs are unavailable.
- Each container’s logs are fetched with the correct container name.

Fixes #8985

Signed-off-by: Armando Ruocco armando.ruocco@enterprisedb.com
(cherry picked from commit eef478d)
cnpg-bot pushed a commit that referenced this pull request Oct 30, 2025
…unavailable (#8992)

Fixes two issues in the pod logs writer that caused log collection to fail:

1. Incorrect handling of previous logs: when the `Previous` option was enabled,
   `sendLogsToWriter` attempted to fetch logs with `Previous=true` twice — once
   for the previous logs and again for the current logs.  This failed when
   containers had never been restarted (no previous terminated container).
   The logic now correctly fetches previous logs first (with proper error
   handling) and then retrieves current logs with `Previous=false`.

2. Incorrect container options: the `Multiple` method was passing `opts`
   instead of `containerOpts` to `sendLogsToWriter`, resulting in missing
   container names in the log collection process.

With these fixes:

- Missing previous logs no longer stop the process.
- Current logs are always collected, even if previous logs are unavailable.
- Each container’s logs are fetched with the correct container name.

Fixes #8985

Signed-off-by: Armando Ruocco armando.ruocco@enterprisedb.com
(cherry picked from commit eef478d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-requested ◀️ This pull request should be backported to all supported releases bug 🐛 Something isn't working lgtm This PR has been approved by a maintainer ok to merge 👌 This PR can be merged release-1.25 release-1.26 release-1.27 size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: kubectl cnpg report cluster --logs fails to gather

4 participants