diff --git a/config/BUILD.bazel b/config/BUILD.bazel index 0c95044a..bf0b6118 100644 --- a/config/BUILD.bazel +++ b/config/BUILD.bazel @@ -1,4 +1,4 @@ -load("@rules_go//go:def.bzl", "go_library") +load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "config", @@ -12,3 +12,13 @@ go_library( visibility = ["//visibility:public"], deps = ["@com_github_goccy_go_yaml//:go-yaml"], ) + +go_test( + name = "config_test", + srcs = ["config_test.go"], + embed = [":config"], + deps = [ + "@com_github_stretchr_testify//assert", + "@com_github_stretchr_testify//require", + ], +) diff --git a/config/config.go b/config/config.go index 77ca4939..ceb52c1c 100644 --- a/config/config.go +++ b/config/config.go @@ -22,6 +22,10 @@ import ( yaml "github.com/goccy/go-yaml" ) +const _defaultBazelQueryTimeoutSeconds = 600 + +var _bzlmodEnabledDefault = true + var _ RepositoryConfigProvider = (*Config)(nil) // Config is the root configuration structure. @@ -58,17 +62,14 @@ func Parse(configFilePath string) (*Config, error) { if config.Storage.Type == "" { config.Storage.Type = StorageTypeMemory } - if config.Service.WorkerRootPath != "" && config.Service.RepoManagerClonePath == "" { - return nil, fmt.Errorf("service.repo_manager_clone_path must be set when worker_root_path is specified") - } - if config.Service.RepoManagerClonePath == "" { - config.Service.RepoManagerClonePath = filepath.Join(os.TempDir(), "tango-repo-manager") + if config.Service.WorkspacesRootPath == "" { + return nil, fmt.Errorf("service.workspaces_root_path must be set") } if config.Service.WorkerRootPath == "" { - config.Service.WorkerRootPath = filepath.Join(config.Service.RepoManagerClonePath, ".workers") + config.Service.WorkerRootPath = filepath.Join(config.Service.WorkspacesRootPath, ".workers") } - if config.Service.WorkerPoolSize <= 0 { - return nil, fmt.Errorf("service.worker_pool_size must be > 0, got %d", config.Service.WorkerPoolSize) + if config.Service.MaxWorkerPoolSize <= 0 { + return nil, fmt.Errorf("service.max_worker_pool_size must be > 0, got %d", config.Service.MaxWorkerPoolSize) } config.repositoryByRemote = make(map[string]*RepositoryConfig, len(config.Repository)) for i := range config.Repository { @@ -79,6 +80,12 @@ func Parse(configFilePath string) (*Config, error) { if _, exists := config.repositoryByRemote[remote]; exists { return nil, fmt.Errorf("duplicate repository remote %q", remote) } + if config.Repository[i].BzlmodEnabled == nil { + config.Repository[i].BzlmodEnabled = &_bzlmodEnabledDefault + } + if config.Repository[i].QueryTimeoutSeconds <= 0 { + config.Repository[i].QueryTimeoutSeconds = _defaultBazelQueryTimeoutSeconds + } config.repositoryByRemote[remote] = &config.Repository[i] } return &config, nil diff --git a/config/config_test.go b/config/config_test.go new file mode 100644 index 00000000..77213d5d --- /dev/null +++ b/config/config_test.go @@ -0,0 +1,159 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const _baseServiceYAML = ` +service: + workspaces_root_path: "/tmp/x" + max_worker_pool_size: 1 +` + +func writeConfig(t *testing.T, contents string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "tango-config.yaml") + require.NoError(t, os.WriteFile(path, []byte(contents), 0o600)) + return path +} + +func TestParse_ServiceValidation(t *testing.T) { + tests := []struct { + name string + give string + }{ + { + name: "workspaces_root_path required", + give: ` +service: + max_worker_pool_size: 1 +repository: + - remote: "r1" +`, + }, + { + name: "max_worker_pool_size must be positive", + give: ` +service: + workspaces_root_path: "/tmp/x" + max_worker_pool_size: 0 +repository: + - remote: "r1" +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := Parse(writeConfig(t, tt.give)) + require.Error(t, err) + }) + } +} + +func TestParse_ServiceDefaults(t *testing.T) { + tests := []struct { + name string + give string + wantWorkerRootPath string + }{ + { + name: "worker_root_path defaults when unset", + give: _baseServiceYAML + ` +repository: + - remote: "r1" +`, + wantWorkerRootPath: filepath.Join("/tmp/x", ".workers"), + }, + { + name: "worker_root_path explicit value preserved", + give: ` +service: + workspaces_root_path: "/tmp/x" + worker_root_path: "/tmp/custom-workers" + max_worker_pool_size: 1 +repository: + - remote: "r1" +`, + wantWorkerRootPath: "/tmp/custom-workers", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg, err := Parse(writeConfig(t, tt.give)) + require.NoError(t, err) + assert.Equal(t, tt.wantWorkerRootPath, cfg.Service.WorkerRootPath) + }) + } +} + +func TestParse_RepositoryDefaults(t *testing.T) { + tests := []struct { + name string + give string + wantBzlmodEnabled bool + wantQueryTimeoutSeconds int64 + }{ + { + name: "bzlmod_enabled and query_timeout_seconds default when unset", + give: _baseServiceYAML + ` +repository: + - remote: "r1" +`, + wantBzlmodEnabled: true, + wantQueryTimeoutSeconds: _defaultBazelQueryTimeoutSeconds, + }, + { + name: "bzlmod_enabled explicit false preserved", + give: _baseServiceYAML + ` +repository: + - remote: "r1" + bzlmod_enabled: false +`, + wantBzlmodEnabled: false, + wantQueryTimeoutSeconds: _defaultBazelQueryTimeoutSeconds, + }, + { + name: "query_timeout_seconds explicit value preserved", + give: _baseServiceYAML + ` +repository: + - remote: "r1" + query_timeout_seconds: 120 +`, + wantBzlmodEnabled: true, + wantQueryTimeoutSeconds: 120, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg, err := Parse(writeConfig(t, tt.give)) + require.NoError(t, err) + repoCfg, ok := cfg.GetRepositoryConfig("r1") + require.True(t, ok) + require.NotNil(t, repoCfg.BzlmodEnabled) + assert.Equal(t, tt.wantBzlmodEnabled, *repoCfg.BzlmodEnabled) + assert.Equal(t, tt.wantQueryTimeoutSeconds, repoCfg.QueryTimeoutSeconds) + }) + } +} diff --git a/config/repository_config.go b/config/repository_config.go index 4052ec4c..71cb0e39 100644 --- a/config/repository_config.go +++ b/config/repository_config.go @@ -16,15 +16,32 @@ package config // RepositoryConfig holds configuration for a single repository. type RepositoryConfig struct { - Remote string `yaml:"remote"` + // Remote is the URL used to `git clone` the repository. Tango clones from + // this URL and uses it as the lookup key for per-repo settings. Must be + // unique across all entries and match exactly what clients send in + // BuildDescription.remote. + Remote string `yaml:"remote"` + // TODO: FullHashRepos, ExcludedFiles, and ExcludeExternalTargets are not + // documented in config/README.md. Delete them if they turn out to be unneeded, + // otherwise document them there. FullHashRepos []string `yaml:"full_hash_repos"` ExcludedFiles []string `yaml:"excluded_files"` ExcludeExternalTargets bool `yaml:"exclude_external_targets"` - BzlmodEnabled bool `yaml:"bzlmod_enabled"` - BazelCommand string `yaml:"bazel_command"` - QueryTimeout int64 `yaml:"query_timeout"` // in seconds - BazelExtraArgs []string `yaml:"bazel_extra_args"` - StreamBazelLogs bool `yaml:"stream_bazel_logs"` + // BzlmodEnabled indicates whether this repository uses Bzlmod for external + // dependency management. Defaults to true if unset. Set to false only for + // repositories still using WORKSPACE. + BzlmodEnabled *bool `yaml:"bzlmod_enabled"` + // BazelCommandPath overrides the Bazel binary path. When empty, Tango + // automatically downloads and caches Bazelisk from GitHub. + BazelCommandPath string `yaml:"bazel_command_path"` + // QueryTimeoutSeconds is the Bazel query timeout in seconds. Defaults to 600. + QueryTimeoutSeconds int64 `yaml:"query_timeout_seconds"` + // BazelExtraArgs are extra arguments passed to `bazel query` invocations, + // inserted between the `query` subcommand and the query expression. + BazelExtraArgs []string `yaml:"bazel_extra_args"` + // TODO: StreamBazelLogs is not documented in config/README.md. Delete it if + // it turns out to be unneeded, otherwise document it there. + StreamBazelLogs bool `yaml:"stream_bazel_logs"` } // RepositoryConfigProvider looks up per-repository configuration by remote. diff --git a/config/service_config.go b/config/service_config.go index 2245f35f..733126b3 100644 --- a/config/service_config.go +++ b/config/service_config.go @@ -16,23 +16,32 @@ package config // ServiceConfig holds operational configuration for the Tango service. type ServiceConfig struct { - WorkerPoolSize int `yaml:"worker_pool_size"` // number of worker workspaces per repo - RepoManagerClonePath string `yaml:"repo_manager_clone_path"` // root directory for origin repo clones - WorkerRootPath string `yaml:"worker_root_path"` // root directory for worker workspace checkouts; defaults to repo_manager_clone_path/.workers - Chunking ChunkConfig `yaml:"chunking"` // streaming chunk sizes; zero values fall back to package defaults + // MaxWorkerPoolSize is the max number of concurrent requests per repository. + // Each worker is a lightweight local clone (hardlinked to the origin, not a + // full copy) that handles one request at a time. Must be greater than 0. + MaxWorkerPoolSize int `yaml:"max_worker_pool_size"` + // WorkspacesRootPath is the root directory where Tango stores repository clones + // and worker checkouts. Required. Layout: // for + // origin clones and /.workers//worker-{1..N}/ for + // worker checkouts. + WorkspacesRootPath string `yaml:"workspaces_root_path"` + // TODO: WorkerRootPath is not documented in config/README.md. Delete it if + // it turns out to be unneeded, otherwise document it there. + WorkerRootPath string `yaml:"worker_root_path"` // root directory for worker workspace checkouts; defaults to workspaces_root_path/.workers + Streaming ChunkConfig `yaml:"streaming"` // streaming chunk sizes; zero values fall back to package defaults } // ChunkConfig controls the number of entries per gRPC stream message. // All fields are optional; a zero value means "use the package default". // Tune these when a monorepo's per-target size causes messages to approach -// the 64MB default gRPC per-message limit. +// gRPC's default 4MB per-message limit. type ChunkConfig struct { - // TargetChunkSize is the max number of OptimizedTarget entries per stream message. - TargetChunkSize int `yaml:"target_chunk_size"` - // ChangedTargetChunkSize is the max number of ChangedTarget entries per stream message. + // MaxNumTargets is the max number of OptimizedTarget entries per stream message. + MaxNumTargets int `yaml:"max_num_targets"` + // MaxNumChangedTargets is the max number of ChangedTarget entries per stream message. // ChangedTarget carries both old and new targets (~2× the size of a regular target). - ChangedTargetChunkSize int `yaml:"changed_target_chunk_size"` - // MetadataMapChunkSize is the max number of entries per metadata map chunk. + MaxNumChangedTargets int `yaml:"max_num_changed_targets"` + // MaxNumMetadataEntries is the max number of entries per metadata map chunk. // Applies to target_id_mapping and attribute_string_value_mapping. - MetadataMapChunkSize int `yaml:"metadata_map_chunk_size"` + MaxNumMetadataEntries int `yaml:"max_num_metadata_entries"` } diff --git a/controller/controller.go b/controller/controller.go index 34ca33c2..30523806 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -64,15 +64,15 @@ func NewController(appCtx context.Context, p Params) pb.TangoYARPCServer { if scope == nil { scope = tally.NoopScope } - targetChunkSize := p.ChunkConfig.TargetChunkSize + targetChunkSize := p.ChunkConfig.MaxNumTargets if targetChunkSize <= 0 { targetChunkSize = common.DefaultTargetChunkSize } - changedTargetChunkSize := p.ChunkConfig.ChangedTargetChunkSize + changedTargetChunkSize := p.ChunkConfig.MaxNumChangedTargets if changedTargetChunkSize <= 0 { changedTargetChunkSize = common.DefaultChangedTargetChunkSize } - metadataMapChunkSize := p.ChunkConfig.MetadataMapChunkSize + metadataMapChunkSize := p.ChunkConfig.MaxNumMetadataEntries if metadataMapChunkSize <= 0 { metadataMapChunkSize = common.DefaultMetadataMapChunkSize } diff --git a/example/main.go b/example/main.go index 51a93a28..6aba8d0c 100644 --- a/example/main.go +++ b/example/main.go @@ -71,7 +71,7 @@ func run() error { logger.Infof("Using storage type: %s", cfg.Storage.Type) // Repo manager and orchestrator - repoManagerClonePath := cfg.Service.RepoManagerClonePath + repoManagerClonePath := cfg.Service.WorkspacesRootPath workerRootPath := cfg.Service.WorkerRootPath if err := os.MkdirAll(repoManagerClonePath, 0o755); err != nil { return fmt.Errorf("failed to create repo manager clone path: %w", err) @@ -87,7 +87,7 @@ func run() error { Logger: logger, RepoManagerClonePath: repoManagerClonePath, WorkerRootPath: workerRootPath, - PoolSize: cfg.Service.WorkerPoolSize, + PoolSize: cfg.Service.MaxWorkerPoolSize, }) if err != nil { return fmt.Errorf("failed to create repo manager: %w", err) @@ -109,6 +109,7 @@ func run() error { Logger: zl, Storage: store, Orchestrator: orch, + ChunkConfig: cfg.Service.Streaming, }) // YARPC transports and dispatcher diff --git a/example/tango-config.yaml b/example/tango-config.yaml index ec43e6ad..86059963 100644 --- a/example/tango-config.yaml +++ b/example/tango-config.yaml @@ -15,12 +15,12 @@ repository: - "^@@?bazel_tools/" exclude_external_targets: true bzlmod_enabled: true - query_timeout: 300 + query_timeout_seconds: 300 # Service configuration service: - worker_pool_size: 5 - # root for origin clones; defaults to os.TempDir()/tango-repo-manager - repo_manager_clone_path: "/tmp/tango-repo-manager" - # root for worker checkouts; defaults to repo_manager_clone_path/.workers + max_worker_pool_size: 5 + # root for origin clones; required + workspaces_root_path: "/tmp/tango-repo-manager" + # root for worker checkouts; defaults to workspaces_root_path/.workers worker_root_path: "/tmp/tango/workers" diff --git a/graphrunner/native.go b/graphrunner/native.go index cf5582d4..0eb1c0af 100644 --- a/graphrunner/native.go +++ b/graphrunner/native.go @@ -94,7 +94,7 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace) KnownSourceHashes: knownSourceHashes, FullHashRepos: g.config.FullHashRepos, ExcludedRegex: append(g.config.ExcludedFiles, g.extraExcludedFiles...), - UseBzlmod: g.config.BzlmodEnabled, + UseBzlmod: g.config.BzlmodEnabled == nil || *g.config.BzlmodEnabled, } hashStart := time.Now() diff --git a/integration/testdata/tango-config.yaml.tmpl b/integration/testdata/tango-config.yaml.tmpl index 8d7c5ee6..a6bfb93c 100644 --- a/integration/testdata/tango-config.yaml.tmpl +++ b/integration/testdata/tango-config.yaml.tmpl @@ -5,15 +5,15 @@ repository: - remote: {{.Remote}} default_branch: "main" {{- if .BazelCommand}} - bazel_command: {{.BazelCommand}} + bazel_command_path: {{.BazelCommand}} {{- end}} full_hash_repos: [""] excluded_files: ["^@@?bazel_tools/"] exclude_external_targets: true bzlmod_enabled: true - query_timeout: 600 + query_timeout_seconds: 600 service: - worker_pool_size: 2 - repo_manager_clone_path: {{.ClonePath}} + max_worker_pool_size: 2 + workspaces_root_path: {{.ClonePath}} worker_root_path: {{.WorkerPath}} diff --git a/orchestrator/native_orchestrator.go b/orchestrator/native_orchestrator.go index 0ca9e389..ff7701b0 100644 --- a/orchestrator/native_orchestrator.go +++ b/orchestrator/native_orchestrator.go @@ -189,8 +189,8 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, req entity.GetT client, err := bazel.NewBazelClient(ctx, bazel.Params{ WorkspacePath: ws.Path(), Logger: b.logger, - BazelCommand: repoCfg.BazelCommand, - QueryTimeout: time.Duration(repoCfg.QueryTimeout) * time.Second, + BazelCommand: repoCfg.BazelCommandPath, + QueryTimeout: time.Duration(repoCfg.QueryTimeoutSeconds) * time.Second, StreamLogs: repoCfg.StreamBazelLogs, }) if err != nil { @@ -210,8 +210,8 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, req entity.GetT return nil, fmt.Errorf("compute target graph: %w", err) } responses, err := common.ResultToGetTargetGraphResponse(ctx, result, - b.config.Service.Chunking.TargetChunkSize, - b.config.Service.Chunking.MetadataMapChunkSize, + b.config.Service.Streaming.MaxNumTargets, + b.config.Service.Streaming.MaxNumMetadataEntries, ) if err != nil { return nil, fmt.Errorf("convert target graph to response: %w", err) diff --git a/orchestrator/testdata/config.yaml b/orchestrator/testdata/config.yaml index 34887e08..4f3d045e 100644 --- a/orchestrator/testdata/config.yaml +++ b/orchestrator/testdata/config.yaml @@ -7,11 +7,12 @@ repository: - "*.gen.go" exclude_external_targets: true bzlmod_enabled: true - query_timeout: 15 + query_timeout_seconds: 15 service: - worker_pool_size: 3 - chunking: - target_chunk_size: 250 - changed_target_chunk_size: 125 - metadata_map_chunk_size: 50000 + max_worker_pool_size: 3 + workspaces_root_path: "/tmp/tango-repo-manager" + streaming: + max_num_targets: 250 + max_num_changed_targets: 125 + max_num_metadata_entries: 50000