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

Support Horizontal Scaling #1048

Merged
merged 26 commits into from
Sep 18, 2023

Conversation

kate-osborn
Copy link
Contributor

@kate-osborn kate-osborn commented Sep 8, 2023

Proposed changes

Problem: NKG cannot be scaled horizontally because all replicas will write statuses to the Gateway API resources.

Solution: Add leader election to the status updater so that only one replica of NKG will write statuses to the Gateway API resources. Leader election is enabled by default but can be disabled via a cli arg --leader-election-disable. The lock name used for leader election can be configured via the cli arg --leader-election-lock-name.

Testing: Verified leader election works with multiple replicas.
Closes #637

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request labels Sep 8, 2023
@kate-osborn kate-osborn marked this pull request as ready for review September 8, 2023 21:38
@kate-osborn kate-osborn requested a review from a team as a code owner September 8, 2023 21:38
Makefile Outdated Show resolved Hide resolved
internal/framework/status/updater.go Outdated Show resolved Hide resolved
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

more comments:
(1) Would it make sense to add something about leader election to the architecture doc?

(2) What removes the leader election lock resource? Do we need to update our deinstallation instructions (helm and manifests) if it is not removed automatically?

cmd/gateway/commands.go Show resolved Hide resolved
cmd/gateway/commands.go Outdated Show resolved Hide resolved
cmd/gateway/commands.go Outdated Show resolved Hide resolved
conformance/provisioner/static-deployment.yaml Outdated Show resolved Hide resolved
internal/mode/static/manager.go Outdated Show resolved Hide resolved
internal/mode/static/manager.go Outdated Show resolved Hide resolved
internal/framework/status/updater.go Outdated Show resolved Hide resolved
internal/framework/status/updater_test.go Show resolved Hide resolved
internal/mode/static/manager.go Outdated Show resolved Hide resolved
internal/mode/static/leader.go Show resolved Hide resolved
@kate-osborn
Copy link
Contributor Author

more comments: (1) Would it make sense to add something about leader election to the architecture doc?

(2) What removes the leader election lock resource? Do we need to update our deinstallation instructions (helm and manifests) if it is not removed automatically?

(1) Sure, I can add something to the architecture doc.
(2). It's removed when you delete the namespace.

Makefile Outdated Show resolved Hide resolved
Copy link
Member

@ciarams87 ciarams87 left a comment

Choose a reason for hiding this comment

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

🚀

docs/architecture.md Show resolved Hide resolved
cmd/gateway/commands.go Show resolved Hide resolved
@kate-osborn kate-osborn force-pushed the feat/horizontal-scaling branch from 50e907a to d4b4423 Compare September 18, 2023 18:17
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

👍

@bjee19
Copy link
Contributor

bjee19 commented Sep 18, 2023

https://github.com/nginxinc/nginx-kubernetes-gateway/blob/main/design/control-data-plane-separation/design.md

There is a line here that is: "Once we add leader election, the control plane will be able to scale.", would we like to change that now?

@kate-osborn
Copy link
Contributor Author

https://github.com/nginxinc/nginx-kubernetes-gateway/blob/main/design/control-data-plane-separation/design.md

There is a line here that is: "Once we add leader election, the control plane will be able to scale.", would we like to change that now?

No, that's in an old design document. We don't need to keep those up-to-date.

Copy link
Contributor

@bjee19 bjee19 left a comment

Choose a reason for hiding this comment

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

🚀🚀🚀

@kate-osborn kate-osborn force-pushed the feat/horizontal-scaling branch from f0f0ed8 to ed875d2 Compare September 18, 2023 21:45
@kate-osborn kate-osborn force-pushed the feat/horizontal-scaling branch from ed875d2 to 28cd3b1 Compare September 18, 2023 21:56
@kate-osborn kate-osborn changed the title Feat/horizontal scaling Allow for Horizontal Scaling Sep 18, 2023
@kate-osborn kate-osborn changed the title Allow for Horizontal Scaling Support Horizontal Scaling Sep 18, 2023
@kate-osborn kate-osborn merged commit 43c1100 into nginxinc:main Sep 18, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Horizontal scaling
5 participants