Skip to content

kernel-test: normalize traffic monitor log names before uploading - #504

Closed
vineetgarc wants to merge 1 commit into
masterfrom
tmon-normalize-on-timeout
Closed

kernel-test: normalize traffic monitor log names before uploading#504
vineetgarc wants to merge 1 commit into
masterfrom
tmon-normalize-on-timeout

Conversation

@vineetgarc

Copy link
Copy Markdown

The traffic monitor names its logs after the test and subtest, so they can contain characters that actions/upload-artifact rejects. A subtest called "INET4: bpf_timestamping" makes the upload fail as below (see [1])

Error: The path for one of the files in artifact is not valid:
/packets-125-15-net_timestamping__INET4:_bpf_timestamping-net_timestamping_ns.log.
Contains the following character: Colon :

run-vmtest already renames these, but only after the VM exits, so a step timeout or a dead VM kills it before that point. The upload step runs with if: always() and then trips over the names that were left behind.

Rename them in a step of its own, also guarded by if: always(), so it happens whatever the outcome of the test step. It is a no-op when the directory is absent, which is the case for every test that does not enable the traffic monitor, and re-running it is harmless.

Link: https://github.com/kernel-patches/vmtest/actions/runs/30710914503/job/9139901138 [1]

The traffic monitor names its logs after the test and subtest, so they can
contain characters that actions/upload-artifact rejects. A subtest called
"INET4: bpf_timestamping" makes the upload fail as below (see [1])

  Error: The path for one of the files in artifact is not valid:
  /packets-125-15-net_timestamping__INET4:_bpf_timestamping-net_timestamping_ns.log.
  Contains the following character:  Colon :

run-vmtest already renames these, but only after the VM exits, so a step
timeout or a dead VM kills it before that point. The upload step runs with
if: always() and then trips over the names that were left behind.

Rename them in a step of its own, also guarded by if: always(), so it happens
whatever the outcome of the test step. It is a no-op when the directory is
absent, which is the case for every test that does not enable the traffic
monitor, and re-running it is harmless.

Link: https://github.com/kernel-patches/vmtest/actions/runs/30710914503/job/9139901138 [1]
Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
@vineetgarc
vineetgarc requested a review from theihor August 6, 2026 21:23
@vineetgarc vineetgarc mentioned this pull request Aug 6, 2026

@theihor theihor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This essentially copy-pastes a script from libbpf/ci/run-vmtest, which is an unnecessary duplication.

always() is the right thing to do, but I think the right spot for it is in run-vmtest/action.yml as a last step.

So basically we need factor out this normalization from run.sh into action.yml.

@vineetgarc

Copy link
Copy Markdown
Author

This is superseded by libbpf/ci#231

@vineetgarc vineetgarc closed this Aug 11, 2026
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.

2 participants