search

gotoz / runq

run regular Docker images in KVM/Qemu
Apache License 2.0
802 stars 45 forks source link

Skip generating resolv.conf when DNS_RUNQ_PRESERVE is defined #11

Closed yoheiueda closed 4 years ago

yoheiueda commented 4 years ago

Kubernetes does not use the Docker builtin name resolver, and provides an appropriate resolv.conf file depending on cluster and pod configurations. A default DNS setting per container runtime does not work well with Kubernetes.

This patch suppresses generation of resolv.conf by runq when DNS settings of /etc/docker/daemon.json or RUNQ_DNS are not specified.

pmorjan commented 4 years ago

Thanks @yoheiueda for this PR. I support this but would prefer a different implementation. This PR changes the current behavior. The RUNQ_DNS* environment variables can no longer be used to force an empty resolv.conf. I suggest a new env variable e.g. "RUNQ_DNS_KEEP=1" to disable overwriting resolve.conf completely. Maybe the unmountResolvConf() function can be simplified. I think the scanner on /proc/mounts is not needed.

yoheiueda commented 4 years ago

OK. I'll update the PR.

yoheiueda commented 4 years ago

@pmorjan

I've updated my patch. When DNS_RUNQ_PRESERVE is defined, runq skips generation of resolv.conf.

I also removed unmountResolvConf(), and try to unmount /etc/resolv.conf. When it fails with EINVAL, it is not a mount point.

pmorjan commented 4 years ago

LGTM. With that patch RUNQ_PRESERVE_DNS="0" also "preserves" resolv.conf. I think this should be fixed (and maybe other env variables as well) via a function like the one below. We also have to document the new env variable in the readme. But all this can be done in a follow commit.

// RepresentsTrue returns true for strings that represent a true value.
func RepresentsTrue(s string) bool {
    switch strings.ToLower(strings.TrimSpace(s)) {
    case "1", "on", "yes", "true":
        return true
    }   
    return false
}