From 97d501251e8b8354724b95585ac75f7ef642bca9 Mon Sep 17 00:00:00 2001
From: Michele Baldessari <michele@acksyn.org>
Date: Mon, 11 Sep 2017 21:45:15 +0200
Subject: [PATCH] Give horizon's stanza in haproxy a per-server cookie

Currently our haproxy.cfg stanza for horizon looks like the following:
server overcloud-controller-0.internalapi.localdomain 172.17.0.22:80 check cookie overcloud-controller-0 fall 5 inter 2000 rise 2
server overcloud-controller-1.internalapi.localdomain 172.17.0.25:80 check cookie overcloud-controller-0 fall 5 inter 2000 rise 2
server overcloud-controller-2.internalapi.localdomain 172.17.0.12:80 check cookie overcloud-controller-0 fall 5 inter 2000 rise 2

We need to make sure that the cookie is set the same as the server:
server overcloud-controller-0.internalapi.localdomain 172.17.0.22:80 check cookie overcloud-controller-0.internalapi.localdomain fall 5 inter 2000 rise 2
server overcloud-controller-1.internalapi.localdomain 172.17.0.25:80 check cookie overcloud-controller-1.internalapi.localdomain fall 5 inter 2000 rise 2
server overcloud-controller-2.internalapi.localdomain 172.17.0.12:80 check cookie overcloud-controller-2.internalapi.localdomain fall 5 inter 2000 rise 2

The problem here is that the cookie is being inserted into the response
by haproxy so that we have session persistence. When logging to horizon
we want the session to be persistent and go to the same backend server.
When haproxy sees a match for the cookie, it does just that. The cookie
value will should match the server name. Prior to this fix
each server was matching on the same cookie ($::hostname) which is not
correct.

Tested by connecting to horizon's VIP and shutting off horizon on each
controller node one at the time. Observed that after each stop, the
correct cookie from the remaining servers was sent to the browser.

Closes-Bug: #1738453
Change-Id: Ieb9cf3c6a8373df288a73ff2dacfc9d0b09e675a
---
 manifests/haproxy/horizon_endpoint.pp | 14 ++++++++------
 spec/classes/tripleo_haproxy_spec.rb  | 15 +++++++++++++++
 spec/fixtures/hieradata/default.yaml  |  4 ++++
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/manifests/haproxy/horizon_endpoint.pp b/manifests/haproxy/horizon_endpoint.pp
index c7dfd88e8..e44c4b88f 100644
--- a/manifests/haproxy/horizon_endpoint.pp
+++ b/manifests/haproxy/horizon_endpoint.pp
@@ -144,11 +144,13 @@ class tripleo::haproxy::horizon_endpoint (
     mode             => 'http',
     collect_exported => false,
   }
-  haproxy::balancermember { 'horizon':
-    listening_service => 'horizon',
-    ports             => $backend_port,
-    ipaddresses       => $ip_addresses,
-    server_names      => $server_names,
-    options           => union($member_options, ["cookie ${::hostname}"]),
+  hash(zip($ip_addresses, $server_names)).each | $ip, $server | {
+    haproxy::balancermember { "horizon_${ip}_${server}":
+      listening_service => 'horizon',
+      ports             => $backend_port,
+      ipaddresses       => $ip,
+      server_names      => $server,
+      options           => union($member_options, ["cookie ${server}"]),
+    }
   }
 }
diff --git a/spec/classes/tripleo_haproxy_spec.rb b/spec/classes/tripleo_haproxy_spec.rb
index cd572f152..0dc0955e5 100644
--- a/spec/classes/tripleo_haproxy_spec.rb
+++ b/spec/classes/tripleo_haproxy_spec.rb
@@ -80,6 +80,21 @@ describe 'tripleo::haproxy' do
       end
     end
 
+    describe "horizon" do
+      before :each do
+        params.merge!({
+          :horizon      => true,
+        })
+      end
+
+      it 'should configure haproxy horizon endpoint' do
+        is_expected.to contain_class('tripleo::haproxy::horizon_endpoint')
+        is_expected.to contain_haproxy__balancermember('horizon_127.0.0.1_controller-1').with(
+          :options => ['check', 'inter 2000', 'rise 2', 'fall 5', 'cookie controller-1'],
+        )
+      end
+    end
+
     describe "override maxconn without clustercheck" do
       before :each do
         params.merge!({
diff --git a/spec/fixtures/hieradata/default.yaml b/spec/fixtures/hieradata/default.yaml
index 1c32bf732..c07b67050 100644
--- a/spec/fixtures/hieradata/default.yaml
+++ b/spec/fixtures/hieradata/default.yaml
@@ -48,6 +48,10 @@ memcached_node_ips:
 # octavia related items
 octavia::rabbit_password: 'password'
 horizon::secret_key: 'secrete'
+horizon_node_ips:
+ - '127.0.0.1'
+horizon_node_names:
+ - 'controller-1'
 #Neutron related
 neutron::rabbit_password: 'password'
 # Pacemaker related