-
Notifications
You must be signed in to change notification settings - Fork 217
Add locket client keepalive time and timeout to jobs #722
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
Add locket client keepalive time and timeout to jobs #722
Conversation
|
Hi @klapkov thank you for adding this with tests!! 👏 In slack you asked:
I looked a routing-release as an example. I suggest:
|
|
I have added another file called run-template-tests-in-docker which we execute in the docker shell script, right before we start the standard docker image. In this file we start a docker container with the ruby:3.0 image. I did this since we cannot execute the test in the other container, since there is not ruby on those images. @ameowlia Please let me know if this way is appropriate. |
|
@klapkov this change looks good to me. Do you plan to have PRs for components separately to use these properties? It would be preferable to merge everything at the same time, so we don't have unused properties in the release. |
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.
This looks consistent with routing release
scripts/docker-shell
Outdated
| exit 1 | ||
| fi | ||
|
|
||
| ./scripts/run-template-tests-in-docker.sh |
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.
Traditionally the docker-shell script has been for just getting into a shell to do various one-off tests, and not actually test anything. Can you remove this call? If we want something to do all the testing, maybe instead add a run-all-tests-in-docker script that calls both run-template-tests-in-docker.sh + docker-test scripts.
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.
Sounds reasonable, please check the new commit.
|
Will merge once concourse goes green. Feel free to bump the locket submodule on this PR as well, now that it's merged |
Context: #722 Submodule src/code.cloudfoundry.org/locket 5e8522d97..6cd541649: > go fmt > Add keepalive timeout
[DIEGO-RELEASE] Add locket client keepalive time and timeout to jobs: cloudfoundry/diego-release#722 [DIEGO-RELEASE] Make max_containers prop configurable: cloudfoundry/diego-release#876 [BBS] Add request metrics for BBS - still open: cloudfoundry/bbs#80 [BBS] Use scheduling info instead of the whole desiredLRP: cloudfoundry/bbs#79 [BBS] Add routing info endpoint: cloudfoundry/bbs#66 [BBS] Remove cpu_weight limit: cloudfoundry/bbs#81 [EXECUTOR] Improve error handling on process start - still open: cloudfoundry/executor#91 [LOCKET] Add a keepalive timeout on the locket client: cloudfoundry/locket#12 [GROOTFS] UsedVolumesSize and UsedStoreInBytes metrics: cloudfoundry/grootfs#155 [ROUTE-EMITTER] Use routing info bbs endpoint when syncing: cloudfoundry/route-emitter#23 [ROUTE-EMITTER] Use routing_info for desired_lrp's when there are missing actual_lrp's: cloudfoundry/route-emitter#26 [SILK-RELEASE] Deduplicate Iptables Rules with Dynamic ASG's: cloudfoundry/silk-release#101 [SILK-RELEASE] Make container_metadata_file_check_timeout on silk-shutdown configurable: cloudfoundry/silk-release#111
[DIEGO-RELEASE] Add locket client keepalive time and timeout to jobs: cloudfoundry/diego-release#722 [DIEGO-RELEASE] Make max_containers prop configurable: cloudfoundry/diego-release#876 [BBS] Add request metrics for BBS - still open: cloudfoundry/bbs#80 [BBS] Use scheduling info instead of the whole desiredLRP: cloudfoundry/bbs#79 [BBS] Add routing info endpoint: cloudfoundry/bbs#66 [BBS] Remove cpu_weight limit: cloudfoundry/bbs#81 [EXECUTOR] Improve error handling on process start - still open: cloudfoundry/executor#91 [LOCKET] Add a keepalive timeout on the locket client: cloudfoundry/locket#12 [GROOTFS] UsedVolumesSize and UsedStoreInBytes metrics: cloudfoundry/grootfs#155 [ROUTE-EMITTER] Use routing info bbs endpoint when syncing: cloudfoundry/route-emitter#23 [ROUTE-EMITTER] Use routing_info for desired_lrp's when there are missing actual_lrp's: cloudfoundry/route-emitter#26 [SILK-RELEASE] Deduplicate Iptables Rules with Dynamic ASG's: cloudfoundry/silk-release#101 [SILK-RELEASE] Make container_metadata_file_check_timeout on silk-shutdown configurable: cloudfoundry/silk-release#111 wip
[DIEGO-RELEASE] Add locket client keepalive time and timeout to jobs: cloudfoundry/diego-release#722 [DIEGO-RELEASE] Make max_containers prop configurable: cloudfoundry/diego-release#876 [BBS] Add request metrics for BBS - still open: cloudfoundry/bbs#80 [BBS] Use scheduling info instead of the whole desiredLRP: cloudfoundry/bbs#79 [BBS] Add routing info endpoint: cloudfoundry/bbs#66 [BBS] Remove cpu_weight limit: cloudfoundry/bbs#81 [EXECUTOR] Improve error handling on process start - still open: cloudfoundry/executor#91 [LOCKET] Add a keepalive timeout on the locket client: cloudfoundry/locket#12 [GROOTFS] UsedVolumesSize and UsedStoreInBytes metrics: cloudfoundry/grootfs#155 [ROUTE-EMITTER] Use routing info bbs endpoint when syncing: cloudfoundry/route-emitter#23 [ROUTE-EMITTER] Use routing_info for desired_lrp's when there are missing actual_lrp's: cloudfoundry/route-emitter#26 [SILK-RELEASE] Deduplicate Iptables Rules with Dynamic ASG's: cloudfoundry/silk-release#101 [SILK-RELEASE] Make container_metadata_file_check_timeout on silk-shutdown configurable: cloudfoundry/silk-release#111 wip
Follow up to this PR: #675. I deleted that fork by mistake. Anyhow I have added template tests for the rep, auctioneer and bbs. We only test the new introduced props to the locket client. I imagine that these tests can be filled with all the needed cases in the future.