Skip to content
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

values.yaml - set default connect inject init cpu resource limits to null to increase service registration times #2008

Merged
merged 8 commits into from
Mar 15, 2023

Conversation

david-yu
Copy link
Contributor

@david-yu david-yu commented Mar 10, 2023

Changes proposed in this PR:

  • Based on internal investigations allowing the connect-inject-init container to utilize more cpu will allow registration times to Consul to improve registration times from 3.5 seconds to 0.25 seconds. This PR removes the default cpu limits of 50 millicores to allow the the init container to temporarily eat up more CPU to speed up registration times during the init container lifecycle.

How I've tested this PR:

Tested manually on kind as shown below following the kind Consul k8s learn guide: https://developer.hashicorp.com/consul/tutorials/kubernetes/kubernetes-kind

dyu@dyu-JRXHVGG467 kind-consulk8s % kind create cluster --name dc1
Creating cluster "dc1" ...
 ✓ Ensuring node image (kindest/node:v1.25.3) 🖼
 ✓ Preparing nodes 📦
 ✓ Writing configuration 📜
 ✓ Starting control-plane 🕹️
 ✓ Installing CNI 🔌
 ✓ Installing StorageClass 💾
Set kubectl context to "kind-dc1"
You can now use your cluster with:

kubectl cluster-info --context kind-dc1

Have a nice day! 👋
dyu@dyu-JRXHVGG467 kind-consulk8s % kind create cluster --name dc1
dyu@dyu-JRXHVGG467 kind-consulk8s % helm install --values values.yaml consul hashicorp/consul --create-namespace --namespace consul --version "1.1.0"
NAME: consul
LAST DEPLOYED: Fri Mar 10 10:45:34 2023
NAMESPACE: consul
STATUS: deployed
REVISION: 1
NOTES:
Thank you for installing HashiCorp Consul!

Your release is named consul.

To learn more about the release, run:

  $ helm status consul --namespace consul
  $ helm get all consul --namespace consul

Consul on Kubernetes Documentation:
https://www.consul.io/docs/platform/k8s

Consul on Kubernetes CLI Reference:
https://www.consul.io/docs/k8s/k8s-cli
dyu@dyu-JRXHVGG467 kind-consulk8s % k get pods -n consul
NAME                                          READY   STATUS    RESTARTS   AGE
consul-connect-injector-54c5c75775-k2rr5      1/1     Running   0          89s
consul-server-0                               1/1     Running   0          89s
consul-webhook-cert-manager-57fd9b6db-drzvw   1/1     Running   0          89s
dyu@dyu-JRXHVGG467 kind-consulk8s % kubectl apply -f counting.yaml && kubectl apply -f dashboard.yaml
serviceaccount/counting created
service/counting created
deployment.apps/counting created
serviceaccount/dashboard created
service/dashboard created
deployment.apps/dashboard created
dyu@dyu-JRXHVGG467 kind-consulk8s % k get pods
NAME                         READY   STATUS    RESTARTS   AGE
counting-7547ff85f7-bxfp6    2/2     Running   0          17s
dashboard-6b957f4b9b-thdjr   2/2     Running   0          16s

How I expect reviewers to test this PR:

Checklist:

  • Tests added
  • CHANGELOG entry added

    HashiCorp engineers only, community PRs should not add a changelog entry.
    Entries should use present tense (e.g. Add support for...)

@david-yu david-yu requested a review from rboyer March 10, 2023 18:33
@david-yu david-yu added backport/0.49.x backport/1.1.x Backport to release/1.1.x branch labels Mar 10, 2023
@david-yu david-yu marked this pull request as ready for review March 10, 2023 18:49
@david-yu david-yu changed the title values.yaml - bump connect inject init cpu resource limits values.yaml - bump default connect inject init cpu resource limits Mar 10, 2023
David Yu added 3 commits March 10, 2023 13:02
Add more documentation around some defaults for resource on connect inject and connect inject init
@david-yu david-yu changed the title values.yaml - bump default connect inject init cpu resource limits values.yaml - set default connect inject init cpu resource limits to null to increase service registration times Mar 10, 2023
@david-yu david-yu requested a review from rboyer March 10, 2023 22:37
@david-yu
Copy link
Contributor Author

Thanks will merge after the 1.1.1 release is out.

Change wording from "Recommended default" to "Recommended production default"
@david-yu
Copy link
Contributor Author

Added some more changes on docs based on @andrewstucki 's review. Will merge when approved since other changes are now merged in.

@david-yu david-yu merged commit 1483c17 into main Mar 15, 2023
@david-yu david-yu deleted the dyu/bump-default-connect-init-cpu branch March 15, 2023 20:10
david-yu pushed a commit to david-yu/consul-k8s that referenced this pull request Mar 15, 2023
…`null` to increase service registration times (hashicorp#2008)

* Update values.yaml
david-yu pushed a commit to david-yu/consul-k8s that referenced this pull request Mar 15, 2023
…`null` to increase service registration times (hashicorp#2008)

* Update values.yaml
david-yu pushed a commit to david-yu/consul-k8s that referenced this pull request Mar 15, 2023
…`null` to increase service registration times (hashicorp#2008)

* Update values.yaml
david-yu pushed a commit that referenced this pull request Mar 15, 2023
…`null` to increase service registration times (#2008)

* Update values.yaml
david-yu pushed a commit that referenced this pull request Mar 15, 2023
…`null` to increase service registration times (#2008)

* Update values.yaml
david-yu pushed a commit that referenced this pull request Mar 15, 2023
…`null` to increase service registration times (#2008) (#2023)

* Update values.yaml
david-yu pushed a commit that referenced this pull request Mar 16, 2023
…`null` to increase service registration times (#2008) (#2024)

* Update values.yaml
absolutelightning pushed a commit that referenced this pull request Aug 4, 2023
…`null` to increase service registration times (#2008)

* Update values.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.1.x Backport to release/1.1.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants