diff --git a/cmd/hi/docker.go b/cmd/hi/docker.go index 698e9d54..bcb6dbc5 100644 --- a/cmd/hi/docker.go +++ b/cmd/hi/docker.go @@ -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) } diff --git a/cmd/hi/stats.go b/cmd/hi/stats.go index 00a6cc4f..173b5332 100644 --- a/cmd/hi/stats.go +++ b/cmd/hi/stats.go @@ -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 diff --git a/integration/dockertestutil/config.go b/integration/dockertestutil/config.go index 88b2712c..75bc872c 100644 --- a/integration/dockertestutil/config.go +++ b/integration/dockertestutil/config.go @@ -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) diff --git a/integration/dockertestutil/network.go b/integration/dockertestutil/network.go index d07841f1..95b69b88 100644 --- a/integration/dockertestutil/network.go +++ b/integration/dockertestutil/network.go @@ -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 } diff --git a/integration/helpers.go b/integration/helpers.go index 38abfdb2..4f9c1018 100644 --- a/integration/helpers.go +++ b/integration/helpers.go @@ -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) } diff --git a/integration/hsic/hsic.go b/integration/hsic/hsic.go index f2fe5b30..1dc5c2d3 100644 --- a/integration/hsic/hsic.go +++ b/integration/hsic/hsic.go @@ -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) } diff --git a/integration/integrationutil/util.go b/integration/integrationutil/util.go index 5604af32..3e257a8e 100644 --- a/integration/integrationutil/util.go +++ b/integration/integrationutil/util.go @@ -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( diff --git a/integration/scenario.go b/integration/scenario.go index d1ebdd51..73a45edd 100644 --- a/integration/scenario.go +++ b/integration/scenario.go @@ -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), "") - 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 { diff --git a/integration/tsic/tsic.go b/integration/tsic/tsic.go index 3136b6ae..b943ee13 100644 --- a/integration/tsic/tsic.go +++ b/integration/tsic/tsic.go @@ -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)) }