diff --git a/etcd/callback.go b/etcd/callback.go index 4ee4a458..ff7caba4 100644 --- a/etcd/callback.go +++ b/etcd/callback.go @@ -23,6 +23,7 @@ import ( "sync" "github.com/purpleidea/mgmt/etcd/interfaces" + etcdUtil "github.com/purpleidea/mgmt/etcd/util" "github.com/purpleidea/mgmt/util" "github.com/purpleidea/mgmt/util/errwrap" @@ -84,7 +85,7 @@ func (obj *EmbdEtcd) endpointApply(data *interfaces.WatcherData) error { // is the endpoint list different? // TODO: do we want to use the skipEndpointApply here too? skipEndpointApply := obj.NoServer && len(endpoints) == 0 && len(obj.endpoints) > 0 - if err := cmpURLsMap(obj.endpoints, endpoints); err != nil && !skipEndpointApply { + if err := etcdUtil.CmpURLsMap(obj.endpoints, endpoints); err != nil && !skipEndpointApply { obj.endpoints = endpoints // set // can happen if a server drops out for example obj.Logf("endpoint list changed to: %+v", endpoints) @@ -153,7 +154,7 @@ func (obj *EmbdEtcd) nominateCb(ctx context.Context) error { var sendError = false var serverErr error obj.Logf("waiting for server...") - nominated, err := copyURLsMap(obj.nominated) + nominated, err := etcdUtil.CopyURLsMap(obj.nominated) if err != nil { return err } @@ -321,11 +322,11 @@ func (obj *EmbdEtcd) volunteerCb(ctx context.Context) error { } // TODO: do we really need to check these errors? - m, err := copyURLsMap(obj.membermap) // list of members... + m, err := etcdUtil.CopyURLsMap(obj.membermap) // list of members... if err != nil { return err } - v, err := copyURLsMap(obj.volunteers) + v, err := etcdUtil.CopyURLsMap(obj.volunteers) if err != nil { return err } diff --git a/etcd/etcd.go b/etcd/etcd.go index 365818fc..6264e37f 100644 --- a/etcd/etcd.go +++ b/etcd/etcd.go @@ -119,6 +119,7 @@ import ( "github.com/purpleidea/mgmt/etcd/chooser" "github.com/purpleidea/mgmt/etcd/client" "github.com/purpleidea/mgmt/etcd/interfaces" + etcdUtil "github.com/purpleidea/mgmt/etcd/util" "github.com/purpleidea/mgmt/util" "github.com/purpleidea/mgmt/util/errwrap" @@ -405,7 +406,7 @@ func (obj *EmbdEtcd) Validate() error { } } - if _, err := copyURLs(obj.Seeds); err != nil { // this will validate + if _, err := etcdUtil.CopyURLs(obj.Seeds); err != nil { // this will validate return errwrap.Wrapf(err, "the Seeds are not valid") } @@ -475,7 +476,7 @@ func (obj *EmbdEtcd) Init() error { // TODO: if we don't have any localhost URLs, should we warn so // that our local client can be able to connect more easily? - if len(localhostURLs(obj.ClientURLs)) == 0 { + if len(etcdUtil.LocalhostURLs(obj.ClientURLs)) == 0 { u, err := url.Parse(DefaultClientURL) if err != nil { return err @@ -592,9 +593,9 @@ func (obj *EmbdEtcd) Close() error { func (obj *EmbdEtcd) curls() (etcdtypes.URLs, error) { // TODO: do we need the copy? if len(obj.AClientURLs) > 0 { - return copyURLs(obj.AClientURLs) + return etcdUtil.CopyURLs(obj.AClientURLs) } - return copyURLs(obj.ClientURLs) + return etcdUtil.CopyURLs(obj.ClientURLs) } // surls returns the server (peer) urls that we should use everywhere except for @@ -602,9 +603,9 @@ func (obj *EmbdEtcd) curls() (etcdtypes.URLs, error) { func (obj *EmbdEtcd) surls() (etcdtypes.URLs, error) { // TODO: do we need the copy? if len(obj.AServerURLs) > 0 { - return copyURLs(obj.AServerURLs) + return etcdUtil.CopyURLs(obj.AServerURLs) } - return copyURLs(obj.ServerURLs) + return etcdUtil.CopyURLs(obj.ServerURLs) } // err is an error helper that sends to the errChan. @@ -1372,7 +1373,7 @@ func (obj *EmbdEtcd) Exited() <-chan struct{} { return obj.exitsSignal } // config returns the config struct to be used during the etcd client connect. func (obj *EmbdEtcd) config() etcd.Config { // FIXME: filter out any urls which wouldn't resolve ? - endpoints := fromURLsMapToStringList(obj.endpoints) // flatten map + endpoints := etcdUtil.FromURLsMapToStringList(obj.endpoints) // flatten map // We don't need to do any sort of priority sort here, since for initial // connect we'd be the only one, so it doesn't matter, and subsequent // changes are made with SetEndpoints, not here, so we never need to diff --git a/etcd/helpers.go b/etcd/helpers.go index 8dc11dcc..251fadb4 100644 --- a/etcd/helpers.go +++ b/etcd/helpers.go @@ -24,6 +24,7 @@ import ( "strings" "github.com/purpleidea/mgmt/etcd/interfaces" + etcdUtil "github.com/purpleidea/mgmt/etcd/util" "github.com/purpleidea/mgmt/util" "github.com/purpleidea/mgmt/util/errwrap" @@ -41,13 +42,13 @@ func (obj *EmbdEtcd) setEndpoints() { return } - eps := fromURLsMapToStringList(obj.endpoints) // get flat list - sort.Strings(eps) // sort for determinism + eps := etcdUtil.FromURLsMapToStringList(obj.endpoints) // get flat list + sort.Strings(eps) // sort for determinism curls, _ := obj.curls() // ignore error, was already validated // prio sort so we connect locally first - urls := fromURLsToStringList(curls) + urls := etcdUtil.FromURLsToStringList(curls) headFn := func(x string) bool { return !util.StrInList(x, urls) } @@ -113,7 +114,7 @@ func applyDeltaEvents(data *interfaces.WatcherData, urlsMap etcdtypes.URLsMap) ( if err := data.Err; err != nil { return nil, errwrap.Wrapf(err, "data contains an error") } - out, err := copyURLsMap(urlsMap) + out, err := etcdUtil.CopyURLsMap(urlsMap) if err != nil { return nil, err } diff --git a/etcd/membership.go b/etcd/membership.go index 73596ea7..ee561a62 100644 --- a/etcd/membership.go +++ b/etcd/membership.go @@ -24,6 +24,7 @@ import ( "sort" "time" + etcdUtil "github.com/purpleidea/mgmt/etcd/util" "github.com/purpleidea/mgmt/util/errwrap" pb "go.etcd.io/etcd/api/v3/etcdserverpb" @@ -216,14 +217,14 @@ func (obj *EmbdEtcd) isLeader(ctx context.Context) (bool, error) { var ep, backup *url.URL if len(obj.ClientURLs) > 0 { // heuristic, but probably correct - addresses := localhostURLs(obj.ClientURLs) + addresses := etcdUtil.LocalhostURLs(obj.ClientURLs) if len(addresses) > 0 { ep = &addresses[0] // arbitrarily pick the first one } backup = &obj.ClientURLs[0] // backup } if ep == nil && len(obj.AClientURLs) > 0 { - addresses := localhostURLs(obj.AClientURLs) + addresses := etcdUtil.LocalhostURLs(obj.AClientURLs) if len(addresses) > 0 { ep = &addresses[0] } diff --git a/etcd/server.go b/etcd/server.go index 92a05680..58434d11 100644 --- a/etcd/server.go +++ b/etcd/server.go @@ -24,6 +24,7 @@ import ( "strings" "time" + etcdUtil "github.com/purpleidea/mgmt/etcd/util" "github.com/purpleidea/mgmt/util" "github.com/purpleidea/mgmt/util/errwrap" @@ -116,7 +117,7 @@ func (obj *EmbdEtcd) runServer(newCluster bool, peerURLsMap etcdtypes.URLsMap) ( if len(obj.ServerURLs) > 0 { peerURLs = obj.ServerURLs } - initialPeerURLsMap, err := copyURLsMap(peerURLsMap) + initialPeerURLsMap, err := etcdUtil.CopyURLsMap(peerURLsMap) if err != nil { return errwrap.Wrapf(err, "error copying URLsMap") } diff --git a/etcd/util.go b/etcd/util/util.go similarity index 82% rename from etcd/util.go rename to etcd/util/util.go index 024651ae..75a10553 100644 --- a/etcd/util.go +++ b/etcd/util/util.go @@ -15,9 +15,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -package etcd - -// TODO: move to sub-package if this expands in utility or is used elsewhere... +package util import ( "fmt" @@ -38,9 +36,9 @@ func copyURL(u *url.URL) (*url.URL, error) { return url.Parse(u.String()) // copy it } -// copyURLs copies a URLs. +// CopyURLs copies a URLs. // TODO: submit this upstream to etcd ? -func copyURLs(urls etcdtypes.URLs) (etcdtypes.URLs, error) { +func CopyURLs(urls etcdtypes.URLs) (etcdtypes.URLs, error) { out := []url.URL{} for _, x := range urls { u, err := copyURL(&x) @@ -52,12 +50,12 @@ func copyURLs(urls etcdtypes.URLs) (etcdtypes.URLs, error) { return out, nil } -// copyURLsMap copies a URLsMap. +// CopyURLsMap copies a URLsMap. // TODO: submit this upstream to etcd ? -func copyURLsMap(urlsMap etcdtypes.URLsMap) (etcdtypes.URLsMap, error) { +func CopyURLsMap(urlsMap etcdtypes.URLsMap) (etcdtypes.URLsMap, error) { out := make(etcdtypes.URLsMap) for k, v := range urlsMap { - urls, err := copyURLs(v) + urls, err := CopyURLs(v) if err != nil { return nil, err } @@ -84,8 +82,8 @@ func cmpURLs(u1, u2 etcdtypes.URLs) error { return nil } -// cmpURLsMap compares two URLsMap's, and returns nil if they are the same. -func cmpURLsMap(m1, m2 etcdtypes.URLsMap) error { +// CmpURLsMap compares two URLsMap's, and returns nil if they are the same. +func CmpURLsMap(m1, m2 etcdtypes.URLsMap) error { if (m1 == nil) != (m2 == nil) { // xor return fmt.Errorf("maps differ") } @@ -112,7 +110,9 @@ func newURLsMap() etcdtypes.URLsMap { return make(etcdtypes.URLsMap) } -func fromURLsToStringList(urls etcdtypes.URLs) []string { +// FromURLsToStringList turns a list of etcd URLs into a list of strings using +// the full URL scheme. +func FromURLsToStringList(urls etcdtypes.URLs) []string { result := []string{} for _, u := range urls { // flatten map result = append(result, u.String()) // use full url including scheme @@ -120,9 +120,9 @@ func fromURLsToStringList(urls etcdtypes.URLs) []string { return result } -// fromURLsMapToStringList flattens a map of URLs into a single string list. +// FromURLsMapToStringList flattens a map of URLs into a single string list. // Remember to sort the result if you want it to be deterministic! -func fromURLsMapToStringList(m etcdtypes.URLsMap) []string { +func FromURLsMapToStringList(m etcdtypes.URLsMap) []string { result := []string{} for _, x := range m { // flatten map for _, u := range x { @@ -134,14 +134,14 @@ func fromURLsMapToStringList(m etcdtypes.URLsMap) []string { // validateURLsMap checks if each embedded URL is parseable correctly. //func validateURLsMap(urlsMap etcdtypes.URLsMap) error { -// _, err := copyURLsMap(urlsMap) // would fail if anything didn't parse +// _, err := CopyURLsMap(urlsMap) // would fail if anything didn't parse // return err //} -// localhostURLs returns the most localhost like URLs for direct connection. +// LocalhostURLs returns the most localhost like URLs for direct connection. // This gets clients to talk to the local servers first before looking remotely. // TODO: improve this algorithm as it's currently a bad heuristic -func localhostURLs(urls etcdtypes.URLs) etcdtypes.URLs { +func LocalhostURLs(urls etcdtypes.URLs) etcdtypes.URLs { out := etcdtypes.URLs{} for _, u := range urls { // "localhost" or anything in 127.0.0.0/8 is valid! diff --git a/etcd/util_test.go b/etcd/util/util_test.go similarity index 96% rename from etcd/util_test.go rename to etcd/util/util_test.go index 5d6a59f5..36ff8d50 100644 --- a/etcd/util_test.go +++ b/etcd/util/util_test.go @@ -17,7 +17,7 @@ //go:build !root -package etcd +package util import ( "net/url" @@ -94,7 +94,7 @@ func TestCopyURLs0(t *testing.T) { urls1 = append(urls1, *u) } - urls2, err := copyURLs(urls1) + urls2, err := CopyURLs(urls1) if err != nil { t.Errorf("urls did not copy: %+v", err) continue @@ -176,13 +176,13 @@ func TestCopyURLsMap0(t *testing.T) { urlsMap1[key] = urls } - urlsMap2, err := copyURLsMap(urlsMap1) + urlsMap2, err := CopyURLsMap(urlsMap1) if err != nil { t.Errorf("urlsMap did not copy: %+v", err) continue } - if err := cmpURLsMap(urlsMap1, urlsMap2); err != nil { + if err := CmpURLsMap(urlsMap1, urlsMap2); err != nil { t.Errorf("urlsMap did not cmp, err: %+v", err) } }