fix: suppress spurious hostsstore NotFound warning on container removal - #5098
Open
pujitha24 wants to merge 1 commit into
Open
fix: suppress spurious hostsstore NotFound warning on container removal#5098pujitha24 wants to merge 1 commit into
pujitha24 wants to merge 1 commit into
Conversation
Motivation: On Windows, `nerdctl run --rm ...` (and `nerdctl rm`) logs a confusing warning after every container removal: level=warning msg="failed to remove hosts file for container \"<id>\"" error="hosts-store error\nnot found\nGetFileAttributesEx ...\\etchosts\\default\\<id>: The system cannot find the file specified." Fixes containerd#4678 Approach: pkg/containerutil/container_network_manager_windows.go never calls hostsstore.Acquire/AllocHostsFile (unlike the Linux and generic network manager code paths), so no hosts-store directory is ever created for a Windows container. As a result, hostsstore.Delete(id) in RemoveContainer (pkg/cmd/container/remove.go) always fails with store.ErrNotFound on Windows, and that failure was unconditionally logged as a warning. This is purely a noisy/confusing log message, not a functional bug: hostsstore.Delete's error was already only logged (soft failure), so container removal succeeds identically before and after this change. The fix mirrors the existing nameStore.Release handling a few lines above in the same function (which already ignores store.ErrNotFound to tolerate double-release with --rm): hostsstore.Delete's NotFound error is now ignored for the same reason - the hosts-store entry may simply never have been allocated for this container/platform/network mode. Validation: - go build ./... passes. - go test ./pkg/... passes (all packages ok, including pkg/dnsutil/hostsstore and pkg/store). - gofmt -l pkg/cmd/container/remove.go reports no issues. - pkg/cmd/container has no unit tests for RemoveContainer (it requires a live containerd.Container); this matches the existing test coverage pattern for that function (integration-only, via cmd/nerdctl/container/container_remove_test.go). Signed-off-by: Pujitha Paladugu <venkat@dema.shop> Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Suppresses a spurious and confusing warning emitted during container removal when the hosts-store entry was never created (notably on Windows), aligning error handling with existing nameStore.Release behavior and resolving the noise reported in #4678.
Changes:
- Ignore
store.ErrNotFoundfromhostsstore.Delete(id)duringRemoveContainercleanup. - Add explanatory comments documenting why
NotFoundis expected on some platforms/network modes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
On Windows,
nerdctl run --rm ...(andnerdctl rm) logs a confusingwarning after every container removal:
level=warning msg="failed to remove hosts file for container """
error="hosts-store error\nnot found\nGetFileAttributesEx
...\etchosts\default\: The system cannot find the file specified."
Fixes #4678
Approach:
pkg/containerutil/container_network_manager_windows.go never calls
hostsstore.Acquire/AllocHostsFile (unlike the Linux and generic network
manager code paths), so no hosts-store directory is ever created for a
Windows container. As a result, hostsstore.Delete(id) in
RemoveContainer (pkg/cmd/container/remove.go) always fails with
store.ErrNotFound on Windows, and that failure was unconditionally
logged as a warning.
This is purely a noisy/confusing log message, not a functional bug:
hostsstore.Delete's error was already only logged (soft failure), so
container removal succeeds identically before and after this change.
The fix mirrors the existing nameStore.Release handling a few lines
above in the same function (which already ignores store.ErrNotFound
to tolerate double-release with --rm): hostsstore.Delete's NotFound
error is now ignored for the same reason - the hosts-store entry may
simply never have been allocated for this container/platform/network
mode.
Validation:
pkg/dnsutil/hostsstore and pkg/store).
a live containerd.Container); this matches the existing test
coverage pattern for that function (integration-only, via
cmd/nerdctl/container/container_remove_test.go).
Signed-off-by: Pujitha Paladugu venkat@dema.shop
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com