From 4ede28031e60bf02fee40c3e97727cdee4fe1438 Mon Sep 17 00:00:00 2001 From: Joao Morais <jcmoraisjr@gmail.com> Date: Mon, 14 Sep 2020 08:08:50 -0300 Subject: [PATCH 1/9] check if socket error is eof `Conn.Read()` returns `io.EOF` error if the server closed the connection. This happens when there isn't an expected response from the server. If so, return an empty string as the socket output instead of an error. --- pkg/haproxy/utils/utils.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/haproxy/utils/utils.go b/pkg/haproxy/utils/utils.go index 66985a280..e4b597bbe 100644 --- a/pkg/haproxy/utils/utils.go +++ b/pkg/haproxy/utils/utils.go @@ -18,6 +18,7 @@ package utils import ( "fmt" + "io" "net" "time" ) @@ -39,10 +40,12 @@ func HAProxyCommand(socket string, observer func(duration time.Duration), comman return msg, fmt.Errorf("incomplete data sent to unix socket %s", socket) } readBuffer := make([]byte, 1024) - if r, err := c.Read(readBuffer); err != nil { + if r, err := c.Read(readBuffer); err != nil && err != io.EOF { return msg, fmt.Errorf("error reading response buffer: %v", err) } else if r > 2 { msg = append(msg, fmt.Sprintf("response from server: %s", string(readBuffer[:r-2]))) + } else { + msg = append(msg, "") } observer(time.Since(start)) } From 2039214cacff2c25846cadd2a3db6abcd384362e Mon Sep 17 00:00:00 2001 From: Joao Morais <jcmoraisjr@gmail.com> Date: Mon, 14 Sep 2020 08:30:45 -0300 Subject: [PATCH 2/9] make observer callback optional in haproxyCommand Some admin socket calls don't have a metric callback, so check if an observer was provided before send the collected response time. --- pkg/haproxy/utils/utils.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/haproxy/utils/utils.go b/pkg/haproxy/utils/utils.go index e4b597bbe..cefda6d69 100644 --- a/pkg/haproxy/utils/utils.go +++ b/pkg/haproxy/utils/utils.go @@ -47,7 +47,9 @@ func HAProxyCommand(socket string, observer func(duration time.Duration), comman } else { msg = append(msg, "") } - observer(time.Since(start)) + if observer != nil { + observer(time.Since(start)) + } } return msg, nil } From 4330e451dab9131a85975f28dc547ceb5ecbd9db Mon Sep 17 00:00:00 2001 From: Joao Morais <jcmoraisjr@gmail.com> Date: Mon, 14 Sep 2020 08:35:03 -0300 Subject: [PATCH 3/9] add daemon command-line option in the reload script Always call the reload script with `-D` command-line option, which enforces haproxy to run in the background even if the `daemon` keyword isn't used in the template file. --- rootfs/haproxy-reload.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rootfs/haproxy-reload.sh b/rootfs/haproxy-reload.sh index 966f9555a..3ee5cb9d9 100755 --- a/rootfs/haproxy-reload.sh +++ b/rootfs/haproxy-reload.sh @@ -53,9 +53,9 @@ case "$1" in HAPROXY_PID=/var/run/haproxy/haproxy.pid OLD_PID=$(cat "$HAPROXY_PID" 2>/dev/null || :) if [ -S "$HAPROXY_SOCKET" ]; then - haproxy -f "$CONFIG" -p "$HAPROXY_PID" -sf $OLD_PID -x "$HAPROXY_SOCKET" + haproxy -f "$CONFIG" -p "$HAPROXY_PID" -D -sf $OLD_PID -x "$HAPROXY_SOCKET" else - haproxy -f "$CONFIG" -p "$HAPROXY_PID" -sf $OLD_PID + haproxy -f "$CONFIG" -p "$HAPROXY_PID" -D -sf $OLD_PID fi ;; *) From 269ec6e4ce8c00b9fc964c3210d5aa3189eb2640 Mon Sep 17 00:00:00 2001 From: Joao Morais <jcmoraisjr@gmail.com> Date: Mon, 14 Sep 2020 08:45:49 -0300 Subject: [PATCH 4/9] option to create a minimum haproxy cfg file HAProxy needs a config file to start, which isn't provided in the default image. This creates a command-line option in the entrypoint script that creates a minimum and valid config file, in order to make an external haproxy happy during the startup process. --- rootfs/start.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/rootfs/start.sh b/rootfs/start.sh index 8112ad0c3..10f3aa06b 100755 --- a/rootfs/start.sh +++ b/rootfs/start.sh @@ -19,6 +19,15 @@ set -e if [ $# -gt 0 ] && [ "$(echo $1 | cut -b1-2)" != "--" ]; then # Probably a `docker run -ti`, so exec and exit exec "$@" +elif [ "$1" = "--init" ]; then + cat >/etc/haproxy/haproxy.cfg <<EOF +defaults + timeout server 1s + timeout client 1s + timeout connect 1s +listen l + bind unix@/tmp/dummy.sock +EOF else # Copy static files to /etc/haproxy, which cannot have static content cp -R -p /etc/lua /etc/haproxy/ From 8a44d6d4391e841abd13a6b77cbbee8f37c67711 Mon Sep 17 00:00:00 2001 From: Joao Morais <jcmoraisjr@gmail.com> Date: Tue, 15 Sep 2020 09:28:52 -0300 Subject: [PATCH 5/9] do not change socket response Move the user friendly response "response from server" from the HAProxyCommand() to the dynupdate logger. This change allows a proper parsing of the socket response. --- pkg/haproxy/dynupdate.go | 4 ++-- pkg/haproxy/utils/utils.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/haproxy/dynupdate.go b/pkg/haproxy/dynupdate.go index 621cc8137..b2e917ba3 100644 --- a/pkg/haproxy/dynupdate.go +++ b/pkg/haproxy/dynupdate.go @@ -285,7 +285,7 @@ func (d *dynUpdater) execDisableEndpoint(backname string, ep *hatypes.Endpoint) } d.logger.InfoV(2, "disabled endpoint '%s' on backend/server '%s/%s'", ep.Target, backname, ep.Name) for _, m := range msg { - d.logger.InfoV(2, m) + d.logger.InfoV(2, "response from server: %s", m) } return true } @@ -307,7 +307,7 @@ func (d *dynUpdater) execEnableEndpoint(backname string, oldEP, curEP *hatypes.E d.logger.InfoV(2, "%s endpoint '%s' weight '%d' state '%s' on backend/server '%s/%s'", event, curEP.Target, curEP.Weight, state, backname, curEP.Name) for _, m := range msg { - d.logger.InfoV(2, m) + d.logger.InfoV(2, "response from server: %s", m) } return true } diff --git a/pkg/haproxy/utils/utils.go b/pkg/haproxy/utils/utils.go index cefda6d69..0d4f91692 100644 --- a/pkg/haproxy/utils/utils.go +++ b/pkg/haproxy/utils/utils.go @@ -43,7 +43,7 @@ func HAProxyCommand(socket string, observer func(duration time.Duration), comman if r, err := c.Read(readBuffer); err != nil && err != io.EOF { return msg, fmt.Errorf("error reading response buffer: %v", err) } else if r > 2 { - msg = append(msg, fmt.Sprintf("response from server: %s", string(readBuffer[:r-2]))) + msg = append(msg, string(readBuffer[:r-2])) } else { msg = append(msg, "") } From 046c34979b58fc61e715515b96efd21d197e79f6 Mon Sep 17 00:00:00 2001 From: Joao Morais <jcmoraisjr@gmail.com> Date: Tue, 15 Sep 2020 21:10:17 -0300 Subject: [PATCH 6/9] add a show proc cmd facade `show proc` is a master cli command used to list the active haproxy process - the master, the actives and old ones which might still have pending connections. The facade parses the current `show proc` lay out and creates a ProcTable instance with parsed data. Master cli goes down and drops its listening socket during a reload, so it's also the responsibility of the facade wait master back to life which will give also the total reload time. A successfull reload happens when the current worker slice has at least one instance, an empty list means that the last reload failed. --- pkg/haproxy/utils/utils.go | 111 +++++++++++++++- pkg/haproxy/utils/utils_test.go | 218 ++++++++++++++++++++++++++++++++ 2 files changed, 326 insertions(+), 3 deletions(-) create mode 100644 pkg/haproxy/utils/utils_test.go diff --git a/pkg/haproxy/utils/utils.go b/pkg/haproxy/utils/utils.go index 0d4f91692..83a6f8225 100644 --- a/pkg/haproxy/utils/utils.go +++ b/pkg/haproxy/utils/utils.go @@ -20,7 +20,13 @@ import ( "fmt" "io" "net" + "strconv" + "strings" "time" + + k8snet "k8s.io/apimachinery/pkg/util/net" + + "github.com/jcmoraisjr/haproxy-ingress/pkg/utils" ) // HAProxyCommand ... @@ -30,18 +36,18 @@ func HAProxyCommand(socket string, observer func(duration time.Duration), comman start := time.Now() c, err := net.Dial("unix", socket) if err != nil { - return msg, fmt.Errorf("error connecting to unix socket %s: %v", socket, err) + return msg, fmt.Errorf("error connecting to unix socket %s: %w", socket, err) } defer c.Close() cmd = cmd + "\n" if sent, err := c.Write([]byte(cmd)); err != nil { - return msg, fmt.Errorf("error sending to unix socket %s: %v", socket, err) + return msg, fmt.Errorf("error sending to unix socket %s: %w", socket, err) } else if sent != len(cmd) { return msg, fmt.Errorf("incomplete data sent to unix socket %s", socket) } readBuffer := make([]byte, 1024) if r, err := c.Read(readBuffer); err != nil && err != io.EOF { - return msg, fmt.Errorf("error reading response buffer: %v", err) + return msg, fmt.Errorf("error reading response buffer: %w", err) } else if r > 2 { msg = append(msg, string(readBuffer[:r-2])) } else { @@ -53,3 +59,102 @@ func HAProxyCommand(socket string, observer func(duration time.Duration), comman } return msg, nil } + +// ProcTable ... +type ProcTable struct { + Master Proc + Workers []Proc + OldWorkers []Proc +} + +// Proc ... +type Proc struct { + Type string + PID int + RPID int + Reloads int +} + +var haproxyCmd func(string, func(duration time.Duration), ...string) ([]string, error) = HAProxyCommand + +// HAProxyProcs reads and converts `show proc` from the master CLI to a ProcTable +// instance. Waits for the reload to complete while master CLI is down and the +// attempt to connect leads to a connection refused. Some context: +// https://www.mail-archive.com/haproxy@formilux.org/msg38415.html +// The amount of time between attempts increases exponentially between 1ms and 64ms, +// and aritmetically betweem 128ms and 1s in order to save CPU on long reload events +// and quit fast on the fastest ones. The whole processing time can be calculated by +// the caller as the haproxy reload time. +func HAProxyProcs(masterSocket string) (*ProcTable, error) { + maxLogWait := 64 * time.Millisecond + logFactor := 2 + maxArithWait := 1024 * time.Millisecond + arithFactor := 32 * time.Millisecond + wait := time.Millisecond + for { + time.Sleep(wait) + out, err := haproxyCmd(masterSocket, nil, "show proc") + if err == nil || !k8snet.IsConnectionRefused(err) { + if len(out) > 0 { + return buildProcTable(out[0]), err + } + if err == nil { + return &ProcTable{}, nil + } + return nil, err + } + if wait < maxLogWait { + wait = time.Duration(logFactor) * wait + } else if wait < maxArithWait { + wait = wait + arithFactor + } + } +} + +// buildProcTable parses `show proc` output and creates a corresponding ProcTable +// +// 1 3 4 6 8 8 +// 0.......|.......6.......|.......2.......|.......8.......|.......4.......|.......0.......|.......8 +// #<PID> <type> <relative PID> <reloads> <uptime> <version> +// 1 master 0 2 0d00h01m28s 2.2.3-0e58a34 +// # workers +// 3 worker 1 0 0d00h00m00s 2.2.3-0e58a34 +// # old workers +// 2 worker [was: 1] 1 0d00h00m28s 2.2.3-0e58a34 +// # programs +// +func buildProcTable(procOutput string) *ProcTable { + atoi := func(s string) int { + i, _ := strconv.Atoi(s) + return i + } + cut := func(s string, i, j int) string { + v := strings.TrimSpace(s[i:j]) + if strings.HasPrefix(v, "[") { + return v[6 : len(v)-1] // `[was: 1]` + } + return v + } + procTable := ProcTable{} + old := false + for _, line := range utils.LineToSlice(procOutput) { + if len(line) > 0 && line[0] != '#' { + proc := Proc{ + PID: atoi(cut(line, 0, 16)), + Type: cut(line, 16, 32), + RPID: atoi(cut(line, 32, 48)), + Reloads: atoi(cut(line, 48, 64)), + } + if proc.Type == "master" { + procTable.Master = proc + } else if old { + procTable.OldWorkers = append(procTable.OldWorkers, proc) + } else { + procTable.Workers = append(procTable.Workers, proc) + } + } else if line == "# old workers" { + old = true + } + } + return &procTable +} diff --git a/pkg/haproxy/utils/utils_test.go b/pkg/haproxy/utils/utils_test.go new file mode 100644 index 000000000..1b1b26b5a --- /dev/null +++ b/pkg/haproxy/utils/utils_test.go @@ -0,0 +1,218 @@ +/* +Copyright 2020 The HAProxy Ingress Controller 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 utils + +import ( + "fmt" + "reflect" + "syscall" + "testing" + "time" +) + +func TestHAProxyProcs(t *testing.T) { + testCases := []struct { + cmdOutput []string + cmdError error + expOutput *ProcTable + expError bool + }{ + // 0 + { + expOutput: &ProcTable{}, + }, + // 1 + { + cmdError: fmt.Errorf("fail"), + expError: true, + }, + // 2 + { + cmdOutput: []string{`#<PID> <type> <relative PID> <reloads> <uptime> <version> +1 master 0 0 0d00h00m08s 2.2.3-0e58a34 +# workers +# old workers +# programs + +`}, + expOutput: &ProcTable{ + Master: Proc{Type: "master", PID: 1, RPID: 0, Reloads: 0}, + }, + }, + // 3 + { + cmdOutput: []string{`#<PID> <type> <relative PID> <reloads> <uptime> <version> +1 master 0 1 0d00h00m28s 2.2.3-0e58a34 +# workers +2 worker 1 0 0d00h00m00s 2.2.3-0e58a34 +# old workers +# programs + +`}, + expOutput: &ProcTable{ + Master: Proc{Type: "master", PID: 1, RPID: 0, Reloads: 1}, + Workers: []Proc{ + {Type: "worker", PID: 2, RPID: 1, Reloads: 0}, + }, + }, + }, + // 4 + { + cmdOutput: []string{`#<PID> <type> <relative PID> <reloads> <uptime> <version> +1 master 0 2 0d00h01m28s 2.2.3-0e58a34 +# workers +3 worker 1 0 0d00h00m00s 2.2.3-0e58a34 +# old workers +2 worker [was: 1] 1 0d00h00m28s 2.2.3-0e58a34 +# programs + +`}, + expOutput: &ProcTable{ + Master: Proc{Type: "master", PID: 1, RPID: 0, Reloads: 2}, + Workers: []Proc{ + {Type: "worker", PID: 3, RPID: 1, Reloads: 0}, + }, + OldWorkers: []Proc{ + {Type: "worker", PID: 2, RPID: 1, Reloads: 1}, + }, + }, + }, + // 5 + { + cmdOutput: []string{`#<PID> <type> <relative PID> <reloads> <uptime> <version> +1 master 0 3 0d00h02m28s 2.2.3-0e58a34 +# workers +# old workers +3 worker [was: 1] 2 0d00h01m28s 2.2.3-0e58a34 +# programs + +`}, + expOutput: &ProcTable{ + Master: Proc{Type: "master", PID: 1, RPID: 0, Reloads: 3}, + OldWorkers: []Proc{ + {Type: "worker", PID: 3, RPID: 1, Reloads: 2}, + }, + }, + }, + // 6 + { + cmdOutput: []string{`#<PID> <type> <relative PID> <reloads> <uptime> <version> +1001 master 0 1128 0d00h02m28s 2.2.3-0e58a34 +# workers +3115 worker 1001 11 0d00h00m00s 2.2.3-0e58a34 +3116 worker 1002 11 0d00h00m00s 2.2.3-0e58a34 +# old workers +2112 worker [was: 1001] 128 0d00h01m28s 2.2.3-0e58a34 +2113 worker [was: 1002] 128 0d00h01m28s 2.2.3-0e58a34 +2114 worker [was: 1003] 128 0d00h01m28s 2.2.3-0e58a34 +# programs + +`}, + expOutput: &ProcTable{ + Master: Proc{Type: "master", PID: 1001, RPID: 0, Reloads: 1128}, + Workers: []Proc{ + {Type: "worker", PID: 3115, RPID: 1001, Reloads: 11}, + {Type: "worker", PID: 3116, RPID: 1002, Reloads: 11}, + }, + OldWorkers: []Proc{ + {Type: "worker", PID: 2112, RPID: 1001, Reloads: 128}, + {Type: "worker", PID: 2113, RPID: 1002, Reloads: 128}, + {Type: "worker", PID: 2114, RPID: 1003, Reloads: 128}, + }, + }, + }, + } + for i, test := range testCases { + c := setup(t) + c.cmdOutput = test.cmdOutput + c.cmdError = test.cmdError + out, err := HAProxyProcs("socket") + if !reflect.DeepEqual(out, test.expOutput) { + t.Errorf("output differs on %d - expected: %+v, actual: %+v", i, test.expOutput, out) + } + if (err != nil) != test.expError { + t.Errorf("error differs on %d - expected: %v, actual: %v", i, test.expError, err) + } + c.tearDown() + } +} + +func TestHAProxyProcsLoop(t *testing.T) { + testCases := []struct { + reload time.Duration + minDelay time.Duration + maxCnt int + }{ + // 0 + { + reload: 0, + minDelay: 1 * time.Millisecond, + maxCnt: 1, + }, + // 1 + { + reload: 20 * time.Millisecond, + minDelay: (1 + 2 + 4 + 8 + 16) * time.Millisecond, + maxCnt: 5, + }, + // 2 + { + reload: 450 * time.Millisecond, + minDelay: (1 + 2 + 4 + 8 + 16 + 32 + 64 + 96 + 128 + 160) * time.Millisecond, + maxCnt: 10, + }, + } + for i, test := range testCases { + c := setup(t) + c.cmdError = syscall.ECONNREFUSED + time.AfterFunc(test.reload, func() { c.cmdError = nil }) + start := time.Now() + _, err := HAProxyProcs("") + if err != nil { + t.Errorf("%d should not return an error: %w", i, err) + } + elapsed := time.Now().Sub(start) + if elapsed < test.minDelay { + t.Errorf("elapsed in %d is '%s' and should not be lower than min '%s'", i, elapsed.String(), test.minDelay.String()) + } + if c.callCnt > test.maxCnt { + t.Errorf("callCnt in %d is '%d' and should not be greater than max '%d'", i, c.callCnt, test.maxCnt) + } + } +} + +type testConfig struct { + t *testing.T + cmdOutput []string + cmdError error + callCnt int +} + +func setup(t *testing.T) *testConfig { + c := &testConfig{ + t: t, + } + haproxyCmd = c.haproxyCommand + return c +} + +func (c *testConfig) tearDown() {} + +func (c *testConfig) haproxyCommand(string, func(duration time.Duration), ...string) ([]string, error) { + c.callCnt++ + return c.cmdOutput, c.cmdError +} From 3b587c698739f28595de6232fc48231ec51a83da Mon Sep 17 00:00:00 2001 From: Joao Morais <jcmoraisjr@gmail.com> Date: Thu, 17 Sep 2020 08:31:38 -0300 Subject: [PATCH 7/9] remove instance cmd options Instance cmd, currently haproxy and reload, was configured in the most external controller code and now it's not compatible with the new external haproxy option and remote commands. This will need another refactor in the future, moving all magic strings to a single place. A new field `fake` is now in place. Unit tests defines this field as true in order to the instance code know that there is no haproxy to be reloaded. Btw this will also need a refactor in the future, using mocks instead of ifs. --- pkg/controller/controller.go | 2 -- pkg/haproxy/instance.go | 18 ++++++++++-------- pkg/haproxy/instance_test.go | 2 ++ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index b3f5cc57f..f235933cc 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -121,8 +121,6 @@ func (hc *HAProxyController) configController() { ) } instanceOptions := haproxy.InstanceOptions{ - HAProxyCmd: "haproxy", - ReloadCmd: "/haproxy-reload.sh", HAProxyCfgDir: "/etc/haproxy", HAProxyMapsDir: ingress.DefaultMapsDirectory, BackendShards: hc.cfg.BackendShards, diff --git a/pkg/haproxy/instance.go b/pkg/haproxy/instance.go index 4bb6d3668..46e3e4ebe 100644 --- a/pkg/haproxy/instance.go +++ b/pkg/haproxy/instance.go @@ -38,15 +38,15 @@ type InstanceOptions struct { AcmeSigner acme.Signer AcmeQueue utils.Queue BackendShards int - HAProxyCmd string HAProxyCfgDir string HAProxyMapsDir string LeaderElector types.LeaderElector MaxOldConfigFiles int Metrics types.Metrics - ReloadCmd string ReloadStrategy string ValidateConfig bool + // TODO Fake is used to skip real haproxy calls. Use a mock instead. + fake bool } // Instance ... @@ -251,8 +251,8 @@ func (i *instance) haproxyUpdate(timer *utils.Timer) { return } timer.Tick("write_maps") - if i.options.HAProxyCmd != "" { - // TODO update tests and remove `if cmd!=""` above + if !i.options.fake { + // TODO update tests and remove `if !fake` above i.logChanged() } updater := i.newDynUpdater() @@ -408,11 +408,12 @@ func (i *instance) updateCertExpiring() { } func (i *instance) check() error { - if i.options.HAProxyCmd == "" { + if i.options.fake { i.logger.Info("(test) check was skipped") return nil } - out, err := exec.Command(i.options.HAProxyCmd, "-c", "-f", i.options.HAProxyCfgDir).CombinedOutput() + // TODO Move all magic strings to a single place + out, err := exec.Command("haproxy", "-c", "-f", i.options.HAProxyCfgDir).CombinedOutput() outstr := string(out) if err != nil { return fmt.Errorf(outstr) @@ -421,11 +422,12 @@ func (i *instance) check() error { } func (i *instance) reload() error { - if i.options.ReloadCmd == "" { + if i.options.fake { i.logger.Info("(test) reload was skipped") return nil } - out, err := exec.Command(i.options.ReloadCmd, i.options.ReloadStrategy, i.options.HAProxyCfgDir).CombinedOutput() + // TODO Move all magic strings to a single place + out, err := exec.Command("/haproxy-reload.sh", i.options.ReloadStrategy, i.options.HAProxyCfgDir).CombinedOutput() outstr := string(out) if len(outstr) > 0 { i.logger.Warn("output from haproxy:\n%v", outstr) diff --git a/pkg/haproxy/instance_test.go b/pkg/haproxy/instance_test.go index 6716400dd..4280b3975 100644 --- a/pkg/haproxy/instance_test.go +++ b/pkg/haproxy/instance_test.go @@ -3050,6 +3050,8 @@ func setupOptions(options testOptions) *testConfig { HAProxyMapsDir: tempdir, Metrics: helper_test.NewMetricsMock(), BackendShards: options.shardCount, + // + fake: true, }).(*instance) if err := instance.haproxyTmpl.NewTemplate( "haproxy.tmpl", From d00617aaedf80d26bb9a615c7dc2e19e0121fc51 Mon Sep 17 00:00:00 2001 From: Joao Morais <jcmoraisjr@gmail.com> Date: Sun, 20 Sep 2020 08:39:10 -0300 Subject: [PATCH 8/9] add compatibility with master-worker and external haproxy Add new global configuration keys that can be used to adjust the config file to an external haproxy deployment: * `username` and `groupname` that configures a non root user that should own the running haproxy process; * `external-has-lua` that currently loads `auth-request.lua`, this script would crash haproxy if loaded and O.S. doesn't have Lua socket installed; * `master-exit-on-failure` that configs such option in the master worker mode; * `use-haproxy-user` continue to behave just like v0.9-v0.11, but now it's an alias to configure username and groupname as haproxy. hatypes.External.IsExternal value is not being updated yet, its value depends on the master socket being configured - this is a task of a future commit. haproxy's `unix-bind` keyword now only changes the user and group of unix sockets if a non root user is used, there is no reason to change the socket user if the haproxy process owner isn't changed - and it also simplifies the embedded/external configuration compatibility. oauth2 was also updated to not be configured if `external-has-lua` wasn't enabled, otherwise it would fail haproxy bootstrap. --- docs/content/en/docs/configuration/keys.md | 67 ++++++++++++++- pkg/converters/ingress/annotations/backend.go | 5 ++ .../ingress/annotations/backend_test.go | 29 +++++++ pkg/converters/ingress/annotations/global.go | 22 +++++ .../ingress/annotations/global_test.go | 82 ++++++++++++++++++ pkg/converters/ingress/annotations/updater.go | 5 +- pkg/converters/ingress/defaults.go | 1 + pkg/converters/ingress/types/global.go | 4 + pkg/haproxy/instance_test.go | 85 ++++++++++++++++++- pkg/haproxy/types/types.go | 23 ++++- rootfs/etc/templates/haproxy/haproxy.tmpl | 17 ++-- 11 files changed, 325 insertions(+), 15 deletions(-) diff --git a/docs/content/en/docs/configuration/keys.md b/docs/content/en/docs/configuration/keys.md index f34325fd5..fdadf5bdc 100644 --- a/docs/content/en/docs/configuration/keys.md +++ b/docs/content/en/docs/configuration/keys.md @@ -147,8 +147,10 @@ The table below describes all supported configuration keys. | [`drain-support`](#drain-support) | [true\|false] | Global | `false` | | [`drain-support-redispatch`](#drain-support) | [true\|false] | Global | `true` | | [`dynamic-scaling`](#dynamic-scaling) | [true\|false] | Backend | `true` | +| [`external-has-lua`](#external) | [true\|false] | Global | `false` | | [`forwardfor`](#forwardfor) | [add\|ignore\|ifmissing] | Global | `add` | | [`fronting-proxy-port`](#fronting-proxy-port) | port number | Global | 0 (do not listen) | +| [`groupname`](#security) | haproxy group name | Global | `haproxy` | | [`headers`](#headers) | multiline header:value pair | Backend | | | [`health-check-addr`](#health-check) | address for health checks | Backend | | | [`health-check-fall-count`](#health-check) | number of failures | Backend | | @@ -171,6 +173,7 @@ The table below describes all supported configuration keys. | [`limit-rps`](#limit) | rate per second | Backend | | | [`limit-whitelist`](#limit) | cidr list | Backend | | | [`load-server-state`](#load-server-state) (experimental) |[true\|false] | Global | `false` | +| [`master-exit-on-failure`](#master-worker) | [true\|false] | Global | `true` | | [`max-connections`](#connection) | number | Global | `2000` | | [`maxconn-server`](#connection) | qty | Backend | | | [`maxqueue-server`](#connection) | qty | Backend | | @@ -247,6 +250,7 @@ The table below describes all supported configuration keys. | [`use-htx`](#use-htx) | [true\|false] | Global | `false` | | [`use-proxy-protocol`](#proxy-protocol) | [true\|false] | Global | `false` | | [`use-resolver`](#dns-resolvers) | resolver name | Backend | | +| [`username`](#security) | haproxy user name | Global | `haproxy` | | [`var-namespace`](#var-namespace) | [true\|false] | Host | `false` | | [`waf`](#waf) | "modsecurity" | Backend | | | [`waf-mode`](#waf) | [deny\|detect] | Backend | `deny` (if waf is set) | @@ -935,6 +939,26 @@ See also: --- +## External + +| Configuration key | Scope | Default | Since | +|--------------------|----------|---------|-------| +| `external-has-lua` | `Global` | `false` | v0.12 | + +Defines features that can be found in the external haproxy deployment, if an +external deployment is used. These options have no effect if using the embedded +haproxy. + +* `external-has-lua`: Define as true if the external haproxy has Lua libraries +installed in the operating system. Currently only [OAuth](#oauth) needs Lua +socket installed and will not work if `external-has-lua` is not enabled. + +See also: + +* [OAuth](#oauth) configuration keys. + +--- + ## Forwardfor | Configuration key | Scope | Default | Since | @@ -1174,6 +1198,23 @@ See also: --- +## Master-worker + +| Configuration key | Scope | Default | Since | +|--------------------------|----------|---------|-------| +| `master-exit-on-failure` | `Global` | `true` | v0.12 | + +Configures master-worker related options. + +* `master-exit-on-failure`: If `true`, kill all the remaining workers and exit +from master in the case of an unexpected failure of a worker, eg a segfault. + +See also: + +* https://cbonte.github.io/haproxy-dconv/2.0/configuration.html#3.1-master-worker + +--- + ## Modsecurity | Configuration key | Scope | Default | Since | @@ -1293,8 +1334,13 @@ on an ingress resource without oauth annotations. In other words, if two ingress the same domain but only one has oauth annotations - the one that has at least the `oauth2_proxy` service - all paths from that domain will be protected. +{{% alert title="Note" %}} +OAuth2 needs [`external-has-lua`](#external) enabled if running on an external haproxy deployment. +{{% /alert %}} + See also: +* [`external-has-lua`](#external) configuration key. * [example](https://github.com/jcmoraisjr/haproxy-ingress/tree/master/examples/auth/oauth) page. --- @@ -1426,15 +1472,26 @@ See also: | Configuration key | Scope | Default | Since | |--------------------|----------|---------|-------| +| `groupname` | `Global` | | v0.12 | | `use-chroot` | `Global` | `false` | v0.9 | | `use-haproxy-user` | `Global` | `false` | v0.9 | +| `username` | `Global` | | v0.12 | Change security options. -* `use-chroot`: If `true`, configures haproxy to perform a `chroot()` in the empty and non-writable directory `/var/empty` during the startup process, just before it drops its own privileges. Only root can perform a `chroot()`, so HAProxy Ingress container should start as UID `0` if this option is configured as `true`. See the note below about `use-chroot` option limitations. -* `use-haproxy-user`: Defines if the haproxy's process should be changed to `haproxy`, UID `1001`. The default value `false` leaves haproxy running as root. Note that even running as root, haproxy always drops its own privileges before start its event loop. +* `username` and `groupname`: Changes the user and group names used to run haproxy as non root. The default value is an empty string, which means leave haproxy running as root. Note that even running as root, haproxy always drops its own privileges before start its event loop. Both options should be declared to the configuration take effect. Note that this configuration means "running haproxy as non root", it's only useful when the haproxy container starts as root. +* `use-chroot`: If `true`, configures haproxy to perform a `chroot()` in the empty and non-writable directory `/var/empty` during the startup process, just before it drops its own privileges. Only root can perform a `chroot()`, so HAProxy Ingress container should start as UID `0` if this option is configured as `true`. See **Using chroot()** section below. +* `use-haproxy-user`: If `true`, configures `username` and `groupname` configuration keys as `haproxy`. See `username` and `groupname` above. Note that this user and group exists in the embedded haproxy, and should exist in the external haproxy if used. In the case of a conflict, `username` and `groupname` declaration will have priority and `use-haproxy-user` will be ignored. If `false`, the default value, user and group names will not be changed. + +**Starting as non root** + +In the default configuration HAProxy Ingress container starts as root. Since v0.9 it's also possible to configure the container to start as `haproxy` user, UID `1001`. + +If using the embedded haproxy, read the [Security considerations](http://cbonte.github.io/haproxy-dconv/2.0/management.html#13) from HAProxy doc before change the starting user. -In the default configuration, HAProxy Ingress container starts as root. Since v0.9 it's also possible to configure the container to start as `haproxy` user, UID `1001`. Read the [Security considerations](http://cbonte.github.io/haproxy-dconv/1.9/management.html#13) from HAProxy doc before change the starting user. The starting user can be changed in the deployment or daemonset's pod template using the following configuration: +If using an external haproxy, configures the pod's securityContext (instead of the container's one) which will make Kubernetes create the shared file system with write access, so the controller can create and update configuration, maps and certificate files. + +The starting user can be changed in the deployment or daemonset's pod template using the following configuration: ```yaml ... @@ -1446,6 +1503,10 @@ In the default configuration, HAProxy Ingress container starts as root. Since v0 Note that ports below 1024 cannot be bound if the container starts as non-root. +**Using chroot()** + +Beware of some chroot limitations: + {{% alert title="Note" %}} HAProxy does not have access to the file system after configure a `chroot()`. Unix sockets located outside the chroot directory are used in the following conditions: diff --git a/pkg/converters/ingress/annotations/backend.go b/pkg/converters/ingress/annotations/backend.go index ebae495ee..d9eec1e43 100644 --- a/pkg/converters/ingress/annotations/backend.go +++ b/pkg/converters/ingress/annotations/backend.go @@ -567,6 +567,11 @@ func (c *updater) buildBackendOAuth(d *backData) { c.logger.Warn("ignoring invalid oauth implementation '%s' on %v", oauth, oauth.Source) return } + external := c.haproxy.Global().External + if external.IsExternal && !external.HasLua { + c.logger.Warn("oauth2_proxy on %v needs Lua socket, install Lua libraries and enable 'external-has-lua' global config", oauth.Source) + return + } uriPrefix := "/oauth2" headers := []string{"X-Auth-Request-Email:auth_response_email"} if prefix := d.mapper.Get(ingtypes.BackOAuthURIPrefix); prefix.Source != nil { diff --git a/pkg/converters/ingress/annotations/backend_test.go b/pkg/converters/ingress/annotations/backend_test.go index 14f227574..06ff61b52 100644 --- a/pkg/converters/ingress/annotations/backend_test.go +++ b/pkg/converters/ingress/annotations/backend_test.go @@ -1099,6 +1099,8 @@ func TestOAuth(t *testing.T) { testCases := []struct { annDefault map[string]string ann map[string]string + external bool + haslua bool backend string oauthExp hatypes.OAuthConfig logging string @@ -1225,6 +1227,31 @@ func TestOAuth(t *testing.T) { }, }, }, + // 10 + { + ann: map[string]string{ + ingtypes.BackOAuth: "oauth2_proxy", + }, + external: true, + backend: "default:back:/oauth2", + oauthExp: hatypes.OAuthConfig{}, + logging: "WARN oauth2_proxy on ingress 'default/ing1' needs Lua socket, install Lua libraries and enable 'external-has-lua' global config", + }, + // 11 + { + ann: map[string]string{ + ingtypes.BackOAuth: "oauth2_proxy", + }, + external: true, + haslua: true, + backend: "default:back:/oauth2", + oauthExp: hatypes.OAuthConfig{ + Impl: "oauth2_proxy", + BackendName: "default_back_8080", + URIPrefix: "/oauth2", + Headers: map[string]string{"X-Auth-Request-Email": "auth_response_email"}, + }, + }, } source := &Source{ @@ -1235,6 +1262,8 @@ func TestOAuth(t *testing.T) { for i, test := range testCases { c := setup(t) d := c.createBackendData("default/app", source, test.ann, test.annDefault) + c.haproxy.Global().External.IsExternal = test.external + c.haproxy.Global().External.HasLua = test.haslua if test.backend != "" { b := strings.Split(test.backend, ":") backend := c.haproxy.Backends().AcquireBackend(b[0], b[1], "8080") diff --git a/pkg/converters/ingress/annotations/global.go b/pkg/converters/ingress/annotations/global.go index 740ac6a87..6c53be193 100644 --- a/pkg/converters/ingress/annotations/global.go +++ b/pkg/converters/ingress/annotations/global.go @@ -206,6 +206,28 @@ func (c *updater) buildGlobalTimeout(d *globalData) { d.global.Timeout.Tunnel = c.validateTime(d.mapper.Get(ingtypes.BackTimeoutTunnel)) } +func (c *updater) buildSecurity(d *globalData) { + username := d.mapper.Get(ingtypes.GlobalUsername).Value + groupname := d.mapper.Get(ingtypes.GlobalGroupname).Value + if (username == "") != (groupname == "") { + c.logger.Warn("if configuring non root user, both username and groupname must be defined") + username = "" + groupname = "" + } + haproxy := "haproxy" + useHaproxyUser := d.mapper.Get(ingtypes.GlobalUseHAProxyUser).Bool() + if useHaproxyUser { + if username == "" { + username, groupname = haproxy, haproxy + } else if username != haproxy || groupname != haproxy { + c.logger.Warn("username and groupname are already defined as '%s' and '%s', ignoring '%s' config", username, groupname, ingtypes.GlobalUseHAProxyUser) + } + } + d.global.Security.Username = username + d.global.Security.Groupname = groupname + d.global.Security.UseChroot = d.mapper.Get(ingtypes.GlobalUseChroot).Bool() +} + func (c *updater) buildGlobalSSL(d *globalData) { ssl := &d.global.SSL ssl.ALPN = d.mapper.Get(ingtypes.HostTLSALPN).Value diff --git a/pkg/converters/ingress/annotations/global_test.go b/pkg/converters/ingress/annotations/global_test.go index f62cb6c31..15aa948bc 100644 --- a/pkg/converters/ingress/annotations/global_test.go +++ b/pkg/converters/ingress/annotations/global_test.go @@ -429,3 +429,85 @@ func TestPathTypeOrder(t *testing.T) { c.teardown() } } + +func TestSecurity(t *testing.T) { + testCases := []struct { + ann map[string]string + expected hatypes.SecurityConfig + logging string + }{ + // 0 + { + ann: map[string]string{ + ingtypes.GlobalUseChroot: "true", + }, + expected: hatypes.SecurityConfig{ + UseChroot: true, + }, + }, + // 1 + { + ann: map[string]string{ + ingtypes.GlobalUsername: "someuser", + }, + expected: hatypes.SecurityConfig{}, + logging: `WARN if configuring non root user, both username and groupname must be defined`, + }, + // 2 + { + ann: map[string]string{ + ingtypes.GlobalUsername: "someuser", + ingtypes.GlobalGroupname: "somegroup", + }, + expected: hatypes.SecurityConfig{ + Username: "someuser", + Groupname: "somegroup", + }, + }, + // 3 + { + ann: map[string]string{ + ingtypes.GlobalUsername: "someuser", + ingtypes.GlobalGroupname: "somegroup", + ingtypes.GlobalUseHAProxyUser: "true", + }, + expected: hatypes.SecurityConfig{ + Username: "someuser", + Groupname: "somegroup", + }, + logging: `WARN username and groupname are already defined as 'someuser' and 'somegroup', ignoring 'use-haproxy-user' config`, + }, + // 4 + { + ann: map[string]string{ + ingtypes.GlobalUsername: "someuser", + ingtypes.GlobalUseHAProxyUser: "true", + }, + expected: hatypes.SecurityConfig{ + Username: "haproxy", + Groupname: "haproxy", + }, + logging: `WARN if configuring non root user, both username and groupname must be defined`, + }, + // 5 + { + ann: map[string]string{ + ingtypes.GlobalUsername: "haproxy", + ingtypes.GlobalGroupname: "haproxy", + ingtypes.GlobalUseHAProxyUser: "true", + }, + expected: hatypes.SecurityConfig{ + Username: "haproxy", + Groupname: "haproxy", + }, + }, + } + for i, test := range testCases { + c := setup(t) + d := c.createGlobalData(test.ann) + c.createUpdater().buildSecurity(d) + c.compareObjects("fronting proxy", i, d.global.Security, test.expected) + c.logger.CompareLogging(test.logging) + c.teardown() + } +} diff --git a/pkg/converters/ingress/annotations/updater.go b/pkg/converters/ingress/annotations/updater.go index 9b53d0d6d..c654f1660 100644 --- a/pkg/converters/ingress/annotations/updater.go +++ b/pkg/converters/ingress/annotations/updater.go @@ -111,10 +111,10 @@ func (c *updater) UpdateGlobalConfig(haproxyConfig haproxy.Config, mapper *Mappe d.global.DrainSupport.Drain = mapper.Get(ingtypes.GlobalDrainSupport).Bool() d.global.DrainSupport.Redispatch = mapper.Get(ingtypes.GlobalDrainSupportRedispatch).Bool() d.global.Cookie.Key = mapper.Get(ingtypes.GlobalCookieKey).Value + d.global.External.HasLua = mapper.Get(ingtypes.GlobalExternalHasLua).Bool() d.global.LoadServerState = mapper.Get(ingtypes.GlobalLoadServerState).Bool() + d.global.Master.ExitOnFailure = mapper.Get(ingtypes.GlobalMasterExitOnFailure).Bool() d.global.StrictHost = mapper.Get(ingtypes.GlobalStrictHost).Bool() - d.global.UseChroot = mapper.Get(ingtypes.GlobalUseChroot).Bool() - d.global.UseHAProxyUser = mapper.Get(ingtypes.GlobalUseHAProxyUser).Bool() d.global.UseHTX = mapper.Get(ingtypes.GlobalUseHTX).Bool() c.buildGlobalAcme(d) c.buildGlobalBind(d) @@ -125,6 +125,7 @@ func (c *updater) UpdateGlobalConfig(haproxyConfig haproxy.Config, mapper *Mappe c.buildGlobalModSecurity(d) c.buildGlobalPathTypeOrder(d) c.buildGlobalProc(d) + c.buildSecurity(d) c.buildGlobalSSL(d) c.buildGlobalStats(d) c.buildGlobalSyslog(d) diff --git a/pkg/converters/ingress/defaults.go b/pkg/converters/ingress/defaults.go index 2efc3a66d..476b6fa1c 100644 --- a/pkg/converters/ingress/defaults.go +++ b/pkg/converters/ingress/defaults.go @@ -77,6 +77,7 @@ func createDefaults() map[string]string { types.GlobalHealthzPort: "10253", types.GlobalHTTPPort: "80", types.GlobalHTTPSPort: "443", + types.GlobalMasterExitOnFailure: "true", types.GlobalMaxConnections: "2000", types.GlobalModsecurityTimeoutConnect: "5s", types.GlobalModsecurityTimeoutHello: "100ms", diff --git a/pkg/converters/ingress/types/global.go b/pkg/converters/ingress/types/global.go index 126fe23c0..be4e546e3 100644 --- a/pkg/converters/ingress/types/global.go +++ b/pkg/converters/ingress/types/global.go @@ -44,8 +44,10 @@ const ( GlobalDNSTimeoutRetry = "dns-timeout-retry" GlobalDrainSupport = "drain-support" GlobalDrainSupportRedispatch = "drain-support-redispatch" + GlobalExternalHasLua = "external-has-lua" GlobalForwardfor = "forwardfor" GlobalFrontingProxyPort = "fronting-proxy-port" + GlobalGroupname = "groupname" GlobalHealthzPort = "healthz-port" GlobalHTTPLogFormat = "http-log-format" GlobalHTTPPort = "http-port" @@ -53,6 +55,7 @@ const ( GlobalHTTPSPort = "https-port" GlobalHTTPStoHTTPPort = "https-to-http-port" GlobalLoadServerState = "load-server-state" + GlobalMasterExitOnFailure = "master-exit-on-failure" GlobalMaxConnections = "max-connections" GlobalModsecurityEndpoints = "modsecurity-endpoints" GlobalModsecurityTimeoutConnect = "modsecurity-timeout-connect" @@ -65,6 +68,7 @@ const ( GlobalNbthread = "nbthread" GlobalNoTLSRedirectLocations = "no-tls-redirect-locations" GlobalPathTypeOrder = "path-type-order" + GlobalUsername = "username" GlobalPrometheusPort = "prometheus-port" GlobalSSLDHDefaultMaxSize = "ssl-dh-default-max-size" GlobalSSLDHParam = "ssl-dh-param" diff --git a/pkg/haproxy/instance_test.go b/pkg/haproxy/instance_test.go index 4280b3975..e3a5f37c5 100644 --- a/pkg/haproxy/instance_test.go +++ b/pkg/haproxy/instance_test.go @@ -804,7 +804,7 @@ func TestInstanceEmpty(t *testing.T) { c.checkConfig(` global daemon - unix-bind user haproxy group haproxy mode 0600 + unix-bind mode 0600 stats socket /var/run/haproxy.sock level admin expose-fd listeners mode 600 maxconn 2000 hard-stop-after 15m @@ -851,6 +851,84 @@ empty/ default_empty_8080`) c.logger.CompareLogging(defaultLogging) } +func TestInstanceEmptyExternal(t *testing.T) { + c := setup(t) + defer c.teardown() + + c.config.global.External.IsExternal = true + c.config.global.Security.Username = "external" + c.config.global.Security.Groupname = "external" + + c.config.Hosts().AcquireHost("empty").AddPath(c.config.Backends().AcquireBackend("default", "empty", "8080"), "/", hatypes.MatchBegin) + c.Update() + + c.checkConfig(` +global + master-worker + user external + group external + unix-bind user external group external mode 0600 + stats socket /var/run/haproxy.sock level admin expose-fd listeners mode 600 + maxconn 2000 + hard-stop-after 15m + lua-load /etc/haproxy/lua/services.lua + ssl-dh-param-file /var/haproxy/tls/dhparam.pem + ssl-default-bind-ciphers ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256 + ssl-default-bind-ciphersuites TLS_AES_128_GCM_SHA256 + ssl-default-bind-options no-sslv3 + ssl-default-server-ciphers ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256 + ssl-default-server-ciphersuites TLS_AES_128_GCM_SHA256 +<<defaults>> +backend default_empty_8080 + mode http +backend _error404 + mode http + http-request use-service lua.send-404 +<<frontends-default>> +<<support>> +`) + c.logger.CompareLogging(defaultLogging) +} + +func TestInstanceSecurity(t *testing.T) { + c := setup(t) + defer c.teardown() + + c.config.global.Security.Username = "haproxy" + c.config.global.Security.Groupname = "haproxy" + + c.config.Hosts().AcquireHost("empty").AddPath(c.config.Backends().AcquireBackend("default", "empty", "8080"), "/", hatypes.MatchBegin) + c.Update() + + c.checkConfig(` +global + daemon + user haproxy + group haproxy + unix-bind user haproxy group haproxy mode 0600 + stats socket /var/run/haproxy.sock level admin expose-fd listeners mode 600 + maxconn 2000 + hard-stop-after 15m + lua-load /etc/haproxy/lua/auth-request.lua + lua-load /etc/haproxy/lua/services.lua + ssl-dh-param-file /var/haproxy/tls/dhparam.pem + ssl-default-bind-ciphers ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256 + ssl-default-bind-ciphersuites TLS_AES_128_GCM_SHA256 + ssl-default-bind-options no-sslv3 + ssl-default-server-ciphers ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES128-GCM-SHA256 + ssl-default-server-ciphersuites TLS_AES_128_GCM_SHA256 +<<defaults>> +backend default_empty_8080 + mode http +backend _error404 + mode http + http-request use-service lua.send-404 +<<frontends-default>> +<<support>> +`) + c.logger.CompareLogging(defaultLogging) +} + func TestInstanceMatch(t *testing.T) { c := setup(t) defer c.teardown() @@ -2293,7 +2371,7 @@ func TestInstanceSyslog(t *testing.T) { c.checkConfig(` global daemon - unix-bind user haproxy group haproxy mode 0600 + unix-bind mode 0600 stats socket /var/run/haproxy.sock level admin expose-fd listeners mode 600 maxconn 2000 hard-stop-after 15m @@ -3097,6 +3175,7 @@ func (c *testConfig) configGlobal(global *hatypes.Global) { global.Bind.HTTPSBind = ":443" global.Cookie.Key = "Ingress" global.Healthz.Port = 10253 + global.Master.ExitOnFailure = true global.MatchOrder = hatypes.DefaultMatchOrder global.MaxConn = 2000 global.SSL.ALPN = "h2,http/1.1" @@ -3208,7 +3287,7 @@ func (c *testConfig) checkConfigFile(expected, fileName string) { replace := map[string]string{ "<<global>>": `global daemon - unix-bind user haproxy group haproxy mode 0600 + unix-bind mode 0600 stats socket /var/run/haproxy.sock level admin expose-fd listeners mode 600 maxconn 2000 hard-stop-after 15m diff --git a/pkg/haproxy/types/types.go b/pkg/haproxy/types/types.go index 1632b613d..a8619bd4c 100644 --- a/pkg/haproxy/types/types.go +++ b/pkg/haproxy/types/types.go @@ -63,13 +63,14 @@ type Global struct { ForwardFor string LoadServerState bool AdminSocket string + External ExternalConfig Healthz HealthzConfig + Master MasterConfig MatchOrder []MatchType Prometheus PromConfig + Security SecurityConfig Stats StatsConfig StrictHost bool - UseChroot bool - UseHAProxyUser bool UseHTX bool CustomConfig []string CustomDefaults []string @@ -177,18 +178,36 @@ type DrainConfig struct { Redispatch bool } +// ExternalConfig ... +type ExternalConfig struct { + HasLua bool + IsExternal bool +} + // HealthzConfig ... type HealthzConfig struct { BindIP string Port int } +// MasterConfig ... +type MasterConfig struct { + ExitOnFailure bool +} + // PromConfig ... type PromConfig struct { BindIP string Port int } +// SecurityConfig ... +type SecurityConfig struct { + Groupname string + UseChroot bool + Username string +} + // StatsConfig ... type StatsConfig struct { AcceptProxy bool diff --git a/rootfs/etc/templates/haproxy/haproxy.tmpl b/rootfs/etc/templates/haproxy/haproxy.tmpl index 686fa4ad2..796bbc3e9 100644 --- a/rootfs/etc/templates/haproxy/haproxy.tmpl +++ b/rootfs/etc/templates/haproxy/haproxy.tmpl @@ -59,13 +59,18 @@ {{- define "global" }} {{- $global := .p1 }} global +{{- if $global.External.IsExternal }} + master-worker{{ if not $global.Master.ExitOnFailure }} no-exit-on-failure{{ end }} +{{- else }} daemon -{{- if $global.UseHAProxyUser }} - user haproxy - group haproxy {{- end }} - unix-bind user haproxy group haproxy mode 0600 -{{- if $global.UseChroot }} +{{- $nonroot := and $global.Security.Username $global.Security.Groupname }} +{{- if $nonroot }} + user {{ $global.Security.Username }} + group {{ $global.Security.Groupname }} +{{- end }} + unix-bind{{ if $nonroot }} user {{ $global.Security.Username }} group {{ $global.Security.Groupname }}{{ end }} mode 0600 +{{- if $global.Security.UseChroot }} chroot /var/empty {{- end }} {{- if gt $global.Procs.Nbproc 1 }} @@ -91,7 +96,9 @@ global log {{ $global.Syslog.Endpoint }} len {{ $global.Syslog.Length }} format {{ $global.Syslog.Format }} local0 log-tag {{ $global.Syslog.Tag }} {{- end }} +{{- if or (not $global.External.IsExternal) $global.External.HasLua }} lua-load /etc/haproxy/lua/auth-request.lua +{{- end }} lua-load /etc/haproxy/lua/services.lua {{- if $global.SSL.DHParam.Filename }} ssl-dh-param-file {{ $global.SSL.DHParam.Filename }} From ed1c79870f4b7ba379ceb37274491ea84ddb67ab Mon Sep 17 00:00:00 2001 From: Joao Morais <jcmoraisjr@gmail.com> Date: Mon, 21 Sep 2020 23:18:16 -0300 Subject: [PATCH 9/9] add master-socket command-line option Add command-line option that enables an external haproxy in master-worker mode and references the unix socket of its master CLI. The master CLI is used to reload haproxy and check if the reload successfully started a new worker. --- .../en/docs/configuration/command-line.md | 24 ++++ docs/content/en/docs/configuration/keys.md | 2 + .../en/docs/examples/external-haproxy.md | 135 ++++++++++++++++++ .../external-haproxy/daemonset-patch.yaml | 56 ++++++++ pkg/common/ingress/controller/controller.go | 3 +- pkg/common/ingress/controller/launch.go | 5 + pkg/controller/controller.go | 5 +- pkg/converters/ingress/annotations/backend.go | 2 +- .../ingress/annotations/backend_test.go | 4 +- pkg/converters/ingress/annotations/updater.go | 4 + pkg/converters/ingress/types/options.go | 1 + pkg/haproxy/instance.go | 44 ++++-- pkg/haproxy/instance_test.go | 8 +- pkg/haproxy/types/global.go | 5 + pkg/haproxy/types/types.go | 4 +- rootfs/start.sh | 9 +- 16 files changed, 290 insertions(+), 21 deletions(-) create mode 100644 docs/content/en/docs/examples/external-haproxy.md create mode 100644 docs/content/en/docs/examples/external-haproxy/daemonset-patch.yaml diff --git a/docs/content/en/docs/configuration/command-line.md b/docs/content/en/docs/configuration/command-line.md index 98118300e..3b2049832 100644 --- a/docs/content/en/docs/configuration/command-line.md +++ b/docs/content/en/docs/configuration/command-line.md @@ -29,6 +29,7 @@ The following command-line options are supported: | [`--ignore-ingress-without-class`](#ignore-ingress-without-class)| [true\|false] | `false` | v0.10 | | [`--ingress-class`](#ingress-class) | name | `haproxy` | | | [`--kubeconfig`](#kubeconfig) | /path/to/kubeconfig | in cluster config | | +| [`--master-socket`](#master-socket) | socket path | use embedded haproxy | v0.12 | | [`--max-old-config-files`](#max-old-config-files) | num of files | `0` | | | [`--profiling`](#stats) | [true\|false] | `true` | | | [`--publish-service`](#publish-service) | namespace/servicename | | | @@ -155,6 +156,29 @@ is deployed outside of the Kubernetes cluster. --- +## --master-socket + +Since v0.12 + +Configures HAProxy Ingress to use an external haproxy deployment in master-worker mode. This option +receives the unix socket of the master CLI. The default value is an empty string, which will +instruct the controller to start and manage the embedded haproxy instead of an external instance. + +The following conditions should be satisfied in order to an external haproxy work properly: + +1. The following paths should be shared between HAProxy Ingress and the external haproxy: `/etc/haproxy`, `/var/lib/haproxy`, `/var/run/haproxy`. HAProxy Ingress must have write access to all of them, external haproxy should have write access to `/var/run/haproxy`. This can be made using a sidecar container and k8s' emptyDir, or a remote file system provided that it updates synchronously and supports unix sockets +1. Start the external haproxy with: + * `-S /var/run/haproxy/master.sock,mode,600`. `mode 600` isn't mandatory but recommended; + * `-f /etc/haproxy` +1. HAProxy Ingress image has a `--init` command-line option which creates an initial valid configuration file, this allows the external haproxy to bootstraps successfully. This option can be used as an init container. + +See also: + +* [example]({{% relref "../examples/external-haproxy" %}}) page. +* [External]({{% relref "keys#external" %}}) and [Master-worker]({{% relref "keys#master-worker" %}}) configuration keys + +--- + ## --max-old-config-files Everytime a configuration change need to update HAProxy, a configuration file is rewritten even if diff --git a/docs/content/en/docs/configuration/keys.md b/docs/content/en/docs/configuration/keys.md index fdadf5bdc..6eb30a5d1 100644 --- a/docs/content/en/docs/configuration/keys.md +++ b/docs/content/en/docs/configuration/keys.md @@ -956,6 +956,7 @@ socket installed and will not work if `external-has-lua` is not enabled. See also: * [OAuth](#oauth) configuration keys. +* [master-socket]({{% relref "command-line#master-socket" %}}) command-line option --- @@ -1212,6 +1213,7 @@ from master in the case of an unexpected failure of a worker, eg a segfault. See also: * https://cbonte.github.io/haproxy-dconv/2.0/configuration.html#3.1-master-worker +* [master-socket]({{% relref "command-line#master-socket" %}}) command-line option --- diff --git a/docs/content/en/docs/examples/external-haproxy.md b/docs/content/en/docs/examples/external-haproxy.md new file mode 100644 index 000000000..f4b445622 --- /dev/null +++ b/docs/content/en/docs/examples/external-haproxy.md @@ -0,0 +1,135 @@ +--- +title: "External haproxy" +linkTitle: "External haproxy" +weight: 20 +description: > + Demonstrate how to configure HAProxy Ingress to use an external haproxy deployment. +--- + +{{% pageinfo %}} +This is a `v0.12` example and need HAProxy Ingress `v0.12-snapshot.1` or above +{{% /pageinfo %}} + +This example demonstrates how to configure HAProxy Ingress to manage an external +haproxy instance deployed as a sidecar container. This approach decouple the +controller and the running haproxy version, allowing the sysadmin to update any +of them independently of the other. + +## Prerequisites + +This document has the following prerequisite: + +* A Kubernetes cluster with a running HAProxy Ingress controller v0.12 or above. +Follow The five minutes deployment in the [getting started]({{% relref "/docs/getting-started" %}}) guide. +* A running and exposed application in the Kubernetes cluster, this getting started +[deployment]({{% relref "/docs/getting-started#try-it-out" %}}) does the job. + +## Update the deployment + +The following instruction patches the current HAProxy Ingress daemonset (this will also revert the command-line arguments to the default value): + +``` +$ kubectl --namespace ingress-controller patch daemonset haproxy-ingress \ +-p "$(curl -sSL https://haproxy-ingress.github.io/v0.12/docs/examples/external-haproxy/daemonset-patch.yaml)" +``` + +Check if the controller restarts without any problem: + +``` +$ kubectl --namespace ingress-controller get pod -w +``` + +## What was changed? + +The sections below have details of what changed in the deployment. + +### Sidecar + +This example configures 2 (two) new containers in the controllers' pod: + +* `haproxy` is the external haproxy deployment with two mandatory arguments: `-S` with the master CLI unix socket, and `-f` with the configuration files path +* `init`, a Kubernetes' [initContainer](https://kubernetes.io/docs/concepts/workloads/pods/init-containers/) used to create an initial and valid haproxy configuration. + +The `haproxy` container references the official Alpine based image `haproxy:alpine`, +but can be any other customized image. The only requisite is to be 2.0 or newer due +to some new keywords used by HAProxy Ingress. + +The `init` container just copy a minimum and valid `haproxy.cfg`. This file is used +to properly starts haproxy and configures its master CLI that HAProxy Ingress uses +to manage the instance. + +A new command-line `--master-socket` was also added to the HAProxy Ingress container. +This option enables an external haproxy instance, pointing to the unix socket path +of its master CLI. + +### Shared filesystem + +HAProxy Ingress sends configuration files to the haproxy instance using a shared +filesystem. A Kubernetes' [`emptyDir`](https://kubernetes.io/docs/concepts/storage/volumes/#emptydir) +works well. + +The following directories must be shared: + +* `/etc/haproxy`: configuration and map files - `init` and `haproxy-ingress` need write access, `haproxy` need read access. +* `/var/lib/haproxy`: mostly ssl related files - `haproxy-ingress` need write access, `haproxy` need read access. +* `/var/run/haproxy`: unix sockets - `haproxy-ingress` and `haproxy` need write access. + +### Liveness probe + +Default HAProxy Ingress deployment has a liveness probe to an haproxy's health +check URI. The patch of this example moves the liveness probe from the HAProxy +Ingress container to the haproxy one. + +## Test + +Open two distinct terminals to follow `haproxy-ingress` and `haproxy` logs: + +``` +$ kubectl --namespace ingress-controller get pod +NAME READY STATUS RESTARTS AGE +haproxy-ingress-6bsvz 3/3 Running 0 17m + +$ kubectl --namespace ingress-controller logs -f haproxy-ingress-6bsvz -c haproxy-ingress +``` + +and + +``` +$ kubectl --namespace ingress-controller logs -f haproxy-ingress-6bsvz -c haproxy +``` + +Update syslog configuration using another terminal, this will make haproxy use stdout: + +``` +$ kubectl --namespace ingress-controller patch configmap haproxy-ingress -p '{"data":{"syslog-endpoint":"stdout","syslog-format":"raw"}}' +``` + +Do some `curl` to an exposed application deployed in the cluster: + +``` +$ kubectl get ing +NAME HOSTS ADDRESS PORTS AGE +nginx nginx.192.168.1.11.nip.io 80, 443 21m + +$ curl nginx.192.168.1.11.nip.io +``` + +During the ConfigMap update and the endpoint calls, HAProxy Ingress and the external +haproxy should be logging its own events: + +`haproxy-ingress` container: + +``` +I0921 16:06:12.699201 6 controller.go:314] starting haproxy update id=8 +I0921 16:06:12.710723 6 instance.go:322] updating 1 host(s): [*.sub.t002.app.domain] +I0921 16:06:12.710752 6 instance.go:339] updating 1 backend(s): [0_echoserver_8080] +I0921 16:06:12.726794 6 instance.go:387] updated main cfg and 1 backend file(s): [002] +I0921 16:06:12.788496 6 instance.go:301] haproxy successfully reloaded (external) +I0921 16:06:12.790696 6 controller.go:346] finish haproxy update id=8: parse_ingress=1.323253ms write_maps=10.101055ms write_config=16.320063ms reload_haproxy=63.587362ms total=91.331733ms +``` + +`haproxy` container: + +``` +192.168.100.1:58167 [21/Sep/2020:16:06:15.003] _front_https~ t015_echoserver_8080/srv001 0/0/2/0/2 200 485 - - ---- 1/1/0/0/0 0/0 "GET https://t004.app.domain/ HTTP/2.0" +``` diff --git a/docs/content/en/docs/examples/external-haproxy/daemonset-patch.yaml b/docs/content/en/docs/examples/external-haproxy/daemonset-patch.yaml new file mode 100644 index 000000000..956a1c8d6 --- /dev/null +++ b/docs/content/en/docs/examples/external-haproxy/daemonset-patch.yaml @@ -0,0 +1,56 @@ +spec: + template: + spec: + containers: + - name: haproxy-ingress + image: quay.io/jcmoraisjr/haproxy-ingress:v0.12-snapshot.1 + args: + - --configmap=ingress-controller/haproxy-ingress + - --master-socket=/var/run/haproxy/master.sock + livenessProbe: null + volumeMounts: + - mountPath: /etc/haproxy + name: etc + - mountPath: /var/lib/haproxy + name: lib + - mountPath: /var/run/haproxy + name: run + - name: haproxy + image: haproxy:alpine + args: + - -W + - -S + - /var/run/haproxy/master.sock,mode,600 + - -f + - /etc/haproxy + livenessProbe: + failureThreshold: 3 + httpGet: + path: /healthz + port: 10253 + scheme: HTTP + periodSeconds: 10 + successThreshold: 1 + timeoutSeconds: 1 + volumeMounts: + - mountPath: /etc/haproxy + name: etc + - mountPath: /var/lib/haproxy + name: lib + - mountPath: /var/run/haproxy + name: run + initContainers: + - name: init + image: quay.io/jcmoraisjr/haproxy-ingress:v0.12-snapshot.1 + args: + - --init + volumeMounts: + - mountPath: /etc/haproxy + name: etc + volumes: + - emptyDir: {} + name: etc + - emptyDir: {} + name: lib + - emptyDir: {} + name: run diff --git a/pkg/common/ingress/controller/controller.go b/pkg/common/ingress/controller/controller.go index ef9f393da..3dde9b984 100644 --- a/pkg/common/ingress/controller/controller.go +++ b/pkg/common/ingress/controller/controller.go @@ -54,7 +54,8 @@ type GenericController struct { // Configuration contains all the settings required by an Ingress controller type Configuration struct { - Client clientset.Interface + Client clientset.Interface + MasterSocket string RateLimitUpdate float32 ResyncPeriod time.Duration diff --git a/pkg/common/ingress/controller/launch.go b/pkg/common/ingress/controller/launch.go index a8bab4c2a..c79f31b3a 100644 --- a/pkg/common/ingress/controller/launch.go +++ b/pkg/common/ingress/controller/launch.go @@ -46,6 +46,10 @@ func NewIngressController(backend ingress.Controller) *GenericController { ingressClass = flags.String("ingress-class", "", `Name of the ingress class to route through this controller.`) + masterSocket = flags.String("master-socket", "", + `Defines the master CLI unix socket of an external HAProxy running in master-worker mode. + Defaults to use the embedded HAProxy if not declared.`) + configMap = flags.String("configmap", "", `Name of the ConfigMap that contains the custom configuration to use`) @@ -296,6 +300,7 @@ func NewIngressController(backend ingress.Controller) *GenericController { UpdateStatus: *updateStatus, ElectionID: *electionID, Client: kubeClient, + MasterSocket: *masterSocket, AcmeServer: *acmeServer, AcmeCheckPeriod: *acmeCheckPeriod, AcmeElectionID: *acmeElectionID, diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index f235933cc..3185ecc71 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -140,6 +140,7 @@ func (hc *HAProxyController) configController() { Logger: hc.logger, Cache: hc.cache, Tracker: hc.tracker, + MasterSocket: hc.cfg.MasterSocket, AnnotationPrefix: hc.cfg.AnnPrefix, DefaultBackend: hc.cfg.DefaultService, DefaultCrtSecret: hc.cfg.DefaultSSLCertificate, @@ -310,7 +311,7 @@ func (hc *HAProxyController) syncIngress(item interface{}) { // ingress converter // hc.updateCount++ - hc.logger.Info("starting HAProxy update id=%d", hc.updateCount) + hc.logger.Info("starting haproxy update id=%d", hc.updateCount) timer := utils.NewTimer(hc.metrics.ControllerProcTime) ingConverter := ingressconverter.NewIngressConverter( hc.converterOptions, @@ -342,5 +343,5 @@ func (hc *HAProxyController) syncIngress(item interface{}) { // update proxy // hc.instance.Update(timer) - hc.logger.Info("finish HAProxy update id=%d: %s", hc.updateCount, timer.AsString("total")) + hc.logger.Info("finish haproxy update id=%d: %s", hc.updateCount, timer.AsString("total")) } diff --git a/pkg/converters/ingress/annotations/backend.go b/pkg/converters/ingress/annotations/backend.go index d9eec1e43..81c6b5092 100644 --- a/pkg/converters/ingress/annotations/backend.go +++ b/pkg/converters/ingress/annotations/backend.go @@ -568,7 +568,7 @@ func (c *updater) buildBackendOAuth(d *backData) { return } external := c.haproxy.Global().External - if external.IsExternal && !external.HasLua { + if external.IsExternal() && !external.HasLua { c.logger.Warn("oauth2_proxy on %v needs Lua socket, install Lua libraries and enable 'external-has-lua' global config", oauth.Source) return } diff --git a/pkg/converters/ingress/annotations/backend_test.go b/pkg/converters/ingress/annotations/backend_test.go index 06ff61b52..d2e88473e 100644 --- a/pkg/converters/ingress/annotations/backend_test.go +++ b/pkg/converters/ingress/annotations/backend_test.go @@ -1262,7 +1262,9 @@ func TestOAuth(t *testing.T) { for i, test := range testCases { c := setup(t) d := c.createBackendData("default/app", source, test.ann, test.annDefault) - c.haproxy.Global().External.IsExternal = test.external + if test.external { + c.haproxy.Global().External.MasterSocket = "/tmp/master.sock" + } c.haproxy.Global().External.HasLua = test.haslua if test.backend != "" { b := strings.Split(test.backend, ":") diff --git a/pkg/converters/ingress/annotations/updater.go b/pkg/converters/ingress/annotations/updater.go index c654f1660..366b29db9 100644 --- a/pkg/converters/ingress/annotations/updater.go +++ b/pkg/converters/ingress/annotations/updater.go @@ -39,6 +39,7 @@ type Updater interface { func NewUpdater(haproxy haproxy.Config, options *ingtypes.ConverterOptions) Updater { return &updater{ haproxy: haproxy, + options: options, logger: options.Logger, cache: options.Cache, tracker: options.Tracker, @@ -48,6 +49,7 @@ func NewUpdater(haproxy haproxy.Config, options *ingtypes.ConverterOptions) Upda type updater struct { haproxy haproxy.Config + options *ingtypes.ConverterOptions logger types.Logger cache convtypes.Cache tracker convtypes.Tracker @@ -106,12 +108,14 @@ 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.MaxConn = mapper.Get(ingtypes.GlobalMaxConnections).Int() d.global.DrainSupport.Drain = mapper.Get(ingtypes.GlobalDrainSupport).Bool() d.global.DrainSupport.Redispatch = mapper.Get(ingtypes.GlobalDrainSupportRedispatch).Bool() d.global.Cookie.Key = mapper.Get(ingtypes.GlobalCookieKey).Value d.global.External.HasLua = mapper.Get(ingtypes.GlobalExternalHasLua).Bool() + d.global.External.MasterSocket = c.options.MasterSocket d.global.LoadServerState = mapper.Get(ingtypes.GlobalLoadServerState).Bool() d.global.Master.ExitOnFailure = mapper.Get(ingtypes.GlobalMasterExitOnFailure).Bool() d.global.StrictHost = mapper.Get(ingtypes.GlobalStrictHost).Bool() diff --git a/pkg/converters/ingress/types/options.go b/pkg/converters/ingress/types/options.go index b28a656d8..eddf95a8a 100644 --- a/pkg/converters/ingress/types/options.go +++ b/pkg/converters/ingress/types/options.go @@ -26,6 +26,7 @@ type ConverterOptions struct { Logger types.Logger Cache convtypes.Cache Tracker convtypes.Tracker + MasterSocket string DefaultConfig func() map[string]string DefaultBackend string DefaultCrtSecret string diff --git a/pkg/haproxy/instance.go b/pkg/haproxy/instance.go index 46e3e4ebe..c3f4abb45 100644 --- a/pkg/haproxy/instance.go +++ b/pkg/haproxy/instance.go @@ -279,7 +279,7 @@ func (i *instance) haproxyUpdate(timer *utils.Timer) { timer.Tick("validate_cfg") i.metrics.UpdateSuccessful(err == nil) } - i.logger.Info("HAProxy updated without needing to reload. Commands sent: %d", updater.cmdCnt) + i.logger.Info("haproxy updated without needing to reload. Commands sent: %d", updater.cmdCnt) i.metrics.IncUpdateDynamic() } else { i.logger.Info("old and new configurations match") @@ -292,11 +292,16 @@ func (i *instance) haproxyUpdate(timer *utils.Timer) { if err := i.reload(); err != nil { i.logger.Error("error reloading server:\n%v", err) i.metrics.UpdateSuccessful(false) + timer.Tick("reload_haproxy") return } i.up = true i.metrics.UpdateSuccessful(true) - i.logger.Info("HAProxy successfully reloaded") + if i.config.Global().External.IsExternal() { + i.logger.Info("haproxy successfully reloaded (external)") + } else { + i.logger.Info("haproxy successfully reloaded (embedded)") + } timer.Tick("reload_haproxy") } @@ -412,11 +417,15 @@ func (i *instance) check() error { i.logger.Info("(test) check was skipped") return nil } - // TODO Move all magic strings to a single place - out, err := exec.Command("haproxy", "-c", "-f", i.options.HAProxyCfgDir).CombinedOutput() - outstr := string(out) - if err != nil { - return fmt.Errorf(outstr) + if i.config.Global().External.IsExternal() { + // TODO check config on remote haproxy + } else { + // TODO Move all magic strings to a single place + out, err := exec.Command("haproxy", "-c", "-f", i.options.HAProxyCfgDir).CombinedOutput() + outstr := string(out) + if err != nil { + return fmt.Errorf(outstr) + } } return nil } @@ -426,14 +435,33 @@ func (i *instance) reload() error { i.logger.Info("(test) reload was skipped") return nil } + if i.config.Global().External.IsExternal() { + return i.reloadExternal() + } + return i.reloadEmbedded() +} + +func (i *instance) reloadEmbedded() error { // TODO Move all magic strings to a single place out, err := exec.Command("/haproxy-reload.sh", i.options.ReloadStrategy, i.options.HAProxyCfgDir).CombinedOutput() outstr := string(out) if len(outstr) > 0 { i.logger.Warn("output from haproxy:\n%v", outstr) } + return err +} + +func (i *instance) reloadExternal() error { + socket := i.config.Global().External.MasterSocket + if _, err := hautils.HAProxyCommand(socket, nil, "reload"); err != nil { + return fmt.Errorf("error sending reload to master socket: %w", err) + } + out, err := hautils.HAProxyProcs(socket) if err != nil { - return err + return fmt.Errorf("error reading procs from master socket: %w", err) + } + if len(out.Workers) == 0 { + return fmt.Errorf("external haproxy was not successfully reloaded") } return nil } diff --git a/pkg/haproxy/instance_test.go b/pkg/haproxy/instance_test.go index e3a5f37c5..abc096fb9 100644 --- a/pkg/haproxy/instance_test.go +++ b/pkg/haproxy/instance_test.go @@ -855,7 +855,7 @@ func TestInstanceEmptyExternal(t *testing.T) { c := setup(t) defer c.teardown() - c.config.global.External.IsExternal = true + c.config.global.External.MasterSocket = "/tmp/master.sock" c.config.global.Security.Username = "external" c.config.global.Security.Groupname = "external" @@ -887,7 +887,9 @@ backend _error404 <<frontends-default>> <<support>> `) - c.logger.CompareLogging(defaultLogging) + c.logger.CompareLogging(` +INFO (test) reload was skipped +INFO haproxy successfully reloaded (external)`) } func TestInstanceSecurity(t *testing.T) { @@ -3266,7 +3268,7 @@ var endpointS41h = &hatypes.Endpoint{ var defaultLogging = ` INFO (test) reload was skipped -INFO HAProxy successfully reloaded` +INFO haproxy successfully reloaded (embedded)` func _yamlMarshal(in interface{}) string { out, _ := yaml.Marshal(in) diff --git a/pkg/haproxy/types/global.go b/pkg/haproxy/types/global.go index f571706b0..1f15c33a1 100644 --- a/pkg/haproxy/types/global.go +++ b/pkg/haproxy/types/global.go @@ -121,6 +121,11 @@ func (c *AcmeCerts) AddDomains(domains []string) { } } +// IsExternal ... +func (e *ExternalConfig) IsExternal() bool { + return e.MasterSocket != "" +} + func (dns *DNSConfig) String() string { return fmt.Sprintf("%+v", *dns) } diff --git a/pkg/haproxy/types/types.go b/pkg/haproxy/types/types.go index a8619bd4c..8bb2d52b6 100644 --- a/pkg/haproxy/types/types.go +++ b/pkg/haproxy/types/types.go @@ -180,8 +180,8 @@ type DrainConfig struct { // ExternalConfig ... type ExternalConfig struct { - HasLua bool - IsExternal bool + HasLua bool + MasterSocket string } // HealthzConfig ... diff --git a/rootfs/start.sh b/rootfs/start.sh index 10f3aa06b..9bd6ebba0 100755 --- a/rootfs/start.sh +++ b/rootfs/start.sh @@ -20,16 +20,19 @@ if [ $# -gt 0 ] && [ "$(echo $1 | cut -b1-2)" != "--" ]; then # Probably a `docker run -ti`, so exec and exit exec "$@" elif [ "$1" = "--init" ]; then + port="${2:-10253}" cat >/etc/haproxy/haproxy.cfg <<EOF defaults timeout server 1s timeout client 1s timeout connect 1s -listen l - bind unix@/tmp/dummy.sock +frontend healthz + mode http + bind :$port + monitor-uri /healthz EOF else # Copy static files to /etc/haproxy, which cannot have static content - cp -R -p /etc/lua /etc/haproxy/ + cp -R /etc/lua /etc/haproxy/ exec /haproxy-ingress-controller "$@" fi