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

Controller: Make Leader Election TTL configurable. #11142

Merged
merged 2 commits into from Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions charts/ingress-nginx/README.md
Expand Up @@ -297,6 +297,7 @@ As of version `1.26.0` of this chart, by simply not providing any clusterIP valu
| controller.dnsConfig | object | `{}` | Optionally customize the pod dnsConfig. |
| controller.dnsPolicy | string | `"ClusterFirst"` | Optionally change this to ClusterFirstWithHostNet in case you have 'hostNetwork: true'. By default, while using host network, name resolution uses the host's DNS. If you wish nginx-controller to keep resolving names inside the k8s network, use ClusterFirstWithHostNet. |
| controller.electionID | string | `""` | Election ID to use for status update, by default it uses the controller name combined with a suffix of 'leader' |
| controller.electionTTL | string | `""` | Duration a leader election is valid before it's getting re-elected, e.g. `15s`, `10m` or `1h`. (Default: 30s) |
| controller.enableAnnotationValidations | bool | `false` | |
| controller.enableMimalloc | bool | `true` | Enable mimalloc as a drop-in replacement for malloc. # ref: https://github.com/microsoft/mimalloc # |
| controller.enableTopologyAwareRouting | bool | `false` | This configuration enables Topology Aware Routing feature, used together with service annotation service.kubernetes.io/topology-mode="auto" Defaults to false |
Expand Down
3 changes: 3 additions & 0 deletions charts/ingress-nginx/templates/_params.tpl
Expand Up @@ -63,6 +63,9 @@
{{- if .Values.controller.disableLeaderElection }}
- --disable-leader-election=true
{{- end }}
{{- if .Values.controller.electionTTL }}
- --election-ttl={{ .Values.controller.electionTTL }}
{{- end }}
{{- range $key, $value := .Values.controller.extraArgs }}
{{- /* Accept keys without values or with false as value */}}
{{- if eq ($value | quote | len) 2 }}
Expand Down
2 changes: 2 additions & 0 deletions charts/ingress-nginx/values.yaml
Expand Up @@ -85,6 +85,8 @@ controller:
enableTopologyAwareRouting: false
# -- This configuration disable Nginx Controller Leader Election
disableLeaderElection: false
# -- Duration a leader election is valid before it's getting re-elected, e.g. `15s`, `10m` or `1h`. (Default: 30s)
electionTTL: ""
# -- This configuration defines if Ingress Controller should allow users to set
# their own *-snippet annotations, otherwise this is forbidden / dropped
# when users add those annotations.
Expand Down
1 change: 1 addition & 0 deletions docs/user-guide/cli-arguments.md
Expand Up @@ -22,6 +22,7 @@ They are set in the container spec of the `ingress-nginx-controller` Deployment
| `--disable-sync-events` | Disables the creation of 'Sync' Event resources, but still logs them |
| `--dynamic-configuration-retries` | Number of times to retry failed dynamic configuration before failing to sync an ingress. (default 15) |
| `--election-id` | Election id to use for Ingress status updates. (default "ingress-controller-leader") |
| `--election-ttl` | Duration a leader election is valid before it's getting re-elected, e.g. `15s`, `10m` or `1h`. (Default: 30s) |
| `--enable-metrics` | Enables the collection of NGINX metrics. (default true) |
| `--enable-ssl-chain-completion` | Autocomplete SSL certificate chains with missing intermediate CA certificates. Certificates uploaded to Kubernetes must have the "Authority Information Access" X.509 v3 extension for this to succeed. (default false)|
| `--enable-ssl-passthrough` | Enable SSL Passthrough. (default false) |
Expand Down
1 change: 1 addition & 0 deletions internal/ingress/controller/controller.go
Expand Up @@ -91,6 +91,7 @@ type Configuration struct {
UpdateStatus bool
UseNodeInternalIP bool
ElectionID string
ElectionTTL time.Duration
UpdateStatusOnShutdown bool

HealthCheckHost string
Expand Down
5 changes: 3 additions & 2 deletions internal/ingress/controller/nginx.go
Expand Up @@ -274,8 +274,9 @@ func (n *NGINXController) Start() {
if !n.cfg.DisableLeaderElection {
electionID := n.cfg.ElectionID
setupLeaderElection(&leaderElectionConfig{
Client: n.cfg.Client,
ElectionID: electionID,
Client: n.cfg.Client,
ElectionID: electionID,
ElectionTTL: n.cfg.ElectionTTL,
OnStartedLeading: func(stopCh chan struct{}) {
if n.syncStatus != nil {
go n.syncStatus.Run(stopCh)
Expand Down
11 changes: 5 additions & 6 deletions internal/ingress/controller/status.go
Expand Up @@ -36,7 +36,8 @@ import (
type leaderElectionConfig struct {
Client clientset.Interface

ElectionID string
ElectionID string
ElectionTTL time.Duration

OnStartedLeading func(chan struct{})
OnStoppedLeading func()
Expand Down Expand Up @@ -107,13 +108,11 @@ func setupLeaderElection(config *leaderElectionConfig) {
LockConfig: resourceLockConfig,
}

ttl := 30 * time.Second

elector, err = leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{
Lock: lock,
LeaseDuration: ttl,
RenewDeadline: ttl / 2,
RetryPeriod: ttl / 4,
LeaseDuration: config.ElectionTTL,
RenewDeadline: config.ElectionTTL / 2,
RetryPeriod: config.ElectionTTL / 4,

Callbacks: callbacks,
})
Expand Down
8 changes: 8 additions & 0 deletions pkg/flags/flags.go
Expand Up @@ -132,6 +132,9 @@ Requires setting the publish-service parameter to a valid Service reference.`)
electionID = flags.String("election-id", "ingress-controller-leader",
`Election id to use for Ingress status updates.`)

electionTTL = flags.Duration("election-ttl", 30*time.Second,
msfidelis marked this conversation as resolved.
Show resolved Hide resolved
`Duration a leader election is valid before it's getting re-elected`)

updateStatusOnShutdown = flags.Bool("update-status-on-shutdown", true,
`Update the load-balancer status of Ingress objects when the controller shuts down.
Requires the update-status parameter.`)
Expand Down Expand Up @@ -314,6 +317,10 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g
}
}

if *electionTTL <= 0 {
*electionTTL = 30 * time.Second
}

histogramBuckets := &collectors.HistogramBuckets{
TimeBuckets: *timeBuckets,
LengthBuckets: *lengthBuckets,
Expand All @@ -327,6 +334,7 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g
KubeConfigFile: *kubeConfigFile,
UpdateStatus: *updateStatus,
ElectionID: *electionID,
ElectionTTL: *electionTTL,
EnableProfiling: *profiling,
EnableMetrics: *enableMetrics,
MetricsPerHost: *metricsPerHost,
Expand Down
69 changes: 69 additions & 0 deletions pkg/flags/flags_test.go
Expand Up @@ -19,6 +19,7 @@ package flags
import (
"os"
"testing"
"time"
)

func TestNoMandatoryFlag(t *testing.T) {
Expand Down Expand Up @@ -143,3 +144,71 @@ func TestIfLeaderElectionDisabledFlagIsFalse(t *testing.T) {
t.Fatalf("Expected --disable-leader-election and conf.DisableLeaderElection as false, but found: %v", conf.DisableLeaderElection)
}
}

func TestLeaderElectionTTLDefaultValue(t *testing.T) {
ResetForTesting(func() { t.Fatal("Parsing failed") })

oldArgs := os.Args
defer func() { os.Args = oldArgs }()
os.Args = []string{"cmd", "--http-port", "80", "--https-port", "443"}

_, conf, err := ParseFlags()
if err != nil {
t.Fatalf("Unexpected error parsing default flags: %v", err)
}

if conf.ElectionTTL != 30*time.Second {
t.Fatalf("Expected --election-ttl and conf.ElectionTTL as 30s, but found: %v", conf.ElectionTTL)
}
}

func TestLeaderElectionTTLParseValueInSeconds(t *testing.T) {
ResetForTesting(func() { t.Fatal("Parsing failed") })

oldArgs := os.Args
defer func() { os.Args = oldArgs }()
os.Args = []string{"cmd", "--http-port", "80", "--https-port", "443", "--election-ttl", "10s"}

_, conf, err := ParseFlags()
if err != nil {
t.Fatalf("Unexpected error parsing default flags: %v", err)
}

if conf.ElectionTTL != 10*time.Second {
t.Fatalf("Expected --election-ttl and conf.ElectionTTL as 10s, but found: %v", conf.ElectionTTL)
}
}

func TestLeaderElectionTTLParseValueInMinutes(t *testing.T) {
ResetForTesting(func() { t.Fatal("Parsing failed") })

oldArgs := os.Args
defer func() { os.Args = oldArgs }()
os.Args = []string{"cmd", "--http-port", "80", "--https-port", "443", "--election-ttl", "10m"}

_, conf, err := ParseFlags()
if err != nil {
t.Fatalf("Unexpected error parsing default flags: %v", err)
}

if conf.ElectionTTL != 10*time.Minute {
t.Fatalf("Expected --election-ttl and conf.ElectionTTL as 10m, but found: %v", conf.ElectionTTL)
}
}

func TestLeaderElectionTTLParseValueInHours(t *testing.T) {
ResetForTesting(func() { t.Fatal("Parsing failed") })

oldArgs := os.Args
defer func() { os.Args = oldArgs }()
os.Args = []string{"cmd", "--http-port", "80", "--https-port", "443", "--election-ttl", "1h"}

_, conf, err := ParseFlags()
if err != nil {
t.Fatalf("Unexpected error parsing default flags: %v", err)
}

if conf.ElectionTTL != 1*time.Hour {
t.Fatalf("Expected --election-ttl and conf.ElectionTTL as 1h, but found: %v", conf.ElectionTTL)
}
}