storage: Move flag out of the package to main.go (#2287)

Move the flag so that it is always initialised after storage providers are imported.

CHANGELOG.md

@@ -13,9 +13,9 @@
     interested in and avoid the Trillian hasher registry+protobuf deps.
  * Moved some packages intended for internal-only use into `internal` packages:
    * InMemoryMerkleTree (indended to only be used by Trillian tests)
- * Move `--quota_system` flag to `main.go` so that it is initialised properly.
-   It might break depending builds relying on this flag, but the fix is easy:
-   put it to your `main.go`.
+ * Moved `--quota_system` and `--storage_system` flags to `main.go` so that they
+   are initialised properly. It might break depending builds relying on these
+   flags. Suggested fix: add the flags to `main.go`.
 
 ## v1.3.11
 [Published 2020-10-06](https://github.com/google/trillian/releases/tag/v1.3.11)

cmd/trillian_log_server/main.go

@@ -75,6 +75,8 @@ var (
 	quotaSystem = flag.String("quota_system", "mysql", fmt.Sprintf("Quota system to use. One of: %v", quota.Providers()))
 	quotaDryRun = flag.Bool("quota_dry_run", false, "If true no requests are blocked due to lack of tokens")
 
+	storageSystem = flag.String("storage_system", "mysql", fmt.Sprintf("Storage system to use. One of: %v", storage.Providers()))
+
 	treeGCEnabled            = flag.Bool("tree_gc", true, "If true, tree garbage collection (hard-deletion) is periodically performed")
 	treeDeleteThreshold      = flag.Duration("tree_delete_threshold", serverutil.DefaultTreeDeleteThreshold, "Minimum period a tree has to remain deleted before being hard-deleted")
 	treeDeleteMinRunInterval = flag.Duration("tree_delete_min_run_interval", serverutil.DefaultTreeDeleteMinInterval, "Minimum interval between tree garbage collection sweeps. Actual runs happen randomly between [minInterval,2*minInterval).")
@@ -115,7 +117,7 @@ func main() {
 		options = append(options, opts...)
 	}
 
-	sp, err := storage.NewProviderFromFlags(mf)
+	sp, err := storage.NewProvider(*storageSystem, mf)
 	if err != nil {
 		glog.Exitf("Failed to get storage provider: %v", err)
 	}

cmd/trillian_log_signer/main.go

@@ -81,6 +81,8 @@ var (
 		"Increase factor for tokens replenished by sequencing-based quotas (1 means a 1:1 relationship between sequenced leaves and replenished tokens)."+
 			"Only effective for --quota_system=etcd.")
 
+	storageSystem = flag.String("storage_system", "mysql", fmt.Sprintf("Storage system to use. One of: %v", storage.Providers()))
+
 	preElectionPause   = flag.Duration("pre_election_pause", 1*time.Second, "Maximum time to wait before starting elections")
 	masterHoldInterval = flag.Duration("master_hold_interval", 60*time.Second, "Minimum interval to hold mastership for")
 	masterHoldJitter   = flag.Duration("master_hold_jitter", 120*time.Second, "Maximal random addition to --master_hold_interval")
@@ -108,7 +110,7 @@ func main() {
 	mf := prometheus.MetricFactory{}
 	monitoring.SetStartSpan(opencensus.StartSpan)
 
-	sp, err := storage.NewProviderFromFlags(mf)
+	sp, err := storage.NewProvider(*storageSystem, mf)
 	if err != nil {
 		glog.Exitf("Failed to get storage provider: %v", err)
 	}

cmd/trillian_map_server/main.go

@@ -71,6 +71,8 @@ var (
 	quotaSystem = flag.String("quota_system", "mysql", fmt.Sprintf("Quota system to use. One of: %v", quota.Providers()))
 	quotaDryRun = flag.Bool("quota_dry_run", false, "If true no requests are blocked due to lack of tokens")
 
+	storageSystem = flag.String("storage_system", "mysql", fmt.Sprintf("Storage system to use. One of: %v", storage.Providers()))
+
 	treeGCEnabled            = flag.Bool("tree_gc", true, "If true, tree garbage collection (hard-deletion) is periodically performed")
 	treeDeleteThreshold      = flag.Duration("tree_delete_threshold", serverutil.DefaultTreeDeleteThreshold, "Minimum period a tree has to remain deleted before being hard-deleted")
 	treeDeleteMinRunInterval = flag.Duration("tree_delete_min_run_interval", serverutil.DefaultTreeDeleteMinInterval, "Minimum interval between tree garbage collection sweeps. Actual runs happen randomly between [minInterval,2*minInterval).")
@@ -111,7 +113,7 @@ func main() {
 		options = append(options, opts...)
 	}
 
-	sp, err := storage.NewProviderFromFlags(mf)
+	sp, err := storage.NewProvider(*storageSystem, mf)
 	if err != nil {
 		glog.Exitf("Failed to get storage provider: %v", err)
 	}

server/storage_provider.go

@@ -36,12 +36,6 @@ type StorageProvider = storage.Provider
 // Deprecated: storage.RegisterProvider should be used directly.
 var RegisterStorageProvider = storage.RegisterProvider
 
-// NewStorageProviderFromFlags returns a new StorageProvider instance of the
-// type specified by flag.
-//
-// Deprecated: storage.NewProviderFromFlags should be used directly.
-var NewStorageProviderFromFlags = storage.NewProviderFromFlags
-
 // NewStorageProvider returns a new StorageProvider instance of the type
 // specified by name.
 //

storage/provider.go

@@ -15,7 +15,6 @@
 package storage
 
 import (
-	"flag"
 	"fmt"
 	"sync"
 
@@ -27,9 +26,6 @@ import (
 type NewProviderFunc func(monitoring.MetricFactory) (Provider, error)
 
 var (
-	// TODO(pavelkalinnikov): Move this flag to main file.
-	storageSystem = flag.String("storage_system", "mysql", fmt.Sprintf("Storage system to use. One of: %v", providers()))
-
 	spMu     sync.RWMutex
 	spByName = make(map[string]NewProviderFunc)
 )
@@ -47,12 +43,6 @@ func RegisterProvider(name string, sp NewProviderFunc) error {
 	return nil
 }
 
-// NewProviderFromFlags returns a new Provider instance of the type
-// specified by flag.
-func NewProviderFromFlags(mf monitoring.MetricFactory) (Provider, error) {
-	return NewProvider(*storageSystem, mf)
-}
-
 // NewProvider returns a new Provider instance of the type specified by name.
 func NewProvider(name string, mf monitoring.MetricFactory) (Provider, error) {
 	spMu.RLock()
@@ -66,8 +56,8 @@ func NewProvider(name string, mf monitoring.MetricFactory) (Provider, error) {
 	return sp(mf)
 }
 
-// providers returns a slice of all registered storage provider names.
-func providers() []string {
+// Providers returns a slice of all registered storage provider names.
+func Providers() []string {
 	spMu.RLock()
 	defer spMu.RUnlock()
 

storage/provider_test.go

@@ -77,7 +77,7 @@ func TestProviders(t *testing.T) {
 	RegisterProvider("b", func(_ monitoring.MetricFactory) (Provider, error) {
 		return &provider{}, nil
 	})
-	sp := providers()
+	sp := Providers()
 
 	if got, want := len(sp), 2; got < want {
 		t.Fatalf("Got %d names, want at least %d", got, want)