openshift / router

Ingress controller for OpenShift
Apache License 2.0
72 stars 116 forks source link

NE-1761: DO NOT MERGE: validate haproxy28-2.8.10-1.rhaos4.17.el9.x86_64.rpm #611

Closed frobware closed 3 months ago

openshift-ci[bot] commented 4 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from frobware. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/openshift/router/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
frobware commented 4 months ago

Patch to be applied to https://pkgs.devel.redhat.com/cgit/rpms/haproxy/log/?h=rhaos-4.17-rhel-9

From 729a34e0ba35cf05fe0ae32a082fbfe0fed7bbe1 Mon Sep 17 00:00:00 2001
From: Andrew McDermott <amcdermo@redhat.com>
Date: Tue, 25 Jun 2024 13:47:50 +0100
Subject: Bump to 2.8.10

Drop old carry patches.
---
 .gitignore                                    |  1 +
 ...ck-Always-clear-retry-flags-in-read-.patch | 78 -------------------
 ...y-only-tid-0-must-not-sleep-if-got-s.patch | 44 -----------
 haproxy.spec                                  | 13 ++--
 sources                                       |  2 +-
 5 files changed, 8 insertions(+), 130 deletions(-)
 delete mode 100644 0001-BUG-MAJOR-ssl_sock-Always-clear-retry-flags-in-read-.patch
 delete mode 100644 0001-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch

diff --git a/.gitignore b/.gitignore
index 87a9319..58118d1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,3 +17,4 @@
 /haproxy-2.6.9.tar.gz
 /haproxy-2.6.13.tar.gz
 /haproxy-2.8.5.tar.gz
+/haproxy-2.8.10.tar.gz
diff --git a/0001-BUG-MAJOR-ssl_sock-Always-clear-retry-flags-in-read-.patch b/0001-BUG-MAJOR-ssl_sock-Always-clear-retry-flags-in-read-.patch
deleted file mode 100644
index c8b3871..0000000
--- a/0001-BUG-MAJOR-ssl_sock-Always-clear-retry-flags-in-read-.patch
+++ /dev/null
@@ -1,78 +0,0 @@
-From 6402ae5c1c81efbf1679e589f94ab05d4b85ccc2 Mon Sep 17 00:00:00 2001
-From: Olivier Houchard <cognet@ci0.org>
-Date: Sat, 27 Jan 2024 22:58:29 +0100
-Subject: BUG/MAJOR: ssl_sock: Always clear retry flags in read/write functions
-MIME-Version: 1.0
-Content-Type: text/plain; charset=UTF-8
-Content-Transfer-Encoding: 8bit
-
-It has been found that under some rare error circumstances,
-SSL_do_handshake() could return with SSL_ERROR_WANT_READ without
-even trying to call the read function, causing permanent wakeups
-that prevent the process from sleeping.
-
-It was established that this only happens if the retry flags are
-not systematically cleared in both directions upon any I/O attempt,
-but, given the lack of documentation on this topic, it is hard to
-say if this rather strange behavior is expected or not, otherwise
-why wouldn't the library always clear the flags by itself before
-proceeding?
-
-In addition, this only seems to affect OpenSSL 1.1.0 and above,
-and does not affect wolfSSL nor aws-lc.
-
-A bisection on haproxy showed that this issue was first triggered by
-commit a8955d57ed ("MEDIUM: ssl: provide our own BIO."), which means
-that OpenSSL's socket BIO does not have this problem. And this one
-does always clear the flags before proceeding. So let's just proceed
-the same way. It was verified that it properly fixes the problem,
-does not affect other implementations, and doesn't cause any freeze
-nor spurious wakeups either.
-
-Many thanks to Valentín Gutiérrez for providing a network capture
-showing the incident as well as a reproducer. This is GH issue #2403.
-
-This patch needs to be backported to all versions that include the
-commit above, i.e. as far as 2.0.
-
-(cherry picked from commit 1ad19917213fac57ee37e581b0ef137e36c6309d)
-Signed-off-by: Willy Tarreau <w@1wt.eu>
----
- src/ssl_sock.c | 8 ++++----
- 1 file changed, 4 insertions(+), 4 deletions(-)
-
-diff --git a/src/ssl_sock.c b/src/ssl_sock.c
-index 36c3490f7..d6cfb227a 100644
---- a/src/ssl_sock.c
-+++ b/src/ssl_sock.c
-@@ -224,11 +224,11 @@ static int ha_ssl_write(BIO *h, const char *buf, int num)
-   tmpbuf.head = 0;
-   flags = (ctx->xprt_st & SSL_SOCK_SEND_MORE) ? CO_SFL_MSG_MORE : 0;
-   ret = ctx->xprt->snd_buf(ctx->conn, ctx->xprt_ctx, &tmpbuf, num, flags);
-+  BIO_clear_retry_flags(h);
-   if (ret == 0 && !(ctx->conn->flags & (CO_FL_ERROR | CO_FL_SOCK_WR_SH))) {
-       BIO_set_retry_write(h);
-       ret = -1;
--  } else if (ret == 0)
--       BIO_clear_retry_flags(h);
-+  }
-   return ret;
- }
- 
-@@ -256,11 +256,11 @@ static int ha_ssl_read(BIO *h, char *buf, int size)
-   tmpbuf.data = 0;
-   tmpbuf.head = 0;
-   ret = ctx->xprt->rcv_buf(ctx->conn, ctx->xprt_ctx, &tmpbuf, size, 0);
-+  BIO_clear_retry_flags(h);
-   if (ret == 0 && !(ctx->conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH))) {
-       BIO_set_retry_read(h);
-       ret = -1;
--  } else if (ret == 0)
--      BIO_clear_retry_flags(h);
-+  }
- 
-   return ret;
- }
--- 
-2.43.0
-
diff --git a/0001-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch b/0001-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch
deleted file mode 100644
index 69be68e..0000000
--- a/0001-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch
+++ /dev/null
@@ -1,44 +0,0 @@
-From 4d895b9e4bee9cdab6630203f935f98afd35ec06 Mon Sep 17 00:00:00 2001
-From: Valentine Krasnobaeva <vkrasnobaeva@haproxy.com>
-Date: Mon, 6 May 2024 14:24:41 +0200
-Subject: BUG/MINOR: haproxy: only tid 0 must not sleep if got signal
-
-This patch fixes the commit eea152ee68
-("BUG/MINOR: signals/poller: ensure wakeup from signals").
-
-There is some probability that run_poll_loop() becomes inifinite, if
-TH_FL_SLEEPING is withdrawn from all threads in the second signal_queue_len
-check, when a signal has received just after the first one.
-
-In such particular case, the 'wake' variable, which is used to terminate
-thread's poll loop is never reset to 0. So, we never enter to the "stopping"
-part of the run_poll_loop() and threads, except the one with id 0 (tid 0
-handles signals), will continue to call _do_poll() eternally and will never
-sleep, as its TH_FL_SLEEPING flag was unset.
-
-This flag needs to be removed only for the tid 0, as it was done in the first
-signal_queue_len check.
-
-This fixes an issue #2537 "infinite loop when shutting down".
-
-This fix must be backported in every stable version.
----
- src/haproxy.c | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
-
-diff --git a/src/haproxy.c b/src/haproxy.c
-index f7a91b9d3..4c7754939 100644
---- a/src/haproxy.c
-+++ b/src/haproxy.c
-@@ -2983,7 +2983,7 @@ void run_poll_loop()
-           if (thread_has_tasks()) {
-               activity[tid].wake_tasks++;
-               _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_SLEEPING);
--          } else if (signal_queue_len) {
-+          } else if (signal_queue_len && tid == 0) {
-               /* this check is required after setting TH_FL_SLEEPING to avoid
-                * a race with wakeup on signals using wake_threads() */
-               _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_SLEEPING);
--- 
-2.44.0
-
diff --git a/haproxy.spec b/haproxy.spec
index 320877f..82e17cd 100644
--- a/haproxy.spec
+++ b/haproxy.spec
@@ -9,8 +9,8 @@
 %define real_name haproxy28

 Name:           haproxy
-Version:        2.8.5
-Release:        2.rhaos4.16%{?dist}
+Version:        2.8.10
+Release:        1.rhaos4.17%{?dist}
 Summary:        Do not ship, install or use this, use %{real_name} subpackage instead

 License:        GPLv2+
@@ -18,9 +18,6 @@ License:        GPLv2+
 URL:            http://www.haproxy.org/
 Source0:        http://www.haproxy.org/download/2.8/src/haproxy-%{version}.tar.gz

-Patch0:         0001-BUG-MAJOR-ssl_sock-Always-clear-retry-flags-in-read-.patch
-Patch1:         0001-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch
-
 BuildRequires:  gcc
 BuildRequires:  make
 BuildRequires:  openssl-devel
@@ -53,8 +50,6 @@ availability environments. Indeed, it can:

 %prep
 %setup -q
-%patch0 -p1
-%patch1 -p1

 %build
 regparm_opts=
@@ -96,6 +91,10 @@ fi
 %{_sbindir}/%{name}

 %changelog
+* Tue Jun 25 2024 Andrew McDermott <amcdermo@redhat.com> - 2.8.10-1.rhaos4.17
+- Bump to 2.8.10
+- Drop carry patches
+
 * Thu May 16 2024 Andrew McDermott <amcdermo@redhat.com> - 2.8.5-2.rhaos4.16
 - Resolve https://issues.redhat.com/browse/OCPBUGS-32369 (HAProxy consuming high cpu usage)
 - Carry fix for https://github.com/haproxy/haproxy/issues/2537
diff --git a/sources b/sources
index 8d0d802..501fe14 100644
--- a/sources
+++ b/sources
@@ -1 +1 @@
-SHA512 (haproxy-2.8.5.tar.gz) = 7634b9f7d85a246ea467335f71def0085ec63f96f862d2e0016b206c266e5c9cafce1431a7ebe1e8cb0e4f2a80cd5d8f9222c93630c74170bb4175000623380b
+SHA512 (haproxy-2.8.10.tar.gz) = 0a36f1e17702f4ab6eccf1c10643f4685e8c8b272cc71cbf5feb61c8c95ea52b5fc47eeefc97390bb8cb4ac1d52db0c9ad3c0510f8ca017bf10204690554c6d2
-- 
2.44.1
From 5a45f2ffa5d3624a910f15de32eb7feee7c9a253 Mon Sep 17 00:00:00 2001
From: Andrew McDermott <amcdermo@redhat.com>
Date: Thu, 27 Jun 2024 13:22:11 +0100
Subject: haproxy.spec: Optionally enable debug builds

Add a new build type variable to switch between 'debug' and 'release'
builds. The default build type is 'release'. In 'debug' mode, adjust the
compilation flags to create a binary suitable for debugging with gdb.
This allows inspection of variables and function parameters.

Usage:
$ rpmbuild --define '_build_type debug' -ba haproxy.spec

Modify %optflags and %__global_ldflags in 'debug' mode to include
debugging information and disable optimisations.
---
 haproxy.spec | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/haproxy.spec b/haproxy.spec
index 320877f..2e754f2 100644
--- a/haproxy.spec
+++ b/haproxy.spec
@@ -6,6 +6,18 @@

 %global _hardened_build 1

+# The variable _build_type allows switching between 'debug' and
+# 'release' builds. The default is 'release'.
+#
+# In 'debug' mode, the compilation flags are adjusted to create a
+# binary suitable for debugging with gdb. 'debug' mode ensures
+# variables and function parameters can be inspected.
+#
+# Usage:
+#
+# $ rpmbuild --define '_build_type debug' -ba haproxy.spec
+%global build_type %{?_build_type}%{!?_build_type:release}
+
 %define real_name haproxy28

 Name:           haproxy
@@ -62,6 +74,11 @@ regparm_opts=
 regparm_opts="USE_REGPARM=1"
 %endif

+%if "%{build_type}" == "debug"
+%define optflags -g -ggdb3 -fno-inline -fno-omit-frame-pointer -O0
+%define __global_ldflags -g -ggdb3
+%endif
+
 %{__make} %{?_smp_mflags} CPU="generic" TARGET="linux-glibc" USE_OPENSSL=1 USE_PCRE=1 USE_ZLIB=1 USE_CRYPT_H=1 USE_LINUX_TPROXY=1 USE_GETADDRINFO=1 ${regparm_opts} ADDINC="%{optflags}" ADDLIB="%{__global_ldflags}"

 %install
-- 
2.45.2
frobware commented 4 months ago

Scratch RPMs: https://github.com/frobware/haproxy-builds/tree/master/rhaos-4.17-rhel-9

frobware commented 4 months ago

/test perfscale-aws-ingress-perf

openshift-ci-robot commented 4 months ago

@frobware: This pull request references NE-1761 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set.

In response to [this](https://github.com/openshift/router/pull/611): > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Frouter). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
frobware commented 4 months ago

/test e2e-upgrade

frobware commented 4 months ago

/hold

frobware commented 4 months ago

bz-Insights Operator] clusteroperator/insights should not change condition/Available expand_less 2h55m39s { 4 unexpected clusteroperator state transitions during e2e test run. These did not match any known exceptions, so they cause this test-case to fail:

/retest

frobware commented 4 months ago

/test perfscale-aws-ingress-perf

Results: https://grafana.rdu2.scalelab.redhat.com:3000/d/d6105ff8-bc26-4d64-951e-56da771b703d/ingress-perf?orgId=1&var-datasource=QE%20Ingress-perf&var-platform=AWS&var-clusterType=self-managed&var-workerNodesCount=9&var-infraNodesType=c5.4xlarge&var-ocpMajorVersion=4.17&var-termination=All&var-latency_metric=avg_lat_us&var-compare_by=haproxyVersion.keyword&var-all_uuids=All&var-uuid=1f3e4de4-c8ec-4178-81f8-d7c5e624517b&var-uuid=5660ff21-6be3-43b8-b043-0ca4b97a0772&var-uuid=5d876577-089c-4974-8f8c-a4d7c0fd40e3&from=now-30d&to=now

Generating an additional sample:

/test perfscale-aws-ingress-perf

frobware commented 4 months ago

Generating an additional perf & scale sample:

/test perfscale-aws-ingress-perf

frobware commented 4 months ago

/test perfscale-aws-ingress-perf

frobware commented 4 months ago

/test perfscale-aws-ingress-perf

frobware commented 4 months ago

I've updated the RPMs to include a new patch for optionally enabling debug builds.

Rebuilding everything as the RPM has changed.

/test all

frobware commented 4 months ago

/test perfscale-aws-ingress-perf

frobware commented 4 months ago

Perf & Scale results: https://grafana.rdu2.scalelab.redhat.com:3000/d/d6105ff8-bc26-4d64-951e-56da771b703d/ingress-perf?orgId=1&from=now-30d&to=now&var-datasource=beefdfd9-800e-430c-afef-383032aa2d1f&var-platform=AWS&var-clusterType=self-managed&var-workerNodesCount=9&var-infraNodesType=c5.4xlarge&var-ocpMajorVersion=4.17&var-ocpMajorVersion=4.16&var-uuid=0cbb85cc-f746-4687-b4c4-b5f7b2af2d41&var-uuid=1f3e4de4-c8ec-4178-81f8-d7c5e624517b&var-uuid=260c2be1-4b79-4d5d-bc09-1af5c75bfcb2&var-uuid=27abe597-ad3c-4cb7-9bf4-04af1516c188&var-uuid=2f3e487c-129a-47b6-8577-3266abfc480f&var-uuid=51d4e9e5-aaa0-4c1b-9d50-a31b55bb6eeb&var-uuid=562cdbc1-8ac1-4bc8-9e98-bcfc2838c0e7&var-uuid=585062c6-2b41-4a90-af0d-08627bee614b&var-uuid=59d9cbb3-5d10-4dcf-9620-5ecd629b0b9b&var-uuid=691471d9-7e75-47b9-bf99-065b7e0cb6f9&var-uuid=8003c69a-0be2-4c72-beb9-cbbec7b32d12&var-uuid=78b339f2-c0db-4065-98f5-0bc71f0b798a&var-uuid=94e85fdb-3731-48e5-b6da-28a8c70c6895&var-uuid=977d6e56-56fb-47b8-a1dc-366e2baa1d95&var-uuid=a86b95f2-bac1-40f0-92eb-913c8e51f287&var-uuid=bd6ab9a8-bde0-46f1-9d55-48d33fa3e030&var-uuid=cade8c7c-f18f-43bc-b250-953a0ff828dd&var-uuid=cd05a6d8-97c6-43d0-91c0-30d57aac51aa&var-uuid=d10a616e-c0c2-44ae-947d-8b32398d65ae&var-uuid=dc3ddcb7-33f9-41bb-ba31-53c1210f695b&var-uuid=e369d021-fc7f-4c63-9528-6f05bf9c11b5&var-uuid=f327075b-a06d-4e3f-ab37-14298248bfd1&var-uuid=fe756f56-f64a-4836-881d-fc93e6bc8151&var-termination=All&var-latency_metric=avg_lat_us&var-compare_by=haproxyVersion.keyword&var-all_uuids=All

gcs278 commented 3 months ago

@frobware the 2.8.10 bump looks good except one small nit:

Should we mention NE-1761 in the commit message? Looks like this is something we generally do with our bumps, and it's good to have some paper trails I suppose.

The debug build looks reasonable, but a couple questions:

  1. What about -U_FORTIFY_SOURCE for turning off GCC fortification? (I'll admit I don't know much about this, I got this from when you used me for off-site slack storage)
  2. What about HAProxy specific debug flags? I think -DDEBUG=DEBUG_FULL?
  3. For debug mode, should we append to optflags and global_ldflags instead of overwrite? To illustrate, it's odd that: rpmbuild --define 'optflags adsfasdf' -ba SPECS/haproxy.spec fails but rpmbuild --define 'optflags adsfasdf' --define '_build_type debug' -ba SPECS/haproxy.spec works.
frobware commented 3 months ago

Should we mention NE-1761 in the commit message? Looks like this is something we generally do with our bumps, and it's good to have some paper trails I suppose.

Done. Will push a commit once we settle on your last point/question (i.e., number 3).

The debug build looks reasonable, but a couple questions:

  1. What about -U_FORTIFY_SOURCE for turning off GCC fortification? (I'll admit I don't know much about this, I got this from when you used me for off-site slack storage)

The way we set optflags means we don't need to use -U_FORTIFY_SOURCE because the flags in optflags do not enable any features that would require fortification.

A normal build looks like:

+ /usr/bin/make -j12 CPU=generic TARGET=linux-glibc USE_OPENSSL=1
USE_PCRE=1 USE_ZLIB=1 USE_CRYPT_H=1 USE_LINUX_TPROXY=1
USE_GETADDRINFO=1 USE_REGPARM=1 'ADDINC=-O2 -flto=auto
-ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall
-Wno-complain-wrong-lang -Werror=format-security
-Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
-fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1
-m64 -march=x86-64 -mtune=generic -fasynchronous-unwind-tables
-fstack-clash-protection -fcf-protection -fno-omit-frame-pointer
-mno-omit-leaf-frame-pointer' 'ADDLIB=-Wl,-z,relro -Wl,--as-needed
-Wl,-z,pack-relative-relocs -Wl,-z,now
-specs=/usr/lib/rpm/redhat/redhat-hardened-ld-errors
-specs=/usr/lib/rpm/redhat/redhat-hardened-ld
-specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -Wl,--build-id=sha1
-specs=/usr/lib/rpm/redhat/redhat-package-notes ' -v

whereas if I invoke a debug build I see:

+ /usr/bin/make -j12 CPU=generic TARGET=linux-glibc USE_OPENSSL=1
USE_PCRE=1 USE_ZLIB=1 USE_CRYPT_H=1 USE_LINUX_TPROXY=1
USE_GETADDRINFO=1 USE_REGPARM=1 'ADDINC=-g -ggdb3 -fno-inline
-fno-omit-frame-pointer -O0 -v' 'ADDLIB=-g -ggdb3'

We completely obliterate the normal hardening flags.

  1. What about HAProxy specific debug flags? I think -DDEBUG=DEBUG_FULL?

I think that's a judgement call for the day you want a debug build. My immediate concern is perturbing the build too much in terms of performance. There's nothing stopping you from adding these if you feel they add value for the issue being debugged. I'd rather go with a minimal change that only involves altering the C compilation flags, allowing us to get some value from a backtrace in gdb without enabling any other HAProxy debug flags.

  1. For debug mode, should we append to optflags and global_ldflags instead of overwrite? To illustrate, it's odd that: rpmbuild --define 'optflags adsfasdf' -ba SPECS/haproxy.spec fails but rpmbuild --define 'optflags adsfasdf' --define '_build_type debug' -ba SPECS/haproxy.spec works.

The ask here is primarily?

%if "%{build_type}" == "debug"
optflags="%{optflags} -g -ggdb3 -fno-inline -fno-omit-frame-pointer -O0 -v"
__global_ldflags="%{__global_ldflags} -g -ggdb3"
%endif

I'd need to test the latter to ensure that fortify source is still disabled.

frobware commented 3 months ago

Patches to be applied to https://pkgs.devel.redhat.com/cgit/rpms/haproxy/log/?h=rhaos-4.17-rhel-9

From 5a45f2ffa5d3624a910f15de32eb7feee7c9a253 Mon Sep 17 00:00:00 2001
From: Andrew McDermott <amcdermo@redhat.com>
Date: Thu, 27 Jun 2024 13:22:11 +0100
Subject: haproxy.spec: Optionally enable debug builds

Add a new build type variable to switch between 'debug' and 'release'
builds. The default build type is 'release'. In 'debug' mode, adjust the
compilation flags to create a binary suitable for debugging with gdb.
This allows inspection of variables and function parameters.

Usage:
$ rpmbuild --define '_build_type debug' -ba haproxy.spec

Modify %optflags and %__global_ldflags in 'debug' mode to include
debugging information and disable optimisations.
---
 haproxy.spec | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/haproxy.spec b/haproxy.spec
index 320877f..2e754f2 100644
--- a/haproxy.spec
+++ b/haproxy.spec
@@ -6,6 +6,18 @@

 %global _hardened_build 1

+# The variable _build_type allows switching between 'debug' and
+# 'release' builds. The default is 'release'.
+#
+# In 'debug' mode, the compilation flags are adjusted to create a
+# binary suitable for debugging with gdb. 'debug' mode ensures
+# variables and function parameters can be inspected.
+#
+# Usage:
+#
+# $ rpmbuild --define '_build_type debug' -ba haproxy.spec
+%global build_type %{?_build_type}%{!?_build_type:release}
+
 %define real_name haproxy28

 Name:           haproxy
@@ -62,6 +74,11 @@ regparm_opts=
 regparm_opts="USE_REGPARM=1"
 %endif

+%if "%{build_type}" == "debug"
+%define optflags -g -ggdb3 -fno-inline -fno-omit-frame-pointer -O0
+%define __global_ldflags -g -ggdb3
+%endif
+
 %{__make} %{?_smp_mflags} CPU="generic" TARGET="linux-glibc" USE_OPENSSL=1 USE_PCRE=1 USE_ZLIB=1 USE_CRYPT_H=1 USE_LINUX_TPROXY=1 USE_GETADDRINFO=1 ${regparm_opts} ADDINC="%{optflags}" ADDLIB="%{__global_ldflags}"

 %install
-- 
2.44.1
From e6f4e37cb71bf91ac17ec87440776625a063cb97 Mon Sep 17 00:00:00 2001
From: Andrew McDermott <amcdermo@redhat.com>
Date: Tue, 25 Jun 2024 13:47:50 +0100
Subject: NE-1761: Bump to 2.8.10

- Resolves https://issues.redhat.com/browse/NE-1761.
- Drop old carry patches.
---
 .gitignore                                    |  1 +
 ...ck-Always-clear-retry-flags-in-read-.patch | 78 -------------------
 ...y-only-tid-0-must-not-sleep-if-got-s.patch | 44 -----------
 haproxy.spec                                  | 14 ++--
 sources                                       |  2 +-
 5 files changed, 9 insertions(+), 130 deletions(-)
 delete mode 100644 0001-BUG-MAJOR-ssl_sock-Always-clear-retry-flags-in-read-.patch
 delete mode 100644 0001-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch

diff --git a/.gitignore b/.gitignore
index 87a9319..58118d1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,3 +17,4 @@
 /haproxy-2.6.9.tar.gz
 /haproxy-2.6.13.tar.gz
 /haproxy-2.8.5.tar.gz
+/haproxy-2.8.10.tar.gz
diff --git a/0001-BUG-MAJOR-ssl_sock-Always-clear-retry-flags-in-read-.patch b/0001-BUG-MAJOR-ssl_sock-Always-clear-retry-flags-in-read-.patch
deleted file mode 100644
index c8b3871..0000000
--- a/0001-BUG-MAJOR-ssl_sock-Always-clear-retry-flags-in-read-.patch
+++ /dev/null
@@ -1,78 +0,0 @@
-From 6402ae5c1c81efbf1679e589f94ab05d4b85ccc2 Mon Sep 17 00:00:00 2001
-From: Olivier Houchard <cognet@ci0.org>
-Date: Sat, 27 Jan 2024 22:58:29 +0100
-Subject: BUG/MAJOR: ssl_sock: Always clear retry flags in read/write functions
-MIME-Version: 1.0
-Content-Type: text/plain; charset=UTF-8
-Content-Transfer-Encoding: 8bit
-
-It has been found that under some rare error circumstances,
-SSL_do_handshake() could return with SSL_ERROR_WANT_READ without
-even trying to call the read function, causing permanent wakeups
-that prevent the process from sleeping.
-
-It was established that this only happens if the retry flags are
-not systematically cleared in both directions upon any I/O attempt,
-but, given the lack of documentation on this topic, it is hard to
-say if this rather strange behavior is expected or not, otherwise
-why wouldn't the library always clear the flags by itself before
-proceeding?
-
-In addition, this only seems to affect OpenSSL 1.1.0 and above,
-and does not affect wolfSSL nor aws-lc.
-
-A bisection on haproxy showed that this issue was first triggered by
-commit a8955d57ed ("MEDIUM: ssl: provide our own BIO."), which means
-that OpenSSL's socket BIO does not have this problem. And this one
-does always clear the flags before proceeding. So let's just proceed
-the same way. It was verified that it properly fixes the problem,
-does not affect other implementations, and doesn't cause any freeze
-nor spurious wakeups either.
-
-Many thanks to Valentín Gutiérrez for providing a network capture
-showing the incident as well as a reproducer. This is GH issue #2403.
-
-This patch needs to be backported to all versions that include the
-commit above, i.e. as far as 2.0.
-
-(cherry picked from commit 1ad19917213fac57ee37e581b0ef137e36c6309d)
-Signed-off-by: Willy Tarreau <w@1wt.eu>
----
- src/ssl_sock.c | 8 ++++----
- 1 file changed, 4 insertions(+), 4 deletions(-)
-
-diff --git a/src/ssl_sock.c b/src/ssl_sock.c
-index 36c3490f7..d6cfb227a 100644
---- a/src/ssl_sock.c
-+++ b/src/ssl_sock.c
-@@ -224,11 +224,11 @@ static int ha_ssl_write(BIO *h, const char *buf, int num)
-   tmpbuf.head = 0;
-   flags = (ctx->xprt_st & SSL_SOCK_SEND_MORE) ? CO_SFL_MSG_MORE : 0;
-   ret = ctx->xprt->snd_buf(ctx->conn, ctx->xprt_ctx, &tmpbuf, num, flags);
-+  BIO_clear_retry_flags(h);
-   if (ret == 0 && !(ctx->conn->flags & (CO_FL_ERROR | CO_FL_SOCK_WR_SH))) {
-       BIO_set_retry_write(h);
-       ret = -1;
--  } else if (ret == 0)
--       BIO_clear_retry_flags(h);
-+  }
-   return ret;
- }
- 
-@@ -256,11 +256,11 @@ static int ha_ssl_read(BIO *h, char *buf, int size)
-   tmpbuf.data = 0;
-   tmpbuf.head = 0;
-   ret = ctx->xprt->rcv_buf(ctx->conn, ctx->xprt_ctx, &tmpbuf, size, 0);
-+  BIO_clear_retry_flags(h);
-   if (ret == 0 && !(ctx->conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH))) {
-       BIO_set_retry_read(h);
-       ret = -1;
--  } else if (ret == 0)
--      BIO_clear_retry_flags(h);
-+  }
- 
-   return ret;
- }
--- 
-2.43.0
-
diff --git a/0001-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch b/0001-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch
deleted file mode 100644
index 69be68e..0000000
--- a/0001-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch
+++ /dev/null
@@ -1,44 +0,0 @@
-From 4d895b9e4bee9cdab6630203f935f98afd35ec06 Mon Sep 17 00:00:00 2001
-From: Valentine Krasnobaeva <vkrasnobaeva@haproxy.com>
-Date: Mon, 6 May 2024 14:24:41 +0200
-Subject: BUG/MINOR: haproxy: only tid 0 must not sleep if got signal
-
-This patch fixes the commit eea152ee68
-("BUG/MINOR: signals/poller: ensure wakeup from signals").
-
-There is some probability that run_poll_loop() becomes inifinite, if
-TH_FL_SLEEPING is withdrawn from all threads in the second signal_queue_len
-check, when a signal has received just after the first one.
-
-In such particular case, the 'wake' variable, which is used to terminate
-thread's poll loop is never reset to 0. So, we never enter to the "stopping"
-part of the run_poll_loop() and threads, except the one with id 0 (tid 0
-handles signals), will continue to call _do_poll() eternally and will never
-sleep, as its TH_FL_SLEEPING flag was unset.
-
-This flag needs to be removed only for the tid 0, as it was done in the first
-signal_queue_len check.
-
-This fixes an issue #2537 "infinite loop when shutting down".
-
-This fix must be backported in every stable version.
----
- src/haproxy.c | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
-
-diff --git a/src/haproxy.c b/src/haproxy.c
-index f7a91b9d3..4c7754939 100644
---- a/src/haproxy.c
-+++ b/src/haproxy.c
-@@ -2983,7 +2983,7 @@ void run_poll_loop()
-           if (thread_has_tasks()) {
-               activity[tid].wake_tasks++;
-               _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_SLEEPING);
--          } else if (signal_queue_len) {
-+          } else if (signal_queue_len && tid == 0) {
-               /* this check is required after setting TH_FL_SLEEPING to avoid
-                * a race with wakeup on signals using wake_threads() */
-               _HA_ATOMIC_AND(&th_ctx->flags, ~TH_FL_SLEEPING);
--- 
-2.44.0
-
diff --git a/haproxy.spec b/haproxy.spec
index 2e754f2..67126cc 100644
--- a/haproxy.spec
+++ b/haproxy.spec
@@ -21,8 +21,8 @@
 %define real_name haproxy28

 Name:           haproxy
-Version:        2.8.5
-Release:        2.rhaos4.16%{?dist}
+Version:        2.8.10
+Release:        1.rhaos4.17%{?dist}
 Summary:        Do not ship, install or use this, use %{real_name} subpackage instead

 License:        GPLv2+
@@ -30,9 +30,6 @@ License:        GPLv2+
 URL:            http://www.haproxy.org/
 Source0:        http://www.haproxy.org/download/2.8/src/haproxy-%{version}.tar.gz

-Patch0:         0001-BUG-MAJOR-ssl_sock-Always-clear-retry-flags-in-read-.patch
-Patch1:         0001-BUG-MINOR-haproxy-only-tid-0-must-not-sleep-if-got-s.patch
-
 BuildRequires:  gcc
 BuildRequires:  make
 BuildRequires:  openssl-devel
@@ -65,8 +62,6 @@ availability environments. Indeed, it can:

 %prep
 %setup -q
-%patch0 -p1
-%patch1 -p1

 %build
 regparm_opts=
@@ -113,6 +108,11 @@ fi
 %{_sbindir}/%{name}

 %changelog
+* Tue Jun 25 2024 Andrew McDermott <amcdermo@redhat.com> - 2.8.10-1.rhaos4.17
+- Resolves https://issues.redhat.com/browse/NE-1761.
+- Drop old carry patches.
+- haproxy.spec: Optionally enable debug builds
+
 * Thu May 16 2024 Andrew McDermott <amcdermo@redhat.com> - 2.8.5-2.rhaos4.16
 - Resolve https://issues.redhat.com/browse/OCPBUGS-32369 (HAProxy consuming high cpu usage)
 - Carry fix for https://github.com/haproxy/haproxy/issues/2537
diff --git a/sources b/sources
index 8d0d802..501fe14 100644
--- a/sources
+++ b/sources
@@ -1 +1 @@
-SHA512 (haproxy-2.8.5.tar.gz) = 7634b9f7d85a246ea467335f71def0085ec63f96f862d2e0016b206c266e5c9cafce1431a7ebe1e8cb0e4f2a80cd5d8f9222c93630c74170bb4175000623380b
+SHA512 (haproxy-2.8.10.tar.gz) = 0a36f1e17702f4ab6eccf1c10643f4685e8c8b272cc71cbf5feb61c8c95ea52b5fc47eeefc97390bb8cb4ac1d52db0c9ad3c0510f8ca017bf10204690554c6d2
-- 
2.44.1
frobware commented 3 months ago

New RPMs:

/test all

gcs278 commented 3 months ago

Patches LGTM @frobware @alebedev87

frobware commented 3 months ago

/test perfscale-aws-ingress-perf

openshift-ci[bot] commented 3 months ago

@frobware: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
ShudiLi commented 3 months ago

/label qe-approved tested it with 4.17.0-0.ci.test-2024-07-26-044736-ci-ln-dyp1tf2-latest, the haproxy version was updated to haproxy28-2.8.10-1, the 4 types of routes work well.

1.
% oc get clusterversion
NAME      VERSION                                                   AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.17.0-0.ci.test-2024-07-26-044736-ci-ln-dyp1tf2-latest   True        False         32m     Cluster version is 4.17.0-0.ci.test-2024-07-26-044736-ci-ln-dyp1tf2-latest

2.
% oc -n openshift-ingress get pods
NAME                             READY   STATUS    RESTARTS   AGE
router-default-886fc6654-7xtck   1/1     Running   0          50m
router-default-886fc6654-8bwsz   1/1     Running   0          50m

3.
% oc -n openshift-ingress rsh router-default-886fc6654-7xtck
sh-5.1$ rpm -qa haproxy28
haproxy28-2.8.10-1.rhaos4.17.el9.x86_64
sh-5.1$ rpm -qa --changelog haproxy28 | head -n 5
* Tue Jun 25 2024 Andrew McDermott <amcdermo@redhat.com> - 2.8.10-1.rhaos4.17
- Resolves https://issues.redhat.com/browse/NE-1761.
- Drop old carry patches.
- haproxy.spec: Optionally enable debug builds

sh-5.1$

4.
% oc -n test get route
NAME          HOST/PORT                                                                     PATH   SERVICES      PORT          TERMINATION   WILDCARD
edge1         edge1-test.apps.ci-ln-dyp1tf2-76ef8.origin-ci-int-aws.dev.rhcloud.com                unsec-apach   unsec-apach   edge          None
pass1         pass1-test.apps.ci-ln-dyp1tf2-76ef8.origin-ci-int-aws.dev.rhcloud.com                sec-apach2    sec-apach2    passthrough   None
reencrypt1    reencrypt1-test.apps.ci-ln-dyp1tf2-76ef8.origin-ci-int-aws.dev.rhcloud.com           sec-apach2    sec-apach2    reencrypt     None
unsec-apach   unsec-apach-test.apps.ci-ln-dyp1tf2-76ef8.origin-ci-int-aws.dev.rhcloud.com          unsec-apach   unsec-apach                 None

5.
% curl http://unsec-apach-test.apps.ci-ln-dyp1tf2-76ef8.origin-ci-int-aws.dev.rhcloud.com
It is a test!
% curl https://edge1-test.apps.ci-ln-dyp1tf2-76ef8.origin-ci-int-aws.dev.rhcloud.com -k
It is a test!
% curl https://pass1-test.apps.ci-ln-dyp1tf2-76ef8.origin-ci-int-aws.dev.rhcloud.com -k
It is a test!
% curl https://reencrypt1-test.apps.ci-ln-dyp1tf2-76ef8.origin-ci-int-aws.dev.rhcloud.com -k
It is a test!
openshift-ci-robot commented 3 months ago

@frobware: This pull request references NE-1761 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target only the "4.17.0" version, but multiple target versions were set.

In response to [this](https://github.com/openshift/router/pull/611): > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Frouter). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.