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

[kong] Release 1.12.0 to next #229

Merged
merged 4 commits into from
Nov 16, 2020
Merged

[kong] Release 1.12.0 to next #229

merged 4 commits into from
Nov 16, 2020

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Nov 14, 2020

What this PR does / why we need it:

Releases 1.12 to next. Includes several additional changes:

  • Bump the default Enterprise tags to the latest 2.1 patch, 2.1.4.1.
  • Increase the default worker count to 2.
  • Handle deprecation warnings via a template to simplify and improve formatting.

Which issue this PR fixes

Special notes for your reviewer:

Re conversations about worker count increase and memory usage from planning:

  • By default, the chart does not apply memory limits to Kong deployments. However, it did include commented example limits with an unreasonably low default limit (128MB). From personal experience, this limit is too low for any deployment (even single-worker deployments), and would cause issues if applied. I've increased the commented example to 256MB, which is much more reasonable, though that's essentially a minimum lower bound for Kong to function at all rather than what I'd expect for most actual deployments. Non-demo/test environments that actually handle traffic will likely start around 1GB, and will usually have even higher limits.
  • There was a brief interlude where additional workers allocated resulted in unexpected higher memory use due to removal of limits elsewhere. AFAIK we've since re-introduced those limits (chore(conf) worker_rlimit_nofile & worker_connections "auto" == max 16384 kong#5864), and increasing worker count no longer causes the consumption issues we observed in earlier 2.x releases, so I don't expect the chart change to cause issues.

We still want to explore resource usage guidelines further, but the minimums proposed here should be reasonable.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • PR is based off the current tip of the next branch and targets next, not main
  • Title of the PR and commit headers start with chart name (e.g. [kong])

Travis Raines added 4 commits November 13, 2020 15:42
Increase the default nginx_worker_processes setting for Kong containers
to 2. This addresses latency issues stemming from config updates
blocking connection handling.

Increase the example memory limit/requests to 256MB. Although these are
not enforced by default, the previous example (128MB) was excessively
low, even for single-worker deployments.
Use a helper function to print deprecation warnings in NOTES.txt.

This addresses an issue with unwanted extra newlines and hard-wraps
warning text automatically.
@mflendrich
Copy link
Contributor

LGTM but:

  • this does not fix [kong] update default worker count to 2 #227 yet - after this PR, the checkbox below remains unsatisfied:

    [ ] Update the default value to 2 in Kong Ingress Controller's repository

  • for future PRs, please avoid mix features and release chores in one PR

Please merge without squashing, too.

@mflendrich
Copy link
Contributor

Part of #232.

@rainest rainest merged commit 542749d into next Nov 16, 2020
@rainest rainest deleted the release/kong-1.12.0 branch November 16, 2020 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants