Skip to content

[release/1.7] Handle empty DNSConfig differently than unspecified#10462

Merged
estesp merged 1 commit intocontainerd:release/1.7from
dims:automated-cherry-pick-of-#9730-upstream-release-1.7
Jul 16, 2024
Merged

[release/1.7] Handle empty DNSConfig differently than unspecified#10462
estesp merged 1 commit intocontainerd:release/1.7from
dims:automated-cherry-pick-of-#9730-upstream-release-1.7

Conversation

@dims
Copy link
Copy Markdown
Member

@dims dims commented Jul 15, 2024

If we find that DNSConfig is provided and empty (not nil), we should not replace it with the host's resolv.conf.

Also adds tests.

@dims
Copy link
Copy Markdown
Member Author

dims commented Jul 15, 2024 •
edited
Loading

There's a cherry pick label for 1.7.x but missing from release/1.7 branch... #9730

@dims dims changed the title CRI: An empty DNSConfig != unspecified [release/1.7] CRI: An empty DNSConfig != unspecified Jul 15, 2024
@samuelkarp samuelkarp added the area/cri Container Runtime Interface (CRI) label Jul 15, 2024
@dims dims force-pushed the automated-cherry-pick-of-#9730-upstream-release-1.7 branch from 66481c3 to 6150f1f Compare July 16, 2024 14:11
@estesp
Copy link
Copy Markdown
Member

estesp commented Jul 16, 2024

This failure looks relevant:

=== Failed
=== FAIL: pkg/cri/server TestSetupSandboxFiles/should_create_empty_/etc/resolv.conf_if_DNSOptions_is_empty (0.00s)
    sandbox_run_linux_test.go:676: 
        	Error Trace:	/home/runner/work/containerd/containerd/pkg/cri/server/sandbox_run_linux_test.go:676
        	Error:      	Not equal: 
        	            	expected: testing.CalledDetail{Name:"WriteFile", Arguments:[]interface {}{"/test/root/sandboxes/test-id/resolv.conf", []uint8{}, 0x1a4}}
        	            	actual  : testing.CalledDetail{Name:"CopyFile", Arguments:[]interface {}{"/etc/resolv.conf", "/test/root/sandboxes/test-id/resolv.conf", 0x1a4}}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,7 +1,6 @@
        	            	 (testing.CalledDetail) {
        	            	- Name: (string) (len=9) "WriteFile",
        	            	+ Name: (string) (len=8) "CopyFile",
        	            	  Arguments: ([]interface {}) (len=3) {
        	            	+  (string) (len=16) "/etc/resolv.conf",
        	            	   (string) (len=40) "/test/root/sandboxes/test-id/resolv.conf",
        	            	-  ([]uint8) {
        	            	-  },
        	            	   (fs.FileMode) 420
        	Test:       	TestSetupSandboxFiles/should_create_empty_/etc/resolv.conf_if_DNSOptions_is_empty

=== FAIL: pkg/cri/server TestSetupSandboxFiles (0.00s)

DONE 1750 tests, 37 skipped, 2 failures in 105.838s
make: *** [Makefile:209: test] Error 1
Error: Process completed with exit code 2.

@thockin @dims
CRI: An empty DNSConfig != unspecified
209ee4f 
If we find that DNSConfig is provided and empty (not nil), we should not
replace it with the host's resolv.conf.

Also adds tests.

Signed-off-by: Tim Hockin <[email protected]>
@dims dims force-pushed the automated-cherry-pick-of-#9730-upstream-release-1.7 branch from 6150f1f to 209ee4f Compare July 16, 2024 16:03
@dims
Copy link
Copy Markdown
Member Author

dims commented Jul 16, 2024

@estesp yep! fixing

mikebrow
mikebrow approved these changes Jul 16, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 87c908e into containerd:release/1.7 Jul 16, 2024
@dmcgowan dmcgowan changed the title [release/1.7] CRI: An empty DNSConfig != unspecified [release/1.7] Handle empty DNSConfig differently than unspecified Jul 17, 2024
@dmcgowan dmcgowan mentioned this pull request Jul 18, 2024
Merged
Mengkzhaoyun pushed a commit to open-beagle/containerd that referenced this pull request Aug 27, 2024
Merge tag 'v1.7.20'
ed7b2f0 
containerd 1.7.20

Welcome to the v1.7.20 release of containerd!

The twentieth patch release for containerd 1.7 contains various fixes
and updates.

* Support for dropping inheritable capabilities ([#10469](containerd/containerd#10469))

* Make PodSandboxStatus friendlier to shim crashes ([#10461](containerd/containerd#10461))
* Handle empty DNSConfig differently than unspecified ([#10462](containerd/containerd#10462))
* Fix for `[cri] ttrpc: closed` during ListPodSandboxStats ([#10423](containerd/containerd#10423))

Please try out the release binaries and report any issues at
https://github.com/containerd/containerd/issues.

* Derek McGowan
* Akihiro Suda
* Phil Estes
* Akhil Mohan
* Bryant Biggs
* Danny Canter
* Davanum Srinivas
* Mike Brown
* Samuel Karp
* Tim Hockin
<details><summary>16 commits</summary>
<p>

* Prepare release notes for v1.7.20 ([#10481](containerd/containerd#10481))
  * [`7f2d4cd97`](containerd/containerd@7f2d4cd) Prepare release notes for v1.7.20
* deps: Update otelgrpc ([#10413](containerd/containerd#10413))
  * [`3a02c523d`](containerd/containerd@3a02c52) deps: Update otelgrpc
* Make PodSandboxStatus friendlier to shim crashes ([#10461](containerd/containerd#10461))
  * [`df86bdd5d`](containerd/containerd@df86bdd) CRI Sbserver: Make PodSandboxStatus friendlier to shim crashes
* Handle empty DNSConfig differently than unspecified ([#10462](containerd/containerd#10462))
  * [`209ee4f10`](containerd/containerd@209ee4f) CRI: An empty DNSConfig != unspecified
* Support for dropping inheritable capabilities ([#10469](containerd/containerd#10469))
  * [`ce65228af`](containerd/containerd@ce65228) Support for dropping inheritable capabilities
* Fix for `[cri] ttrpc: closed` during ListPodSandboxStats ([#10423](containerd/containerd#10423))
  * [`610498df7`](containerd/containerd@610498d) Fix for `[cri] ttrpc: closed` during ListPodSandboxStats
* update to go1.21.12 / go1.22.5 ([#10426](containerd/containerd#10426))
  * [`e61c7932e`](containerd/containerd@e61c793) update to go1.21.12 / go1.22.5
* errdefs: denote deprecation as a godoc comment ([#10424](containerd/containerd#10424))
  * [`c7d5e430a`](containerd/containerd@c7d5e43) errdefs: denote deprecation as a godoc comment
</p>
</details>

* **github.com/go-logr/logr**                                                      v1.2.4 -> v1.3.0
* **github.com/google/go-cmp**                                                     v0.5.9 -> v0.6.0
* **github.com/google/uuid**                                                       v1.3.1 -> v1.4.0
* **go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc**  v0.45.0 -> v0.46.1
* **go.opentelemetry.io/otel**                                                     v1.19.0 -> v1.21.0
* **go.opentelemetry.io/otel/metric**                                              v1.19.0 -> v1.21.0
* **go.opentelemetry.io/otel/sdk**                                                 v1.19.0 -> v1.21.0
* **go.opentelemetry.io/otel/trace**                                               v1.19.0 -> v1.21.0
* **google.golang.org/genproto**                                                   e6e6cdab5c13 -> 989df2bf70f3
* **google.golang.org/genproto/googleapis/api**                                    007df8e322eb -> 83a465c0220f
* **google.golang.org/genproto/googleapis/rpc**                                    d307bd883b97 -> 995d672761c0

Previous release can be found at [v1.7.19](https://github.com/containerd/containerd/releases/tag/v1.7.19)
@lefterisALEX lefterisALEX mentioned this pull request Jan 9, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estesp estesp estesp approved these changes

@mikebrow mikebrow mikebrow approved these changes

Assignees

No one assigned

Labels

area/cri Container Runtime Interface (CRI) impact/changelog size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@dims @estesp @mikebrow @dmcgowan @samuelkarp @k8s-ci-robot @thockin