Revert "Add draining for cc_uploader (#529)" #544
Conversation
This reverts commit 45d2a21. This original commit introduces the requirement that cc_uploader is co-located on the same VM as the cloud_controller_ng job. While this is the case in cf-deployment, this is not the case for all deployments of CF (like the VMware one). As this commit has broken our deployment, and its not easy for us simply to relocate the job, we propose we revert this and discuss options going forwards. The cc_uploader originally was designed an independent process, typically processes that require to be co-located are within the same job. E.g. nginx, cloud_controller_ng and cloud_controller_local_worker. Some options: * Investigate a way to generically drain uploads from any source in nginx rather than specifically in cc_uploader. * Current timeout is [10 seconds](https://github.com/cloudfoundry/cloud_controller_ng/blob/5c4dac049bd28979284aeab1efa48c7075676131/lib/cloud_controller/drain.rb#L9) * Find a way to sync draining between jobs without requiring co-location. * Keep the co-location requirement for draining but behind a capi-property.
| @@ -6,8 +6,6 @@ $LOAD_PATH.unshift('/var/vcap/packages/cloud_controller_ng/cloud_controller_ng/l | |||
| require 'cloud_controller/drain' | |||
|
|
|||
| @drain = VCAP::CloudController::Drain.new('/var/vcap/sys/log/cloud_controller_ng') | |||
|
|
|||
| @drain.shutdown_cc_uploader('/var/vcap/sys/run/bpm/cc_uploader/cc_uploader.pid') | |||
There was a problem hiding this comment.
This change orphans some code in CC https://github.com/cloudfoundry/cloud_controller_ng/blob/b922faf661a4203a0089a2a1c80c212c47e75c9c/lib/cloud_controller/drain.rb#L61
I'm fine with this for the short term, but we probably want a PR reverting the CC changes also.
|
We can revert the change and see how we can implement this without breaking your deployment, for us a draining of cc-uploader would be valuable. Is there a possibility to have your set up in a test scenario in the capi pipeline so that we have a chance to see that it won't work? |
It would take a lot to replicate how we deploy CF in OSS pipelines, its using custom tooling to build the overall manifest and maintaining it would also be a lot of engineering effort. From our side I think we need to better at reviewing PRs and releases and identifying these possible issues as early as possible. |
|
@kathap could you please open an issue where we can discuss implementation options? Thanks! |
|
This reverts commit 45d2a21.
This original commit introduces the requirement that cc_uploader is co-located on the same VM as the cloud_controller_ng job. While this is the case in cf-deployment, this is not the case for all deployments of CF (like the VMware one).
As this commit has broken our deployment, and its not easy for us simply to relocate the job, we propose we revert this and discuss options going forwards.
The cc_uploader originally was designed an independent process, typically processes that require to be co-located are within the same job. E.g. nginx, cloud_controller_ng and cloud_controller_local_worker.
Some options:
We can discuss options async or at the next office hours (14th May)
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:
An explanation of the use cases your change solves
Links to any other associated PRs
I have viewed signed and have submitted the Contributor License Agreement
I have made this pull request to the
developbranchI have run CF Acceptance Tests on bosh lite