From bff94ec6bf116557387ca113ca876105216be324 Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Mon, 13 Feb 2023 15:36:32 -0600 Subject: [PATCH 1/2] Store version-check file in the configured data-directory This also fixes what I think is a bug. It also un-exports everything. I have questions. --- server.go | 7 ++++--- verchk.go | 58 +++++++++++++++++++++++++++---------------------------- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/server.go b/server.go index 007cd6add..35f133a0b 100644 --- a/server.go +++ b/server.go @@ -604,11 +604,12 @@ func (s *Server) Open() error { log.Println(errors.Wrap(err, "logging startup")) } - // Do version check in. This is in a goroutine so that we don't block server startup if the server endpoint is down/having issues. + // Do version check in. This is in a goroutine so that we don't block server + // startup if the server endpoint is down/having issues. go func() { s.logger.Printf("Beginning featurebase version check-in") - vc := VersionChecker{URL: "https://analytics.featurebase.com/v2/featurebase/metrics"} - resp, err := vc.CheckIn() + vc := newVersionChecker(s.cluster.Path, "https://analytics.featurebase.com/v2/featurebase/metrics") + resp, err := vc.checkIn() if err != nil { s.logger.Errorf("doing version checkin. Error was %s", err) return diff --git a/verchk.go b/verchk.go index 0041a9ca1..9f2ca28b3 100644 --- a/verchk.go +++ b/verchk.go @@ -6,25 +6,31 @@ import ( "io" "net/http" "os" + "path/filepath" "strings" "github.com/google/uuid" ) -type VersionChecker struct { - URL string +const ( + versionCheckerFilename = ".client_id.txt" +) + +type versionChecker struct { + path string + url string } -func NewVersionChecker(endpoint string) *VersionChecker { - v := VersionChecker{ - URL: endpoint, +func newVersionChecker(path, endpoint string) *versionChecker { + v := versionChecker{ + path: path, + url: endpoint, } return &v } -func (v *VersionChecker) CheckIn() (*VerCheckResponse, error) { - - id, err := v.WriteClientUUID() +func (v *versionChecker) checkIn() (*verCheckResponse, error) { + id, err := v.writeClientUUID() if err != nil { return nil, err } @@ -38,60 +44,54 @@ func (v *VersionChecker) CheckIn() (*VerCheckResponse, error) { if err != nil { return nil, err } - wReq := bytes.NewReader(req) + var jsonResp verCheckResponse + r, err := http.Post(v.url, "application/json", wReq) if err != nil { return nil, err } - var json_resp VerCheckResponse - r, err := http.Post(v.URL, "application/json", wReq) - if err != nil { - return nil, err - } data, err := io.ReadAll(r.Body) - if err != nil { return nil, err } - err = json.Unmarshal(data, &json_resp) + err = json.Unmarshal(data, &jsonResp) if err != nil { return nil, err } - return &json_resp, nil + return &jsonResp, nil } -func (v *VersionChecker) GenerateClientUUID() (string, error) { +func (v *versionChecker) generateClientUUID() (string, error) { clientUUID := uuid.New() cleanedUUID := strings.Replace(clientUUID.String(), "-", "", -1) return cleanedUUID, nil } -func (v *VersionChecker) WriteClientUUID() (string, error) { - filename := ".client_id.txt" - _, err := os.Stat(filename) - if err != nil { +func (v *versionChecker) writeClientUUID() (string, error) { + filename := filepath.Join(v.path, versionCheckerFilename) + if _, err := os.Stat(filename); err != nil { if os.IsNotExist(err) { fh, err := os.Create(filename) if err != nil { return "", err } defer fh.Close() - id, err := v.GenerateClientUUID() + + id, err := v.generateClientUUID() if err != nil { return "", err } - _, err = fh.WriteString(id) - if err != nil { + if _, err := fh.WriteString(id); err != nil { return "", err } - return "", err + return id, nil } else { return "", err } @@ -102,15 +102,15 @@ func (v *VersionChecker) WriteClientUUID() (string, error) { return "", err } defer fh.Close() - buf, err := os.ReadFile(filename) + buf, err := os.ReadFile(filename) if err != nil { return "", err } - return string(buf), nil + return string(buf), nil } -type VerCheckResponse struct { +type verCheckResponse struct { Version string `json:"latest_version"` } From e09aa8487b8ba86f099c291fa981087e9399e7ef Mon Sep 17 00:00:00 2001 From: Lory Cloutier Date: Wed, 15 Feb 2023 17:54:55 -0600 Subject: [PATCH 2/2] Version checking: clean up code, add server flags FB-1975 Cleaned up version checking, removed a race condition, and added error checking. Added server flags for check-in endpoint and UUID storage file. Incorporates Travis's changes to store UUID file in data directory and unexport most of verchk.go. --- ctl/server.go | 2 ++ ctl/server_test.go | 6 +++++ server.go | 28 +++++++++++++++++++-- server/config.go | 11 ++++++++ server/server.go | 2 ++ verchk.go | 63 +++++++++++++++++++++------------------------- 6 files changed, 76 insertions(+), 36 deletions(-) diff --git a/ctl/server.go b/ctl/server.go index 03261ffc2..0c03dca24 100644 --- a/ctl/server.go +++ b/ctl/server.go @@ -49,6 +49,8 @@ func serverFlagSet(srv *server.Config, prefix string) *pflag.FlagSet { flags.DurationVar((*time.Duration)(&srv.LongQueryTime), pre("long-query-time"), time.Duration(srv.LongQueryTime), "Duration that will trigger log and stat messages for slow queries. Zero to disable.") flags.IntVar(&srv.QueryHistoryLength, pre("query-history-length"), srv.QueryHistoryLength, "Number of queries to remember in history.") flags.Int64Var(&srv.MaxQueryMemory, pre("max-query-memory"), srv.MaxQueryMemory, "Maximum memory allowed per Extract() or SELECT query.") + flags.StringVar(&srv.VerChkAddress, pre("verchk-address"), srv.VerChkAddress, "Address to contact to check for latest version.") + flags.StringVar(&srv.UUIDFile, pre("uuid-file"), srv.UUIDFile, "File to store UUID used in checking latest version. If this is a relative path, the file will be stored in the server's data directory.") // TLS SetTLSConfig(flags, pre(""), &srv.TLS.CertificatePath, &srv.TLS.CertificateKeyPath, &srv.TLS.CACertPath, &srv.TLS.SkipVerify, &srv.TLS.EnableClientVerification) diff --git a/ctl/server_test.go b/ctl/server_test.go index 495c9b03c..1bc4cff30 100644 --- a/ctl/server_test.go +++ b/ctl/server_test.go @@ -20,4 +20,10 @@ func TestBuildServerFlags(t *testing.T) { if cm.Flags().Lookup("log-path").Name == "" { t.Fatal("log-path flag is required") } + if cm.Flags().Lookup("verchk-address").Name == "" { + t.Fatal("verchk-address flag is required") + } + if cm.Flags().Lookup("uuid-file").Name == "" { + t.Fatal("uuid-file flag is required") + } } diff --git a/server.go b/server.go index 35f133a0b..b04feea7f 100644 --- a/server.go +++ b/server.go @@ -89,6 +89,8 @@ type Server struct { // nolint: maligned defaultClient *InternalClient dataDir string + verChkAddress string + uuidFile string // Threshold for logging long-running queries longQueryTime time.Duration @@ -167,6 +169,24 @@ func OptServerDataDir(dir string) ServerOption { } } +// OptServerVerChkAddress is a functional option on Server +// used to set the address to check for the current version. +func OptServerVerChkAddress(addr string) ServerOption { + return func(s *Server) error { + s.verChkAddress = addr + return nil + } +} + +// OptServerUUIDFile is a functional option on Server +// used to set the file name for storing the checkin UUID. +func OptServerUUIDFile(uf string) ServerOption { + return func(s *Server) error { + s.uuidFile = uf + return nil + } +} + // OptServerViewsRemovalInterval is a functional option on Server // used to set the ttl removal interval. func OptServerViewsRemovalInterval(interval time.Duration) ServerOption { @@ -608,13 +628,17 @@ func (s *Server) Open() error { // startup if the server endpoint is down/having issues. go func() { s.logger.Printf("Beginning featurebase version check-in") - vc := newVersionChecker(s.cluster.Path, "https://analytics.featurebase.com/v2/featurebase/metrics") + vc := newVersionChecker(s.cluster.Path, s.verChkAddress, s.uuidFile) resp, err := vc.checkIn() if err != nil { s.logger.Errorf("doing version checkin. Error was %s", err) return } - s.logger.Printf("Version check-in complete. Latest version is %s", resp.Version) + if resp.Error != "" { + s.logger.Printf("Version check-in failed, endpoint response was %s", resp.Error) + } else { + s.logger.Printf("Version check-in complete. Latest version is %s", resp.Version) + } }() // Start DisCo. diff --git a/server/config.go b/server/config.go index f2d42e8ac..92e86c10d 100644 --- a/server/config.go +++ b/server/config.go @@ -152,6 +152,15 @@ type Config struct { // Limits the total amount of memory to be used by Extract() & SELECT queries. MaxQueryMemory int64 `toml:"max-query-memory"` + // On startup, featurebase server contacts a web server to check the latest version. + // This stores the address for that check + VerChkAddress string `toml:"verchk-address"` + + // When checking version, server sends a UUID so that we can keep track of + // how many unique Featurebase installs are out there. The file is stored in + // the data directory; this stores the filename to use. + UUIDFile string `toml:"uuid-file"` + Cluster struct { ReplicaN int `toml:"replicas"` Name string `toml:"name"` @@ -383,6 +392,8 @@ func NewConfig() *Config { LongQueryTime: toml.Duration(-time.Minute), CheckInInterval: 60 * time.Second, + VerChkAddress: "https://analytics.featurebase.com/v2/featurebase/metrics", + UUIDFile: ".client_id.txt", } // Cluster config. diff --git a/server/server.go b/server/server.go index 0ebdb513d..fbea5f3f8 100644 --- a/server/server.go +++ b/server/server.go @@ -596,6 +596,8 @@ func (m *Command) setupServer() error { pilosa.OptServerServerlessStorage(m.serverlessStorage), pilosa.OptServerIsDataframeEnabled(m.Config.Dataframe.Enable), pilosa.OptServerDataframeUseParquet(m.Config.Dataframe.UseParquet), + pilosa.OptServerVerChkAddress(m.Config.VerChkAddress), + pilosa.OptServerUUIDFile(m.Config.UUIDFile), } if m.isComputeNode { diff --git a/verchk.go b/verchk.go index 9f2ca28b3..1dcf93558 100644 --- a/verchk.go +++ b/verchk.go @@ -12,19 +12,17 @@ import ( "github.com/google/uuid" ) -const ( - versionCheckerFilename = ".client_id.txt" -) - type versionChecker struct { - path string - url string + path string + url string + idfile string } -func newVersionChecker(path, endpoint string) *versionChecker { +func newVersionChecker(path, endpoint, idfile string) *versionChecker { v := versionChecker{ - path: path, - url: endpoint, + path: path, + url: endpoint, + idfile: idfile, } return &v } @@ -62,7 +60,6 @@ func (v *versionChecker) checkIn() (*verCheckResponse, error) { return nil, err } - return &jsonResp, nil } @@ -73,31 +70,15 @@ func (v *versionChecker) generateClientUUID() (string, error) { } func (v *versionChecker) writeClientUUID() (string, error) { - filename := filepath.Join(v.path, versionCheckerFilename) - if _, err := os.Stat(filename); err != nil { - if os.IsNotExist(err) { - fh, err := os.Create(filename) - if err != nil { - return "", err - } - defer fh.Close() - - id, err := v.generateClientUUID() - if err != nil { - return "", err - } - - if _, err := fh.WriteString(id); err != nil { - return "", err - } - - return id, nil - } else { - return "", err - } + var filename string + // if v.idfile starts with a path separator then it's an absolute path + // otherwise it's a relative path and the file goes in the data directory + if v.idfile[0] == os.PathSeparator { + filename = v.idfile + } else { + filename = filepath.Join(v.path, v.idfile) } - - fh, err := os.Open(filename) + fh, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE, 0744) if err != nil { return "", err } @@ -107,10 +88,24 @@ func (v *versionChecker) writeClientUUID() (string, error) { if err != nil { return "", err } + // this just checks to see if there was anything at all in the file. + // we should probably check to make sure it's a valid UUID + if string(buf) == "" { + id, err := v.generateClientUUID() + if err != nil { + return "", err + } + _, err = fh.WriteString(id) + if err != nil { + return "", err + } + return id, nil + } return string(buf), nil } type verCheckResponse struct { Version string `json:"latest_version"` + Error string `json:"error"` }