diff --git a/CHANGELOG.md b/CHANGELOG.md index fd2b0a30389..4305f4ed270 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * [ENHANCEMENT] Distributor: Add new `cortex_reduced_resolution_histogram_samples_total` metric to track the number of histogram samples which resolution was reduced. #6182 * [ENHANCEMENT] StoreGateway: Implement metadata API limit in queryable. #6195 * [ENHANCEMENT] Ingester: Add matchers to ingester LabelNames() and LabelNamesStream() RPC. #6209 +* [BUGFIX] Runtime-config: Handle absolute file paths when working directory is not / #6224 ## 1.18.0 2024-09-03 diff --git a/integration/e2e/scenario_test.go b/integration/e2e/scenario_test.go index 50b61dbc07f..12249f3900d 100644 --- a/integration/e2e/scenario_test.go +++ b/integration/e2e/scenario_test.go @@ -146,3 +146,17 @@ func TestScenario(t *testing.T) { _, err = bkt.Get(context.Background(), "recipe") require.Error(t, err) } + +// TestStartStop tests for ensuring that when the container is stopped, it can be started again. +// This is to test that the stop waits for the container to be stopped and cleaned up before returning. +func TestStartStop(t *testing.T) { + s, err := e2e.NewScenario("e2e-scenario-test") + require.NoError(t, err) + + m1 := e2edb.NewMinio(9000, bktName) + + for i := 0; i < 10; i++ { + require.NoError(t, s.Start(m1)) + require.NoError(t, s.Stop(m1)) + } +} diff --git a/integration/e2e/service.go b/integration/e2e/service.go index 50fc0b83011..0d021e19a39 100644 --- a/integration/e2e/service.go +++ b/integration/e2e/service.go @@ -48,6 +48,9 @@ type ConcreteService struct { // docker NetworkName used to start this container. // If empty it means service is stopped. usedNetworkName string + + // workDir is the working directory inside the container + workDir string } func NewConcreteService( @@ -92,6 +95,10 @@ func (s *ConcreteService) SetUser(user string) { s.user = user } +func (s *ConcreteService) SetWorkDir(workDir string) { + s.workDir = workDir +} + func (s *ConcreteService) Start(networkName, sharedDir string) (err error) { // In case of any error, if the container was already created, we // have to cleanup removing it. We ignore the error of the "docker rm" @@ -151,6 +158,9 @@ func (s *ConcreteService) Stop() error { logger.Log(string(out)) return err } + + s.Wait() + s.usedNetworkName = "" return nil @@ -168,15 +178,22 @@ func (s *ConcreteService) Kill() error { return err } - // Wait until the container actually stopped. However, this could fail if - // the container already exited, so we just ignore the error. - _, _ = RunCommandAndGetOutput("docker", "wait", s.containerName()) + s.Wait() s.usedNetworkName = "" return nil } +// Wait waits until the service is stopped. +func (s *ConcreteService) Wait() { + // Wait until the container actually stopped. However, this could fail if + // the container already exited, so we just ignore the error. + if out, err := RunCommandAndGetOutput("docker", "wait", s.containerName()); err != nil { + logger.Log(string(out)) + } +} + // Endpoint returns external (from host perspective) service endpoint (host:port) for given internal port. // External means that it will be accessible only from host, but not from docker containers. // @@ -309,6 +326,10 @@ func (s *ConcreteService) buildDockerRunArgs(networkName, sharedDir string) []st args = append(args, "--user", s.user) } + if s.workDir != "" { + args = append(args, "--workdir", s.workDir) + } + // Published ports for _, port := range s.networkPorts { args = append(args, "-p", strconv.Itoa(port)) diff --git a/integration/runtime_config_test.go b/integration/runtime_config_test.go index f8f722084bd..8952b59f7c6 100644 --- a/integration/runtime_config_test.go +++ b/integration/runtime_config_test.go @@ -33,8 +33,9 @@ func TestLoadRuntimeConfigFromStorageBackend(t *testing.T) { filePath := filepath.Join(e2e.ContainerSharedDir, runtimeConfigFile) tests := []struct { - name string - flags map[string]string + name string + flags map[string]string + workDir string }{ { name: "no storage backend provided", @@ -57,6 +58,28 @@ func TestLoadRuntimeConfigFromStorageBackend(t *testing.T) { "-alertmanager-storage.local.path": filepath.Join(e2e.ContainerSharedDir, "alertmanager_configs"), }, }, + { + name: "runtime-config.file is a relative path", + flags: map[string]string{ + "-runtime-config.file": runtimeConfigFile, + // alert manager + "-alertmanager.web.external-url": "http://localhost/alertmanager", + "-alertmanager-storage.backend": "local", + "-alertmanager-storage.local.path": filepath.Join(e2e.ContainerSharedDir, "alertmanager_configs"), + }, + workDir: e2e.ContainerSharedDir, + }, + { + name: "runtime-config.file is an absolute path but working directory is not /", + flags: map[string]string{ + "-runtime-config.file": filePath, + // alert manager + "-alertmanager.web.external-url": "http://localhost/alertmanager", + "-alertmanager-storage.backend": "local", + "-alertmanager-storage.local.path": filepath.Join(e2e.ContainerSharedDir, "alertmanager_configs"), + }, + workDir: "/var/lib/cortex", + }, } // make alert manager config dir require.NoError(t, writeFileToSharedDir(s, "alertmanager_configs", []byte{})) @@ -64,6 +87,8 @@ func TestLoadRuntimeConfigFromStorageBackend(t *testing.T) { for i, tt := range tests { t.Run(tt.name, func(t *testing.T) { cortexSvc := e2ecortex.NewSingleBinaryWithConfigFile(fmt.Sprintf("cortex-%d", i), cortexConfigFile, tt.flags, "", 9009, 9095) + cortexSvc.SetWorkDir(tt.workDir) + require.NoError(t, s.StartAndWaitReady(cortexSvc)) assertRuntimeConfigLoadedCorrectly(t, cortexSvc) diff --git a/pkg/cortex/modules.go b/pkg/cortex/modules.go index b8d39daeba0..bcb7517b9e9 100644 --- a/pkg/cortex/modules.go +++ b/pkg/cortex/modules.go @@ -160,6 +160,20 @@ func (t *Cortex) initRuntimeConfig() (services.Service, error) { registerer := prometheus.WrapRegistererWithPrefix("cortex_", prometheus.DefaultRegisterer) logger := util_log.Logger bucketClientFactory := func(ctx context.Context) (objstore.Bucket, error) { + // When directory is an empty string but the runtime-config.file is an absolute path, + // the filesystem.NewBucketClient will treat it as a relative path based on the current working directory + // that the process is running in. + if t.Cfg.RuntimeConfig.StorageConfig.Backend == bucket.Filesystem { + if t.Cfg.RuntimeConfig.StorageConfig.Filesystem.Directory == "" { + // Check if runtime-config.file is an absolute path + if t.Cfg.RuntimeConfig.LoadPath[0] == '/' { + // If it is, set the directory to the root directory so that the filesystem bucket + // will treat it as an absolute path. This is to maintain backwards compatibility + // with the previous behavior of the runtime-config.file of allowing relative and absolute paths. + t.Cfg.RuntimeConfig.StorageConfig.Filesystem.Directory = "/" + } + } + } return bucket.NewClient(ctx, t.Cfg.RuntimeConfig.StorageConfig, "runtime-config", logger, registerer) } serv, err := runtimeconfig.New(t.Cfg.RuntimeConfig, registerer, logger, bucketClientFactory)