Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Fix containerd resolv.conf + DHCP behavior #441

Merged
merged 2 commits into from
Sep 19, 2019
Merged

Fix containerd resolv.conf + DHCP behavior #441

merged 2 commits into from
Sep 19, 2019

Conversation

stealthybox
Copy link
Contributor

@stealthybox stealthybox commented Sep 19, 2019 •
edited
Loading

Fixes #437

Patches:

  • Add heuristic resolv.conf builder supporting nameserver+search
  • Fix containerd DHCP using resolvconf + Add id to runtime.RunContainer()

Having the user be able to setup custom DNS is great, but it would be better if it first worked well without any tweaking.

This resolv.conf library supports /etc/resolv.conf, systemd-resolved, and will even work with nothing at all.
All loopback nameservers are filtered, and in the event that there's still no usable DNS, it falls back to Google DNS servers. (other default suggestions are welcome)

The containerd runtime then uses this lib to populate the vmDir specfic to the ignite vm's id.
A modification to the runtime interface was needed to pass the id so this could be done.

It might be better to try and use the container's snapshot instead of the vmDir to accomplish this.
However, using the vmDir is a generic solution since adding mounts for the runtime container is simple.
We could use this mechanism to more uniformly configure DHCP /w docker while disabling the default resolver for ignite-spawn as mentioned in #438.

I'd also be fine not actually writing any file and just adding fields to the vm manifest for dhcp.Nameservers and dhcp.Search.
We can still use readDNSConfig() and its accompanying functions to calculate the proper values from the host.

The benefit of using /etc/resolv.conf is it's well integrated with docker networks which the vm will benefit from through it's DHCP config.

example behavior:

make build-all
~/Repos/ignite-scratch/ignite-clean.sh
~/Repos/ignite-scratch/iptables-clean-cni-ignite.sh

sudo bin/ignite vm run weaveworks/ignite-ubuntu --log-level debug --ssh --name test441
DEBU[0000] Ensuring image weaveworks/ignite-ubuntu:latest exists, or importing it... 
DEBU[0000] Found image with UID 35c47bfb397b922b        
DEBU[0000] Ensuring kernel weaveworks/ignite-kernel:4.19.47 exists, or importing it... 
DEBU[0000] Found kernel with UID 5dfb8abbb5d4c79a       
INFO[0000] Created VM with ID "030cd0a728543694" and name "test441" 
DEBU[0001] containerd: Inspecting image "weaveworks/ignite:v0.6.0-67-585d77f9804381" 
DEBU[0001] Writing "/var/lib/firecracker/vm/030cd0a728543694/runtime.containerd.resolv.conf" with new hash: "cca33446e2ce0f418633a6ea9ef9c54859cb8a755f90dcc74274200c31fa85e4", old hash: "" 
INFO[0001] Networking is handled by "cni"               
INFO[0001] Started Firecracker VM "030cd0a728543694" in a container with ID "ignite-030cd0a728543694" 

sudo bin/ignite exec test441 cat /etc/resolv.conf
#PROTO: DHCP
nameserver 1.0.0.1
bootserver 0.0.0.0

export vm_id="$(sudo bin/ignite ps | grep test441 | awk '{print $1}')"

ls -1 /var/lib/firecracker/vm/${vm_id}/*resolv.conf*
/var/lib/firecracker/vm/030cd0a728543694/runtime.containerd.resolv.conf
/var/lib/firecracker/vm/030cd0a728543694/runtime.containerd.resolv.conf.sha256

cat /var/lib/firecracker/vm/${vm_id}/*resolv.conf*
# The following config was built by ignite:
nameserver 1.0.0.1
search stealthybox.local
cca33446e2ce0f418633a6ea9ef9c54859cb8a755f90dcc74274200c31fa85e4

sudo ctr -n firecracker t exec --exec-id 0 "ignite-${vm_id}" cat /etc/resolv.conf
# The following config was built by ignite:
nameserver 1.0.0.1
search stealthybox.local

sudo bin/ignite vm stop test441 --log-level debug
INFO[0000] Removing the container with ID "ignite-030cd0a728543694" from the "cni" network 
INFO[0008] Stopped VM with name "test441" and ID "030cd0a728543694" 

sudo bin/ignite vm start test441 --log-level debug
DEBU[0000] containerd: Inspecting image "weaveworks/ignite:v0.6.0-67-585d77f9804381" 
DEBU[0000] "/var/lib/firecracker/vm/030cd0a728543694/runtime.containerd.resolv.conf" with hash "cca33446e2ce0f418633a6ea9ef9c54859cb8a755f90dcc74274200c31fa85e4" is unchanged 
INFO[0000] Networking is handled by "cni"               
INFO[0000] Started Firecracker VM "030cd0a728543694" in a container with ID "ignite-030cd0a728543694" 

sudo bin/ignite exec test441 cat /etc/resolv.conf
#PROTO: DHCP
nameserver 1.0.0.1
bootserver 0.0.0.0

@stealthybox stealthybox requested a review from twelho as a code owner September 19, 2019 08:27
stealthybox
stealthybox commented Sep 19, 2019
View reviewed changes
pkg/runtime/containerd/client.go
// - maybe we can use containerd.NewContainerOpts{} to do it just-in-time
// - write this file to snapshot mount instead of vmDir
// - commit snapshot?
// - deprecate vm id from this function's signature
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This extensive TODO is here for posterity, but I can move it into an issue or just remove it altogether depending on what we want.

stealthybox
stealthybox commented Sep 19, 2019
View reviewed changes
pkg/resolvconf/resolvconf_test.go
@@ -0,0 +1,201 @@
package resolvconf
Copy link
Contributor Author

@stealthybox stealthybox Sep 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This is its own package because it's very decoupled from runtime needs.
I couldn't find a more sensible home for it.

Also, not including it in runtime/containerd allows you to run the tests without root.

Copy link
Member

Choose a reason for hiding this comment

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

Yes. I have no problem with this in a separate package :-)

@stealthybox stealthybox requested review from chanwit and removed request for twelho September 19, 2019 09:08
chanwit
chanwit approved these changes Sep 19, 2019
View reviewed changes
pkg/resolvconf/resolvconf_test.go
@@ -0,0 +1,201 @@
package resolvconf
Copy link
Member

Choose a reason for hiding this comment

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

Yes. I have no problem with this in a separate package :-)

pkg/util/fs.go

// WriteFileIfChanged stores a sha of data at <filename>.sha256 and determines whether to
// rewrite the file; it has the same signature as ioutil.WriteFile().
func WriteFileIfChanged(filename string, data []byte, perm os.FileMode) error {
Copy link
Member

Choose a reason for hiding this comment

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

I like this function!

pkg/resolvconf/resolvconf.go
}

if isSystemdResolved(cfg) {
systemdCfg, err := dns.ClientConfigFromFile(resolvSystemd)
Copy link
Member

Choose a reason for hiding this comment

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

This systemd bit could also solve my problem too.

@chanwit chanwit added the area/networking Issues related to networking label Sep 19, 2019
@chanwit chanwit added this to the v0.6.1 milestone Sep 19, 2019
@chanwit chanwit added the kind/bug Categorizes issue or PR as related to a bug. label Sep 19, 2019
@chanwit chanwit merged commit 0ed3780 into weaveworks:master Sep 19, 2019
@stealthybox stealthybox deleted the resolvconf branch September 19, 2019 16:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@chanwit chanwit chanwit approved these changes

Assignees
No one assigned
Labels
area/networking Issues related to networking kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Milestone
v0.6.1
Development

Successfully merging this pull request may close these issues.

DHCPv4 DNS incorrect on host /w systemd-resolved using --runtime=containerd
2 participants
@stealthybox @chanwit