diff --git a/go.mod b/go.mod index 2dc739a06e6..3323e01ac92 100644 --- a/go.mod +++ b/go.mod @@ -201,7 +201,7 @@ require ( github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399 // indirect github.com/ulikunitz/xz v0.5.12 // indirect github.com/uptrace/opentelemetry-go-extra/otelutil v0.3.2 // indirect - github.com/vbatts/tar-split v0.11.7 // indirect + github.com/vbatts/tar-split v0.12.2 // indirect github.com/vbauerster/mpb/v8 v8.9.1 // indirect github.com/vishvananda/netns v0.0.4 // indirect github.com/x448/float16 v0.8.4 // indirect diff --git a/go.sum b/go.sum index 8cc60da01aa..b3f00b312c7 100644 --- a/go.sum +++ b/go.sum @@ -1272,8 +1272,8 @@ github.com/uptrace/opentelemetry-go-extra/otelutil v0.3.2 h1:3/aHKUq7qaFMWxyQV0W github.com/uptrace/opentelemetry-go-extra/otelutil v0.3.2/go.mod h1:Zit4b8AQXaXvA68+nzmbyDzqiyFRISyw1JiD5JqUBjw= github.com/urfave/cli/v2 v2.27.5 h1:WoHEJLdsXr6dDWoJgMq/CboDmyY/8HMMH1fTECbih+w= github.com/urfave/cli/v2 v2.27.5/go.mod h1:3Sevf16NykTbInEnD0yKkjDAeZDS0A6bzhBH5hrMvTQ= -github.com/vbatts/tar-split v0.11.7 h1:ixZ93pO/GmvaZw4Vq9OwmfZK/kc2zKdPfu0B+gYqs3U= -github.com/vbatts/tar-split v0.11.7/go.mod h1:eF6B6i6ftWQcDqEn3/iGFRFRo8cBIMSJVOpnNdfTMFA= +github.com/vbatts/tar-split v0.12.2 h1:w/Y6tjxpeiFMR47yzZPlPj/FcPLpXbTUi/9H7d3CPa4= +github.com/vbatts/tar-split v0.12.2/go.mod h1:eF6B6i6ftWQcDqEn3/iGFRFRo8cBIMSJVOpnNdfTMFA= github.com/vbauerster/mpb/v8 v8.9.1 h1:LH5R3lXPfE2e3lIGxN7WNWv3Hl5nWO6LRi2B0L0ERHw= github.com/vbauerster/mpb/v8 v8.9.1/go.mod h1:4XMvznPh8nfe2NpnDo1QTPvW9MVkUhbG90mPWvmOzcQ= github.com/vishvananda/netlink v1.3.0 h1:X7l42GfcV4S6E4vHTsw48qbrV+9PVojNfIhZcwQdrZk= diff --git a/internal/version/version.go b/internal/version/version.go index fb9d16e9720..1ae61ccc983 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -21,7 +21,7 @@ import ( ) // Version is the version of the build. -const Version = "1.32.10" +const Version = "1.32.11" // ReleaseMinorVersions are the currently supported minor versions. var ReleaseMinorVersions = []string{"1.31", "1.30", "1.29"} diff --git a/server/sandbox_network.go b/server/sandbox_network.go index 26675e4320a..af75693ac47 100644 --- a/server/sandbox_network.go +++ b/server/sandbox_network.go @@ -197,7 +197,16 @@ func (s *Server) networkStop(ctx context.Context, sb *sandbox.Sandbox) error { // to prevent IP leaks, but we'll mark the network as stopped regardless of the outcome. netnsValid := true - if podNetwork.NetNS != "" { + if podNetwork.NetNS == "" { + // Network namespace path is unexpectedly empty. This can happen when: + // 1) infra container process died + // 2) namespace not properly initialized + // 3) namespace was already cleaned up + log.Warnf(ctx, "Network namespace path is empty for pod sandbox %s(%s), attempting CNI teardown with cached info", + sb.Name(), sb.ID()) + + netnsValid = false + } else { if _, statErr := os.Stat(podNetwork.NetNS); statErr != nil { // Network namespace file doesn't exist, but we should still attempt CNI teardown log.Debugf(ctx, "Network namespace file %s does not exist for pod sandbox %s(%s), attempting CNI teardown with cached info", @@ -216,6 +225,16 @@ func (s *Server) networkStop(ctx context.Context, sb *sandbox.Sandbox) error { // Always attempt CNI teardown to prevent IP leaks, even if netns is invalid. if err := s.config.CNIPlugin().TearDownPodWithContext(stopCtx, podNetwork); err != nil { + if !netnsValid { + // This is expected when the network namespace is missing/invalid. + log.Debugf(ctx, "CNI teardown failed due to missing/invalid network namespace for pod sandbox %s(%s): %v", sb.Name(), sb.ID(), err) + + // Clean up CNI result files even when NetNS is invalid. + s.cleanupCNIResultFiles(ctx, sb.ID()) + + return sb.SetNetworkStopped(ctx, true) + } + log.Warnf(ctx, "Failed to destroy network for pod sandbox %s(%s): %v", sb.Name(), sb.ID(), err) // If the network namespace exists but CNI teardown failed, try to clean it up. diff --git a/server/sandbox_stop_test.go b/server/sandbox_stop_test.go index f7267e4b905..06e25fc7507 100644 --- a/server/sandbox_stop_test.go +++ b/server/sandbox_stop_test.go @@ -5,7 +5,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "go.uber.org/mock/gomock" types "k8s.io/cri-api/pkg/apis/runtime/v1" ) @@ -25,7 +24,7 @@ var _ = t.Describe("PodSandboxStatus", func() { // Given addContainerAndSandbox() testSandbox.SetStopped(ctx, false) - Expect(testSandbox.SetNetworkStopped(ctx, false)).To(Succeed()) + Expect(testSandbox.SetNetworkStopped(ctx, true)).To(Succeed()) // When _, err := sut.StopPodSandbox(context.Background(), @@ -45,22 +44,6 @@ var _ = t.Describe("PodSandboxStatus", func() { Expect(err).ToNot(HaveOccurred()) }) - It("should fail when container is not stopped", func() { - // Given - addContainerAndSandbox() - gomock.InOrder( - cniPluginMock.EXPECT().GetDefaultNetworkName().Return(""), - cniPluginMock.EXPECT().TearDownPodWithContext(gomock.Any(), gomock.Any()).Return(t.TestError), - ) - - // When - _, err := sut.StopPodSandbox(context.Background(), - &types.StopPodSandboxRequest{PodSandboxId: testSandbox.ID()}) - - // Then - Expect(err).To(HaveOccurred()) - }) - It("should fail with empty sandbox ID", func() { // Given // When diff --git a/test/network.bats b/test/network.bats index 6c4348e0f47..af6def8b08c 100644 --- a/test/network.bats +++ b/test/network.bats @@ -253,3 +253,31 @@ function check_networking() { # Verify the pod is gone. run ! crictl inspectp "$pod_id" } + +@test "pod deletion succeeds when NetNS path is missing" { + start_crio + + pod_id=$(crictl runp "$TESTDATA"/sandbox_config.json) + ctr_id=$(crictl create "$pod_id" "$TESTDATA"/container_redis.json "$TESTDATA"/sandbox_config.json) + crictl start "$ctr_id" + + crictl ps --id "$ctr_id" | grep Running + + netns_path=$(crictl inspectp "$pod_id" | jq -r '.status.network.namespace_path') + + # Simulating the scenario where infra container dies and NetNS becomes invalid + # by removing the network namespace file. + if [[ -n "$netns_path" && -f "$netns_path" ]]; then + rm -f "$netns_path" + fi + + crictl stop "$ctr_id" + crictl rm "$ctr_id" + + crictl stopp "$pod_id" + + # Pod deletion should succeed even with missing NetNS + crictl rmp "$pod_id" + + run ! crictl inspectp "$pod_id" +} diff --git a/vendor/github.com/vbatts/tar-split/archive/tar/common.go b/vendor/github.com/vbatts/tar-split/archive/tar/common.go index dee9e47e4ae..e687a08c966 100644 --- a/vendor/github.com/vbatts/tar-split/archive/tar/common.go +++ b/vendor/github.com/vbatts/tar-split/archive/tar/common.go @@ -34,6 +34,7 @@ var ( errMissData = errors.New("archive/tar: sparse file references non-existent data") errUnrefData = errors.New("archive/tar: sparse file contains unreferenced data") errWriteHole = errors.New("archive/tar: write non-NUL byte in sparse hole") + errSparseTooLong = errors.New("archive/tar: sparse map too long") ) type headerError []string diff --git a/vendor/github.com/vbatts/tar-split/archive/tar/reader.go b/vendor/github.com/vbatts/tar-split/archive/tar/reader.go index 248a7ccb15a..a645c41605f 100644 --- a/vendor/github.com/vbatts/tar-split/archive/tar/reader.go +++ b/vendor/github.com/vbatts/tar-split/archive/tar/reader.go @@ -581,12 +581,17 @@ func readGNUSparseMap1x0(r io.Reader) (sparseDatas, error) { cntNewline int64 buf bytes.Buffer blk block + totalSize int ) // feedTokens copies data in blocks from r into buf until there are // at least cnt newlines in buf. It will not read more blocks than needed. feedTokens := func(n int64) error { for cntNewline < n { + totalSize += len(blk) + if totalSize > maxSpecialFileSize { + return errSparseTooLong + } if _, err := mustReadFull(r, blk[:]); err != nil { return err } @@ -619,8 +624,8 @@ func readGNUSparseMap1x0(r io.Reader) (sparseDatas, error) { } // Parse for all member entries. - // numEntries is trusted after this since a potential attacker must have - // committed resources proportional to what this library used. + // numEntries is trusted after this since feedTokens limits the number of + // tokens based on maxSpecialFileSize. if err := feedTokens(2 * numEntries); err != nil { return nil, err } diff --git a/vendor/github.com/vbatts/tar-split/archive/tar/writer.go b/vendor/github.com/vbatts/tar-split/archive/tar/writer.go index e80498d03e3..893eac00ae2 100644 --- a/vendor/github.com/vbatts/tar-split/archive/tar/writer.go +++ b/vendor/github.com/vbatts/tar-split/archive/tar/writer.go @@ -199,6 +199,9 @@ func (tw *Writer) writePAXHeader(hdr *Header, paxHdrs map[string]string) error { flag = TypeXHeader } data := buf.String() + if len(data) > maxSpecialFileSize { + return ErrFieldTooLong + } if err := tw.writeRawFile(name, data, flag, FormatPAX); err != nil || isGlobal { return err // Global headers return here } diff --git a/vendor/modules.txt b/vendor/modules.txt index c945d9cc2b0..b91da6ef68e 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1028,7 +1028,7 @@ github.com/uptrace/opentelemetry-go-extra/otelutil # github.com/urfave/cli/v2 v2.27.5 ## explicit; go 1.18 github.com/urfave/cli/v2 -# github.com/vbatts/tar-split v0.11.7 +# github.com/vbatts/tar-split v0.12.2 ## explicit; go 1.17 github.com/vbatts/tar-split/archive/tar github.com/vbatts/tar-split/tar/asm