Skip to content

Commit

Permalink
Merge branch 'cortexproject:master' into master
Browse files Browse the repository at this point in the history
  • Loading branch information
afhassan authored Sep 27, 2024
2 parents a9a8a62 + fbe118b commit 557f3d0
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 14 additions & 0 deletions integration/e2e/scenario_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
27 changes: 24 additions & 3 deletions integration/e2e/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -151,6 +158,9 @@ func (s *ConcreteService) Stop() error {
logger.Log(string(out))
return err
}

s.Wait()

s.usedNetworkName = ""

return nil
Expand All @@ -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.
//
Expand Down Expand Up @@ -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))
Expand Down
29 changes: 27 additions & 2 deletions integration/runtime_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -57,13 +58,37 @@ 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{}))

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)
Expand Down
14 changes: 14 additions & 0 deletions pkg/cortex/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 557f3d0

Please sign in to comment.