-
Notifications
You must be signed in to change notification settings - Fork 31
Add HTCondor integration tests for file transfer plugin #2900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This test validates that the Pelican file transfer plugin works correctly with HTCondor for transferring files using the pelican:// protocol. Key features: - Uses dynamic port allocation (COLLECTOR_HOST=127.0.0.1:0) to avoid race conditions - Tests pelican_plugin naming (not stash_plugin) - Validates PelicanCfg_ job classad attribute mechanism (+PelicanCfg_TLSSkipVerify) - Sets XRD_PLUGINCONFDIR so cache can handle pelican:// protocol - Creates mini HTCondor instance with all necessary daemons - Submits a job that downloads input file and uploads output file - Verifies successful job completion and file transfer The test starts a full Pelican federation (director, registry, origin, cache) and a mini HTCondor setup, then submits a job that transfers files through the plugin, validating end-to-end functionality. Addresses: #2900
bbockelm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another batch of changes to work on.
cmd/plugin_htcondor_test.go
Outdated
| EnableWrites: true | ||
| `) | ||
|
|
||
| // Wait for federation to be ready |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poll for readiness; don't have an arbitrary sleep.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(the address file is dropped by the daemon when it is ready).
cmd/plugin_htcondor_test.go
Outdated
| error = %s/test.err | ||
|
|
||
| # Transfer the input file from the federation | ||
| transfer_input_files = %s/test/%s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The last bug was only triggered if you had two input files coming from the data federation. Can you add a second?
This test validates that the Pelican file transfer plugin works correctly with HTCondor for transferring files using the pelican:// protocol. Key features: - Uses dynamic port allocation (COLLECTOR_HOST=127.0.0.1:0) to avoid race conditions - Tests pelican_plugin naming (not stash_plugin) - Validates PelicanCfg_ job classad attribute mechanism (+PelicanCfg_TLSSkipVerify) - Sets XRD_PLUGINCONFDIR so cache can handle pelican:// protocol - Creates mini HTCondor instance with all necessary daemons - Submits a job that downloads input file and uploads output file - Verifies successful job completion and file transfer The test starts a full Pelican federation (director, registry, origin, cache) and a mini HTCondor setup, then submits a job that transfers files through the plugin, validating end-to-end functionality. Addresses: #2900
effd06d to
355e564
Compare
- Update development Dockerfile to install HTCondor package - Create plugin_htcondor_test.go with integration test - Test configures mini HTCondor with pelican plugin - Test starts data federation and verifies file transfers
355e564 to
86fecb8
Compare
The removeBufferedHook function was directly accessing log.StandardLogger().Hooks and iterating over it while the background log level manager goroutine could be triggering logging operations that modify the same map concurrently. Changed to use ReplaceHooks() which provides internal locking, preventing the race condition that caused 'fatal error: concurrent map iteration and map write' in TestGetOIDCProvider and other tests.
Replace test_utils.GetUniqueAvailablePorts anti-pattern with port 0 binding to eliminate race conditions where ports can be taken between check and use. Changes: - Set Origin_Port and Server_WebPort to 0 in all origin tests - OS automatically assigns available ports via net.Listen - Clear cached issuer URL after listener creation to ensure correct port - Move issuer URL retrieval in TestMultiExportOrigin to after server startup This prevents 'address already in use' errors in CI.
The test was experiencing a race condition where HTTP requests were made through the broker before the Director had processed the Origin's advertisement and registered the broker endpoint. Changes: - Add HasBrokerEndpoint() method to broker.BrokerDialer to check if an endpoint is registered - Add HasBrokerForAddr() function to director package to expose the check - Update TestBrokerApi to poll until the Director has registered the origin's broker endpoint before proceeding with the test This ensures the broker connection infrastructure is fully set up before the test makes requests, eliminating the race condition that caused sporadic TLS handshake timeout failures in CI (20% failure rate).
The v7.22 release was delayed by a plugin bug that unit tests missed due to differences in how HTCondor invokes the plugin versus test mocks.
Changes
pelican-devimage for running integration testscmd/plugin_htcondor_test.gowith full plugin lifecycle test:pelican://URLscondor_masternot in PATHTest configuration
The integration test uses HTCondor's shared port feature and dynamic port allocation:
Run with:
go test -tags=integration ./cmd -run TestHTCondorPluginOriginal prompt
💡 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.