Fb 1975 (#2254)

* 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.

* 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.

---------

Co-authored-by: Travis Turner <travis@molecula.com>
Co-authored-by: seebs <seebs@molecula.com>

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)

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")
+	}
 }

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 {
@@ -604,16 +624,21 @@ 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, 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.

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.

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 {

verchk.go

@@ -6,25 +6,29 @@ import (
 	"io"
 	"net/http"
 	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/google/uuid"
 )
 
-type VersionChecker struct {
-	URL string
+type versionChecker struct {
+	path   string
+	url    string
+	idfile string
 }
 
-func NewVersionChecker(endpoint string) *VersionChecker {
-	v := VersionChecker{
-		URL: endpoint,
+func newVersionChecker(path, endpoint, idfile string) *versionChecker {
+	v := versionChecker{
+		path:   path,
+		url:    endpoint,
+		idfile: idfile,
 	}
 	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,79 +42,70 @@ 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 {
-		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
-			}
-
-			_, err = fh.WriteString(id)
-			if err != nil {
-				return "", err
-			}
-
-			return "", err
-		} else {
-			return "", err
-		}
+func (v *versionChecker) writeClientUUID() (string, error) {
+	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
 	}
 	defer fh.Close()
-	buf, err := os.ReadFile(filename)
 
+	buf, err := os.ReadFile(filename)
 	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 {
+type verCheckResponse struct {
 	Version string `json:"latest_version"`
+	Error   string `json:"error"`
 }