chore(ci): Refine SeaweedFS S3 auth setup and enhance wait logic in deployment scripts#12772
chore(ci): Refine SeaweedFS S3 auth setup and enhance wait logic in deployment scripts#12772hbelmiro wants to merge 3 commits intokubeflow:masterfrom
Conversation
…eployment scripts Signed-off-by: Helber Belmiro <helber.belmiro@gmail.com>
|
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a race condition in CI tests where SeaweedFS S3 authentication was not fully configured before tests started, causing intermittent failures with "Signed request requires setting up SeaweedFS S3 authentication" errors.
The root cause was that PR #12387 removed an init Job and moved S3 configuration to a postStart lifecycle hook, but the wait_for_seaweedfs_init() function was not updated and became a no-op. This PR fixes the wait function to properly poll for S3 authentication completion.
Changes:
- Replaced no-op wait logic with active polling that verifies SeaweedFS S3 authentication is configured
- Updated error messages to reflect the new postStart-based initialization approach
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/resources/scripts/helper-functions.sh |
Rewrote wait_for_seaweedfs_init() to poll SeaweedFS and verify kubeflow-admin identity exists via s3.configure command, with timeout handling and error logging |
.github/resources/scripts/deploy-kfp.sh |
Updated error messages from "init job" terminology to "S3 authentication setup" to reflect the postStart lifecycle hook approach |
… for consistency Signed-off-by: Helber Belmiro <helber.belmiro@gmail.com>
|
/lgtm |
|
|
||
| wait_for_seaweedfs_init () { | ||
| # Wait for SeaweedFS init job to complete to ensure S3 auth is configured | ||
| # Wait for SeaweedFS S3 authentication to be configured via postStart lifecycle hook. |
There was a problem hiding this comment.
I think there is a better way to handle this, because this keeps on coming up now and then, even after this fix, I believe we can still see failures because there is no retry logic within the seaweed poststart hook to retry bucket/user creation. Lets take this offline.
…es, and streamlined scripts Signed-off-by: Helber Belmiro <helber.belmiro@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/lgtm |
This PR resolves a flakiness in the CI.
Resolves: #12771
Description of your changes:
E2E tests intermittently fail with
Signed request requires setting up SeaweedFS S3 authenticationdue to a race condition.The
wait_for_seaweedfs_init()function checks for aninit-seaweedfsJob that was removed in PR #12387. The function became a no-op, allowing tests to start before the SeaweedFSpostStartlifecycle hook completed S3 authentication setup.Solution
Updated
wait_for_seaweedfs_init()to poll SeaweedFS and verify that thekubeflow-adminidentity is configured by checkings3.configureoutput viaweed shell.Checklist: