Skip to content

fix(unikontainers): warn on vAccel misconfiguration#699

Merged
cmainas merged 2 commits into
urunc-dev:main-pr699from
Chennamma-Hotkar:fix/issue-vaccel-silent-error
Jun 2, 2026
Merged

fix(unikontainers): warn on vAccel misconfiguration#699
cmainas merged 2 commits into
urunc-dev:main-pr699from
Chennamma-Hotkar:fix/issue-vaccel-silent-error

Conversation

@Chennamma-Hotkar

@Chennamma-Hotkar Chennamma-Hotkar commented May 18, 2026

Copy link
Copy Markdown
Contributor

Description

When calling resolveVAccelConfig() in pkg/unikontainers/unikontainers.go, we only print the error at DEBUG level. The function returns the same error type in 2 different cases:
1) when vAccel is disabled and
2) when a bad configuration is detected (i.e. missing or bad RPCAddress).
Therefore the error is always ignored and the container launched in with no vAccel but with no warning of the situation.

The fix adds a sentinel error ErrVAccelDisabled to treat the two cases differently. When vAccel is not configured, we expect an error and log it at Debug. When there is a genuine misconfiguration, we Warn so operators can see the
issue without having to enable debug log level. Added unit tests for all four paths in resolveVAccelConfig.

Files changed:

  • pkg/unikontainers/vaccel.go — added ErrVAccelDisabled sentinel error
  • pkg/unikontainers/unikontainers.go — log Warn on misconfiguration
  • pkg/unikontainers/vaccel_test.go — 4 new unit tests

Related issues

How was this tested?

All existing unit tests pass (make unittest). Build passes (make). Four new unit tests added covering disabled, missing address, malformed address and valid address cases.

LLM usage

N/A

Checklist

  • I have read the contribution guide.
  • The linter passes locally (make lint).
  • The e2e tests of at least one tool pass locally (make test_ctr, make test_nerdctl, make test_docker, make test_crictl).
  • If LLMs were used: I have read the llm policy.

@netlify

netlify Bot commented May 18, 2026

Copy link
Copy Markdown

Deploy Preview for urunc canceled.

Name Link
🔨 Latest commit a1a0bb5
🔍 Latest deploy log https://app.netlify.com/projects/urunc/deploys/6a0b3ac4789ec20008682771

@netlify

netlify Bot commented May 18, 2026

Copy link
Copy Markdown

Deploy Preview for urunc canceled.

Name Link
🔨 Latest commit cfd4cdb
🔍 Latest deploy log https://app.netlify.com/projects/urunc/deploys/6a1eb48896f92e00089550ed

@cmainas

cmainas commented May 19, 2026

Copy link
Copy Markdown
Contributor

Hello @Chennamma-Hotkar , please read the contribution guide

@cmainas cmainas added do-not-merge invalid This doesn't seem right and removed invalid This doesn't seem right labels May 19, 2026
@Chennamma-Hotkar

Copy link
Copy Markdown
Contributor Author

Hi @cmainas, I added the contribution guide template to the PR description including the following things. A description of the files changed, how it was tested, LLM disclosure and a checklist. Please let me know if there are any other things that need to be fixed.

@cmainas

cmainas commented May 20, 2026

Copy link
Copy Markdown
Contributor

Hello @Chennamma-Hotkar , you will also need to add yourself on https://github.com/urunc-dev/urunc/blob/main/.github/contributors.yaml

@Chennamma-Hotkar

Chennamma-Hotkar commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

Hi @cmainas, I have included my name in .github/contributors.yaml.

@Chennamma-Hotkar Chennamma-Hotkar force-pushed the fix/issue-vaccel-silent-error branch 3 times, most recently from bd5f83b to 1e044a8 Compare May 20, 2026 14:07
@cmainas

cmainas commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Hello @Chennamma-Hotkar ,

can you rebase over main and remove the merge commit?

Introduce ErrVAccelDisabled to distinguish missing vAccel config
from actual misconfiguration. Log Warn instead of Debug on error.

Fixes: urunc-dev#698
Signed-off-by: Chennamma-Hotkar <channuhotkar@gmail.com>
Signed-off-by: Chennamma-Hotkar <channuhotkar@gmail.com>
@Chennamma-Hotkar Chennamma-Hotkar force-pushed the fix/issue-vaccel-silent-error branch from 953b7a2 to cfd4cdb Compare June 2, 2026 10:46
@Chennamma-Hotkar

Copy link
Copy Markdown
Contributor Author

Hi @cmainas, I have rebased over main and removed the merge commit.

@urunc-bot urunc-bot Bot changed the base branch from main to main-pr699 June 2, 2026 11:38

@cmainas cmainas 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.

Thank you @Chennamma-Hotkar for the fix.

@cmainas cmainas merged commit d3f801a into urunc-dev:main-pr699 Jun 2, 2026
34 checks passed
github-actions Bot pushed a commit that referenced this pull request Jun 2, 2026
Introduce ErrVAccelDisabled to distinguish missing vAccel config
from actual misconfiguration. Log Warn instead of Debug on error.

PR: #699
Fixes: #698
Signed-off-by: Chennamma-Hotkar <channuhotkar@gmail.com>
Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk>
Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>
github-actions Bot pushed a commit that referenced this pull request Jun 2, 2026
PR: #699
Signed-off-by: Chennamma-Hotkar <channuhotkar@gmail.com>
Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk>
Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>
urunc-bot Bot pushed a commit that referenced this pull request Jun 2, 2026
Introduce ErrVAccelDisabled to distinguish missing vAccel config
from actual misconfiguration. Log Warn instead of Debug on error.

PR: #699
Fixes: #698
Signed-off-by: Chennamma-Hotkar <channuhotkar@gmail.com>
Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk>
Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>
urunc-bot Bot pushed a commit that referenced this pull request Jun 2, 2026
PR: #699
Signed-off-by: Chennamma-Hotkar <channuhotkar@gmail.com>
Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk>
Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: vAccel misconfiguration errors silently swallowed in Exec()

2 participants