Skip to content
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

Increase the maximum buffer size of the Kubernetes agent bootstrapRunner's stream readers #1055

Merged
merged 3 commits into from
Jan 13, 2025

Conversation

kevjt
Copy link
Contributor

@kevjt kevjt commented Jan 9, 2025

Background

Part of OctopusDeploy/Issues#9202

This PR addresses an issue where long text cannot be written to Octopus output variables when deploying via the Octopus Kubernetes Agent. The problem is caused by the default max buffer size of the bufio.Scanner in the bootstrap runner, which is limited to 64 KiB. This causes large outputs (e.g., from Terraform plans) to not be written to service message logs.

[sc-100196]

Results

  • The max buffer size for the Scanner has been increased to allow larger outputs, with the maximum size set to 10 MiB, which aligns with Kubernetes' default max container log size.

  • Ensures the reader is closed after scanning (regardless of error) to properly close the pipe, allowing the command process to terminate without hanging while waiting for the pipe to clear before writing stdout.

@kevjt kevjt marked this pull request as ready for review January 9, 2025 03:05
@kevjt kevjt requested a review from a team as a code owner January 9, 2025 03:05
for scanner.Scan() {
Write(stream, scanner.Text(), counter, gcm)
}

err := readCloser.Close()
if scanner.Err() != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QQ: is it worth printing the scanner error before closing the reader?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scanning error will be available even after closing the reader, because the error is associated with the scanner object itself, not the reader.

Copy link
Contributor

@flin-8 flin-8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM + minor comments

@kevjt kevjt merged commit 930b260 into main Jan 13, 2025
51 of 52 checks passed
@kevjt kevjt deleted the lm/add-buffer branch January 13, 2025 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants