Skip to content

Conversation

djeebus
Copy link
Contributor

@djeebus djeebus commented Oct 4, 2025

Pre-requisite for supporting multiple orchestrators running on a single node.


Note

Introduce a typed env-based config and refactor logging, service discovery, pools, and main to use it (with ports as uint16) instead of raw env vars.

  • Config:
    • Add internal/cfg with Config and ServiceDiscoveryConfig parsed via caarlos0/env (Parse).
    • New env fields for ports, Loki, Redis, SD providers; add SkipInitialOrchestratorReadinessCheck.
  • Service Discovery:
    • Replace env-var builder with BuildServiceDiscoveryProvider(cfg, port); create provider constructors accepting cfg.ServiceDiscoveryConfig.
    • Change ServiceDiscoveryItem.NodePort and all SD ports to uint16; update DNS/K8s/Nomad/Static implementations.
    • Add clear error vars for missing config.
  • Logging (Loki):
    • GetLogsQueryProvider(cfg) and NewLokiQueryProvider(cfg); use config.LokiURL/User/Password; add ErrEmptyLokiURL.
  • Edge API:
    • handlers.NewStore and edge.NewEdgeAPIStore accept cfg.Config; use SkipInitialOrchestratorReadinessCheck.
  • Orchestrator/Edge Pools:
    • Port parameters switched to uint16.
  • Main:
    • Parse config at startup; remove internal getters and direct env reads.
    • Build SD for Edge/Orchestrator separately; use config for ports, Redis, and flags; update logs to use Uint16 fields.
  • Deps/Tests:
    • Add github.com/caarlos0/env/v11 and direct testify dependency.
    • Add internal/cfg/model_test.go for basic parsing.

Written by Cursor Bugbot for commit 5a7f2f3. This will update automatically on new commits. Configure here.

cursor[bot]

This comment was marked as outdated.

# Conflicts:
#	packages/client-proxy/internal/edge/handlers/store.go
#	packages/client-proxy/internal/edge/service.go
#	packages/client-proxy/main.go
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

return nil, fmt.Errorf("loki address is empty, please set the %s environment variable", lokiAddressEnvName)
func NewLokiQueryProvider(config cfg.Config) (*LokiQueryProvider, error) {
if config.LokiURL == "" {
return nil, ErrEmptyLokiURL
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Unreachable Empty Check for Required URL

The empty check for config.LokiURL in NewLokiQueryProvider is unreachable. Since LokiURL is a required configuration field, cfg.Parse() will prevent application startup if the URL is missing or empty, making the explicit check here redundant.

Fix in Cursor Fix in Web

@ValentaTomas ValentaTomas self-assigned this Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants