From e37c4efc6e4e4b32185cf3f39d9d1f6027265bcf Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 3 Jul 2023 12:30:51 -0700 Subject: [PATCH] fix: upon DNS refresh() failure use previous values (#17561) DNS refresh() in-case of MinIO can safely re-use the previous values on bare-metal setups, since bare-metal arrangements do not change DNS in any manner commonly. This PR simplifies that, we only ever need DNS caching on bare-metal setups. - On containerized setups do not enable DNS caching at all, as it may have adverse effects on the overall effectiveness of k8s DNS systems. k8s DNS systems are dynamic and expect applications to avoid managing DNS caching themselves, instead provide a cleaner container native caching implementations that must be used. - update IsDocker() detection, including podman runtime - move to minio/dnscache fork for a simpler package --- CREDITS | 55 +++++++++++++++++----------------- cmd/common-main.go | 24 ++++----------- cmd/globals.go | 2 +- cmd/update.go | 24 +++++++++++---- cmd/utils.go | 49 +++++++++++++++++++++++------- go.mod | 4 +-- go.sum | 8 ++--- internal/http/dial_dnscache.go | 23 +++++++++----- internal/http/transports.go | 11 ++----- 9 files changed, 113 insertions(+), 87 deletions(-) diff --git a/CREDITS b/CREDITS index a0dc5a89a..0e6b554a8 100644 --- a/CREDITS +++ b/CREDITS @@ -16022,6 +16022,34 @@ For more information on this, and how to apply and follow the GNU AGPL, see ================================================================ +github.com/minio/dnscache +https://github.com/minio/dnscache +---------------------------------------------------------------- +MIT License + +Copyright (c) 2023 MinIO, Inc. +Copyright (c) 2018 Olivier Poitrey + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + +================================================================ + github.com/minio/dperf https://github.com/minio/dperf ---------------------------------------------------------------- @@ -25164,33 +25192,6 @@ THE SOFTWARE. ================================================================ -github.com/rs/dnscache -https://github.com/rs/dnscache ----------------------------------------------------------------- -MIT License - -Copyright (c) 2018 Olivier Poitrey - -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included in all -copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. - -================================================================ - github.com/rs/xid https://github.com/rs/xid ---------------------------------------------------------------- diff --git a/cmd/common-main.go b/cmd/common-main.go index 11e00e7ba..94523e332 100644 --- a/cmd/common-main.go +++ b/cmd/common-main.go @@ -66,7 +66,6 @@ import ( "github.com/minio/pkg/ellipses" "github.com/minio/pkg/env" xnet "github.com/minio/pkg/net" - "github.com/rs/dnscache" ) // serverDebugLog will enable debug printing @@ -96,11 +95,6 @@ func init() { initGlobalContext() - options := dnscache.ResolverRefreshOptions{ - ClearUnused: true, - PersistOnFailure: false, - } - t, _ := minioVersionToReleaseTime(Version) if !t.IsZero() { globalVersionUnix = uint64(t.Unix()) @@ -108,24 +102,16 @@ func init() { globalIsCICD = env.Get("MINIO_CI_CD", "") != "" || env.Get("CI", "") != "" - containers := IsKubernetes() || IsDocker() || IsBOSH() || IsDCOS() || IsPCFTile() - - // Call to refresh will refresh names in cache. If you pass true, it will also - // remove cached names not looked up since the last call to Refresh. It is a good idea - // to call this method on a regular interval. + // Call to refresh will refresh names in cache. go func() { - var t *time.Ticker - if containers { - // k8s DNS TTL is 30s (Attempt a refresh only after) - t = time.NewTicker(30 * time.Second) - } else { - t = time.NewTicker(10 * time.Minute) - } + // Baremetal setups set DNS refresh window to 10 minutes. + t := time.NewTicker(10 * time.Minute) defer t.Stop() for { select { case <-t.C: - globalDNSCache.RefreshWithOptions(options) + globalDNSCache.Refresh() + case <-GlobalContext.Done(): return } diff --git a/cmd/globals.go b/cmd/globals.go index d3b0d002f..45f146a7b 100644 --- a/cmd/globals.go +++ b/cmd/globals.go @@ -27,6 +27,7 @@ import ( "time" "github.com/minio/console/restapi" + "github.com/minio/dnscache" "github.com/minio/madmin-go/v3" "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/set" @@ -34,7 +35,6 @@ import ( "github.com/minio/minio/internal/config" "github.com/minio/minio/internal/handlers" "github.com/minio/minio/internal/kms" - "github.com/rs/dnscache" "github.com/dustin/go-humanize" "github.com/minio/minio/internal/auth" diff --git a/cmd/update.go b/cmd/update.go index e8ffbe77e..a9d1f413e 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -121,13 +121,27 @@ func GetCurrentReleaseTime() (releaseTime time.Time, err error) { // IsDocker - returns if the environment minio is running in docker or // not. The check is a simple file existence check. // -// https://github.com/moby/moby/blob/master/daemon/initlayer/setup_unix.go#L25 +// https://github.com/moby/moby/blob/master/daemon/initlayer/setup_unix.go +// https://github.com/containers/podman/blob/master/libpod/runtime.go // -// "/.dockerenv": "file", +// "/.dockerenv": "file", +// "/run/.containerenv": "file", func IsDocker() bool { - _, err := os.Stat("/.dockerenv") + var err error + for _, envfile := range []string{ + "/.dockerenv", + "/run/.containerenv", + } { + _, err = os.Stat(envfile) + if err == nil { + return true + } + } if osIsNotExist(err) { - return false + // if none of the files are present we may be running inside + // CRI-O, Containerd etc.. + // Fallback to our container specific ENVs if they are set. + return env.IsSet("MINIO_ACCESS_KEY_FILE") } // Log error, as we will not propagate it to caller @@ -523,7 +537,7 @@ const ( defaultMinisignPubkey = "RWTx5Zr1tiHQLwG9keckT0c45M3AGeHD6IvimQHpyRywVWGbP1aVSGav" ) -func verifyBinary(u *url.URL, sha256Sum []byte, releaseInfo string, mode string, reader []byte) (err error) { +func verifyBinary(u *url.URL, sha256Sum []byte, releaseInfo, mode string, reader []byte) (err error) { if !atomic.CompareAndSwapUint32(&updateInProgress, 0, 1) { return errors.New("update already in progress") } diff --git a/cmd/utils.go b/cmd/utils.go index 9e1511c14..2bf15f0cb 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -557,8 +557,13 @@ func ToS3ETag(etag string) string { // GetDefaultConnSettings returns default HTTP connection settings. func GetDefaultConnSettings() xhttp.ConnSettings { + lookupHost := globalDNSCache.LookupHost + if IsKubernetes() || IsDocker() { + lookupHost = nil + } + return xhttp.ConnSettings{ - DNSCache: globalDNSCache, + LookupHost: lookupHost, DialTimeout: rest.DefaultTimeout, RootCAs: globalRootCAs, TCPOptions: globalTCPOptions, @@ -568,8 +573,13 @@ func GetDefaultConnSettings() xhttp.ConnSettings { // NewInternodeHTTPTransport returns a transport for internode MinIO // connections. func NewInternodeHTTPTransport() func() http.RoundTripper { + lookupHost := globalDNSCache.LookupHost + if IsKubernetes() || IsDocker() { + lookupHost = nil + } + return xhttp.ConnSettings{ - DNSCache: globalDNSCache, + LookupHost: lookupHost, DialTimeout: rest.DefaultTimeout, RootCAs: globalRootCAs, CipherSuites: fips.TLSCiphers(), @@ -582,8 +592,13 @@ func NewInternodeHTTPTransport() func() http.RoundTripper { // NewCustomHTTPProxyTransport is used only for proxied requests, specifically // only supports HTTP/1.1 func NewCustomHTTPProxyTransport() func() *http.Transport { + lookupHost := globalDNSCache.LookupHost + if IsKubernetes() || IsDocker() { + lookupHost = nil + } + return xhttp.ConnSettings{ - DNSCache: globalDNSCache, + LookupHost: lookupHost, DialTimeout: rest.DefaultTimeout, RootCAs: globalRootCAs, CipherSuites: fips.TLSCiphers(), @@ -596,8 +611,13 @@ func NewCustomHTTPProxyTransport() func() *http.Transport { // NewHTTPTransportWithClientCerts returns a new http configuration // used while communicating with the cloud backends. func NewHTTPTransportWithClientCerts(clientCert, clientKey string) *http.Transport { + lookupHost := globalDNSCache.LookupHost + if IsKubernetes() || IsDocker() { + lookupHost = nil + } + s := xhttp.ConnSettings{ - DNSCache: globalDNSCache, + LookupHost: lookupHost, DialTimeout: defaultDialTimeout, RootCAs: globalRootCAs, TCPOptions: globalTCPOptions, @@ -609,8 +629,7 @@ func NewHTTPTransportWithClientCerts(clientCert, clientKey string) *http.Transpo defer cancel() transport, err := s.NewHTTPTransportWithClientCerts(ctx, clientCert, clientKey) if err != nil { - logger.LogIf(ctx, fmt.Errorf("failed to load client key and cert, please check your endpoint configuration: %s", - err.Error())) + logger.LogIf(ctx, fmt.Errorf("Unable to load client key and cert, please check your client certificate configuration: %w", err)) } return transport } @@ -629,9 +648,14 @@ const defaultDialTimeout = 5 * time.Second // NewHTTPTransportWithTimeout allows setting a timeout. func NewHTTPTransportWithTimeout(timeout time.Duration) *http.Transport { + lookupHost := globalDNSCache.LookupHost + if IsKubernetes() || IsDocker() { + lookupHost = nil + } + return xhttp.ConnSettings{ DialContext: newCustomDialContext(), - DNSCache: globalDNSCache, + LookupHost: lookupHost, DialTimeout: defaultDialTimeout, RootCAs: globalRootCAs, TCPOptions: globalTCPOptions, @@ -639,10 +663,8 @@ func NewHTTPTransportWithTimeout(timeout time.Duration) *http.Transport { }.NewHTTPTransportWithTimeout(timeout) } -type dialContext func(ctx context.Context, network, addr string) (net.Conn, error) - // newCustomDialContext setups a custom dialer for any external communication and proxies. -func newCustomDialContext() dialContext { +func newCustomDialContext() xhttp.DialContext { return func(ctx context.Context, network, addr string) (net.Conn, error) { dialer := &net.Dialer{ Timeout: 15 * time.Second, @@ -665,9 +687,14 @@ func newCustomDialContext() dialContext { // NewRemoteTargetHTTPTransport returns a new http configuration // used while communicating with the remote replication targets. func NewRemoteTargetHTTPTransport(insecure bool) func() *http.Transport { + lookupHost := globalDNSCache.LookupHost + if IsKubernetes() || IsDocker() { + lookupHost = nil + } + return xhttp.ConnSettings{ DialContext: newCustomDialContext(), - DNSCache: globalDNSCache, + LookupHost: lookupHost, RootCAs: globalRootCAs, TCPOptions: globalTCPOptions, EnableHTTP2: false, diff --git a/go.mod b/go.mod index 91ce6c36d..4b736a252 100644 --- a/go.mod +++ b/go.mod @@ -45,10 +45,10 @@ require ( github.com/minio/cli v1.24.2 github.com/minio/console v0.30.1-0.20230623034122-b7b0271ec78c github.com/minio/csvparser v1.0.0 + github.com/minio/dnscache v0.1.1 github.com/minio/dperf v0.4.10 github.com/minio/highwayhash v1.0.2 github.com/minio/kes-go v0.1.0 - github.com/minio/madmin-go/v2 v2.2.1 github.com/minio/madmin-go/v3 v3.0.4 github.com/minio/minio-go/v7 v7.0.58 github.com/minio/mux v1.9.0 @@ -75,9 +75,7 @@ require ( github.com/prometheus/procfs v0.11.0 github.com/rabbitmq/amqp091-go v1.8.1 github.com/rs/cors v1.9.0 - github.com/rs/dnscache v0.0.0-20211102005908-e0241e321417 github.com/secure-io/sio-go v0.3.1 - github.com/shirou/gopsutil v3.21.11+incompatible github.com/shirou/gopsutil/v3 v3.23.5 github.com/tidwall/gjson v1.14.4 github.com/tinylib/msgp v1.1.8 diff --git a/go.sum b/go.sum index d9b60796c..3ffc57077 100644 --- a/go.sum +++ b/go.sum @@ -473,6 +473,8 @@ github.com/minio/console v0.30.1-0.20230623034122-b7b0271ec78c h1:vYs64wXByx1CzM github.com/minio/console v0.30.1-0.20230623034122-b7b0271ec78c/go.mod h1:YtBQzrZK/BwQhAq6X7ZcoYQnl90T78QxMzx7ttmfcc4= github.com/minio/csvparser v1.0.0 h1:xJEHcYK8ZAjeW4hNV9Zu30u+/2o4UyPnYgyjWp8b7ZU= github.com/minio/csvparser v1.0.0/go.mod h1:lKXskSLzPgC5WQyzP7maKH7Sl1cqvANXo9YCto8zbtM= +github.com/minio/dnscache v0.1.1 h1:AMYLqomzskpORiUA1ciN9k7bZT1oB3YZN4cEIi88W5o= +github.com/minio/dnscache v0.1.1/go.mod h1:WCumm6offO4rQ/82oTCSoHnlhTmc81+vXgKpBtSYRbg= github.com/minio/dperf v0.4.10 h1:PXxA9WBIB0S6ZTjce+gTygY2H+YVUSkZ81XD2xWpvHc= github.com/minio/dperf v0.4.10/go.mod h1:pd52YVTOQU2CYQ8xlyKcP8lFa72R9SEfsvoKAFPifTQ= github.com/minio/filepath v1.0.0 h1:fvkJu1+6X+ECRA6G3+JJETj4QeAYO9sV43I79H8ubDY= @@ -481,8 +483,6 @@ github.com/minio/highwayhash v1.0.2 h1:Aak5U0nElisjDCfPSG79Tgzkn2gl66NxOMspRrKnA github.com/minio/highwayhash v1.0.2/go.mod h1:BQskDq+xkJ12lmlUUi7U0M5Swg3EWR+dLTk+kldvVxY= github.com/minio/kes-go v0.1.0 h1:h201DyOYP5sTqajkxFGxmXz/kPbT8HQNX1uh3Yx2PFc= github.com/minio/kes-go v0.1.0/go.mod h1:VorHLaIYis9/MxAHAtXN4d8PUMNKhIxTIlvFt0hBOEo= -github.com/minio/madmin-go/v2 v2.2.1 h1:zyRcXBm013VF6+7wefOpJEU6N2f7/7uFKsOrcn44DpM= -github.com/minio/madmin-go/v2 v2.2.1/go.mod h1:8bL1RMNkblIENFSgGYjeHrzUx9PxROb7OqfNuMU9ivE= github.com/minio/madmin-go/v3 v3.0.4 h1:nINToRlCFRKKINGIvn+RUkYgnQregTEi6xVO6XwSUtA= github.com/minio/madmin-go/v3 v3.0.4/go.mod h1:lPrMoc1aeiIWmmrxBthkDqzMPQwC/Lu9ByuyM2wenJk= github.com/minio/mc v0.0.0-20230620210040-4b06db8e171f h1:YWTPtK2aX2YF+A6FJ9aoTXY0AvyIkDrAS7iSR03rx+Q= @@ -642,16 +642,12 @@ github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTE github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= github.com/rs/cors v1.9.0 h1:l9HGsTsHJcvW14Nk7J9KFz8bzeAWXn3CG6bgt7LsrAE= github.com/rs/cors v1.9.0/go.mod h1:XyqrcTp5zjWr1wsJ8PIRZssZ8b/WMcMf71DJnit4EMU= -github.com/rs/dnscache v0.0.0-20211102005908-e0241e321417 h1:Lt9DzQALzHoDwMBGJ6v8ObDPR0dzr2a6sXTB1Fq7IHs= -github.com/rs/dnscache v0.0.0-20211102005908-e0241e321417/go.mod h1:qe5TWALJ8/a1Lqznoc5BDHpYX/8HU60Hm2AwRmqzxqA= github.com/rs/xid v1.5.0 h1:mKX4bl4iPYJtEIxp6CYiUuLQ/8DYMoz0PUdtGgMFRVc= github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g= github.com/scylladb/termtables v0.0.0-20191203121021-c4c0b6d42ff4/go.mod h1:C1a7PQSMz9NShzorzCiG2fk9+xuCgLkPeCvMHYR2OWg= github.com/secure-io/sio-go v0.3.1 h1:dNvY9awjabXTYGsTF1PiCySl9Ltofk9GA3VdWlo7rRc= github.com/secure-io/sio-go v0.3.1/go.mod h1:+xbkjDzPjwh4Axd07pRKSNriS9SCiYksWnZqdnfpQxs= -github.com/shirou/gopsutil v3.21.11+incompatible h1:+1+c1VGhc88SSonWP6foOcLhvnKlUeu/erjjvaPEYiI= -github.com/shirou/gopsutil v3.21.11+incompatible/go.mod h1:5b4v6he4MtMOwMlS0TUMTu2PcXUg8+E1lC7eC3UO/RA= github.com/shirou/gopsutil/v3 v3.23.5 h1:5SgDCeQ0KW0S4N0znjeM/eFHXXOKyv2dVNgRq/c9P6Y= github.com/shirou/gopsutil/v3 v3.23.5/go.mod h1:Ng3Maa27Q2KARVJ0SPZF5NdrQSC3XHKP8IIWrHgMeLY= github.com/shoenig/go-m1cpu v0.1.6 h1:nxdKQNcEB6vzgA2E2bvzKIYRuNj7XNJ4S/aRSwKzFtM= diff --git a/internal/http/dial_dnscache.go b/internal/http/dial_dnscache.go index a0e991e74..3acf4c149 100644 --- a/internal/http/dial_dnscache.go +++ b/internal/http/dial_dnscache.go @@ -21,13 +21,17 @@ import ( "context" "net" "time" - - "github.com/rs/dnscache" ) -// DialContextWithDNSCache is a helper function which returns `net.DialContext` function. -// It randomly fetches an IP from the DNS cache and dials it by the given dial -// function. It dials one by one and returns first connected `net.Conn`. +// LookupHost is a function to make custom lookupHost for optional cached DNS requests +type LookupHost func(ctx context.Context, host string) (addrs []string, err error) + +// DialContextWithLookupHost is a helper function which returns `net.DialContext` function. +// It randomly fetches an IP via custom LookupHost function and dials it by the given dial +// function. LookupHost may implement an internal DNS caching implementation, lookupHost +// input if nil then net.DefaultResolver.LookupHost is used. +// +// It dials one by one and returns first connected `net.Conn`. // If it fails to dial all IPs from cache it returns first error. If no baseDialFunc // is given, it sets default dial function. // @@ -35,7 +39,11 @@ import ( // // In this function, it uses functions from `rand` package. To make it really random, // you MUST call `rand.Seed` and change the value from the default in your application -func DialContextWithDNSCache(resolver *dnscache.Resolver, baseDialCtx DialContext) DialContext { +func DialContextWithLookupHost(lookupHost LookupHost, baseDialCtx DialContext) DialContext { + if lookupHost == nil { + lookupHost = net.DefaultResolver.LookupHost + } + if baseDialCtx == nil { // This is same as which `http.DefaultTransport` uses. baseDialCtx = (&net.Dialer{ @@ -43,6 +51,7 @@ func DialContextWithDNSCache(resolver *dnscache.Resolver, baseDialCtx DialContex KeepAlive: 30 * time.Second, }).DialContext } + return func(ctx context.Context, network, addr string) (conn net.Conn, err error) { host, port, err := net.SplitHostPort(addr) if err != nil { @@ -54,7 +63,7 @@ func DialContextWithDNSCache(resolver *dnscache.Resolver, baseDialCtx DialContex return baseDialCtx(ctx, "tcp", addr) } - ips, err := resolver.LookupHost(ctx, host) + ips, err := lookupHost(ctx, host) if err != nil { return nil, err } diff --git a/internal/http/transports.go b/internal/http/transports.go index e8c4d31be..eba989ba5 100644 --- a/internal/http/transports.go +++ b/internal/http/transports.go @@ -21,13 +21,11 @@ import ( "context" "crypto/tls" "crypto/x509" - "net" "net/http" "syscall" "time" "github.com/minio/pkg/certs" - "github.com/rs/dnscache" ) // tlsClientSessionCacheSize is the cache size for client sessions. @@ -35,11 +33,8 @@ var tlsClientSessionCacheSize = 100 // ConnSettings - contains connection settings. type ConnSettings struct { - // If this is non-nil, DNSCache and DialTimeout are ignored. - DialContext func(ctx context.Context, network, addr string) (net.Conn, error) - - // Dial settings, used if DialContext is nil. - DNSCache *dnscache.Resolver + DialContext DialContext // Custom dialContext, DialTimeout is ignored if this is already setup. + LookupHost LookupHost // Custom lookupHost, is nil on containerized deployments. DialTimeout time.Duration // TLS Settings @@ -57,7 +52,7 @@ type ConnSettings struct { func (s ConnSettings) getDefaultTransport() *http.Transport { dialContext := s.DialContext if dialContext == nil { - dialContext = DialContextWithDNSCache(s.DNSCache, NewInternodeDialContext(s.DialTimeout, s.TCPOptions)) + dialContext = DialContextWithLookupHost(s.LookupHost, NewInternodeDialContext(s.DialTimeout, s.TCPOptions)) } tlsClientConfig := tls.Config{