all: extract remaining magic numbers to named constants

Define named constants for various timeout and configuration values:
- Connection validation and retry timeouts in helpers
- Peer sync timeouts in integrationutil
- Run ID hash length and parts in dockertestutil
- Container memory limits and directory permissions
- HTML parsing split count in scenario
- Container restart and backoff timeouts in tsic
- Stats calculation constants in cmd/hi
This commit is contained in:
Kristoffer Dalby 2026-01-20 16:18:07 +00:00
parent b1463dff1e
commit 12b3da0181
9 changed files with 87 additions and 33 deletions

View file

@ -30,6 +30,13 @@ var (
ErrMemoryLimitExceeded = errors.New("container exceeded memory limits")
)
// Docker container constants.
const (
containerFinalStateWait = 10 * time.Second
containerStateCheckInterval = 500 * time.Millisecond
dirPermissions = 0o755
)
// runTestContainer executes integration tests in a Docker container.
func runTestContainer(ctx context.Context, config *RunConfig) error {
cli, err := createDockerClient()
@ -351,8 +358,8 @@ func waitForContainerFinalization(ctx context.Context, cli *client.Client, testC
testContainers := getCurrentTestContainers(containers, testContainerID, verbose)
// Wait for all test containers to reach a final state
maxWaitTime := 10 * time.Second
checkInterval := 500 * time.Millisecond
maxWaitTime := containerFinalStateWait
checkInterval := containerStateCheckInterval
timeout := time.After(maxWaitTime)
ticker := time.NewTicker(checkInterval)
@ -720,7 +727,7 @@ func getCurrentTestContainers(containers []container.Summary, testContainerID st
// extractContainerArtifacts saves logs and tar files from a container.
func extractContainerArtifacts(ctx context.Context, cli *client.Client, containerID, containerName, logsDir string, verbose bool) error {
// Ensure the logs directory exists
if err := os.MkdirAll(logsDir, 0o755); err != nil {
if err := os.MkdirAll(logsDir, dirPermissions); err != nil {
return fmt.Errorf("failed to create logs directory: %w", err)
}

View file

@ -21,6 +21,12 @@ import (
// Sentinel errors for stats collection.
var ErrStatsCollectionAlreadyStarted = errors.New("stats collection already started")
// Stats calculation constants.
const (
bytesPerKB = 1024
percentageMultiplier = 100.0
)
// ContainerStats represents statistics for a single container.
type ContainerStats struct {
ContainerID string
@ -269,7 +275,7 @@ func (sc *StatsCollector) collectStatsForContainer(ctx context.Context, containe
}
// Calculate memory usage in MB
memoryMB := float64(stats.MemoryStats.Usage) / (1024 * 1024)
memoryMB := float64(stats.MemoryStats.Usage) / (bytesPerKB * bytesPerKB)
// Store the sample (skip first sample since CPU calculation needs previous stats)
if prevStats != nil {
@ -314,7 +320,7 @@ func calculateCPUPercent(prevStats, stats *container.StatsResponse) float64 {
numCPUs = 1.0
}
return (cpuDelta / systemDelta) * numCPUs * 100.0
return (cpuDelta / systemDelta) * numCPUs * percentageMultiplier
}
return 0.0

View file

@ -14,6 +14,11 @@ const (
// TimestampFormatRunID is used for generating unique run identifiers
// Format: "20060102-150405" provides compact date-time for file/directory names.
TimestampFormatRunID = "20060102-150405"
// runIDHashLength is the length of the random hash in run IDs.
runIDHashLength = 6
// runIDParts is the number of parts in a run ID (YYYYMMDD-HHMMSS-HASH).
runIDParts = 3
)
// GetIntegrationRunID returns the run ID for the current integration test session.
@ -46,7 +51,7 @@ func GenerateRunID() string {
timestamp := now.Format(TimestampFormatRunID)
// Add a short random hash to ensure uniqueness
randomHash := util.MustGenerateRandomStringDNSSafe(6)
randomHash := util.MustGenerateRandomStringDNSSafe(runIDHashLength)
return fmt.Sprintf("%s-%s", timestamp, randomHash)
}
@ -55,9 +60,9 @@ func GenerateRunID() string {
// Expects format: "prefix-YYYYMMDD-HHMMSS-HASH".
func ExtractRunIDFromContainerName(containerName string) string {
parts := strings.Split(containerName, "-")
if len(parts) >= 3 {
if len(parts) >= runIDParts {
// Return the last three parts as the run ID (YYYYMMDD-HHMMSS-HASH)
return strings.Join(parts[len(parts)-3:], "-")
return strings.Join(parts[len(parts)-runIDParts:], "-")
}
panic("unexpected container name format: " + containerName)

View file

@ -13,6 +13,12 @@ import (
var ErrContainerNotFound = errors.New("container not found")
// Docker memory constants.
const (
bytesPerKB = 1024
containerMemoryGB = 2
)
func GetFirstOrCreateNetwork(pool *dockertest.Pool, name string) (*dockertest.Network, error) {
networks, err := pool.NetworksByName(name)
if err != nil {
@ -172,6 +178,6 @@ func DockerAllowNetworkAdministration(config *docker.HostConfig) {
// DockerMemoryLimit sets memory limit and disables OOM kill for containers.
func DockerMemoryLimit(config *docker.HostConfig) {
config.Memory = 2 * 1024 * 1024 * 1024 // 2GB in bytes
config.Memory = containerMemoryGB * bytesPerKB * bytesPerKB * bytesPerKB // 2GB in bytes
config.OOMKillDisable = true
}

View file

@ -53,6 +53,16 @@ const (
// TimestampFormatRunID is used for generating unique run identifiers
// Format: "20060102-150405" provides compact date-time for file/directory names.
TimestampFormatRunID = "20060102-150405"
// Connection validation timeouts.
connectionValidationTimeout = 120 * time.Second
onlineCheckRetryInterval = 2 * time.Second
batcherValidationTimeout = 15 * time.Second
nodestoreValidationTimeout = 20 * time.Second
mapResponseTimeout = 60 * time.Second
netInfoRetryInterval = 5 * time.Second
backoffMaxElapsedTime = 10 * time.Second
backoffRetryInterval = 500 * time.Millisecond
)
// NodeSystemStatus represents the status of a node across different systems.
@ -134,7 +144,7 @@ func collectExpectedNodeIDs(t *testing.T, clients []TailscaleClient) []types.Nod
func validateInitialConnection(t *testing.T, headscale ControlServer, expectedNodes []types.NodeID) {
t.Helper()
requireAllClientsOnline(t, headscale, expectedNodes, true, "all clients should be connected after initial login", 120*time.Second)
requireAllClientsOnline(t, headscale, expectedNodes, true, "all clients should be connected after initial login", connectionValidationTimeout)
requireAllClientsNetInfoAndDERP(t, headscale, expectedNodes, "all clients should have NetInfo and DERP after initial login")
}
@ -144,7 +154,7 @@ func validateInitialConnection(t *testing.T, headscale ControlServer, expectedNo
func validateLogoutComplete(t *testing.T, headscale ControlServer, expectedNodes []types.NodeID) {
t.Helper()
requireAllClientsOnline(t, headscale, expectedNodes, false, "all nodes should be offline after logout", 120*time.Second)
requireAllClientsOnline(t, headscale, expectedNodes, false, "all nodes should be offline after logout", connectionValidationTimeout)
}
// validateReloginComplete performs comprehensive validation after client relogin.
@ -153,7 +163,7 @@ func validateLogoutComplete(t *testing.T, headscale ControlServer, expectedNodes
func validateReloginComplete(t *testing.T, headscale ControlServer, expectedNodes []types.NodeID) {
t.Helper()
requireAllClientsOnline(t, headscale, expectedNodes, true, "all clients should be connected after relogin", 120*time.Second)
requireAllClientsOnline(t, headscale, expectedNodes, true, "all clients should be connected after relogin", connectionValidationTimeout)
requireAllClientsNetInfoAndDERP(t, headscale, expectedNodes, "all clients should have NetInfo and DERP after relogin")
}
@ -369,7 +379,7 @@ func requireAllClientsOnlineWithSingleTimeout(t *testing.T, headscale ControlSer
}
assert.True(c, allMatch, fmt.Sprintf("Not all %d nodes are %s across all systems (batcher, mapresponses, nodestore)", len(expectedNodes), stateStr))
}, timeout, 2*time.Second, message)
}, timeout, onlineCheckRetryInterval, message)
}
// requireAllClientsOfflineStaged validates offline state with staged timeouts for different components.
@ -398,7 +408,7 @@ func requireAllClientsOfflineStaged(t *testing.T, headscale ControlServer, expec
}
assert.True(c, allBatcherOffline, "All nodes should be disconnected from batcher")
}, 15*time.Second, 1*time.Second, "batcher disconnection validation")
}, batcherValidationTimeout, 1*time.Second, "batcher disconnection validation")
// Stage 2: Verify nodestore offline status (up to 15 seconds due to disconnect detection delay)
t.Logf("Stage 2: Verifying nodestore offline status for %d nodes (allowing for 10s disconnect detection delay)", len(expectedNodes))
@ -424,7 +434,7 @@ func requireAllClientsOfflineStaged(t *testing.T, headscale ControlServer, expec
}
assert.True(c, allNodeStoreOffline, "All nodes should be offline in nodestore")
}, 20*time.Second, 1*time.Second, "nodestore offline validation")
}, nodestoreValidationTimeout, 1*time.Second, "nodestore offline validation")
// Stage 3: Verify map response propagation (longest delay due to peer update timing)
t.Logf("Stage 3: Verifying map response propagation for %d nodes (allowing for peer map update delays)", len(expectedNodes))
@ -466,7 +476,7 @@ func requireAllClientsOfflineStaged(t *testing.T, headscale ControlServer, expec
}
assert.True(c, allMapResponsesOffline, "All nodes should be absent from peer map responses")
}, 60*time.Second, 2*time.Second, "map response propagation validation")
}, mapResponseTimeout, onlineCheckRetryInterval, "map response propagation validation")
t.Logf("All stages completed: nodes are fully offline across all systems")
}
@ -528,7 +538,7 @@ func requireAllClientsNetInfoAndDERP(t *testing.T, headscale ControlServer, expe
t.Logf("Node %d (%s) has valid NetInfo with DERP server %d at %s", nodeID, node.Hostname, preferredDERP, time.Now().Format(TimestampFormat))
}
}, timeout, 5*time.Second, message)
}, timeout, netInfoRetryInterval, message)
endTime := time.Now()
duration := endTime.Sub(startTime)
@ -658,7 +668,7 @@ func assertCommandOutputContains(t *testing.T, c TailscaleClient, command []stri
}
return struct{}{}, nil
}, backoff.WithBackOff(backoff.NewExponentialBackOff()), backoff.WithMaxElapsedTime(10*time.Second))
}, backoff.WithBackOff(backoff.NewExponentialBackOff()), backoff.WithMaxElapsedTime(backoffMaxElapsedTime))
assert.NoError(t, err)
}
@ -890,7 +900,7 @@ func (s *Scenario) AddAndLoginClient(
}
return struct{}{}, nil
}, backoff.WithBackOff(backoff.NewConstantBackOff(500*time.Millisecond)), backoff.WithMaxElapsedTime(10*time.Second))
}, backoff.WithBackOff(backoff.NewConstantBackOff(backoffRetryInterval)), backoff.WithMaxElapsedTime(backoffMaxElapsedTime))
if err != nil {
return nil, fmt.Errorf("timeout waiting for new client: %w", err)
}

View file

@ -46,6 +46,7 @@ const (
tlsKeyPath = "/etc/headscale/tls.key"
headscaleDefaultPort = 8080
IntegrationTestDockerFileName = "Dockerfile.integration"
dirPermissions = 0o755
)
var (
@ -720,7 +721,7 @@ func (t *HeadscaleInContainer) SaveMetrics(savePath string) error {
// extractTarToDirectory extracts a tar archive to a directory.
func extractTarToDirectory(tarData []byte, targetDir string) error {
if err := os.MkdirAll(targetDir, 0o755); err != nil {
if err := os.MkdirAll(targetDir, dirPermissions); err != nil {
return fmt.Errorf("failed to create directory %s: %w", targetDir, err)
}
@ -784,7 +785,7 @@ func extractTarToDirectory(tarData []byte, targetDir string) error {
}
case tar.TypeReg:
// Ensure parent directories exist
if err := os.MkdirAll(filepath.Dir(targetPath), 0o755); err != nil {
if err := os.MkdirAll(filepath.Dir(targetPath), dirPermissions); err != nil {
return fmt.Errorf("failed to create parent directories for %s: %w", targetPath, err)
}

View file

@ -22,19 +22,29 @@ import (
"tailscale.com/tailcfg"
)
// Integration test timing constants.
const (
// peerSyncTimeoutCI is the peer sync timeout for CI environments.
peerSyncTimeoutCI = 120 * time.Second
// peerSyncTimeoutDev is the peer sync timeout for development environments.
peerSyncTimeoutDev = 60 * time.Second
// peerSyncRetryIntervalMs is the retry interval for peer sync checks.
peerSyncRetryIntervalMs = 100
)
// PeerSyncTimeout returns the timeout for peer synchronization based on environment:
// 60s for dev, 120s for CI.
func PeerSyncTimeout() time.Duration {
if util.IsCI() {
return 120 * time.Second
return peerSyncTimeoutCI
}
return 60 * time.Second
return peerSyncTimeoutDev
}
// PeerSyncRetryInterval returns the retry interval for peer synchronization checks.
func PeerSyncRetryInterval() time.Duration {
return 100 * time.Millisecond
return peerSyncRetryIntervalMs * time.Millisecond
}
func WriteFileToContainer(

View file

@ -46,6 +46,8 @@ import (
const (
scenarioHashLength = 6
// expectedHTMLSplitParts is the expected number of parts when splitting HTML for key extraction.
expectedHTMLSplitParts = 2
)
var usePostgresForTest = envknob.Bool("HEADSCALE_INTEGRATION_POSTGRES")
@ -1153,17 +1155,17 @@ var errParseAuthPage = errors.New("failed to parse auth page")
func (s *Scenario) runHeadscaleRegister(userStr string, body string) error {
// see api.go HTML template
codeSep := strings.Split(string(body), "</code>")
if len(codeSep) != 2 {
if len(codeSep) != expectedHTMLSplitParts {
return errParseAuthPage
}
keySep := strings.Split(codeSep[0], "key ")
if len(keySep) != 2 {
if len(keySep) != expectedHTMLSplitParts {
return errParseAuthPage
}
key := keySep[1]
key = strings.SplitN(key, " ", 2)[0]
key = strings.SplitN(key, " ", expectedHTMLSplitParts)[0]
log.Printf("registering node %s", key)
if headscale, err := s.Headscale(); err == nil {

View file

@ -44,6 +44,13 @@ const (
dockerContextPath = "../."
caCertRoot = "/usr/local/share/ca-certificates"
dockerExecuteTimeout = 60 * time.Second
// Container restart and backoff timeouts.
containerRestartTimeout = 30 // seconds, used by Docker API
tailscaleVersionTimeout = 5 * time.Second
containerRestartBackoff = 30 * time.Second
backoffMaxElapsedTime = 10 * time.Second
curlFailFastMaxTime = 2 * time.Second
)
var (
@ -747,7 +754,7 @@ func (t *TailscaleInContainer) Restart() error {
}
// Use Docker API to restart the container
err := t.pool.Client.RestartContainer(t.container.Container.ID, 30)
err := t.pool.Client.RestartContainer(t.container.Container.ID, containerRestartTimeout)
if err != nil {
return fmt.Errorf("failed to restart container %s: %w", t.hostname, err)
}
@ -756,13 +763,13 @@ func (t *TailscaleInContainer) Restart() error {
// We use exponential backoff to poll until we can successfully execute a command
_, err = backoff.Retry(context.Background(), func() (struct{}, error) {
// Try to execute a simple command to verify the container is responsive
_, _, err := t.Execute([]string{"tailscale", "version"}, dockertestutil.ExecuteCommandTimeout(5*time.Second))
_, _, err := t.Execute([]string{"tailscale", "version"}, dockertestutil.ExecuteCommandTimeout(tailscaleVersionTimeout))
if err != nil {
return struct{}{}, fmt.Errorf("container not ready: %w", err)
}
return struct{}{}, nil
}, backoff.WithBackOff(backoff.NewExponentialBackOff()), backoff.WithMaxElapsedTime(30*time.Second))
}, backoff.WithBackOff(backoff.NewExponentialBackOff()), backoff.WithMaxElapsedTime(containerRestartBackoff))
if err != nil {
return fmt.Errorf("timeout waiting for container %s to restart and become ready: %w", t.hostname, err)
}
@ -847,7 +854,7 @@ func (t *TailscaleInContainer) IPs() ([]netip.Addr, error) {
}
return ips, nil
}, backoff.WithBackOff(backoff.NewExponentialBackOff()), backoff.WithMaxElapsedTime(10*time.Second))
}, backoff.WithBackOff(backoff.NewExponentialBackOff()), backoff.WithMaxElapsedTime(backoffMaxElapsedTime))
if err != nil {
return nil, fmt.Errorf("failed to get IPs for %s after retries: %w", t.hostname, err)
}
@ -1151,7 +1158,7 @@ func (t *TailscaleInContainer) FQDN() (string, error) {
}
return status.Self.DNSName, nil
}, backoff.WithBackOff(backoff.NewExponentialBackOff()), backoff.WithMaxElapsedTime(10*time.Second))
}, backoff.WithBackOff(backoff.NewExponentialBackOff()), backoff.WithMaxElapsedTime(backoffMaxElapsedTime))
if err != nil {
return "", fmt.Errorf("failed to get FQDN for %s after retries: %w", t.hostname, err)
}
@ -1507,7 +1514,7 @@ func (t *TailscaleInContainer) CurlFailFast(url string) (string, error) {
// Use aggressive timeouts for fast failure detection
return t.Curl(url,
WithCurlConnectionTimeout(1*time.Second),
WithCurlMaxTime(2*time.Second),
WithCurlMaxTime(curlFailFastMaxTime),
WithCurlRetry(1))
}