Skip to content

Conversation

@apostasie
Copy link
Contributor

  • commit 1 has a few assorted minor fixes and changes (see commit message for details)
  • commit 2 does make use of recently introduced env Whitelisting to tame Environ display

timeout := gc.Timeout
if timeout == 0 {
timeout = defaultTimeout
if gc.Timeout == 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix 0s being displayed (instead of displaying defaultTimeout) when no explicit timeout has been passed.

Stderr string
ExitCode int
Signal os.Signal
Duration time.Duration
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Store command execution time.


if len(chunks) > maxLines {
chunks = append(chunks[0:maxLines], "...")
abbreviator := "..."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of stripping anything after line maxLines, we now keep maxLines/2 from the start and from the end.


duration := result.Duration.String()
if result.Duration < time.Second {
duration = "<1s"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do not want to see 123.456789ms

[]any{"Environ", strings.Join(result.Environ, "\n")},
[]any{envDecorator, strings.Join(result.Environ, "\n")},
[]any{timeoutDecorator, duration + " (limit: " + gc.cmd.Timeout.String() + ")"},
[]any{cwdDecorator, gc.cmd.WorkingDir},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Show cwd and better duration info.

"NETCONFPATH",
"NERDCTL_EXPERIMENTAL",
"NERDCTL_HOST_GATEWAY_IP",
ret.WithWhitelist([]string{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use whitelist to tame os.Environ on the CI.

@apostasie apostasie force-pushed the ci-fix-debug branch 2 times, most recently from acaa710 to ace5258 Compare April 16, 2025 05:01
@apostasie apostasie marked this pull request as ready for review April 16, 2025 05:50
@apostasie
Copy link
Contributor Author

Failures are unrelated (#3556 and some old logs issue in legacy tests)

- fix timeout showing 0s instead of default when no explicit timeout is provided
- add actual execution time
- fix long logs (stdout, stderr) that were showing only the beginning to also show the end
- fix formatting issue for "..."
- marginally better output for command contextual information

Signed-off-by: apostasie <[email protected]>
@apostasie
Copy link
Contributor Author

Rebased to get another CI run with #4104 fix.

@apostasie
Copy link
Contributor Author

Latest failure is #3510, TestIPFSCompNoBuild getting content digest not found.

My hunch is that it is calling containerd Convert under the hood, which we know is buggy.
Will investigate.

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda added this to the v2.0.5 milestone Apr 18, 2025
@AkihiroSuda AkihiroSuda added the area/ci e.g., CI failure label Apr 18, 2025
@AkihiroSuda AkihiroSuda merged commit df5fc78 into containerd:main Apr 18, 2025
52 of 55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci e.g., CI failure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants