From 5dbd3db61be78d7154a3facff5c988e527016458 Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Fri, 19 Jun 2020 13:25:48 -0700 Subject: [PATCH 01/23] Add ModuleOption to ModuleManager Signed-off-by: Alvin Lin --- pkg/util/modules/modules.go | 33 +++++++++++++++ pkg/util/modules/modules_test.go | 70 ++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index b021227c42f..508708a7819 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -15,6 +15,8 @@ type module struct { // initFn for this module (can return nil) initFn func() (services.Service, error) + + option ModuleOption } // Manager is a component that initialises modules of the application @@ -23,6 +25,12 @@ type Manager struct { modules map[string]*module } +// ModuleOption contains options of a module. +// public option indicates if a module can be use as top level module (i.e. pass in as command line argument). +type ModuleOption struct { + public bool +} + // NewManager creates a new Manager func NewManager() *Manager { return &Manager{ @@ -34,8 +42,16 @@ func NewManager() *Manager { // 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)) { + m.RegisterModuleWithOption(name, initFn, defaultModuleOption()) +} + +// RegisterModule registers a new module with name, init function, and option +// name must be unique to avoid overwriting modules +// if initFn is nil, the module will not initialise +func (m *Manager) RegisterModuleWithOption(name string, initFn func() (services.Service, error), option ModuleOption) { m.modules[name] = &module{ initFn: initFn, + option: option, } } @@ -89,6 +105,19 @@ func (m *Manager) InitModuleServices(target string) (map[string]services.Service return servicesMap, nil } +// PublicModuleNames gets list of module names that are +// public module. +func (m *Manager) PublicModuleNames() []string { + var result []string + for key, val := range m.modules { + if val.option.public { + result = append(result, key) + } + } + + return result +} + // listDeps recursively gets a list of dependencies for a passed moduleName func (m *Manager) listDeps(mod string) []string { deps := m.modules[mod].deps @@ -150,3 +179,7 @@ func (m *Manager) findInverseDependencies(mod string, mods []string) []string { return result } + +func defaultModuleOption() ModuleOption { + return ModuleOption{false} +} diff --git a/pkg/util/modules/modules_test.go b/pkg/util/modules/modules_test.go index 3812779c2d2..8053af2b25a 100644 --- a/pkg/util/modules/modules_test.go +++ b/pkg/util/modules/modules_test.go @@ -47,3 +47,73 @@ func TestDependencies(t *testing.T) { assert.Nil(t, svcs) assert.Error(t, err, fmt.Errorf("unrecognised module name: service_unknown")) } + +func TestRegisterModuleWithOptions(t *testing.T) { + option := ModuleOption{ + public: true, + } + sut := NewManager() + sut.RegisterModuleWithOption("module1", mockInitFunc, option) + + m := sut.modules["module1"] + + assert.NotNil(t, mockInitFunc, m.initFn, "initFn not assigned") + assert.Equal(t, option, m.option, "option not assigned") +} + +func TestRegisterModuleSetsDefaultOption(t *testing.T) { + option := ModuleOption{ + public: true, + } + sut := NewManager() + sut.RegisterModule("module1", mockInitFunc) + + m := sut.modules["module1"] + + assert.NotNil(t, option, m.option, "option not assigned") +} + +func TestGetAllPublicModulesNames(t *testing.T) { + sut := NewManager() + sut.RegisterModuleWithOption("public1", mockInitFunc, ModuleOption{public: true}) + sut.RegisterModuleWithOption("public2", mockInitFunc, ModuleOption{public: true}) + sut.RegisterModuleWithOption("public3", mockInitFunc, ModuleOption{public: true}) + sut.RegisterModuleWithOption("private1", mockInitFunc, ModuleOption{public: false}) + sut.RegisterModule("private2", mockInitFunc) + + pm := sut.PublicModuleNames() + + assert.Len(t, pm, 3, "wrong result slice size") + assert.Contains(t, pm, "public1", "missing public module") + assert.Contains(t, pm, "public2", "missing public module") + assert.Contains(t, pm, "public3", "missing public module") +} + +func TestGetAllPublicModulesNamesHasNoDupWithDependency(t *testing.T) { + sut := NewManager() + sut.RegisterModuleWithOption("public1", mockInitFunc, ModuleOption{public: true}) + sut.RegisterModuleWithOption("public2", mockInitFunc, ModuleOption{public: true}) + sut.RegisterModuleWithOption("public3", mockInitFunc, ModuleOption{public: true}) + + assert.NoError(t, sut.AddDependency("public1", "public2", "public3")) + + pm := sut.PublicModuleNames() + + // make sure we don't include any module twice because there is a dependency + assert.Len(t, pm, 3, "wrong result slice size") + assert.Contains(t, pm, "public1", "missing public module") + assert.Contains(t, pm, "public2", "missing public module") + assert.Contains(t, pm, "public3", "missing public module") +} + +func TestGetEmptyListWhenThereIsNoPublicModule(t *testing.T) { + sut := NewManager() + sut.RegisterModuleWithOption("private1", mockInitFunc, ModuleOption{public: false}) + sut.RegisterModuleWithOption("private2", mockInitFunc, ModuleOption{public: false}) + sut.RegisterModuleWithOption("private3", mockInitFunc, ModuleOption{public: false}) + sut.RegisterModuleWithOption("private4", mockInitFunc, ModuleOption{public: false}) + + pm := sut.PublicModuleNames() + + assert.Len(t, pm, 0, "wrong result slice size") +} From 79fd05000dc8a47d3114cab486570275a58c836c Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Fri, 19 Jun 2020 15:33:46 -0700 Subject: [PATCH 02/23] add support for -modules option Signed-off-by: Alvin Lin --- cmd/cortex/main.go | 9 +++++++++ pkg/cortex/modules.go | 28 ++++++++++++++-------------- pkg/util/modules/modules.go | 4 ++-- pkg/util/modules/modules_test.go | 26 +++++++++++++------------- 4 files changed, 38 insertions(+), 29 deletions(-) diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index 72494b2268b..b11a3a02e2f 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -49,6 +49,7 @@ func main() { eventSampleRate int ballastBytes int mutexProfileFraction int + listModules bool ) configFile, expandENV := parseConfigFileParameter(os.Args[1:]) @@ -74,6 +75,7 @@ func main() { flag.IntVar(&eventSampleRate, "event.sample-rate", 0, "How often to sample observability events (0 = never).") flag.IntVar(&ballastBytes, "mem-ballast-size-bytes", 0, "Size of memory ballast to allocate.") flag.IntVar(&mutexProfileFraction, "debug.mutex-profile-fraction", 0, "Fraction at which mutex profile vents will be reported, 0 to disable") + flag.BoolVar(&listModules, "modules", false, "List available values to use as target.") usage := flag.CommandLine.Usage flag.CommandLine.Usage = func() { /* don't do anything by default, we will print usage ourselves, but only when requested. */ } @@ -130,6 +132,13 @@ func main() { t, err := cortex.New(cfg) util.CheckFatal("initializing cortex", err) + if listModules { + for _, m := range t.ModuleManager.PublicModuleNames() { + fmt.Fprintln(flag.CommandLine.Output(), m) + } + os.Exit(2) + } + level.Info(util.Logger).Log("msg", "Starting Cortex", "version", version.Info()) err = t.Run() diff --git a/pkg/cortex/modules.go b/pkg/cortex/modules.go index 4cd1ed61720..dce25521541 100644 --- a/pkg/cortex/modules.go +++ b/pkg/cortex/modules.go @@ -511,7 +511,7 @@ func (t *Cortex) initDataPurger() (services.Service, error) { func (t *Cortex) setupModuleManager() error { mm := modules.NewManager() - + publicModule := modules.ModuleOption{Public: true} // Register all modules here. // RegisterModule(name string, initFn func()(services.Service, error)) mm.RegisterModule(Server, t.initServer) @@ -520,23 +520,23 @@ func (t *Cortex) setupModuleManager() error { mm.RegisterModule(MemberlistKV, t.initMemberlistKV) mm.RegisterModule(Ring, t.initRing) mm.RegisterModule(Overrides, t.initOverrides) - mm.RegisterModule(Distributor, t.initDistributor) mm.RegisterModule(Store, t.initStore) mm.RegisterModule(DeleteRequestsStore, t.initDeleteRequestsStore) - mm.RegisterModule(Ingester, t.initIngester) - mm.RegisterModule(Flusher, t.initFlusher) - mm.RegisterModule(Querier, t.initQuerier) mm.RegisterModule(StoreQueryable, t.initStoreQueryable) - mm.RegisterModule(QueryFrontend, t.initQueryFrontend) - mm.RegisterModule(TableManager, t.initTableManager) - mm.RegisterModule(Ruler, t.initRuler) - mm.RegisterModule(Configs, t.initConfig) - mm.RegisterModule(AlertManager, t.initAlertManager) - mm.RegisterModule(Compactor, t.initCompactor) - mm.RegisterModule(StoreGateway, t.initStoreGateway) - mm.RegisterModule(DataPurger, t.initDataPurger) - mm.RegisterModule(All, nil) mm.RegisterModule(StoreGateway, t.initStoreGateway) + mm.RegisterModuleWithOption(DataPurger, t.initDataPurger, publicModule) + mm.RegisterModuleWithOption(StoreGateway, t.initStoreGateway, publicModule) + mm.RegisterModuleWithOption(Compactor, t.initCompactor, publicModule) + mm.RegisterModuleWithOption(Flusher, t.initFlusher, publicModule) + mm.RegisterModuleWithOption(QueryFrontend, t.initQueryFrontend, publicModule) + mm.RegisterModuleWithOption(TableManager, t.initTableManager, publicModule) + mm.RegisterModuleWithOption(Ruler, t.initRuler, publicModule) + mm.RegisterModuleWithOption(Configs, t.initConfig, publicModule) + mm.RegisterModuleWithOption(AlertManager, t.initAlertManager, publicModule) + mm.RegisterModuleWithOption(Querier, t.initQuerier, publicModule) + mm.RegisterModuleWithOption(Ingester, t.initIngester, publicModule) + mm.RegisterModuleWithOption(Distributor, t.initDistributor, publicModule) + mm.RegisterModuleWithOption(All, nil, publicModule) // Add dependencies deps := map[string][]string{ diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index 508708a7819..a256d1f8940 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -28,7 +28,7 @@ type Manager struct { // ModuleOption contains options of a module. // public option indicates if a module can be use as top level module (i.e. pass in as command line argument). type ModuleOption struct { - public bool + Public bool } // NewManager creates a new Manager @@ -110,7 +110,7 @@ func (m *Manager) InitModuleServices(target string) (map[string]services.Service func (m *Manager) PublicModuleNames() []string { var result []string for key, val := range m.modules { - if val.option.public { + if val.option.Public { result = append(result, key) } } diff --git a/pkg/util/modules/modules_test.go b/pkg/util/modules/modules_test.go index 8053af2b25a..03fb7ff12cd 100644 --- a/pkg/util/modules/modules_test.go +++ b/pkg/util/modules/modules_test.go @@ -50,7 +50,7 @@ func TestDependencies(t *testing.T) { func TestRegisterModuleWithOptions(t *testing.T) { option := ModuleOption{ - public: true, + Public: true, } sut := NewManager() sut.RegisterModuleWithOption("module1", mockInitFunc, option) @@ -63,7 +63,7 @@ func TestRegisterModuleWithOptions(t *testing.T) { func TestRegisterModuleSetsDefaultOption(t *testing.T) { option := ModuleOption{ - public: true, + Public: true, } sut := NewManager() sut.RegisterModule("module1", mockInitFunc) @@ -75,10 +75,10 @@ func TestRegisterModuleSetsDefaultOption(t *testing.T) { func TestGetAllPublicModulesNames(t *testing.T) { sut := NewManager() - sut.RegisterModuleWithOption("public1", mockInitFunc, ModuleOption{public: true}) - sut.RegisterModuleWithOption("public2", mockInitFunc, ModuleOption{public: true}) - sut.RegisterModuleWithOption("public3", mockInitFunc, ModuleOption{public: true}) - sut.RegisterModuleWithOption("private1", mockInitFunc, ModuleOption{public: false}) + sut.RegisterModuleWithOption("public1", mockInitFunc, ModuleOption{Public: true}) + sut.RegisterModuleWithOption("public2", mockInitFunc, ModuleOption{Public: true}) + sut.RegisterModuleWithOption("public3", mockInitFunc, ModuleOption{Public: true}) + sut.RegisterModuleWithOption("private1", mockInitFunc, ModuleOption{Public: false}) sut.RegisterModule("private2", mockInitFunc) pm := sut.PublicModuleNames() @@ -91,9 +91,9 @@ func TestGetAllPublicModulesNames(t *testing.T) { func TestGetAllPublicModulesNamesHasNoDupWithDependency(t *testing.T) { sut := NewManager() - sut.RegisterModuleWithOption("public1", mockInitFunc, ModuleOption{public: true}) - sut.RegisterModuleWithOption("public2", mockInitFunc, ModuleOption{public: true}) - sut.RegisterModuleWithOption("public3", mockInitFunc, ModuleOption{public: true}) + sut.RegisterModuleWithOption("public1", mockInitFunc, ModuleOption{Public: true}) + sut.RegisterModuleWithOption("public2", mockInitFunc, ModuleOption{Public: true}) + sut.RegisterModuleWithOption("public3", mockInitFunc, ModuleOption{Public: true}) assert.NoError(t, sut.AddDependency("public1", "public2", "public3")) @@ -108,10 +108,10 @@ func TestGetAllPublicModulesNamesHasNoDupWithDependency(t *testing.T) { func TestGetEmptyListWhenThereIsNoPublicModule(t *testing.T) { sut := NewManager() - sut.RegisterModuleWithOption("private1", mockInitFunc, ModuleOption{public: false}) - sut.RegisterModuleWithOption("private2", mockInitFunc, ModuleOption{public: false}) - sut.RegisterModuleWithOption("private3", mockInitFunc, ModuleOption{public: false}) - sut.RegisterModuleWithOption("private4", mockInitFunc, ModuleOption{public: false}) + sut.RegisterModuleWithOption("private1", mockInitFunc, ModuleOption{Public: false}) + sut.RegisterModuleWithOption("private2", mockInitFunc, ModuleOption{Public: false}) + sut.RegisterModuleWithOption("private3", mockInitFunc, ModuleOption{Public: false}) + sut.RegisterModuleWithOption("private4", mockInitFunc, ModuleOption{Public: false}) pm := sut.PublicModuleNames() From 84a899d063ab05bd43462b083f16aa63e0690ebc Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Fri, 19 Jun 2020 16:09:11 -0700 Subject: [PATCH 03/23] refactor so the -modules flag is defied in cortex.go instead of main.go Signed-off-by: Alvin Lin --- cmd/cortex/main.go | 4 +--- docs/configuration/config-file-reference.md | 8 ++++++-- pkg/cortex/cortex.go | 4 +++- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index b11a3a02e2f..75b11457c21 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -49,7 +49,6 @@ func main() { eventSampleRate int ballastBytes int mutexProfileFraction int - listModules bool ) configFile, expandENV := parseConfigFileParameter(os.Args[1:]) @@ -75,7 +74,6 @@ func main() { flag.IntVar(&eventSampleRate, "event.sample-rate", 0, "How often to sample observability events (0 = never).") flag.IntVar(&ballastBytes, "mem-ballast-size-bytes", 0, "Size of memory ballast to allocate.") flag.IntVar(&mutexProfileFraction, "debug.mutex-profile-fraction", 0, "Fraction at which mutex profile vents will be reported, 0 to disable") - flag.BoolVar(&listModules, "modules", false, "List available values to use as target.") usage := flag.CommandLine.Usage flag.CommandLine.Usage = func() { /* don't do anything by default, we will print usage ourselves, but only when requested. */ } @@ -132,7 +130,7 @@ func main() { t, err := cortex.New(cfg) util.CheckFatal("initializing cortex", err) - if listModules { + if t.Cfg.ListModules { for _, m := range t.ModuleManager.PublicModuleNames() { fmt.Fprintln(flag.CommandLine.Output(), m) } diff --git a/docs/configuration/config-file-reference.md b/docs/configuration/config-file-reference.md index e55c8774bc1..b0fcfe0bf9d 100644 --- a/docs/configuration/config-file-reference.md +++ b/docs/configuration/config-file-reference.md @@ -49,8 +49,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: | default = "all"] @@ -62,6 +62,10 @@ Where default_value is the value to use if the environment variable is undefined # CLI flag: -http.prefix [http_prefix: | default = "/api/prom"] +# List available values for target. +# CLI flag: -modules +[listmodules: | default = false] + api: # HTTP URL path under which the Alertmanager ui and api will be served. # CLI flag: -http.alertmanager-http-prefix diff --git a/pkg/cortex/cortex.go b/pkg/cortex/cortex.go index 4c6faa4e105..52dbd871a81 100644 --- a/pkg/cortex/cortex.go +++ b/pkg/cortex/cortex.go @@ -76,6 +76,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"` @@ -110,7 +111,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.") From fa062f9810b8bcd79eb2ba906d0eaa970e8a83aa Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Fri, 19 Jun 2020 16:47:28 -0700 Subject: [PATCH 04/23] Error out if given invalid target name Signed-off-by: Alvin Lin --- docs/configuration/config-file-reference.md | 4 ---- pkg/cortex/cortex.go | 2 +- pkg/cortex/cortex_test.go | 13 +++++++++++++ pkg/util/modules/modules.go | 6 ++++++ 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/docs/configuration/config-file-reference.md b/docs/configuration/config-file-reference.md index b0fcfe0bf9d..71724b3ec01 100644 --- a/docs/configuration/config-file-reference.md +++ b/docs/configuration/config-file-reference.md @@ -62,10 +62,6 @@ Where default_value is the value to use if the environment variable is undefined # CLI flag: -http.prefix [http_prefix: | default = "/api/prom"] -# List available values for target. -# CLI flag: -modules -[listmodules: | default = false] - api: # HTTP URL path under which the Alertmanager ui and api will be served. # CLI flag: -http.alertmanager-http-prefix diff --git a/pkg/cortex/cortex.go b/pkg/cortex/cortex.go index 52dbd871a81..5b8f9ad5e44 100644 --- a/pkg/cortex/cortex.go +++ b/pkg/cortex/cortex.go @@ -76,7 +76,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. + ListModules bool `yaml:"-"` // No yaml for this, it only works with flags. API api.Config `yaml:"api"` Server server.Config `yaml:"server"` diff --git a/pkg/cortex/cortex_test.go b/pkg/cortex/cortex_test.go index 14c44e21a27..526e1b577d3 100644 --- a/pkg/cortex/cortex_test.go +++ b/pkg/cortex/cortex_test.go @@ -68,3 +68,16 @@ func TestCortex(t *testing.T) { require.NotNil(t, serviceMap[Ring]) require.NotNil(t, serviceMap[Distributor]) } + +func TestGivenBadTarget(t *testing.T) { + cfg := Config{ + Target: Ring, + } + + c, err := New(cfg) + require.NoError(t, err) + require.NotNil(t, c) + + require.Error(t, c.Run()) + +} diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index a256d1f8940..4b9c09f8b0b 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -2,6 +2,7 @@ package modules import ( "fmt" + "strings" "github.com/pkg/errors" @@ -73,6 +74,11 @@ 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) } + + if !m.modules[target].option.Public { + return nil, fmt.Errorf("invalid target: %s; valid targets are: %s", target, strings.Join(m.PublicModuleNames(), ", ")) + } + servicesMap := map[string]services.Service{} // initialize all of our dependencies first From 1bb501094347826287703dd7660e103386b9a071 Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Fri, 19 Jun 2020 22:48:05 -0700 Subject: [PATCH 05/23] Fix tests by defaulting moudules to public Signed-off-by: Alvin Lin --- pkg/cortex/cortex_test.go | 7 ++--- pkg/cortex/modules.go | 46 ++++++++++++++++---------------- pkg/util/modules/modules.go | 4 +-- pkg/util/modules/modules_test.go | 18 +++++++++---- 4 files changed, 42 insertions(+), 33 deletions(-) diff --git a/pkg/cortex/cortex_test.go b/pkg/cortex/cortex_test.go index 526e1b577d3..5055150d457 100644 --- a/pkg/cortex/cortex_test.go +++ b/pkg/cortex/cortex_test.go @@ -69,15 +69,16 @@ func TestCortex(t *testing.T) { require.NotNil(t, serviceMap[Distributor]) } -func TestGivenBadTarget(t *testing.T) { +func TestGivenNonPublicTarget(t *testing.T) { cfg := Config{ - Target: Ring, + Target: API, } c, err := New(cfg) require.NoError(t, err) require.NotNil(t, c) - require.Error(t, c.Run()) + _, err2 := c.ModuleManager.InitModuleServices(API) + require.Error(t, err2) } diff --git a/pkg/cortex/modules.go b/pkg/cortex/modules.go index dce25521541..ff3b3e4336b 100644 --- a/pkg/cortex/modules.go +++ b/pkg/cortex/modules.go @@ -511,32 +511,32 @@ func (t *Cortex) initDataPurger() (services.Service, error) { func (t *Cortex) setupModuleManager() error { mm := modules.NewManager() - publicModule := modules.ModuleOption{Public: true} + privateModule := modules.ModuleOption{Public: false} // 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(Store, t.initStore) - mm.RegisterModule(DeleteRequestsStore, t.initDeleteRequestsStore) - mm.RegisterModule(StoreQueryable, t.initStoreQueryable) + mm.RegisterModuleWithOption(Server, t.initServer, privateModule) + mm.RegisterModuleWithOption(API, t.initAPI, privateModule) + mm.RegisterModuleWithOption(RuntimeConfig, t.initRuntimeConfig, privateModule) + mm.RegisterModuleWithOption(MemberlistKV, t.initMemberlistKV, privateModule) + mm.RegisterModuleWithOption(Ring, t.initRing, privateModule) + mm.RegisterModuleWithOption(Overrides, t.initOverrides, privateModule) + mm.RegisterModuleWithOption(Store, t.initStore, privateModule) + mm.RegisterModuleWithOption(DeleteRequestsStore, t.initDeleteRequestsStore, privateModule) + mm.RegisterModuleWithOption(StoreQueryable, t.initStoreQueryable, privateModule) + mm.RegisterModuleWithOption(StoreGateway, t.initStoreGateway, privateModule) + mm.RegisterModule(DataPurger, t.initDataPurger) mm.RegisterModule(StoreGateway, t.initStoreGateway) - mm.RegisterModuleWithOption(DataPurger, t.initDataPurger, publicModule) - mm.RegisterModuleWithOption(StoreGateway, t.initStoreGateway, publicModule) - mm.RegisterModuleWithOption(Compactor, t.initCompactor, publicModule) - mm.RegisterModuleWithOption(Flusher, t.initFlusher, publicModule) - mm.RegisterModuleWithOption(QueryFrontend, t.initQueryFrontend, publicModule) - mm.RegisterModuleWithOption(TableManager, t.initTableManager, publicModule) - mm.RegisterModuleWithOption(Ruler, t.initRuler, publicModule) - mm.RegisterModuleWithOption(Configs, t.initConfig, publicModule) - mm.RegisterModuleWithOption(AlertManager, t.initAlertManager, publicModule) - mm.RegisterModuleWithOption(Querier, t.initQuerier, publicModule) - mm.RegisterModuleWithOption(Ingester, t.initIngester, publicModule) - mm.RegisterModuleWithOption(Distributor, t.initDistributor, publicModule) - mm.RegisterModuleWithOption(All, nil, publicModule) + mm.RegisterModule(Compactor, t.initCompactor) + mm.RegisterModule(Flusher, t.initFlusher) + mm.RegisterModule(QueryFrontend, t.initQueryFrontend) + mm.RegisterModule(TableManager, t.initTableManager) + mm.RegisterModule(Ruler, t.initRuler) + mm.RegisterModule(Configs, t.initConfig) + mm.RegisterModule(AlertManager, t.initAlertManager) + mm.RegisterModule(Querier, t.initQuerier) + mm.RegisterModule(Ingester, t.initIngester) + mm.RegisterModule(Distributor, t.initDistributor) + mm.RegisterModule(All, nil) // Add dependencies deps := map[string][]string{ diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index 4b9c09f8b0b..7278e1fe5fc 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -39,7 +39,7 @@ func NewManager() *Manager { } } -// RegisterModule registers a new module with name and init function +// RegisterModule registers a new module with name and init function. By default a module is public. // 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)) { @@ -187,5 +187,5 @@ func (m *Manager) findInverseDependencies(mod string, mods []string) []string { } func defaultModuleOption() ModuleOption { - return ModuleOption{false} + return ModuleOption{true} // default to public module to keep backward compatibility } diff --git a/pkg/util/modules/modules_test.go b/pkg/util/modules/modules_test.go index 03fb7ff12cd..68ce1204c28 100644 --- a/pkg/util/modules/modules_test.go +++ b/pkg/util/modules/modules_test.go @@ -48,6 +48,14 @@ func TestDependencies(t *testing.T) { assert.Error(t, err, fmt.Errorf("unrecognised module name: service_unknown")) } +func TestCannotInitNonPublicModule(t *testing.T) { + sut := NewManager() + sut.RegisterModuleWithOption("module1", mockInitFunc, ModuleOption{Public: false}) + + _, err := sut.InitModuleServices("module1") + assert.Error(t, err, "Expect error when init private module") +} + func TestRegisterModuleWithOptions(t *testing.T) { option := ModuleOption{ Public: true, @@ -75,11 +83,11 @@ func TestRegisterModuleSetsDefaultOption(t *testing.T) { func TestGetAllPublicModulesNames(t *testing.T) { sut := NewManager() - sut.RegisterModuleWithOption("public1", mockInitFunc, ModuleOption{Public: true}) + sut.RegisterModule("public1", mockInitFunc) sut.RegisterModuleWithOption("public2", mockInitFunc, ModuleOption{Public: true}) sut.RegisterModuleWithOption("public3", mockInitFunc, ModuleOption{Public: true}) sut.RegisterModuleWithOption("private1", mockInitFunc, ModuleOption{Public: false}) - sut.RegisterModule("private2", mockInitFunc) + sut.RegisterModuleWithOption("private2", mockInitFunc, ModuleOption{Public: false}) pm := sut.PublicModuleNames() @@ -91,9 +99,9 @@ func TestGetAllPublicModulesNames(t *testing.T) { func TestGetAllPublicModulesNamesHasNoDupWithDependency(t *testing.T) { sut := NewManager() - sut.RegisterModuleWithOption("public1", mockInitFunc, ModuleOption{Public: true}) - sut.RegisterModuleWithOption("public2", mockInitFunc, ModuleOption{Public: true}) - sut.RegisterModuleWithOption("public3", mockInitFunc, ModuleOption{Public: true}) + sut.RegisterModule("public1", mockInitFunc) + sut.RegisterModule("public2", mockInitFunc) + sut.RegisterModule("public3", mockInitFunc) assert.NoError(t, sut.AddDependency("public1", "public2", "public3")) From 016e180f0d647b987b77b721e0efef4032eefd4d Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Fri, 19 Jun 2020 23:10:44 -0700 Subject: [PATCH 06/23] Update changelog Signed-off-by: Alvin Lin --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c3596e4d3e..e91745e3eba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ * [FEATURE] TLS config options added for GRPC clients in Querier (Query-frontend client & Ingester client), Ruler, Store Gateway, as well as HTTP client in Config store client. #2502 * [FEATURE] The flag `-frontend.max-cache-freshness` is now supported within the limits overrides, to specify per-tenant max cache freshness values. The corresponding YAML config parameter has been changed from `results_cache.max_freshness` to `limits_config.max_cache_freshness`. The legacy YAML config parameter (`results_cache.max_freshness`) will continue to be supported till Cortex release `v1.4.0`. #2609 * [FEATURE] Experimental gRPC Store: Added support to 3rd parties index and chunk stores using gRPC client/server plugin mechanism. #2220 +* [ENHANCEMENT] Add command line flag to list possible values for -target. Also throw error if the given target is not public. #2710 * [ENHANCEMENT] Propagate GOPROXY value when building `build-image`. This is to help the builders building the code in a Network where default Go proxy is not accessible (e.g. when behind some corporate VPN). #2741 * [ENHANCEMENT] Querier: Added metric `cortex_querier_request_duration_seconds` for all requests to the querier. #2708 * [ENHANCEMENT] Experimental TSDB: added the following metrics to the ingester: #2580 #2583 #2589 #2654 From b8f80218aa5f1777c9c3cf72bbb3ee4af1339212 Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Fri, 19 Jun 2020 23:11:17 -0700 Subject: [PATCH 07/23] Update changelog Signed-off-by: Alvin Lin --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e91745e3eba..49ee776d3e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,7 @@ * [FEATURE] TLS config options added for GRPC clients in Querier (Query-frontend client & Ingester client), Ruler, Store Gateway, as well as HTTP client in Config store client. #2502 * [FEATURE] The flag `-frontend.max-cache-freshness` is now supported within the limits overrides, to specify per-tenant max cache freshness values. The corresponding YAML config parameter has been changed from `results_cache.max_freshness` to `limits_config.max_cache_freshness`. The legacy YAML config parameter (`results_cache.max_freshness`) will continue to be supported till Cortex release `v1.4.0`. #2609 * [FEATURE] Experimental gRPC Store: Added support to 3rd parties index and chunk stores using gRPC client/server plugin mechanism. #2220 -* [ENHANCEMENT] Add command line flag to list possible values for -target. Also throw error if the given target is not public. #2710 +* [ENHANCEMENT] Add command line flag to list possible values for -target. Also, throw error if the given target is not public. #2710 * [ENHANCEMENT] Propagate GOPROXY value when building `build-image`. This is to help the builders building the code in a Network where default Go proxy is not accessible (e.g. when behind some corporate VPN). #2741 * [ENHANCEMENT] Querier: Added metric `cortex_querier_request_duration_seconds` for all requests to the querier. #2708 * [ENHANCEMENT] Experimental TSDB: added the following metrics to the ingester: #2580 #2583 #2589 #2654 From e92692f1df3fc991a3f1ec11c9958920756751ce Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Mon, 22 Jun 2020 22:11:02 -0700 Subject: [PATCH 08/23] Implemented functional options for RegisterModule Signed-off-by: Alvin Lin --- pkg/util/modules/modules.go | 11 ++++-- pkg/util/modules/modules_test.go | 59 +++++++++++++++++++++++--------- 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index 7278e1fe5fc..44714459de7 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -42,8 +42,15 @@ func NewManager() *Manager { // RegisterModule registers a new module with name and init function. By default a module is public. // 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)) { - m.RegisterModuleWithOption(name, initFn, defaultModuleOption()) +func (m *Manager) RegisterModule(name string, initFn func() (services.Service, error), options ...func(option *ModuleOption)) { + m.modules[name] = &module{ + initFn: initFn, + option: defaultModuleOption(), + } + + for _, o := range options { + o(&(m.modules[name].option)) + } } // RegisterModule registers a new module with name, init function, and option diff --git a/pkg/util/modules/modules_test.go b/pkg/util/modules/modules_test.go index 68ce1204c28..5b71e22e972 100644 --- a/pkg/util/modules/modules_test.go +++ b/pkg/util/modules/modules_test.go @@ -50,44 +50,66 @@ func TestDependencies(t *testing.T) { func TestCannotInitNonPublicModule(t *testing.T) { sut := NewManager() - sut.RegisterModuleWithOption("module1", mockInitFunc, ModuleOption{Public: false}) + privateMod := func(option *ModuleOption) { + option.Public = false + } + sut.RegisterModule("module1", mockInitFunc, privateMod) _, err := sut.InitModuleServices("module1") assert.Error(t, err, "Expect error when init private module") } func TestRegisterModuleWithOptions(t *testing.T) { - option := ModuleOption{ - Public: true, + publicMod := func(option *ModuleOption) { + option.Public = true } sut := NewManager() - sut.RegisterModuleWithOption("module1", mockInitFunc, option) + sut.RegisterModule("module1", mockInitFunc, publicMod) m := sut.modules["module1"] assert.NotNil(t, mockInitFunc, m.initFn, "initFn not assigned") - assert.Equal(t, option, m.option, "option not assigned") + assert.Equal(t, true, m.option.Public, "option not assigned") } func TestRegisterModuleSetsDefaultOption(t *testing.T) { - option := ModuleOption{ - Public: true, - } sut := NewManager() sut.RegisterModule("module1", mockInitFunc) m := sut.modules["module1"] - assert.NotNil(t, option, m.option, "option not assigned") + assert.NotNil(t, true, m.option.Public, "option not assigned") +} + +func TestFunctionalOptAtTheEndWins(t *testing.T) { + privateMod := func(option *ModuleOption) { + option.Public = false + } + publicMod := func(option *ModuleOption) { + option.Public = false + } + sut := NewManager() + sut.RegisterModule("mod1", mockInitFunc, privateMod, publicMod, privateMod) + + m := sut.modules["mod1"] + + assert.NotNil(t, mockInitFunc, m.initFn, "initFn not assigned") + assert.Equal(t, false, m.option.Public, "option not assigned") } func TestGetAllPublicModulesNames(t *testing.T) { + publicMod := func(option *ModuleOption) { + option.Public = true + } + privateMod := func(option *ModuleOption) { + option.Public = false + } sut := NewManager() sut.RegisterModule("public1", mockInitFunc) - sut.RegisterModuleWithOption("public2", mockInitFunc, ModuleOption{Public: true}) - sut.RegisterModuleWithOption("public3", mockInitFunc, ModuleOption{Public: true}) - sut.RegisterModuleWithOption("private1", mockInitFunc, ModuleOption{Public: false}) - sut.RegisterModuleWithOption("private2", mockInitFunc, ModuleOption{Public: false}) + sut.RegisterModule("public2", mockInitFunc, publicMod) + sut.RegisterModule("public3", mockInitFunc, publicMod) + sut.RegisterModule("private1", mockInitFunc, privateMod) + sut.RegisterModule("private2", mockInitFunc, privateMod) pm := sut.PublicModuleNames() @@ -115,11 +137,14 @@ func TestGetAllPublicModulesNamesHasNoDupWithDependency(t *testing.T) { } func TestGetEmptyListWhenThereIsNoPublicModule(t *testing.T) { + privateMod := func(option *ModuleOption) { + option.Public = false + } sut := NewManager() - sut.RegisterModuleWithOption("private1", mockInitFunc, ModuleOption{Public: false}) - sut.RegisterModuleWithOption("private2", mockInitFunc, ModuleOption{Public: false}) - sut.RegisterModuleWithOption("private3", mockInitFunc, ModuleOption{Public: false}) - sut.RegisterModuleWithOption("private4", mockInitFunc, ModuleOption{Public: false}) + sut.RegisterModule("private1", mockInitFunc, privateMod) + sut.RegisterModule("private2", mockInitFunc, privateMod) + sut.RegisterModule("private3", mockInitFunc, privateMod) + sut.RegisterModule("private4", mockInitFunc, privateMod) pm := sut.PublicModuleNames() From 726f292602a5c9d8d3c61751c4e96721c9fa05ce Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Mon, 22 Jun 2020 22:26:20 -0700 Subject: [PATCH 09/23] Remove RegisterModuleWithOption method Signed-off-by: Alvin Lin --- pkg/cortex/modules.go | 24 +++++++++++++----------- pkg/util/modules/modules.go | 13 ++----------- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/pkg/cortex/modules.go b/pkg/cortex/modules.go index ff3b3e4336b..ffcc8b492fc 100644 --- a/pkg/cortex/modules.go +++ b/pkg/cortex/modules.go @@ -511,19 +511,21 @@ func (t *Cortex) initDataPurger() (services.Service, error) { func (t *Cortex) setupModuleManager() error { mm := modules.NewManager() - privateModule := modules.ModuleOption{Public: false} + privateModule := func(option *modules.ModuleOption) { + option.Public = false + } // Register all modules here. // RegisterModule(name string, initFn func()(services.Service, error)) - mm.RegisterModuleWithOption(Server, t.initServer, privateModule) - mm.RegisterModuleWithOption(API, t.initAPI, privateModule) - mm.RegisterModuleWithOption(RuntimeConfig, t.initRuntimeConfig, privateModule) - mm.RegisterModuleWithOption(MemberlistKV, t.initMemberlistKV, privateModule) - mm.RegisterModuleWithOption(Ring, t.initRing, privateModule) - mm.RegisterModuleWithOption(Overrides, t.initOverrides, privateModule) - mm.RegisterModuleWithOption(Store, t.initStore, privateModule) - mm.RegisterModuleWithOption(DeleteRequestsStore, t.initDeleteRequestsStore, privateModule) - mm.RegisterModuleWithOption(StoreQueryable, t.initStoreQueryable, privateModule) - mm.RegisterModuleWithOption(StoreGateway, t.initStoreGateway, privateModule) + mm.RegisterModule(Server, t.initServer, privateModule) + mm.RegisterModule(API, t.initAPI, privateModule) + mm.RegisterModule(RuntimeConfig, t.initRuntimeConfig, privateModule) + mm.RegisterModule(MemberlistKV, t.initMemberlistKV, privateModule) + mm.RegisterModule(Ring, t.initRing, privateModule) + mm.RegisterModule(Overrides, t.initOverrides, privateModule) + mm.RegisterModule(Store, t.initStore, privateModule) + mm.RegisterModule(DeleteRequestsStore, t.initDeleteRequestsStore, privateModule) + mm.RegisterModule(StoreQueryable, t.initStoreQueryable, privateModule) + mm.RegisterModule(StoreGateway, t.initStoreGateway, privateModule) mm.RegisterModule(DataPurger, t.initDataPurger) mm.RegisterModule(StoreGateway, t.initStoreGateway) mm.RegisterModule(Compactor, t.initCompactor) diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index 44714459de7..f30a6d96a7e 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -39,9 +39,10 @@ func NewManager() *Manager { } } -// RegisterModule registers a new module with name and init function. By default a module is public. +// RegisterModule registers a new module with name, init function, and option functions. // name must be unique to avoid overwriting modules // if initFn is nil, the module will not initialise +// if options is not given, then by default module is public func (m *Manager) RegisterModule(name string, initFn func() (services.Service, error), options ...func(option *ModuleOption)) { m.modules[name] = &module{ initFn: initFn, @@ -53,16 +54,6 @@ func (m *Manager) RegisterModule(name string, initFn func() (services.Service, e } } -// RegisterModule registers a new module with name, init function, and option -// name must be unique to avoid overwriting modules -// if initFn is nil, the module will not initialise -func (m *Manager) RegisterModuleWithOption(name string, initFn func() (services.Service, error), option ModuleOption) { - m.modules[name] = &module{ - initFn: initFn, - option: option, - } -} - // AddDependency adds a dependency from name(source) to dependsOn(targets) // An error is returned if the source module name is not found func (m *Manager) AddDependency(name string, dependsOn ...string) error { From 648ada6eddac7ab78511d2c5c7c6d9adc81a08a9 Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Mon, 22 Jun 2020 23:45:17 -0700 Subject: [PATCH 10/23] Handled majority of PR comments Signed-off-by: Alvin Lin --- cmd/cortex/main.go | 2 +- pkg/cortex/cortex.go | 7 ++++ pkg/cortex/cortex_test.go | 14 -------- pkg/util/modules/modules.go | 24 ++++++++++---- pkg/util/modules/modules_test.go | 55 ++++++++++++++++++++++++-------- 5 files changed, 66 insertions(+), 36 deletions(-) diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index 75b11457c21..c1481384045 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -132,7 +132,7 @@ func main() { if t.Cfg.ListModules { for _, m := range t.ModuleManager.PublicModuleNames() { - fmt.Fprintln(flag.CommandLine.Output(), m) + fmt.Fprintln(os.Stdout, m) } os.Exit(2) } diff --git a/pkg/cortex/cortex.go b/pkg/cortex/cortex.go index 5b8f9ad5e44..0ef753a6958 100644 --- a/pkg/cortex/cortex.go +++ b/pkg/cortex/cortex.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "os" + "strings" "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" @@ -293,6 +294,12 @@ func (t *Cortex) setupThanosTracing() { // Run starts Cortex running, and blocks until a Cortex stops. func (t *Cortex) Run() error { + if !t.ModuleManager.IsPublicModule(t.Cfg.Target) { + level.Warn(util.Logger).Log( + "msg", fmt.Sprintf("'%v' is not a public module, is this intended?", t.Cfg.Target), + "public-modules", strings.Join(t.ModuleManager.PublicModuleNames(), ", ")) + } + serviceMap, err := t.ModuleManager.InitModuleServices(t.Cfg.Target) if err != nil { return err diff --git a/pkg/cortex/cortex_test.go b/pkg/cortex/cortex_test.go index 5055150d457..14c44e21a27 100644 --- a/pkg/cortex/cortex_test.go +++ b/pkg/cortex/cortex_test.go @@ -68,17 +68,3 @@ func TestCortex(t *testing.T) { require.NotNil(t, serviceMap[Ring]) require.NotNil(t, serviceMap[Distributor]) } - -func TestGivenNonPublicTarget(t *testing.T) { - cfg := Config{ - Target: API, - } - - c, err := New(cfg) - require.NoError(t, err) - require.NotNil(t, c) - - _, err2 := c.ModuleManager.InitModuleServices(API) - require.Error(t, err2) - -} diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index f30a6d96a7e..84ab9673991 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -2,7 +2,7 @@ package modules import ( "fmt" - "strings" + "sort" "github.com/pkg/errors" @@ -50,7 +50,7 @@ func (m *Manager) RegisterModule(name string, initFn func() (services.Service, e } for _, o := range options { - o(&(m.modules[name].option)) + o(&m.modules[name].option) } } @@ -73,10 +73,6 @@ func (m *Manager) InitModuleServices(target string) (map[string]services.Service return nil, fmt.Errorf("unrecognised module name: %s", target) } - if !m.modules[target].option.Public { - return nil, fmt.Errorf("invalid target: %s; valid targets are: %s", target, strings.Join(m.PublicModuleNames(), ", ")) - } - servicesMap := map[string]services.Service{} // initialize all of our dependencies first @@ -119,9 +115,23 @@ func (m *Manager) PublicModuleNames() []string { } } + sort.Strings(result) + return result } +// IsPublicModule 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) IsPublicModule(mod string) bool { + val, ok := m.modules[mod] + + if ok { + return val.option.Public + } + + 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 @@ -185,5 +195,5 @@ func (m *Manager) findInverseDependencies(mod string, mods []string) []string { } func defaultModuleOption() ModuleOption { - return ModuleOption{true} // default to public module to keep backward compatibility + return ModuleOption{true} // default to public module to keep backward compatibility } diff --git a/pkg/util/modules/modules_test.go b/pkg/util/modules/modules_test.go index 5b71e22e972..f9ace54b7de 100644 --- a/pkg/util/modules/modules_test.go +++ b/pkg/util/modules/modules_test.go @@ -48,17 +48,6 @@ func TestDependencies(t *testing.T) { assert.Error(t, err, fmt.Errorf("unrecognised module name: service_unknown")) } -func TestCannotInitNonPublicModule(t *testing.T) { - sut := NewManager() - privateMod := func(option *ModuleOption) { - option.Public = false - } - sut.RegisterModule("module1", mockInitFunc, privateMod) - - _, err := sut.InitModuleServices("module1") - assert.Error(t, err, "Expect error when init private module") -} - func TestRegisterModuleWithOptions(t *testing.T) { publicMod := func(option *ModuleOption) { option.Public = true @@ -69,7 +58,7 @@ func TestRegisterModuleWithOptions(t *testing.T) { m := sut.modules["module1"] assert.NotNil(t, mockInitFunc, m.initFn, "initFn not assigned") - assert.Equal(t, true, m.option.Public, "option not assigned") + assert.True(t, m.option.Public, "module should be public") } func TestRegisterModuleSetsDefaultOption(t *testing.T) { @@ -78,7 +67,7 @@ func TestRegisterModuleSetsDefaultOption(t *testing.T) { m := sut.modules["module1"] - assert.NotNil(t, true, m.option.Public, "option not assigned") + assert.True(t, m.option.Public, "mould should be public") } func TestFunctionalOptAtTheEndWins(t *testing.T) { @@ -94,7 +83,7 @@ func TestFunctionalOptAtTheEndWins(t *testing.T) { m := sut.modules["mod1"] assert.NotNil(t, mockInitFunc, m.initFn, "initFn not assigned") - assert.Equal(t, false, m.option.Public, "option not assigned") + assert.False(t, m.option.Public, "module should be public") } func TestGetAllPublicModulesNames(t *testing.T) { @@ -150,3 +139,41 @@ func TestGetEmptyListWhenThereIsNoPublicModule(t *testing.T) { assert.Len(t, pm, 0, "wrong result slice size") } + +func TestPublicModuleNamesReturnsSortedList(t *testing.T) { + publicMod := func(option *ModuleOption) { + option.Public = true + } + sut := NewManager() + sut.RegisterModule("c", mockInitFunc, publicMod) + sut.RegisterModule("b", mockInitFunc, publicMod) + sut.RegisterModule("a", mockInitFunc, publicMod) + + pm := sut.PublicModuleNames() + + assert.Len(t, pm, 3, "wrong result slice size") + assert.Equal(t, []string{"a", "b", "c"}, pm, "module names list is not sorted in ascending order") +} + +func TestIsPublicModule(t *testing.T) { + publicMod := func(option *ModuleOption) { + option.Public = true + } + privateMod := func(option *ModuleOption) { + option.Public = false + } + pubModName := "public" + privateModName := "private" + sut := NewManager() + sut.RegisterModule(pubModName, mockInitFunc, publicMod) + sut.RegisterModule(privateModName, mockInitFunc, privateMod) + + var result = sut.IsPublicModule(pubModName) + assert.True(t, result, "module '%v' should be public", pubModName) + + result = sut.IsPublicModule(privateModName) + assert.False(t, result, "module '%v' should be private", privateModName) + + result = sut.IsPublicModule("ghost") + assert.False(t, result, "expects result be false when module does not exist") +} From 954ad5e8c545c12dafe1b0328acca78ff0a4a961 Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Tue, 23 Jun 2020 01:23:48 -0700 Subject: [PATCH 11/23] More PR review comments handling Signed-off-by: Alvin Lin --- CHANGELOG.md | 2 +- cmd/cortex/main.go | 23 +++++++++++++++++------ cmd/cortex/main_test.go | 12 ++++++++++++ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49ee776d3e9..ca20c462b07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,7 @@ * [FEATURE] TLS config options added for GRPC clients in Querier (Query-frontend client & Ingester client), Ruler, Store Gateway, as well as HTTP client in Config store client. #2502 * [FEATURE] The flag `-frontend.max-cache-freshness` is now supported within the limits overrides, to specify per-tenant max cache freshness values. The corresponding YAML config parameter has been changed from `results_cache.max_freshness` to `limits_config.max_cache_freshness`. The legacy YAML config parameter (`results_cache.max_freshness`) will continue to be supported till Cortex release `v1.4.0`. #2609 * [FEATURE] Experimental gRPC Store: Added support to 3rd parties index and chunk stores using gRPC client/server plugin mechanism. #2220 -* [ENHANCEMENT] Add command line flag to list possible values for -target. Also, throw error if the given target is not public. #2710 +* [ENHANCEMENT] Add command line flag to list possible values for -target. Also, log warning if given target is not public. #2710 * [ENHANCEMENT] Propagate GOPROXY value when building `build-image`. This is to help the builders building the code in a Network where default Go proxy is not accessible (e.g. when behind some corporate VPN). #2741 * [ENHANCEMENT] Querier: Added metric `cortex_querier_request_duration_seconds` for all requests to the querier. #2708 * [ENHANCEMENT] Experimental TSDB: added the following metrics to the ingester: #2580 #2583 #2589 #2654 diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index c1481384045..5f83f6de2a8 100644 --- a/cmd/cortex/main.go +++ b/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. @@ -134,6 +140,11 @@ func main() { for _, m := range t.ModuleManager.PublicModuleNames() { 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) } diff --git a/cmd/cortex/main_test.go b/cmd/cortex/main_test.go index bbe042f9e7c..f5c41cd8778 100644 --- a/cmd/cortex/main_test.go +++ b/cmd/cortex/main_test.go @@ -63,6 +63,18 @@ func TestFlagParsing(t *testing.T) { stdoutMessage: "target: distributor\n", }, + "public module listing": { + arguments: []string{"-modules"}, + stdoutMessage: "ingester\n", + stderrExcluded: "ingester\n", + }, + + "public 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) { From 0e6f54c877baa91b7672f0653233ea4734108ad4 Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Tue, 23 Jun 2020 23:32:12 -0700 Subject: [PATCH 12/23] put functional options for modules inside module package Signed-off-by: Alvin Lin --- pkg/cortex/modules.go | 24 ++++++------ pkg/util/modules/modules.go | 25 ++++++------- pkg/util/modules/modules_test.go | 64 ++++++++++---------------------- 3 files changed, 42 insertions(+), 71 deletions(-) diff --git a/pkg/cortex/modules.go b/pkg/cortex/modules.go index ffcc8b492fc..5a138fa2c74 100644 --- a/pkg/cortex/modules.go +++ b/pkg/cortex/modules.go @@ -511,21 +511,19 @@ func (t *Cortex) initDataPurger() (services.Service, error) { func (t *Cortex) setupModuleManager() error { mm := modules.NewManager() - privateModule := func(option *modules.ModuleOption) { - option.Public = false - } + // Register all modules here. // RegisterModule(name string, initFn func()(services.Service, error)) - mm.RegisterModule(Server, t.initServer, privateModule) - mm.RegisterModule(API, t.initAPI, privateModule) - mm.RegisterModule(RuntimeConfig, t.initRuntimeConfig, privateModule) - mm.RegisterModule(MemberlistKV, t.initMemberlistKV, privateModule) - mm.RegisterModule(Ring, t.initRing, privateModule) - mm.RegisterModule(Overrides, t.initOverrides, privateModule) - mm.RegisterModule(Store, t.initStore, privateModule) - mm.RegisterModule(DeleteRequestsStore, t.initDeleteRequestsStore, privateModule) - mm.RegisterModule(StoreQueryable, t.initStoreQueryable, privateModule) - mm.RegisterModule(StoreGateway, t.initStoreGateway, privateModule) + mm.RegisterModule(Server, t.initServer, modules.PrivateModule) + mm.RegisterModule(API, t.initAPI, modules.PrivateModule) + mm.RegisterModule(RuntimeConfig, t.initRuntimeConfig, modules.PrivateModule) + mm.RegisterModule(MemberlistKV, t.initMemberlistKV, modules.PrivateModule) + mm.RegisterModule(Ring, t.initRing, modules.PrivateModule) + mm.RegisterModule(Overrides, t.initOverrides, modules.PrivateModule) + mm.RegisterModule(Store, t.initStore, modules.PrivateModule) + mm.RegisterModule(DeleteRequestsStore, t.initDeleteRequestsStore, modules.PrivateModule) + mm.RegisterModule(StoreQueryable, t.initStoreQueryable, modules.PrivateModule) + mm.RegisterModule(StoreGateway, t.initStoreGateway, modules.PrivateModule) mm.RegisterModule(DataPurger, t.initDataPurger) mm.RegisterModule(StoreGateway, t.initStoreGateway) mm.RegisterModule(Compactor, t.initCompactor) diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index 84ab9673991..314ab324921 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -17,7 +17,8 @@ type module struct { // initFn for this module (can return nil) initFn func() (services.Service, error) - option ModuleOption + // is this module public + public bool } // Manager is a component that initialises modules of the application @@ -26,10 +27,10 @@ type Manager struct { modules map[string]*module } -// ModuleOption contains options of a module. -// public option indicates if a module can be use as top level module (i.e. pass in as command line argument). -type ModuleOption struct { - Public bool +// PrivateModule is a functional option that can be used +// when registering a module. +func PrivateModule(m *module) { + m.public = false } // NewManager creates a new Manager @@ -43,14 +44,14 @@ func NewManager() *Manager { // name must be unique to avoid overwriting modules // if initFn is nil, the module will not initialise // if options is not given, then by default module is public -func (m *Manager) RegisterModule(name string, initFn func() (services.Service, error), options ...func(option *ModuleOption)) { +func (m *Manager) RegisterModule(name string, initFn func() (services.Service, error), options ...func(option *module)) { m.modules[name] = &module{ initFn: initFn, - option: defaultModuleOption(), + public: true, } for _, o := range options { - o(&m.modules[name].option) + o(m.modules[name]) } } @@ -110,7 +111,7 @@ func (m *Manager) InitModuleServices(target string) (map[string]services.Service func (m *Manager) PublicModuleNames() []string { var result []string for key, val := range m.modules { - if val.option.Public { + if val.public { result = append(result, key) } } @@ -126,7 +127,7 @@ func (m *Manager) IsPublicModule(mod string) bool { val, ok := m.modules[mod] if ok { - return val.option.Public + return val.public } return false @@ -193,7 +194,3 @@ func (m *Manager) findInverseDependencies(mod string, mods []string) []string { return result } - -func defaultModuleOption() ModuleOption { - return ModuleOption{true} // default to public module to keep backward compatibility -} diff --git a/pkg/util/modules/modules_test.go b/pkg/util/modules/modules_test.go index f9ace54b7de..68ad9bc5aec 100644 --- a/pkg/util/modules/modules_test.go +++ b/pkg/util/modules/modules_test.go @@ -49,16 +49,13 @@ func TestDependencies(t *testing.T) { } func TestRegisterModuleWithOptions(t *testing.T) { - publicMod := func(option *ModuleOption) { - option.Public = true - } sut := NewManager() - sut.RegisterModule("module1", mockInitFunc, publicMod) + sut.RegisterModule("module1", mockInitFunc) m := sut.modules["module1"] assert.NotNil(t, mockInitFunc, m.initFn, "initFn not assigned") - assert.True(t, m.option.Public, "module should be public") + assert.True(t, m.public, "module should be public") } func TestRegisterModuleSetsDefaultOption(t *testing.T) { @@ -67,38 +64,29 @@ func TestRegisterModuleSetsDefaultOption(t *testing.T) { m := sut.modules["module1"] - assert.True(t, m.option.Public, "mould should be public") + assert.True(t, m.public, "mould should be public") } func TestFunctionalOptAtTheEndWins(t *testing.T) { - privateMod := func(option *ModuleOption) { - option.Public = false - } - publicMod := func(option *ModuleOption) { - option.Public = false + publicMod := func(option *module) { + option.public = true } sut := NewManager() - sut.RegisterModule("mod1", mockInitFunc, privateMod, publicMod, privateMod) + sut.RegisterModule("mod1", mockInitFunc, PrivateModule, publicMod, PrivateModule) m := sut.modules["mod1"] assert.NotNil(t, mockInitFunc, m.initFn, "initFn not assigned") - assert.False(t, m.option.Public, "module should be public") + assert.False(t, m.public, "module should be public") } func TestGetAllPublicModulesNames(t *testing.T) { - publicMod := func(option *ModuleOption) { - option.Public = true - } - privateMod := func(option *ModuleOption) { - option.Public = false - } sut := NewManager() sut.RegisterModule("public1", mockInitFunc) - sut.RegisterModule("public2", mockInitFunc, publicMod) - sut.RegisterModule("public3", mockInitFunc, publicMod) - sut.RegisterModule("private1", mockInitFunc, privateMod) - sut.RegisterModule("private2", mockInitFunc, privateMod) + sut.RegisterModule("public2", mockInitFunc) + sut.RegisterModule("public3", mockInitFunc) + sut.RegisterModule("private1", mockInitFunc, PrivateModule) + sut.RegisterModule("private2", mockInitFunc, PrivateModule) pm := sut.PublicModuleNames() @@ -126,14 +114,11 @@ func TestGetAllPublicModulesNamesHasNoDupWithDependency(t *testing.T) { } func TestGetEmptyListWhenThereIsNoPublicModule(t *testing.T) { - privateMod := func(option *ModuleOption) { - option.Public = false - } sut := NewManager() - sut.RegisterModule("private1", mockInitFunc, privateMod) - sut.RegisterModule("private2", mockInitFunc, privateMod) - sut.RegisterModule("private3", mockInitFunc, privateMod) - sut.RegisterModule("private4", mockInitFunc, privateMod) + sut.RegisterModule("private1", mockInitFunc, PrivateModule) + sut.RegisterModule("private2", mockInitFunc, PrivateModule) + sut.RegisterModule("private3", mockInitFunc, PrivateModule) + sut.RegisterModule("private4", mockInitFunc, PrivateModule) pm := sut.PublicModuleNames() @@ -141,13 +126,10 @@ func TestGetEmptyListWhenThereIsNoPublicModule(t *testing.T) { } func TestPublicModuleNamesReturnsSortedList(t *testing.T) { - publicMod := func(option *ModuleOption) { - option.Public = true - } sut := NewManager() - sut.RegisterModule("c", mockInitFunc, publicMod) - sut.RegisterModule("b", mockInitFunc, publicMod) - sut.RegisterModule("a", mockInitFunc, publicMod) + sut.RegisterModule("c", mockInitFunc) + sut.RegisterModule("b", mockInitFunc) + sut.RegisterModule("a", mockInitFunc) pm := sut.PublicModuleNames() @@ -156,17 +138,11 @@ func TestPublicModuleNamesReturnsSortedList(t *testing.T) { } func TestIsPublicModule(t *testing.T) { - publicMod := func(option *ModuleOption) { - option.Public = true - } - privateMod := func(option *ModuleOption) { - option.Public = false - } pubModName := "public" privateModName := "private" sut := NewManager() - sut.RegisterModule(pubModName, mockInitFunc, publicMod) - sut.RegisterModule(privateModName, mockInitFunc, privateMod) + sut.RegisterModule(pubModName, mockInitFunc) + sut.RegisterModule(privateModName, mockInitFunc, PrivateModule) var result = sut.IsPublicModule(pubModName) assert.True(t, result, "module '%v' should be public", pubModName) From a853d26b39ce150b4d86c28d395ccdf4d0b5719b Mon Sep 17 00:00:00 2001 From: alvinlin123 Date: Wed, 24 Jun 2020 01:28:20 -0700 Subject: [PATCH 13/23] Update pkg/util/modules/modules.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Peter Štibraný Signed-off-by: Alvin Lin --- pkg/util/modules/modules.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index 314ab324921..b01cae9ed63 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -27,8 +27,7 @@ type Manager struct { modules map[string]*module } -// PrivateModule is a functional option that can be used -// when registering a module. +// PrivateModule is an option for `RegisterModule` that marks module as private. Modules are public by default. func PrivateModule(m *module) { m.public = false } From 614f04eb3443f96d1509af53ac83dd3bed10d43a Mon Sep 17 00:00:00 2001 From: alvinlin123 Date: Wed, 24 Jun 2020 01:37:40 -0700 Subject: [PATCH 14/23] Update pkg/util/modules/modules.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Peter Štibraný Signed-off-by: Alvin Lin --- pkg/util/modules/modules.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index b01cae9ed63..005dec00c69 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -39,7 +39,7 @@ func NewManager() *Manager { } } -// RegisterModule registers a new module with name, init function, and option functions. +// 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 // if options is not given, then by default module is public From 7c8f400d74fa9f6df2156a59ade05783dba228ca Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Wed, 24 Jun 2020 02:21:58 -0700 Subject: [PATCH 15/23] Address more PR comments Signed-off-by: Alvin Lin --- pkg/util/modules/modules.go | 7 +++---- pkg/util/modules/modules_test.go | 35 +++++--------------------------- 2 files changed, 8 insertions(+), 34 deletions(-) diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index 005dec00c69..7d701044bf3 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -39,10 +39,9 @@ func NewManager() *Manager { } } -// 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 -// if options is not given, then by default module is public +// 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 public by default. func (m *Manager) RegisterModule(name string, initFn func() (services.Service, error), options ...func(option *module)) { m.modules[name] = &module{ initFn: initFn, diff --git a/pkg/util/modules/modules_test.go b/pkg/util/modules/modules_test.go index 68ad9bc5aec..2e2a597ffe6 100644 --- a/pkg/util/modules/modules_test.go +++ b/pkg/util/modules/modules_test.go @@ -48,7 +48,7 @@ func TestDependencies(t *testing.T) { assert.Error(t, err, fmt.Errorf("unrecognised module name: service_unknown")) } -func TestRegisterModuleWithOptions(t *testing.T) { +func TestRegisterModuleDefaultsToPublic(t *testing.T) { sut := NewManager() sut.RegisterModule("module1", mockInitFunc) @@ -58,15 +58,6 @@ func TestRegisterModuleWithOptions(t *testing.T) { assert.True(t, m.public, "module should be public") } -func TestRegisterModuleSetsDefaultOption(t *testing.T) { - sut := NewManager() - sut.RegisterModule("module1", mockInitFunc) - - m := sut.modules["module1"] - - assert.True(t, m.public, "mould should be public") -} - func TestFunctionalOptAtTheEndWins(t *testing.T) { publicMod := func(option *module) { option.public = true @@ -82,18 +73,16 @@ func TestFunctionalOptAtTheEndWins(t *testing.T) { func TestGetAllPublicModulesNames(t *testing.T) { sut := NewManager() - sut.RegisterModule("public1", mockInitFunc) - sut.RegisterModule("public2", mockInitFunc) sut.RegisterModule("public3", mockInitFunc) + sut.RegisterModule("public2", mockInitFunc) + sut.RegisterModule("public1", mockInitFunc) sut.RegisterModule("private1", mockInitFunc, PrivateModule) sut.RegisterModule("private2", mockInitFunc, PrivateModule) pm := sut.PublicModuleNames() assert.Len(t, pm, 3, "wrong result slice size") - assert.Contains(t, pm, "public1", "missing public module") - assert.Contains(t, pm, "public2", "missing public module") - assert.Contains(t, pm, "public3", "missing public module") + assert.Equal(t, []string{"public1", "public2", "public3"}, pm, "module list contains wrong element and/or not sorted") } func TestGetAllPublicModulesNamesHasNoDupWithDependency(t *testing.T) { @@ -108,9 +97,7 @@ func TestGetAllPublicModulesNamesHasNoDupWithDependency(t *testing.T) { // make sure we don't include any module twice because there is a dependency assert.Len(t, pm, 3, "wrong result slice size") - assert.Contains(t, pm, "public1", "missing public module") - assert.Contains(t, pm, "public2", "missing public module") - assert.Contains(t, pm, "public3", "missing public module") + assert.Equal(t, []string{"public1", "public2", "public3"}, pm, "module list contains wrong elements and/or not sorted") } func TestGetEmptyListWhenThereIsNoPublicModule(t *testing.T) { @@ -125,18 +112,6 @@ func TestGetEmptyListWhenThereIsNoPublicModule(t *testing.T) { assert.Len(t, pm, 0, "wrong result slice size") } -func TestPublicModuleNamesReturnsSortedList(t *testing.T) { - sut := NewManager() - sut.RegisterModule("c", mockInitFunc) - sut.RegisterModule("b", mockInitFunc) - sut.RegisterModule("a", mockInitFunc) - - pm := sut.PublicModuleNames() - - assert.Len(t, pm, 3, "wrong result slice size") - assert.Equal(t, []string{"a", "b", "c"}, pm, "module names list is not sorted in ascending order") -} - func TestIsPublicModule(t *testing.T) { pubModName := "public" privateModName := "private" From ede9b7cc3f97c6d260a3f1a84e8e134008ec379e Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Wed, 24 Jun 2020 02:28:15 -0700 Subject: [PATCH 16/23] Remove unneeded asserts Signed-off-by: Alvin Lin --- pkg/util/modules/modules_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/util/modules/modules_test.go b/pkg/util/modules/modules_test.go index 2e2a597ffe6..04c5cb76eb5 100644 --- a/pkg/util/modules/modules_test.go +++ b/pkg/util/modules/modules_test.go @@ -81,7 +81,6 @@ func TestGetAllPublicModulesNames(t *testing.T) { pm := sut.PublicModuleNames() - assert.Len(t, pm, 3, "wrong result slice size") assert.Equal(t, []string{"public1", "public2", "public3"}, pm, "module list contains wrong element and/or not sorted") } @@ -96,7 +95,6 @@ func TestGetAllPublicModulesNamesHasNoDupWithDependency(t *testing.T) { pm := sut.PublicModuleNames() // make sure we don't include any module twice because there is a dependency - assert.Len(t, pm, 3, "wrong result slice size") assert.Equal(t, []string{"public1", "public2", "public3"}, pm, "module list contains wrong elements and/or not sorted") } From 6bc6e27e60864139d6c4dc45bea4403ddee44ea1 Mon Sep 17 00:00:00 2001 From: alvinlin123 Date: Thu, 25 Jun 2020 00:43:18 -0700 Subject: [PATCH 17/23] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Peter Štibraný Signed-off-by: Alvin Lin --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbaca65c382..e30d52569cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,7 +56,7 @@ * [FEATURE] TLS config options added for GRPC clients in Querier (Query-frontend client & Ingester client), Ruler, Store Gateway, as well as HTTP client in Config store client. #2502 * [FEATURE] The flag `-frontend.max-cache-freshness` is now supported within the limits overrides, to specify per-tenant max cache freshness values. The corresponding YAML config parameter has been changed from `results_cache.max_freshness` to `limits_config.max_cache_freshness`. The legacy YAML config parameter (`results_cache.max_freshness`) will continue to be supported till Cortex release `v1.4.0`. #2609 * [FEATURE] Experimental gRPC Store: Added support to 3rd parties index and chunk stores using gRPC client/server plugin mechanism. #2220 -* [ENHANCEMENT] Add command line flag to list possible values for -target. Also, log warning if given target is not public. #2710 +* [ENHANCEMENT] Add `-modules` command line flag to list possible values for `-target`. Also, log warning if given target is internal component. #2752 * [ENHANCEMENT] Propagate GOPROXY value when building `build-image`. This is to help the builders building the code in a Network where default Go proxy is not accessible (e.g. when behind some corporate VPN). #2741 * [ENHANCEMENT] Querier: Added metric `cortex_querier_request_duration_seconds` for all requests to the querier. #2708 * [ENHANCEMENT] Experimental TSDB: added the following metrics to the ingester: #2580 #2583 #2589 #2654 From 83b2f3704c707bf838a2818ef1fcb0faea83dd7b Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Thu, 25 Jun 2020 00:58:59 -0700 Subject: [PATCH 18/23] Minimize diff with upstream/master Signed-off-by: Alvin Lin --- pkg/cortex/modules.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/cortex/modules.go b/pkg/cortex/modules.go index 89d8cf2b31b..75eeb5deb96 100644 --- a/pkg/cortex/modules.go +++ b/pkg/cortex/modules.go @@ -520,21 +520,21 @@ func (t *Cortex) setupModuleManager() error { mm.RegisterModule(MemberlistKV, t.initMemberlistKV, modules.PrivateModule) mm.RegisterModule(Ring, t.initRing, modules.PrivateModule) mm.RegisterModule(Overrides, t.initOverrides, modules.PrivateModule) + mm.RegisterModule(Distributor, t.initDistributor) mm.RegisterModule(Store, t.initStore, modules.PrivateModule) mm.RegisterModule(DeleteRequestsStore, t.initDeleteRequestsStore, modules.PrivateModule) - mm.RegisterModule(StoreQueryable, t.initStoreQueryable, modules.PrivateModule) - mm.RegisterModule(StoreGateway, t.initStoreGateway) - mm.RegisterModule(Purger, t.initPurger) - mm.RegisterModule(Compactor, t.initCompactor) + mm.RegisterModule(Ingester, t.initIngester) mm.RegisterModule(Flusher, t.initFlusher) + mm.RegisterModule(Querier, t.initQuerier) + mm.RegisterModule(StoreQueryable, t.initStoreQueryable, modules.PrivateModule) mm.RegisterModule(QueryFrontend, t.initQueryFrontend) mm.RegisterModule(TableManager, t.initTableManager) mm.RegisterModule(Ruler, t.initRuler) mm.RegisterModule(Configs, t.initConfig) mm.RegisterModule(AlertManager, t.initAlertManager) - mm.RegisterModule(Querier, t.initQuerier) - mm.RegisterModule(Ingester, t.initIngester) - mm.RegisterModule(Distributor, t.initDistributor) + mm.RegisterModule(Compactor, t.initCompactor) + mm.RegisterModule(StoreGateway, t.initStoreGateway) + mm.RegisterModule(Purger, t.initPurger) mm.RegisterModule(All, nil) // Add dependencies From 3890d9cc49301c7f8685748ee0213d240a2af83b Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Thu, 25 Jun 2020 02:14:13 -0700 Subject: [PATCH 19/23] Make code more readible Signed-off-by: Alvin Lin --- pkg/util/modules/modules.go | 26 ++++++++++++++++++-------- pkg/util/modules/modules_test.go | 6 +++--- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index 7d701044bf3..071e82949be 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -9,6 +9,16 @@ import ( "github.com/cortexproject/cortex/pkg/util/services" ) +type moduleVisibility int + +const ( + // A public module is intended to be passed into `InitModuleServices`. + public moduleVisibility = iota + + // A private module is not intended to be passed into `InitModuleServices`. + private +) + // module is the basic building block of the application type module struct { // dependencies of this module @@ -17,8 +27,8 @@ type module struct { // initFn for this module (can return nil) initFn func() (services.Service, error) - // is this module public - public bool + // visibility of this module + visibility moduleVisibility } // Manager is a component that initialises modules of the application @@ -29,7 +39,7 @@ type Manager struct { // PrivateModule is an option for `RegisterModule` that marks module as private. Modules are public by default. func PrivateModule(m *module) { - m.public = false + m.visibility = private } // NewManager creates a new Manager @@ -44,8 +54,8 @@ func NewManager() *Manager { // Modules are public by default. func (m *Manager) RegisterModule(name string, initFn func() (services.Service, error), options ...func(option *module)) { m.modules[name] = &module{ - initFn: initFn, - public: true, + initFn: initFn, + visibility: public, } for _, o := range options { @@ -105,11 +115,11 @@ func (m *Manager) InitModuleServices(target string) (map[string]services.Service } // PublicModuleNames gets list of module names that are -// public module. +// public module. Returned list is sorted in increasing order. func (m *Manager) PublicModuleNames() []string { var result []string for key, val := range m.modules { - if val.public { + if val.visibility == public { result = append(result, key) } } @@ -125,7 +135,7 @@ func (m *Manager) IsPublicModule(mod string) bool { val, ok := m.modules[mod] if ok { - return val.public + return val.visibility == public } return false diff --git a/pkg/util/modules/modules_test.go b/pkg/util/modules/modules_test.go index 04c5cb76eb5..c494d3ca410 100644 --- a/pkg/util/modules/modules_test.go +++ b/pkg/util/modules/modules_test.go @@ -55,12 +55,12 @@ func TestRegisterModuleDefaultsToPublic(t *testing.T) { m := sut.modules["module1"] assert.NotNil(t, mockInitFunc, m.initFn, "initFn not assigned") - assert.True(t, m.public, "module should be public") + assert.Equal(t, public, m.visibility, "module should be public") } func TestFunctionalOptAtTheEndWins(t *testing.T) { publicMod := func(option *module) { - option.public = true + option.visibility = public } sut := NewManager() sut.RegisterModule("mod1", mockInitFunc, PrivateModule, publicMod, PrivateModule) @@ -68,7 +68,7 @@ func TestFunctionalOptAtTheEndWins(t *testing.T) { m := sut.modules["mod1"] assert.NotNil(t, mockInitFunc, m.initFn, "initFn not assigned") - assert.False(t, m.public, "module should be public") + assert.Equal(t, private, m.visibility, "module should be private") } func TestGetAllPublicModulesNames(t *testing.T) { From a791200b2df6e8884d428cc89ccfc3c5c0b0ebea Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Thu, 25 Jun 2020 02:59:35 -0700 Subject: [PATCH 20/23] avoid creating new type for simple concept Signed-off-by: Alvin Lin --- cmd/cortex/main.go | 2 +- cmd/cortex/main_test.go | 4 +- pkg/cortex/cortex.go | 6 +-- pkg/cortex/modules.go | 18 ++++---- pkg/util/modules/modules.go | 40 +++++++---------- pkg/util/modules/modules_test.go | 74 ++++++++++++++++---------------- 6 files changed, 67 insertions(+), 77 deletions(-) diff --git a/cmd/cortex/main.go b/cmd/cortex/main.go index 5f83f6de2a8..5ac443665e6 100644 --- a/cmd/cortex/main.go +++ b/cmd/cortex/main.go @@ -137,7 +137,7 @@ func main() { util.CheckFatal("initializing cortex", err) if t.Cfg.ListModules { - for _, m := range t.ModuleManager.PublicModuleNames() { + for _, m := range t.ModuleManager.UserVisibleModuleNames() { fmt.Fprintln(os.Stdout, m) } diff --git a/cmd/cortex/main_test.go b/cmd/cortex/main_test.go index f5c41cd8778..5941d667caa 100644 --- a/cmd/cortex/main_test.go +++ b/cmd/cortex/main_test.go @@ -63,13 +63,13 @@ func TestFlagParsing(t *testing.T) { stdoutMessage: "target: distributor\n", }, - "public module listing": { + "user visible module listing": { arguments: []string{"-modules"}, stdoutMessage: "ingester\n", stderrExcluded: "ingester\n", }, - "public module listing flag take precedence over target flag": { + "user visible module listing flag take precedence over target flag": { arguments: []string{"-modules", "-target=blah"}, stdoutMessage: "ingester\n", stderrExcluded: "ingester\n", diff --git a/pkg/cortex/cortex.go b/pkg/cortex/cortex.go index 57c484cd49e..439ee8ea915 100644 --- a/pkg/cortex/cortex.go +++ b/pkg/cortex/cortex.go @@ -294,10 +294,10 @@ func (t *Cortex) setupThanosTracing() { // Run starts Cortex running, and blocks until a Cortex stops. func (t *Cortex) Run() error { - if !t.ModuleManager.IsPublicModule(t.Cfg.Target) { + if !t.ModuleManager.IsUserVisibleModule(t.Cfg.Target) { level.Warn(util.Logger).Log( - "msg", fmt.Sprintf("'%v' is not a public module, is this intended?", t.Cfg.Target), - "public-modules", strings.Join(t.ModuleManager.PublicModuleNames(), ", ")) + "msg", fmt.Sprintf("'%v' is not a user visible module, is this intended?", t.Cfg.Target), + "user-visible-modules", strings.Join(t.ModuleManager.UserVisibleModuleNames(), ", ")) } serviceMap, err := t.ModuleManager.InitModuleServices(t.Cfg.Target) diff --git a/pkg/cortex/modules.go b/pkg/cortex/modules.go index 75eeb5deb96..c9a3202ce25 100644 --- a/pkg/cortex/modules.go +++ b/pkg/cortex/modules.go @@ -514,19 +514,19 @@ func (t *Cortex) setupModuleManager() error { // Register all modules here. // RegisterModule(name string, initFn func()(services.Service, error)) - mm.RegisterModule(Server, t.initServer, modules.PrivateModule) - mm.RegisterModule(API, t.initAPI, modules.PrivateModule) - mm.RegisterModule(RuntimeConfig, t.initRuntimeConfig, modules.PrivateModule) - mm.RegisterModule(MemberlistKV, t.initMemberlistKV, modules.PrivateModule) - mm.RegisterModule(Ring, t.initRing, modules.PrivateModule) - mm.RegisterModule(Overrides, t.initOverrides, modules.PrivateModule) + 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.initStore, modules.PrivateModule) - mm.RegisterModule(DeleteRequestsStore, t.initDeleteRequestsStore, modules.PrivateModule) + mm.RegisterModule(Store, t.initStore, 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.initStoreQueryable, modules.PrivateModule) + mm.RegisterModule(StoreQueryable, t.initStoreQueryable, modules.UserInvisibleModule) mm.RegisterModule(QueryFrontend, t.initQueryFrontend) mm.RegisterModule(TableManager, t.initTableManager) mm.RegisterModule(Ruler, t.initRuler) diff --git a/pkg/util/modules/modules.go b/pkg/util/modules/modules.go index 071e82949be..8875f9dfbdc 100644 --- a/pkg/util/modules/modules.go +++ b/pkg/util/modules/modules.go @@ -9,16 +9,6 @@ import ( "github.com/cortexproject/cortex/pkg/util/services" ) -type moduleVisibility int - -const ( - // A public module is intended to be passed into `InitModuleServices`. - public moduleVisibility = iota - - // A private module is not intended to be passed into `InitModuleServices`. - private -) - // module is the basic building block of the application type module struct { // dependencies of this module @@ -27,8 +17,8 @@ type module struct { // initFn for this module (can return nil) initFn func() (services.Service, error) - // visibility of this module - visibility moduleVisibility + // 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 @@ -37,9 +27,9 @@ type Manager struct { modules map[string]*module } -// PrivateModule is an option for `RegisterModule` that marks module as private. Modules are public by default. -func PrivateModule(m *module) { - m.visibility = private +// 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 @@ -51,11 +41,11 @@ func NewManager() *Manager { // 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 public by default. +// 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, - visibility: public, + initFn: initFn, + userVisible: true, } for _, o := range options { @@ -114,12 +104,12 @@ func (m *Manager) InitModuleServices(target string) (map[string]services.Service return servicesMap, nil } -// PublicModuleNames gets list of module names that are -// public module. Returned list is sorted in increasing order. -func (m *Manager) PublicModuleNames() []string { +// 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.visibility == public { + if val.userVisible { result = append(result, key) } } @@ -129,13 +119,13 @@ func (m *Manager) PublicModuleNames() []string { return result } -// IsPublicModule check if given module is public or not. Returns true +// 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) IsPublicModule(mod string) bool { +func (m *Manager) IsUserVisibleModule(mod string) bool { val, ok := m.modules[mod] if ok { - return val.visibility == public + return val.userVisible } return false diff --git a/pkg/util/modules/modules_test.go b/pkg/util/modules/modules_test.go index c494d3ca410..6a689045f21 100644 --- a/pkg/util/modules/modules_test.go +++ b/pkg/util/modules/modules_test.go @@ -48,81 +48,81 @@ func TestDependencies(t *testing.T) { assert.Error(t, err, fmt.Errorf("unrecognised module name: service_unknown")) } -func TestRegisterModuleDefaultsToPublic(t *testing.T) { +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.Equal(t, public, m.visibility, "module should be public") + assert.True(t, m.userVisible, "module should be user visible") } func TestFunctionalOptAtTheEndWins(t *testing.T) { - publicMod := func(option *module) { - option.visibility = public + userVisibleMod := func(option *module) { + option.userVisible = true } sut := NewManager() - sut.RegisterModule("mod1", mockInitFunc, PrivateModule, publicMod, PrivateModule) + sut.RegisterModule("mod1", mockInitFunc, UserInvisibleModule, userVisibleMod, UserInvisibleModule) m := sut.modules["mod1"] assert.NotNil(t, mockInitFunc, m.initFn, "initFn not assigned") - assert.Equal(t, private, m.visibility, "module should be private") + assert.False(t, m.userVisible, "module should be internal") } -func TestGetAllPublicModulesNames(t *testing.T) { +func TestGetAllUserVisibleModulesNames(t *testing.T) { sut := NewManager() - sut.RegisterModule("public3", mockInitFunc) - sut.RegisterModule("public2", mockInitFunc) - sut.RegisterModule("public1", mockInitFunc) - sut.RegisterModule("private1", mockInitFunc, PrivateModule) - sut.RegisterModule("private2", mockInitFunc, PrivateModule) + sut.RegisterModule("userVisible3", mockInitFunc) + sut.RegisterModule("userVisible2", mockInitFunc) + sut.RegisterModule("userVisible1", mockInitFunc) + sut.RegisterModule("internal1", mockInitFunc, UserInvisibleModule) + sut.RegisterModule("internal2", mockInitFunc, UserInvisibleModule) - pm := sut.PublicModuleNames() + pm := sut.UserVisibleModuleNames() - assert.Equal(t, []string{"public1", "public2", "public3"}, pm, "module list contains wrong element and/or not sorted") + assert.Equal(t, []string{"userVisible1", "userVisible2", "userVisible3"}, pm, "module list contains wrong element and/or not sorted") } -func TestGetAllPublicModulesNamesHasNoDupWithDependency(t *testing.T) { +func TestGetAllUserVisibleModulesNamesHasNoDupWithDependency(t *testing.T) { sut := NewManager() - sut.RegisterModule("public1", mockInitFunc) - sut.RegisterModule("public2", mockInitFunc) - sut.RegisterModule("public3", mockInitFunc) + sut.RegisterModule("userVisible1", mockInitFunc) + sut.RegisterModule("userVisible2", mockInitFunc) + sut.RegisterModule("userVisible3", mockInitFunc) - assert.NoError(t, sut.AddDependency("public1", "public2", "public3")) + assert.NoError(t, sut.AddDependency("userVisible1", "userVisible2", "userVisible3")) - pm := sut.PublicModuleNames() + pm := sut.UserVisibleModuleNames() // make sure we don't include any module twice because there is a dependency - assert.Equal(t, []string{"public1", "public2", "public3"}, pm, "module list contains wrong elements and/or not sorted") + assert.Equal(t, []string{"userVisible1", "userVisible2", "userVisible3"}, pm, "module list contains wrong elements and/or not sorted") } -func TestGetEmptyListWhenThereIsNoPublicModule(t *testing.T) { +func TestGetEmptyListWhenThereIsNoUserVisibleModule(t *testing.T) { sut := NewManager() - sut.RegisterModule("private1", mockInitFunc, PrivateModule) - sut.RegisterModule("private2", mockInitFunc, PrivateModule) - sut.RegisterModule("private3", mockInitFunc, PrivateModule) - sut.RegisterModule("private4", mockInitFunc, PrivateModule) + sut.RegisterModule("internal1", mockInitFunc, UserInvisibleModule) + sut.RegisterModule("internal2", mockInitFunc, UserInvisibleModule) + sut.RegisterModule("internal3", mockInitFunc, UserInvisibleModule) + sut.RegisterModule("internal4", mockInitFunc, UserInvisibleModule) - pm := sut.PublicModuleNames() + pm := sut.UserVisibleModuleNames() assert.Len(t, pm, 0, "wrong result slice size") } -func TestIsPublicModule(t *testing.T) { - pubModName := "public" - privateModName := "private" +func TestIsUserVisibleModule(t *testing.T) { + userVisibleModName := "userVisible" + internalModName := "internal" sut := NewManager() - sut.RegisterModule(pubModName, mockInitFunc) - sut.RegisterModule(privateModName, mockInitFunc, PrivateModule) + sut.RegisterModule(userVisibleModName, mockInitFunc) + sut.RegisterModule(internalModName, mockInitFunc, UserInvisibleModule) - var result = sut.IsPublicModule(pubModName) - assert.True(t, result, "module '%v' should be public", pubModName) + var result = sut.IsUserVisibleModule(userVisibleModName) + assert.True(t, result, "module '%v' should be user visible", userVisibleModName) - result = sut.IsPublicModule(privateModName) - assert.False(t, result, "module '%v' should be private", privateModName) + result = sut.IsUserVisibleModule(internalModName) + assert.False(t, result, "module '%v' should be internal", internalModName) - result = sut.IsPublicModule("ghost") + result = sut.IsUserVisibleModule("ghost") assert.False(t, result, "expects result be false when module does not exist") } From 3cf8f2ae06702bcf880d56ac314f2f2f3e4cb666 Mon Sep 17 00:00:00 2001 From: alvinlin123 Date: Thu, 25 Jun 2020 03:21:10 -0700 Subject: [PATCH 21/23] Update pkg/cortex/cortex.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Peter Štibraný Signed-off-by: Alvin Lin --- pkg/cortex/cortex.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/cortex/cortex.go b/pkg/cortex/cortex.go index 439ee8ea915..fed017980de 100644 --- a/pkg/cortex/cortex.go +++ b/pkg/cortex/cortex.go @@ -295,9 +295,7 @@ 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", fmt.Sprintf("'%v' is not a user visible module, is this intended?", t.Cfg.Target), - "user-visible-modules", strings.Join(t.ModuleManager.UserVisibleModuleNames(), ", ")) + 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) From b4e1d65d43b8cb7c4c623d61306454295ab10bac Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Thu, 25 Jun 2020 03:27:03 -0700 Subject: [PATCH 22/23] remove unused import Signed-off-by: Alvin Lin --- pkg/cortex/cortex.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/cortex/cortex.go b/pkg/cortex/cortex.go index fed017980de..11e15165fb6 100644 --- a/pkg/cortex/cortex.go +++ b/pkg/cortex/cortex.go @@ -7,7 +7,6 @@ import ( "fmt" "net/http" "os" - "strings" "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" From 9823de48f6836ff788941faec961fe3932b99714 Mon Sep 17 00:00:00 2001 From: Alvin Lin Date: Fri, 26 Jun 2020 22:35:44 -0700 Subject: [PATCH 23/23] Resolve more conflict Signed-off-by: Alvin Lin --- pkg/cortex/modules.go | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/pkg/cortex/modules.go b/pkg/cortex/modules.go index 2a73ab4416a..d2be3e29438 100644 --- a/pkg/cortex/modules.go +++ b/pkg/cortex/modules.go @@ -565,21 +565,12 @@ func (t *Cortex) setupModuleManager() error { mm.RegisterModule(Ring, t.initRing, modules.UserInvisibleModule) mm.RegisterModule(Overrides, t.initOverrides, modules.UserInvisibleModule) mm.RegisterModule(Distributor, t.initDistributor) -<<<<<<< HEAD - mm.RegisterModule(Store, t.initStore, modules.UserInvisibleModule) + 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.initStoreQueryable, modules.UserInvisibleModule) -======= - mm.RegisterModule(Store, t.initChunkStore) - mm.RegisterModule(DeleteRequestsStore, t.initDeleteRequestsStore) - mm.RegisterModule(Ingester, t.initIngester) - mm.RegisterModule(Flusher, t.initFlusher) - mm.RegisterModule(Querier, t.initQuerier) - mm.RegisterModule(StoreQueryable, t.initStoreQueryables) ->>>>>>> upstream/master + mm.RegisterModule(StoreQueryable, t.initStoreQueryables, modules.UserInvisibleModule) mm.RegisterModule(QueryFrontend, t.initQueryFrontend) mm.RegisterModule(TableManager, t.initTableManager) mm.RegisterModule(Ruler, t.initRuler)