Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 20, 2025

Test scripts that probe for open ports and hardcode them into config suffer from a race condition: concurrent tests may grab those ports between probing and server startup, causing failures.

Changes

Core Implementation

  • Add config/address_file.go with WriteAddressFile() that atomically writes address file to the runtime directory after all services start and health checks pass
  • File location depends on execution context:
    • Root execution: /run/pelican/pelican.addresses
    • Non-root with XDG_RUNTIME_DIR: $XDG_RUNTIME_DIR/pelican/pelican.addresses
    • Non-root without XDG_RUNTIME_DIR: Falls back to ${ConfigDir}/pelican.addresses
  • File contains shell-sourceable KEY=VALUE pairs: SERVER_EXTERNAL_WEB_URL, ORIGIN_URL (if enabled), CACHE_URL (if enabled)
  • File permissions set to 0600 (owner read/write only)
  • Called from launchers/launcher.go after WaitUntilWorking() completes

Test Script Update

  • github_scripts/stat_test.sh now uses --port 0 to bind random ports
  • Waits for address file creation in the appropriate runtime directory
  • Sources the file to get actual URLs
  • Eliminates port probing entirely

Example

# Server writes after startup (location depends on runtime context):
$ cat $XDG_RUNTIME_DIR/pelican/pelican.addresses
SERVER_EXTERNAL_WEB_URL=https://hostname:34567
ORIGIN_URL=https://hostname:34568

# Script sources it from the correct location:
if [ -n "$XDG_RUNTIME_DIR" ]; then
    source "$XDG_RUNTIME_DIR/pelican/pelican.addresses"
else
    source "$PELICAN_CONFIGDIR/pelican.addresses"
fi
TOKEN=$(./pelican origin token create --issuer "$SERVER_EXTERNAL_WEB_URL" ...)

Address file appears only when server is fully operational, eliminating the window for race conditions. The file is properly placed in the runtime directory hierarchy, not the config directory, as it contains mutable runtime state.

Original prompt

This section details on the original issue you should resolve

<issue_title>Add an "address file" written after startup</issue_title>
<issue_description>When debugging github_scripts/stat_test.sh for #2905, I noticed an anti-pattern/race condition. The script checks for open ports and then hardcodes them into the config. If a concurrently-running unit test also probed for open ports, the other test could have grabbed them between probing and startup, resulting in failure.

Let's close the race condition.

Start Pelican on port 0 (indicating you can bind to a random address) and have it drop an "address file" in its runtime directory. The address file should be KEY=VALUE pairs (one per newline) indicating the internet addresses that were selected by Pelican for its components. The file should be written atomically after any cache/origin xrootd service has been started and the main web UI port probed for successful connectivity. It should contain the external address in HTTPS URL format (i.e., https://example.com:8412).

The scripts can then probe for the existence of the address file and source it to set the API endpoints as locations, reducing the likelihood of error.</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Add address file generation after startup Add address file written after startup to eliminate port probing race condition Dec 20, 2025
Copilot AI requested a review from bbockelm December 20, 2025 14:52
Copy link
Contributor Author

Copilot AI commented Dec 20, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • abc123
    • Triggering command: /tmp/go-build3171483306/b001/config.test /tmp/go-build3171483306/b001/config.test -test.testlogfile=/tmp/go-build3171483306/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s rev-�� m7yN-GZz6 Njc4/dQHRr_zbbAvvL-F7Njc4 x64/pkg/tool/linux_amd64/compile--64 x64/src/runtime//opt/hostedtoolcache/go/1.24.11/x64/pkg/tool/linux_amd64/link fips140hash (dns block)
    • Triggering command: /tmp/go-build884809855/b001/config.test /tmp/go-build884809855/b001/config.test -test.testlogfile=/tmp/go-build884809855/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -mtune=generic -march=x86-64 -fasynchronous-unwind-tables -fstack-protector-strong -Wformat -Wformat-security -fstack-clash-protection -o pter.go -trimpath ux_amd64/vet 8615158/b153/ crypto/internal/--64 ctor ux_amd64/vet (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from bbockelm December 20, 2025 23:15
@bbockelm bbockelm added test Improvements to the test suite infrastructure GitHub Actions, Release management, and CI internal Internal code improvements, not user-facing labels Dec 20, 2025
@bbockelm bbockelm force-pushed the copilot/add-address-file-on-startup branch from 49c628e to 7e978b3 Compare December 21, 2025 03:06
@bbockelm bbockelm force-pushed the copilot/add-address-file-on-startup branch from 558b964 to e7bc72b Compare December 21, 2025 04:51
Copilot AI and others added 5 commits December 20, 2025 22:52
The success of some of the `config_test.go` functions relied on global
state; if any test ran before them and reset the config, they would
fail.  When `TestAddressConfig` was added, it went first and triggered
that exact condition.

This moves the setup into a standalone helper function which is executed
only by tests that need it.
@bbockelm bbockelm force-pushed the copilot/add-address-file-on-startup branch from e7bc72b to 7aa82d3 Compare December 21, 2025 04:52
@bbockelm bbockelm force-pushed the copilot/add-address-file-on-startup branch from 2f665cb to 8befa92 Compare December 21, 2025 05:37
@bbockelm bbockelm force-pushed the copilot/add-address-file-on-startup branch from 8befa92 to fc09b45 Compare December 21, 2025 13:48
@bbockelm bbockelm marked this pull request as ready for review December 21, 2025 14:13
@bbockelm
Copy link
Collaborator

Another set of race condition bugs bite the dust! More reliable tests = happier developers.

@bbockelm bbockelm merged commit bf0b668 into main Dec 21, 2025
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure GitHub Actions, Release management, and CI internal Internal code improvements, not user-facing test Improvements to the test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an "address file" written after startup

2 participants