Dynamically generate available options for -target commandline arg (#…

…2752)

* Add ModuleOption to ModuleManager

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* add support for -modules option

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* refactor so the -modules flag is defied in cortex.go instead of main.go

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Error out if given invalid target name

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Fix tests by defaulting moudules to public

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Update changelog

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Update changelog

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Implemented functional options for RegisterModule

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Remove RegisterModuleWithOption method

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Handled majority of PR comments

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* More PR review comments handling

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* put functional options for modules inside module package

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Update pkg/util/modules/modules.go

Co-authored-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Update pkg/util/modules/modules.go

Co-authored-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Address more PR comments

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Remove unneeded asserts

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Update CHANGELOG.md

Co-authored-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Minimize diff with upstream/master

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Make code more readible

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* avoid creating new type for simple concept

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Update pkg/cortex/cortex.go

Co-authored-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* remove unused import

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

* Resolve more conflict

Signed-off-by: Alvin Lin <alvinlin@amazon.com>

Co-authored-by: Peter Štibraný <pstibrany@gmail.com>

CHANGELOG.md

@@ -22,6 +22,7 @@
   * `cortex_prometheus_notifications_queue_length`
   * `cortex_prometheus_notifications_queue_capacity`
   * `cortex_prometheus_notifications_alertmanagers_discovered`
+* [ENHANCEMENT] Add `-modules` command line flag to list possible values for `-target`. Also, log warning if given target is internal component. #2752
 * [ENHANCEMENT] Added `-ingester.flush-on-shutdown-with-wal-enabled` option to enable chunks flushing even when WAL is enabled. #2780
 * [BUGFIX] Fixed a bug in the index intersect code causing storage to return more chunks/series than required. #2796
 * [BUGFIX] Fixed the number of reported keys in the background cache queue. #2764

cmd/cortex/main.go

@@ -94,7 +94,9 @@ func main() {
 		}
 	}
 
-	if testMode {
+	// Continue on if -modules flag is given. Code handling the
+	// -modules flag will not start cortex.
+	if testMode && !cfg.ListModules {
 		DumpYaml(&cfg)
 		return
 	}
@@ -117,11 +119,15 @@ func main() {
 
 	util.InitEvents(eventSampleRate)
 
-	// Setting the environment variable JAEGER_AGENT_HOST enables tracing
-	if trace, err := tracing.NewFromEnv("cortex-" + cfg.Target); err != nil {
-		level.Error(util.Logger).Log("msg", "Failed to setup tracing", "err", err.Error())
-	} else {
-		defer trace.Close()
+	// In testing mode skip JAEGER setup to avoid panic due to
+	// "duplicate metrics collector registration attempted"
+	if !testMode {
+		// Setting the environment variable JAEGER_AGENT_HOST enables tracing.
+		if trace, err := tracing.NewFromEnv("cortex-" + cfg.Target); err != nil {
+			level.Error(util.Logger).Log("msg", "Failed to setup tracing", "err", err.Error())
+		} else {
+			defer trace.Close()
+		}
 	}
 
 	// Initialise seed for randomness usage.
@@ -130,6 +136,18 @@ func main() {
 	t, err := cortex.New(cfg)
 	util.CheckFatal("initializing cortex", err)
 
+	if t.Cfg.ListModules {
+		for _, m := range t.ModuleManager.UserVisibleModuleNames() {
+			fmt.Fprintln(os.Stdout, m)
+		}
+
+		// in test mode we cannot call os.Exit, it will stop to whole test process.
+		if testMode {
+			return
+		}
+		os.Exit(2)
+	}
+
 	level.Info(util.Logger).Log("msg", "Starting Cortex", "version", version.Info())
 
 	err = t.Run()

cmd/cortex/main_test.go

@@ -63,6 +63,18 @@ func TestFlagParsing(t *testing.T) {
 			stdoutMessage: "target: distributor\n",
 		},
 
+		"user visible module listing": {
+			arguments:      []string{"-modules"},
+			stdoutMessage:  "ingester\n",
+			stderrExcluded: "ingester\n",
+		},
+
+		"user visible module listing flag take precedence over target flag": {
+			arguments:      []string{"-modules", "-target=blah"},
+			stdoutMessage:  "ingester\n",
+			stderrExcluded: "ingester\n",
+		},
+
 		// we cannot test the happy path, as cortex would then fully start
 	} {
 		t.Run(name, func(t *testing.T) {

docs/configuration/config-file-reference.md

@@ -50,8 +50,8 @@ Where default_value is the value to use if the environment variable is undefined
 ### Supported contents and default values of the config file
 
 ```yaml
-# The Cortex service to run. Supported values are: all, distributor, ingester,
-# querier, query-frontend, table-manager, ruler, alertmanager, configs.
+# The Cortex service to run. Use "-modules" command line flag to get a list of
+# available options.
 # CLI flag: -target
 [target: <string> | default = "all"]
 

pkg/cortex/cortex.go

@@ -75,6 +75,7 @@ type Config struct {
 	AuthEnabled bool   `yaml:"auth_enabled"`
 	PrintConfig bool   `yaml:"-"`
 	HTTPPrefix  string `yaml:"http_prefix"`
+	ListModules bool   `yaml:"-"` // No yaml for this, it only works with flags.
 
 	API            api.Config               `yaml:"api"`
 	Server         server.Config            `yaml:"server"`
@@ -109,7 +110,8 @@ type Config struct {
 func (c *Config) RegisterFlags(f *flag.FlagSet) {
 	c.Server.MetricsNamespace = "cortex"
 	c.Server.ExcludeRequestInLog = true
-	f.StringVar(&c.Target, "target", All, "The Cortex service to run. Supported values are: all, distributor, ingester, querier, query-frontend, table-manager, ruler, alertmanager, configs.")
+	f.StringVar(&c.Target, "target", All, "The Cortex service to run. Use \"-modules\" command line flag to get a list of available options.")
+	f.BoolVar(&c.ListModules, "modules", false, "List available values to be use as target. Cannot be used in YAML config.")
 	f.BoolVar(&c.AuthEnabled, "auth.enabled", true, "Set to false to disable auth.")
 	f.BoolVar(&c.PrintConfig, "print.config", false, "Print the config and exit.")
 	f.StringVar(&c.HTTPPrefix, "http.prefix", "/api/prom", "HTTP path prefix for Cortex API.")
@@ -290,6 +292,10 @@ func (t *Cortex) setupThanosTracing() {
 
 // Run starts Cortex running, and blocks until a Cortex stops.
 func (t *Cortex) Run() error {
+	if !t.ModuleManager.IsUserVisibleModule(t.Cfg.Target) {
+		level.Warn(util.Logger).Log("msg", "selected target is an internal module, is this intended?", "target", t.Cfg.Target)
+	}
+
 	serviceMap, err := t.ModuleManager.InitModuleServices(t.Cfg.Target)
 	if err != nil {
 		return err

pkg/cortex/modules.go

@@ -558,19 +558,19 @@ func (t *Cortex) setupModuleManager() error {
 
 	// Register all modules here.
 	// RegisterModule(name string, initFn func()(services.Service, error))
-	mm.RegisterModule(Server, t.initServer)
-	mm.RegisterModule(API, t.initAPI)
-	mm.RegisterModule(RuntimeConfig, t.initRuntimeConfig)
-	mm.RegisterModule(MemberlistKV, t.initMemberlistKV)
-	mm.RegisterModule(Ring, t.initRing)
-	mm.RegisterModule(Overrides, t.initOverrides)
+	mm.RegisterModule(Server, t.initServer, modules.UserInvisibleModule)
+	mm.RegisterModule(API, t.initAPI, modules.UserInvisibleModule)
+	mm.RegisterModule(RuntimeConfig, t.initRuntimeConfig, modules.UserInvisibleModule)
+	mm.RegisterModule(MemberlistKV, t.initMemberlistKV, modules.UserInvisibleModule)
+	mm.RegisterModule(Ring, t.initRing, modules.UserInvisibleModule)
+	mm.RegisterModule(Overrides, t.initOverrides, modules.UserInvisibleModule)
 	mm.RegisterModule(Distributor, t.initDistributor)
-	mm.RegisterModule(Store, t.initChunkStore)
-	mm.RegisterModule(DeleteRequestsStore, t.initDeleteRequestsStore)
+	mm.RegisterModule(Store, t.initChunkStore, modules.UserInvisibleModule)
+	mm.RegisterModule(DeleteRequestsStore, t.initDeleteRequestsStore, modules.UserInvisibleModule)
 	mm.RegisterModule(Ingester, t.initIngester)
 	mm.RegisterModule(Flusher, t.initFlusher)
 	mm.RegisterModule(Querier, t.initQuerier)
-	mm.RegisterModule(StoreQueryable, t.initStoreQueryables)
+	mm.RegisterModule(StoreQueryable, t.initStoreQueryables, modules.UserInvisibleModule)
 	mm.RegisterModule(QueryFrontend, t.initQueryFrontend)
 	mm.RegisterModule(TableManager, t.initTableManager)
 	mm.RegisterModule(Ruler, t.initRuler)
@@ -580,7 +580,6 @@ func (t *Cortex) setupModuleManager() error {
 	mm.RegisterModule(StoreGateway, t.initStoreGateway)
 	mm.RegisterModule(Purger, t.initPurger)
 	mm.RegisterModule(All, nil)
-	mm.RegisterModule(StoreGateway, t.initStoreGateway)
 
 	// Add dependencies
 	deps := map[string][]string{

pkg/util/modules/modules.go

@@ -2,6 +2,7 @@ package modules
 
 import (
 	"fmt"
+	"sort"
 
 	"github.com/pkg/errors"
 
@@ -15,6 +16,9 @@ type module struct {
 
 	// initFn for this module (can return nil)
 	initFn func() (services.Service, error)
+
+	// is this module user visible (i.e intended to be passed to `InitModuleServices`)
+	userVisible bool
 }
 
 // Manager is a component that initialises modules of the application
@@ -23,19 +27,29 @@ type Manager struct {
 	modules map[string]*module
 }
 
+// UserInvisibleModule is an option for `RegisterModule` that marks module not visible to user. Modules are user visible by default.
+func UserInvisibleModule(m *module) {
+	m.userVisible = false
+}
+
 // NewManager creates a new Manager
 func NewManager() *Manager {
 	return &Manager{
 		modules: make(map[string]*module),
 	}
 }
 
-// RegisterModule registers a new module with name and init function
-// name must be unique to avoid overwriting modules
-// if initFn is nil, the module will not initialise
-func (m *Manager) RegisterModule(name string, initFn func() (services.Service, error)) {
+// RegisterModule registers a new module with name, init function, and options. Name must
+// be unique to avoid overwriting modules. If initFn is nil, the module will not initialise.
+// Modules are user visible by default.
+func (m *Manager) RegisterModule(name string, initFn func() (services.Service, error), options ...func(option *module)) {
 	m.modules[name] = &module{
-		initFn: initFn,
+		initFn:      initFn,
+		userVisible: true,
+	}
+
+	for _, o := range options {
+		o(m.modules[name])
 	}
 }
 
@@ -57,6 +71,7 @@ func (m *Manager) InitModuleServices(target string) (map[string]services.Service
 	if _, ok := m.modules[target]; !ok {
 		return nil, fmt.Errorf("unrecognised module name: %s", target)
 	}
+
 	servicesMap := map[string]services.Service{}
 
 	// initialize all of our dependencies first
@@ -89,6 +104,33 @@ func (m *Manager) InitModuleServices(target string) (map[string]services.Service
 	return servicesMap, nil
 }
 
+// UserVisibleModuleNames gets list of module names that are
+// user visible. Returned list is sorted in increasing order.
+func (m *Manager) UserVisibleModuleNames() []string {
+	var result []string
+	for key, val := range m.modules {
+		if val.userVisible {
+			result = append(result, key)
+		}
+	}
+
+	sort.Strings(result)
+
+	return result
+}
+
+// IsUserVisibleModule check if given module is public or not. Returns true
+// if and only if the given module is registered and is public.
+func (m *Manager) IsUserVisibleModule(mod string) bool {
+	val, ok := m.modules[mod]
+
+	if ok {
+		return val.userVisible
+	}
+
+	return false
+}
+
 // listDeps recursively gets a list of dependencies for a passed moduleName
 func (m *Manager) listDeps(mod string) []string {
 	deps := m.modules[mod].deps

pkg/util/modules/modules_test.go

@@ -47,3 +47,82 @@ func TestDependencies(t *testing.T) {
 	assert.Nil(t, svcs)
 	assert.Error(t, err, fmt.Errorf("unrecognised module name: service_unknown"))
 }
+
+func TestRegisterModuleDefaultsToUserVisible(t *testing.T) {
+	sut := NewManager()
+	sut.RegisterModule("module1", mockInitFunc)
+
+	m := sut.modules["module1"]
+
+	assert.NotNil(t, mockInitFunc, m.initFn, "initFn not assigned")
+	assert.True(t, m.userVisible, "module should be user visible")
+}
+
+func TestFunctionalOptAtTheEndWins(t *testing.T) {
+	userVisibleMod := func(option *module) {
+		option.userVisible = true
+	}
+	sut := NewManager()
+	sut.RegisterModule("mod1", mockInitFunc, UserInvisibleModule, userVisibleMod, UserInvisibleModule)
+
+	m := sut.modules["mod1"]
+
+	assert.NotNil(t, mockInitFunc, m.initFn, "initFn not assigned")
+	assert.False(t, m.userVisible, "module should be internal")
+}
+
+func TestGetAllUserVisibleModulesNames(t *testing.T) {
+	sut := NewManager()
+	sut.RegisterModule("userVisible3", mockInitFunc)
+	sut.RegisterModule("userVisible2", mockInitFunc)
+	sut.RegisterModule("userVisible1", mockInitFunc)
+	sut.RegisterModule("internal1", mockInitFunc, UserInvisibleModule)
+	sut.RegisterModule("internal2", mockInitFunc, UserInvisibleModule)
+
+	pm := sut.UserVisibleModuleNames()
+
+	assert.Equal(t, []string{"userVisible1", "userVisible2", "userVisible3"}, pm, "module list contains wrong element and/or not sorted")
+}
+
+func TestGetAllUserVisibleModulesNamesHasNoDupWithDependency(t *testing.T) {
+	sut := NewManager()
+	sut.RegisterModule("userVisible1", mockInitFunc)
+	sut.RegisterModule("userVisible2", mockInitFunc)
+	sut.RegisterModule("userVisible3", mockInitFunc)
+
+	assert.NoError(t, sut.AddDependency("userVisible1", "userVisible2", "userVisible3"))
+
+	pm := sut.UserVisibleModuleNames()
+
+	// make sure we don't include any module twice because there is a dependency
+	assert.Equal(t, []string{"userVisible1", "userVisible2", "userVisible3"}, pm, "module list contains wrong elements and/or not sorted")
+}
+
+func TestGetEmptyListWhenThereIsNoUserVisibleModule(t *testing.T) {
+	sut := NewManager()
+	sut.RegisterModule("internal1", mockInitFunc, UserInvisibleModule)
+	sut.RegisterModule("internal2", mockInitFunc, UserInvisibleModule)
+	sut.RegisterModule("internal3", mockInitFunc, UserInvisibleModule)
+	sut.RegisterModule("internal4", mockInitFunc, UserInvisibleModule)
+
+	pm := sut.UserVisibleModuleNames()
+
+	assert.Len(t, pm, 0, "wrong result slice size")
+}
+
+func TestIsUserVisibleModule(t *testing.T) {
+	userVisibleModName := "userVisible"
+	internalModName := "internal"
+	sut := NewManager()
+	sut.RegisterModule(userVisibleModName, mockInitFunc)
+	sut.RegisterModule(internalModName, mockInitFunc, UserInvisibleModule)
+
+	var result = sut.IsUserVisibleModule(userVisibleModName)
+	assert.True(t, result, "module '%v' should be user visible", userVisibleModName)
+
+	result = sut.IsUserVisibleModule(internalModName)
+	assert.False(t, result, "module '%v' should be internal", internalModName)
+
+	result = sut.IsUserVisibleModule("ghost")
+	assert.False(t, result, "expects result be false when module does not exist")
+}