Skip to content

Commit

Permalink
feature(leader_election): flag to disable leader election feature on …
Browse files Browse the repository at this point in the history
…controller (#11064)
  • Loading branch information
msfidelis committed Mar 6, 2024
1 parent a302cc5 commit 9b63559
Show file tree
Hide file tree
Showing 11 changed files with 196 additions and 19 deletions.
1 change: 1 addition & 0 deletions charts/ingress-nginx/README.md
Expand Up @@ -293,6 +293,7 @@ As of version `1.26.0` of this chart, by simply not providing any clusterIP valu
| controller.containerSecurityContext | object | `{}` | Security context for controller containers |
| controller.customTemplate.configMapKey | string | `""` | |
| controller.customTemplate.configMapName | string | `""` | |
| controller.disableLeaderElection | bool | `false` | This configuration disable Nginx Controller Leader Election |
| 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' |
Expand Down
3 changes: 3 additions & 0 deletions charts/ingress-nginx/templates/_params.tpl
Expand Up @@ -60,6 +60,9 @@
{{- if .Values.controller.enableTopologyAwareRouting }}
- --enable-topology-aware-routing=true
{{- end }}
{{- if .Values.controller.disableLeaderElection }}
- --disable-leader-election=true
{{- 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 @@ -83,6 +83,8 @@ controller:
# -- This configuration enables Topology Aware Routing feature, used together with service annotation service.kubernetes.io/topology-mode="auto"
# Defaults to false
enableTopologyAwareRouting: false
# -- This configuration disable Nginx Controller Leader Election
disableLeaderElection: false
# -- 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 @@ -25,6 +25,7 @@ They are set in the container spec of the `ingress-nginx-controller` Deployment
| `--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) |
| `--disable-leader-election` | Disable Leader Election on Nginx Controller. (default false) |
| `--enable-topology-aware-routing` | Enable topology aware routing feature, needs service object annotation service.kubernetes.io/topology-mode sets to auto. (default false) |
| `--exclude-socket-metrics` | Set of socket request metrics to exclude which won't be exported nor being calculated. The possible socket request metrics to exclude are documented in the monitoring guide e.g. 'nginx_ingress_controller_request_duration_seconds,nginx_ingress_controller_response_size'|
| `--health-check-path` | URL path of the health check endpoint. Configured inside the NGINX status server. All requests received on the port defined by the healthz-port parameter are forwarded internally to this path. (default "/healthz") |
Expand Down
2 changes: 2 additions & 0 deletions internal/ingress/controller/controller.go
Expand Up @@ -100,6 +100,8 @@ type Configuration struct {

EnableSSLPassthrough bool

DisableLeaderElection bool

EnableProfiling bool

EnableMetrics bool
Expand Down
40 changes: 21 additions & 19 deletions internal/ingress/controller/nginx.go
Expand Up @@ -271,26 +271,28 @@ func (n *NGINXController) Start() {
// TODO: For now, as the the IngressClass logics has changed, is up to the
// cluster admin to create different Leader Election IDs.
// Should revisit this in a future
electionID := n.cfg.ElectionID

setupLeaderElection(&leaderElectionConfig{
Client: n.cfg.Client,
ElectionID: electionID,
OnStartedLeading: func(stopCh chan struct{}) {
if n.syncStatus != nil {
go n.syncStatus.Run(stopCh)
}

n.metricCollector.OnStartedLeading(electionID)
// manually update SSL expiration metrics
// (to not wait for a reload)
n.metricCollector.SetSSLExpireTime(n.runningConfig.Servers)
n.metricCollector.SetSSLInfo(n.runningConfig.Servers)
},
OnStoppedLeading: func() {
n.metricCollector.OnStoppedLeading(electionID)
},
})
if !n.cfg.DisableLeaderElection {
electionID := n.cfg.ElectionID
setupLeaderElection(&leaderElectionConfig{
Client: n.cfg.Client,
ElectionID: electionID,
OnStartedLeading: func(stopCh chan struct{}) {
if n.syncStatus != nil {
go n.syncStatus.Run(stopCh)
}

n.metricCollector.OnStartedLeading(electionID)
// manually update SSL expiration metrics
// (to not wait for a reload)
n.metricCollector.SetSSLExpireTime(n.runningConfig.Servers)
n.metricCollector.SetSSLInfo(n.runningConfig.Servers)
},
OnStoppedLeading: func() {
n.metricCollector.OnStoppedLeading(electionID)
},
})
}

cmd := n.command.ExecCommand()

Expand Down
4 changes: 4 additions & 0 deletions pkg/flags/flags.go
Expand Up @@ -146,6 +146,9 @@ Requires the update-status parameter.`)
enableSSLPassthrough = flags.Bool("enable-ssl-passthrough", false,
`Enable SSL Passthrough.`)

disableLeaderElection = flags.Bool("disable-leader-election", false,
`Disable Leader Election on NGINX Controller.`)

disableServiceExternalName = flags.Bool("disable-svc-external-name", false,
`Disable support for Services of type ExternalName.`)

Expand Down Expand Up @@ -333,6 +336,7 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g
MonitorMaxBatchSize: *monitorMaxBatchSize,
DisableServiceExternalName: *disableServiceExternalName,
EnableSSLPassthrough: *enableSSLPassthrough,
DisableLeaderElection: *disableLeaderElection,
ResyncPeriod: *resyncPeriod,
DefaultService: *defaultSvc,
Namespace: *watchNamespace,
Expand Down
34 changes: 34 additions & 0 deletions pkg/flags/flags_test.go
Expand Up @@ -109,3 +109,37 @@ func TestMaxmindRetryDownload(t *testing.T) {
t.Fatalf("Expected an error parsing flags but none returned")
}
}

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

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

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

if !conf.DisableLeaderElection {
t.Fatalf("Expected --disable-leader-election and conf.DisableLeaderElection as true, but found: %v", conf.DisableLeaderElection)
}
}

func TestIfLeaderElectionDisabledFlagIsFalse(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.DisableLeaderElection {
t.Fatalf("Expected --disable-leader-election and conf.DisableLeaderElection as false, but found: %v", conf.DisableLeaderElection)
}
}
@@ -0,0 +1,34 @@
# TODO: remove the need to use fullnameOverride
fullnameOverride: nginx-ingress
controller:
image:
repository: ingress-controller/controller
chroot: true
tag: 1.0.0-dev
digest:
digestChroot:
scope:
# Necessary to allow the ingress controller to get the topology information from the nodes
enabled: false
config:
worker-processes: "1"
readinessProbe:
initialDelaySeconds: 3
periodSeconds: 1
livenessProbe:
initialDelaySeconds: 3
periodSeconds: 1
service:
type: NodePort
extraArgs:
# e2e tests do not require information about ingress status
update-status: "false"
terminationGracePeriodSeconds: 1
admissionWebhooks:
enabled: false

disableLeaderElection: true

rbac:
create: true
scope: false
93 changes: 93 additions & 0 deletions test/e2e/disableleaderelection/disable_leader.go
@@ -0,0 +1,93 @@
/*
Copyright 2024 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package disableleaderelection

import (
"net/http"
"strings"

"github.com/onsi/ginkgo/v2"

"k8s.io/ingress-nginx/test/e2e/framework"
)

var _ = framework.IngressNginxDescribe("[Disable Leader] Routing works when leader election was disabled", func() {
f := framework.NewDefaultFramework("disableleaderelection")

ginkgo.BeforeEach(func() {
f.NewEchoDeployment()
})

ginkgo.It("should create multiple ingress routings rules when leader election has disabled", func() {
host1 := "leader.election.disabled.com"
host2 := "leader.election.disabled2.com"

ing1 := framework.NewSingleIngress(host1, "/foo", host1, f.Namespace, framework.EchoService, 80, nil)
f.EnsureIngress(ing1)

ing2 := framework.NewSingleIngress(host2, "/ping", host2, f.Namespace, framework.EchoService, 80, nil)
f.EnsureIngress(ing2)

f.WaitForNginxServer(host1,
func(server string) bool {
return strings.Contains(server, host1) &&
strings.Contains(server, "location /foo")
})

f.WaitForNginxServer(host2,
func(server string) bool {
return strings.Contains(server, host2) &&
strings.Contains(server, "location /ping")
})

f.HTTPTestClient().
GET("/foo").
WithHeader("Host", host1).
Expect().
Status(http.StatusOK)

f.HTTPTestClient().
GET("/bar").
WithHeader("Host", host1).
Expect().
Status(http.StatusNotFound)

f.HTTPTestClient().
GET("/foo").
WithHeader("Host", host2).
Expect().
Status(http.StatusNotFound)

f.HTTPTestClient().
GET("/ping").
WithHeader("Host", host2).
Expect().
Status(http.StatusOK)

f.HTTPTestClient().
GET("/pong").
WithHeader("Host", host2).
Expect().
Status(http.StatusNotFound)

f.HTTPTestClient().
GET("/ping").
WithHeader("Host", host1).
Expect().
Status(http.StatusNotFound)
})
})
1 change: 1 addition & 0 deletions test/e2e/e2e.go
Expand Up @@ -34,6 +34,7 @@ import (
_ "k8s.io/ingress-nginx/test/e2e/annotations/modsecurity"
_ "k8s.io/ingress-nginx/test/e2e/dbg"
_ "k8s.io/ingress-nginx/test/e2e/defaultbackend"
_ "k8s.io/ingress-nginx/test/e2e/disableleaderelection"
_ "k8s.io/ingress-nginx/test/e2e/endpointslices"
_ "k8s.io/ingress-nginx/test/e2e/gracefulshutdown"
_ "k8s.io/ingress-nginx/test/e2e/ingress"
Expand Down

0 comments on commit 9b63559

Please sign in to comment.