Skip to content

Add cc uploader configurable drain timeout - #562

Merged
kathap merged 5 commits into
developfrom
cc-uploader-drain-timeout
Sep 16, 2025
Merged

Add cc uploader configurable drain timeout#562
kathap merged 5 commits into
developfrom
cc-uploader-drain-timeout

Conversation

@kathap

@kathap kathap commented Jul 18, 2025

Copy link
Copy Markdown
Contributor
  • Add capi.cc_uploader.cc_uploader_drain_timeout_in_minutes (type: time, default: 15) to job spec
  • Inject --shutdownTimeoutInMinutes=<%= p("capi.cc_uploader.cc_uploader_drain_timeout_in_minutes") %> into bpm.yml.erb
  • Update drain.sh.erb to read the same property, apply the timeout when waiting for PID exit
  • Retain 15-minute fallback in the script for robustness against missing property values

Thanks for contributing to the capi_release. To speed up the process of reviewing your pull request please provide us with:

  • A short explanation of the proposed change:
    Allow operators to tune how long the CC-Uploader waits for in-flight uploads to finish during drain by introducing a new BOSH property (capi.cc_uploader.cc_uploader_drain_timeout_in_minutes) with a sane default of 15 minutes, wiring it through both the BPM startup flags and the drain script.

  • An explanation of the use cases your change solves
    Let operators configure how long CC-Uploader waits (with a default of 15 minutes) during VM shutdown so active uploads complete gracefully.

  • Links to any other associated PRs
    Improve droplet upload draining cc-uploader#245

  • I have viewed signed and have submitted the Contributor License Agreement

  • I have made this pull request to the develop branch

  • [] I have run CF Acceptance Tests on bosh lite

- Add `capi.cc_uploader.cc_uploader_drain_timeout_in_minutes` (type: time, default: 15m) to job spec
- Inject `--shutdownTimeoutInMinutes=<%= p("capi.cc_uploader.cc_uploader_drain_timeout_in_minutes") %>` into `bpm.yml.erb`
- Update `drain.sh.erb` to read the same property, apply the timeout when waiting for PID exit
- Retain 15-minute fallback in the script for robustness against missing property values
@kathap
kathap marked this pull request as draft July 18, 2025 16:51
@kathap
kathap force-pushed the cc-uploader-drain-timeout branch from b7f8344 to ae3cd55 Compare July 22, 2025 13:55
@kathap
kathap marked this pull request as ready for review July 24, 2025 07:05
Comment thread jobs/cc_uploader/templates/drain.sh.erb Outdated
Comment thread jobs/cc_uploader/templates/drain.sh.erb Outdated
Comment thread jobs/cc_uploader/templates/drain.sh.erb Outdated
NOW_TS=$(date +%s)
ELAPSED=$(( NOW_TS - START_TS ))
if [ "$ELAPSED" -ge "$DRAIN_TIMEOUT" ]; then
echo "$(date): drain timeout reached after ${DRAIN_TIMEOUT}s, forcing exit" >> "$LOG_FILE"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So here the timeout is reached and the process has not yet exited. Is it correct to exit the drain script with 0 and send 0 to BOSH in that case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in this case it is correct, the drain script is not failing, the upload is just taking too long, so we waited for 15 minutes and then we let bosh go ahead and stop the jobs, which adds another 60s waiting time. If the upload does not finish in that time, diego will send another upload request to another VM.

Comment thread jobs/cc_uploader/templates/drain.sh.erb

@jochenehret jochenehret left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just consider renaming the drain template file.

@kathap
kathap merged commit 4023f9f into develop Sep 16, 2025
2 checks passed
@moleske
moleske deleted the cc-uploader-drain-timeout branch September 16, 2025 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants