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

Add close-sessions-duration config key #827

Merged
merged 1 commit into from
Aug 8, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions docs/content/en/docs/configuration/command-line.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ The following command-line options are supported:
| [`--stats-collect-processing-period`](#stats) | time | `500ms` | v0.10 |
| [`--sync-period`](#sync-period) | time | `10m` | |
| [`--tcp-services-configmap`](#tcp-services-configmap) | namespace/configmapname | no tcp svc | |
| [`--track-old-instances`](#track-old-instances) | [true\|false] | `false` | v0.14 |
| [`--update-status`](#update-status) | [true\|false] | `true` | |
| [`--update-status-on-shutdown`](#update-status-on-shutdown) | [true\|false] | `true` | |
| [`--v`](#v) | log level as integer | `1` | |
Expand Down Expand Up @@ -486,6 +487,24 @@ See also:

---

## --track-old-instances

Since v0.14

Creates an internal list of connections to old HAProxy instances. These connections are used to
read or send data to stopping instances, which is usually serving long lived connections like
TCP services or websockets.

Enabling this option will make old HAProxy instances to not stop before `timeout-stop` timeout,
even if all the remaining sessions finish, so only enable it if using a feature that requests
it.

See also:

* [`close-sessions-duration`]({{% relref "keys#close-sessions-duration" %}}) configuration key

---

## --update-status

Indicates whether the ingress controller should update the `status` attribute of all the Ingress
Expand Down
38 changes: 38 additions & 0 deletions docs/content/en/docs/configuration/keys.md
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ The table below describes all supported configuration keys.
| [`blue-green-header`](#blue-green) | `HeaderName:LabelName` pair | Backend | |
| [`blue-green-mode`](#blue-green) | [pod\|deploy] | Backend | |
| [`cert-signer`](#acme) | "acme" | Host | |
| [`close-sessions-duration`](#close-sessions-duration) | time with suffix or percentage | Global | leave sessions open |
| [`config-backend`](#configuration-snippet) | multiline backend config | Backend | |
| [`config-defaults`](#configuration-snippet) | multiline config for the defaults section | Global | |
| [`config-frontend`](#configuration-snippet) | multiline HTTP and HTTPS frontend config | Global | |
Expand Down Expand Up @@ -1135,6 +1136,43 @@ See also:

---

## Close sessions duration

| Configuration key | Scope | Default | Since |
|---------------------------|----------|----------|-------|
| `close-sessions-duration` | `Global` | | v0.14 |

Defines the amount of time used to close active sessions before a stopping instance times out
and terminates. A stopping instance is an haproxy that doesn't listen sockets anymore, has an
old configuration, and it's just waiting remaining connections to terminate.

Long lived sessions, like websockets or TCP connections, are usually closed only when the
`timeout-stop` of the old instance expires. Depending on how the clients are configured,
all the disconnected clients will reconnect almost at the same time. `close-sessions-duration`
configures the amount of time used to fairly distribute the sessions shutdown, so distributing
client reconnections to the new HAProxy instance.

The default behavior is to not anticipate the disconnections, so all the active sessions will
be closed at the same time when `timeout-stop` expires. `close-sessions-duration` will only
take effect if `timeout-stop` configuration key and `--track-old-instances` command-line option
are also configured.

The duration needs a suffix, which can be a time suffix like `s` (seconds), `m` (minutes) or
`h` (hours), or a `%` that represents a percentage of the `timeout-stop` configuration:

* `10m` means that the last 10 minutes of the `timeout-stop` will be used to distribute sessions shutdown
* `10%` and a `timeout-stop` of `1h`, means that the last 6 minutes of the `timeout-stop` will be used to distribute sessions shutdown

If the suffix is a time unit, the resulting value should be lower than the `timeout-stop`
configuration. If the suffix is a percentage, the value should be between `2%` and `98%`.

See also:

* [`track-old-instances`]({{% relref "command-line#track-old-instances" %}}) command-line option
* [`timeout-stop`](#timeout) configuration key

---

## Configuration snippet

| Configuration key | Scope | Default | Since |
Expand Down
1 change: 1 addition & 0 deletions pkg/common/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ type Configuration struct {
DefaultHealthzURL string
StatsCollectProcPeriod time.Duration
PublishService string
TrackOldInstances bool
Backend ingress.Controller

UpdateStatus bool
Expand Down
6 changes: 6 additions & 0 deletions pkg/common/ingress/controller/launch.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,11 @@ k8s endpoint order (default); 'name' - server/endpoint name;
'ip' - server/endpoint IP and port; 'random' - shuffle endpoints on every
haproxy reload`)

trackOldInstances = flags.Bool("track-old-instances", false,
`Creates an internal list of connections to old HAProxy instances. These
connections are used to read or send data to stopping instances, which is
usually serving long lived connections like TCP services or websockets.`)

useNodeInternalIP = flags.Bool("report-node-internal-ip-address", false,
`Defines if the nodes IP address to be returned in the ingress status should be
the internal instead of the external IP address`)
Expand Down Expand Up @@ -481,6 +486,7 @@ tracked.`)
DisablePodList: *disablePodList,
DisableExternalName: *disableExternalName,
DisableConfigKeywords: *disableConfigKeywords,
TrackOldInstances: *trackOldInstances,
UpdateStatusOnShutdown: *updateStatusOnShutdown,
BackendShards: *backendShards,
SortEndpointsBy: sortEndpoints,
Expand Down
7 changes: 6 additions & 1 deletion pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ func (hc *HAProxyController) configController() {
instanceOptions := haproxy.InstanceOptions{
HAProxyCfgDir: "/etc/haproxy",
HAProxyMapsDir: ingress.DefaultMapsDirectory,
MasterSocket: hc.cfg.MasterSocket,
AdminSocket: "/var/run/haproxy/admin.sock",
BackendShards: hc.cfg.BackendShards,
AcmeSigner: acmeSigner,
AcmeQueue: hc.acmeQueue,
Expand All @@ -132,6 +134,7 @@ func (hc *HAProxyController) configController() {
MaxOldConfigFiles: hc.cfg.MaxOldConfigFiles,
SortEndpointsBy: hc.cfg.SortEndpointsBy,
StopCh: hc.stopCh,
TrackInstances: hc.cfg.TrackOldInstances,
ValidateConfig: hc.cfg.ValidateConfig,
}
hc.instance = haproxy.CreateInstance(hc.logger, instanceOptions)
Expand All @@ -143,14 +146,16 @@ func (hc *HAProxyController) configController() {
Cache: hc.cache,
Tracker: hc.tracker,
DynamicConfig: hc.dynamicConfig,
MasterSocket: hc.cfg.MasterSocket,
MasterSocket: instanceOptions.MasterSocket,
AdminSocket: instanceOptions.AdminSocket,
AnnotationPrefix: hc.cfg.AnnPrefix,
DefaultBackend: hc.cfg.DefaultService,
DefaultCrtSecret: hc.cfg.DefaultSSLCertificate,
FakeCrtFile: hc.createFakeCrtFile(),
FakeCAFile: hc.createFakeCAFile(),
DisableKeywords: strings.Split(hc.cfg.DisableConfigKeywords, ","),
AcmeTrackTLSAnn: hc.cfg.AcmeTrackTLSAnn,
TrackInstances: hc.cfg.TrackOldInstances,
HasGateway: hc.cache.hasGateway(),
}
}
Expand Down
46 changes: 46 additions & 0 deletions pkg/converters/ingress/annotations/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,49 @@ func (c *updater) buildGlobalBind(d *globalData) {
}
}

func (c *updater) buildGlobalCloseSessions(d *globalData) {
durationCfg := d.mapper.Get(ingtypes.GlobalCloseSessionsDuration).Value
if durationCfg == "" {
return
}
if !c.options.TrackInstances {
c.logger.Warn("ignoring close-sessions-duration config: tracking old instances is disabled")
return
}
timeoutCfg := d.mapper.Get(ingtypes.GlobalTimeoutStop).Value
if timeoutCfg == "" {
c.logger.Warn("ignoring close-sessions-duration config: timeout-stop need to be configured")
return
}
timeout, err := time.ParseDuration(timeoutCfg)
if err != nil {
c.logger.Warn("ignoring close-sessions-duration due to invalid timeout-stop config: %v", err)
return
}
var duration time.Duration
if strings.HasSuffix(durationCfg, "%") {
pct, _ := strconv.Atoi(durationCfg[:len(durationCfg)-1])
if pct < 2 || pct > 98 {
c.logger.Warn("ignoring '%s' for close-sessions-duration value: value should be between 5%% and 95%%", durationCfg)
return
}
duration = timeout * time.Duration(pct) / 100
} else {
duration, err = time.ParseDuration(durationCfg)
if err == nil {
if duration >= timeout {
err = fmt.Errorf("close-sessions-duration should be lower than timeout-stop")
}
}
if err != nil {
c.logger.Warn("ignoring invalid close-sessions-duration config: %v", err)
return
}
}
d.global.CloseSessionsDuration = duration
d.global.Timeout.Stats = timeoutCfg
}

func (c *updater) buildGlobalPathTypeOrder(d *globalData) {
matchTypes := make(map[hatypes.MatchType]struct{}, len(hatypes.DefaultMatchOrder))
for _, match := range hatypes.DefaultMatchOrder {
Expand Down Expand Up @@ -225,6 +268,9 @@ func (c *updater) buildGlobalTimeout(d *globalData) {
d.global.Timeout.ServerFin = c.validateTime(d.mapper.Get(ingtypes.BackTimeoutServerFin))
d.global.Timeout.Stop = c.validateTime(d.mapper.Get(ingtypes.GlobalTimeoutStop))
d.global.Timeout.Tunnel = c.validateTime(d.mapper.Get(ingtypes.BackTimeoutTunnel))
if timeoutStop, err := time.ParseDuration(d.global.Timeout.Stop); err == nil {
d.global.TimeoutStopDuration = timeoutStop
}
}

func (c *updater) buildSecurity(d *globalData) {
Expand Down
83 changes: 83 additions & 0 deletions pkg/converters/ingress/annotations/global_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package annotations
import (
"reflect"
"testing"
"time"

ingtypes "github.com/jcmoraisjr/haproxy-ingress/pkg/converters/ingress/types"
convtypes "github.com/jcmoraisjr/haproxy-ingress/pkg/converters/types"
Expand Down Expand Up @@ -149,6 +150,88 @@ func TestBind(t *testing.T) {
}
}

func TestCloseSessions(t *testing.T) {
testCases := []struct {
annDuration string
annStop string
expDuration time.Duration
untrack bool
logging string
}{
// 0
{},
// 1
{
annDuration: "5m",
annStop: "10m",
untrack: true,
logging: `WARN ignoring close-sessions-duration config: tracking old instances is disabled`,
},
// 2
{
annDuration: "10m",
logging: `WARN ignoring close-sessions-duration config: timeout-stop need to be configured`,
},
// 3
{
annDuration: "10m",
annStop: "10%",
logging: `WARN ignoring close-sessions-duration due to invalid timeout-stop config: time: unknown unit "%" in duration "10%"`,
},
// 4
{
annDuration: "1%",
annStop: "10m",
logging: `WARN ignoring '1%' for close-sessions-duration value: value should be between 5% and 95%`,
},
// 5
{
annDuration: "99%",
annStop: "10m",
logging: `WARN ignoring '99%' for close-sessions-duration value: value should be between 5% and 95%`,
},
// 6
{
annDuration: "10x",
annStop: "10m",
logging: `WARN ignoring invalid close-sessions-duration config: time: unknown unit "x" in duration "10x"`,
},
// 7
{
annDuration: "10m",
annStop: "10m",
logging: `WARN ignoring invalid close-sessions-duration config: close-sessions-duration should be lower than timeout-stop`,
},
// 8
{
annDuration: "5m",
annStop: "10m",
expDuration: 5 * time.Minute,
},
// 9
{
annDuration: "5%",
annStop: "10m",
expDuration: 30 * time.Second,
},
}
for i, test := range testCases {
c := setup(t)
d := c.createGlobalData(map[string]string{
ingtypes.GlobalCloseSessionsDuration: test.annDuration,
ingtypes.GlobalTimeoutStop: test.annStop,
})
u := c.createUpdater()
if !test.untrack {
u.options.TrackInstances = true
}
u.buildGlobalCloseSessions(d)
c.compareObjects("close sessions duration", i, d.global.CloseSessionsDuration, test.expDuration)
c.logger.CompareLogging(test.logging)
c.teardown()
}
}

func TestCustomConfigProxy(t *testing.T) {
testCases := []struct {
config string
Expand Down
4 changes: 2 additions & 2 deletions pkg/converters/ingress/annotations/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,7 @@ func (c *updater) UpdateGlobalConfig(haproxyConfig haproxy.Config, mapper *Mappe
global: haproxyConfig.Global(),
mapper: mapper,
}
// TODO Move all magic strings to a single place
d.global.AdminSocket = "/var/run/haproxy/admin.sock"
d.global.AdminSocket = c.options.AdminSocket
d.global.MaxConn = mapper.Get(ingtypes.GlobalMaxConnections).Int()
d.global.DefaultBackendRedir = mapper.Get(ingtypes.GlobalDefaultBackendRedirect).String()
d.global.DefaultBackendRedirCode = mapper.Get(ingtypes.GlobalDefaultBackendRedirectCode).Int()
Expand All @@ -164,6 +163,7 @@ func (c *updater) UpdateGlobalConfig(haproxyConfig haproxy.Config, mapper *Mappe
c.buildGlobalAcme(d)
c.buildGlobalAuthProxy(d)
c.buildGlobalBind(d)
c.buildGlobalCloseSessions(d)
c.buildGlobalCustomConfig(d)
c.buildGlobalDNS(d)
c.buildGlobalDynamic(d)
Expand Down
1 change: 1 addition & 0 deletions pkg/converters/ingress/types/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const (
GlobalBindIPAddrPrometheus = "bind-ip-addr-prometheus"
GlobalBindIPAddrStats = "bind-ip-addr-stats"
GlobalBindIPAddrTCP = "bind-ip-addr-tcp"
GlobalCloseSessionsDuration = "close-sessions-duration"
GlobalConfigDefaults = "config-defaults"
GlobalConfigFrontend = "config-frontend"
GlobalConfigGlobal = "config-global"
Expand Down
2 changes: 2 additions & 0 deletions pkg/converters/types/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type ConverterOptions struct {
Tracker Tracker
DynamicConfig *DynamicConfig
MasterSocket string
AdminSocket string
DefaultConfig func() map[string]string
DefaultBackend string
DefaultCrtSecret string
Expand All @@ -35,6 +36,7 @@ type ConverterOptions struct {
AnnotationPrefix []string
DisableKeywords []string
AcmeTrackTLSAnn bool
TrackInstances bool
HasGateway bool
}

Expand Down
Loading