Re: [PATCH] skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
On Thu, May 28, 2020 at 01:53:10AM +0500, Илья Шипицин wrote:
> Hello,
>
> let us skip new test on CentOS6
>
>
> Cheers,
> Ilya Shipitcin
> From 4585b4f3b3f6dcbef071b36e7a589cd89757818e Mon Sep 17 00:00:00 2001
> From: Ilya Shipitsin
> Date: Thu, 28 May 2020 01:50:57 +0500
> Subject: [PATCH] CI: cirrus-ci: skip
> reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
>
> as reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc requires ALPN,
> which is not
> supported on CentOS 6, let us skip that test
> ---
> .cirrus.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.cirrus.yml b/.cirrus.yml
> index 07e1bb285..4e2a3330f 100644
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -27,5 +27,5 @@ centos_6_task:
> - ./haproxy -vv
> - ldd haproxy
> # remove some reg-tests (CentOS 6 does not support alpn)
> -- rm reg-tests/connection/proxy_protocol_random_fail.vtc
> reg-tests/checks/tcp-check-ssl.vtc
> +- rm
> reg-tests/{connection/proxy_protocol_random_fail,checks/tcp-check-ssl,connection/proxy_protocol_send_unique_id_alpn}.vtc
> - env VTEST_PROGRAM=../vtest/vtest make reg-tests || (for folder in
> /tmp/*regtest*/vtc.*; do cat $folder/INFO $folder/LOG; done && exit 1)
Thanks I pushed it.
--
William Lallemand
Re: [PATCH] skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
On Thu, May 28, 2020 at 02:26:50PM +0200, Willy Tarreau wrote:
> I'm seeing other benefits of adopting a feature-based model. One of
> them is that we could report them based not only on what was enabled,
> but the real usability status which involves haproxy, the system and
> the libraries. For example we could report "+0RTT" to indicate that
> 0RTT is enabled and supposed to work or "-TFO" to indicate that TFO
> was built in but was detected as non-functional (e.g. OS limitation).
> The pollers could be reported there as well.
That would be confusing I think, because if you show +/- in the
dump, when looking at it you will suppose that every options are there.
For example, if we follow this reasoning, if you have an openssl with
OPENSSL_NO_OCSP, you can't display "-SSL_OCSP" because the support was
not built in haproxy since it requires to link with an ocsp function.
> Just as what we currently have to register build options using
> hap_register_build_opts(), we could have another one to register a
> simple word representing a feature with a boolean to indicate the
> working status, and an equivalent of REGISTER_BUILD_OPTS() to make
> them always available (useful inside #ifdefs). This could become
> really nice 1) to take into account library variants at build time,
> and 2) to mention the availability of certain new features that some
> could want to backport later. For example we've had some VTCs which
> couldn't be backported recently because they were using the new
> "http-after-response" rules, though if a user backports them for his
> own use, it makes sense to report that it's available.
>
> However I think we'd rather report such features in a different
> output from haproxy -vv, which was still made for the end user. Here
> we're now talking about a technical output. I'd be fine with a line
> such as "extra-features" which lists the stuff that's not necessarily
> the base for a given version though, in order to easily spot backports
> or optional stuff.
>
Well, it could be haproxy -vvv, It doesn't bother me if it's in -vv
either honestly. Look at vim for example, vim --version gives you every
feature activated or not. But I agree that is should be on a new line,
because they are inherited options, not build one activated on build.
> That even makes me think that a register line could look like this:
>
> hap_register_feature(name, base_version, status);
>
> where:
> - name is the feature name ("SSL_ALPN", "HTTP_AFTER_RESPONSE", ...)
> - base_version is the haproxy base version expected to expose that
> feature (eg "2.2") resulting in all earlier versions to report it
> on the line of "-vv". This will be particularly nice during
> development because all features added before the final release
> will appear there, and this can be particularly helpful in issue
> reports
>
> And with the full dump, the whole list of features (and their base
> version) would be listed, which shows backports and also helps admins
> figure that the feature they're relying on is fresh new and will
> possibly not be usable if they have to roll back to a previous version.
>
> This should still remain a bit limited so that we don't spend our time
> registering features for each and every new option. Possibly that the
> main driver for this should remain VTC first, and we'll see how that
> evolves.
>
> I'm not suggesting we go down that route immediately, that's just food
> for a possibly durable design. Maybe as a first step we should at least
> plan for a dedicated command line option to list them all and implement
> very basic registration (without target version) as mentioned above.
>
I agree, could be a good idea.
--
William Lallemand
Re: [PATCH] skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
On Thu, May 28, 2020 at 01:30:56PM +0200, William Lallemand wrote:
> On Thu, May 28, 2020 at 03:39:50PM +0500, ??? wrote:
> > anyway, I can install for example openssl-1.1.0 without APLN support.
> > version is not very good indicator (and we try to
> > use features in ifdef wherever possible)
>
> Also, some features in SSL could be enabled only by rebuilding HAProxy,
> so it can be kind of confusing if you have the right openssl lib with
> this feature, but without the feature activated in haproxy. It's 3not a
> common case but it could happen. For example few days ago we rebuilt an
> OpenSSL version with SSLv3 support, but without rebuilding haproxy. So
> haproxy -vv wasn't showing the SSLv3 feature. But if you disable
> something else, let's say OCSP for example, you won't be able to see it
> in haproxy -vv.
I'm seeing other benefits of adopting a feature-based model. One of
them is that we could report them based not only on what was enabled,
but the real usability status which involves haproxy, the system and
the libraries. For example we could report "+0RTT" to indicate that
0RTT is enabled and supposed to work or "-TFO" to indicate that TFO
was built in but was detected as non-functional (e.g. OS limitation).
The pollers could be reported there as well.
Just as what we currently have to register build options using
hap_register_build_opts(), we could have another one to register a
simple word representing a feature with a boolean to indicate the
working status, and an equivalent of REGISTER_BUILD_OPTS() to make
them always available (useful inside #ifdefs). This could become
really nice 1) to take into account library variants at build time,
and 2) to mention the availability of certain new features that some
could want to backport later. For example we've had some VTCs which
couldn't be backported recently because they were using the new
"http-after-response" rules, though if a user backports them for his
own use, it makes sense to report that it's available.
However I think we'd rather report such features in a different
output from haproxy -vv, which was still made for the end user. Here
we're now talking about a technical output. I'd be fine with a line
such as "extra-features" which lists the stuff that's not necessarily
the base for a given version though, in order to easily spot backports
or optional stuff.
That even makes me think that a register line could look like this:
hap_register_feature(name, base_version, status);
where:
- name is the feature name ("SSL_ALPN", "HTTP_AFTER_RESPONSE", ...)
- base_version is the haproxy base version expected to expose that
feature (eg "2.2") resulting in all earlier versions to report it
on the line of "-vv". This will be particularly nice during
development because all features added before the final release
will appear there, and this can be particularly helpful in issue
reports
And with the full dump, the whole list of features (and their base
version) would be listed, which shows backports and also helps admins
figure that the feature they're relying on is fresh new and will
possibly not be usable if they have to roll back to a previous version.
This should still remain a bit limited so that we don't spend our time
registering features for each and every new option. Possibly that the
main driver for this should remain VTC first, and we'll see how that
evolves.
I'm not suggesting we go down that route immediately, that's just food
for a possibly durable design. Maybe as a first step we should at least
plan for a dedicated command line option to list them all and implement
very basic registration (without target version) as mentioned above.
Just my two cents,
Willy
Re: [PATCH] skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
On Thu, May 28, 2020 at 03:39:50PM +0500, Илья Шипицин wrote: > anyway, I can install for example openssl-1.1.0 without APLN support. > version is not very good indicator (and we try to > use features in ifdef wherever possible) Also, some features in SSL could be enabled only by rebuilding HAProxy, so it can be kind of confusing if you have the right openssl lib with this feature, but without the feature activated in haproxy. It's 3not a common case but it could happen. For example few days ago we rebuilt an OpenSSL version with SSLv3 support, but without rebuilding haproxy. So haproxy -vv wasn't showing the SSLv3 feature. But if you disable something else, let's say OCSP for example, you won't be able to see it in haproxy -vv. > > as for current failures, as a short term solution, I think we can merge > patch and figure out "feature" approach later. > I agree for now, but once we have something we should remove all these "rm vtc" in the CI. -- William Lallemand
Re: [PATCH] skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
чт, 28 мая 2020 г. в 14:35, William Lallemand : > On Thu, May 28, 2020 at 09:32:25AM +0200, Willy Tarreau wrote: > > On Thu, May 28, 2020 at 12:21:20AM +0200, Tim Düsterhus wrote: > > > Ilya, > > > > > > Am 27.05.20 um 22:53 schrieb ???: > > > > Hello, > > > > > > > > let us skip new test on CentOS6 > > > > > > > > > > There definitely should be a smarter solution than "delete test" to > skip > > > tests that depend on OpenSSL's features. > > > > Ideally we'd need to add a REQUIRE_OPENSSL_VERSION directive I think. > > We'd make a special case of openssl but it simply matches the reality > > which is that we have a very tight relation with it. I wouldn't be > > surprized if in the future we also have to add a kernel version test > > for certain advanced features. > > > > > Or maybe we should just get rid of CentOS 6 tests, it will be end of > > > life on November, 30th anyway. > > > > It depends. Extended support is still valid for 4 more years, however > > I can't imagine users running a newer haproxy version on that old a > > system. Those relying on old systems just do that because they can't > > afford to have moving pieces in mission critical deployments. They'd > > definitely not upgrade haproxy on such a system! > > > > So yes, I think it can make sense to remove RHEL6 from the tests if > > it's too much of a burden to maintain. Or maybe we can adopt Ilya's > > workaround until 2.2 is released, and completely drop it afterwards ? > > > I'm against it, because it allows us to check if HAProxy still builds > with old version of OpenSSL, even if they are not supported anymore, and > even if no new features are available. HAProxy is an toolbox which is > useful sometimes in old environments. People could use an old version > of HAProxy, of course, but honestly it's not that painful to check if > 0.9.8 still build. > > In my opinion it's not Centos 6 the problem, but the fact that we don't > check the features available in OpenSSL during the tests. And we will > have more problems with that in the future if we use features available > only in newer versions of OpenSSL. And we probably already remove > manually some tests because of that. > > We could also have this problem with BoringSSL etc. What I'm seeing in > the sourcecode is that we still have a lot of #ifdef with checks on the > version and the library. > > Instead we could do a cleaner thing, set constants for SSL features > depending on the lib and version, and checks these constants instead of > the version in the code. The constant could then be reported like the > build options in haproxy -vv, and use that in REQUIRE_OPTIONS or in a > new variable like REQUIRE_FEATURES. > > For example we could have in the .vtc: > > REQUIRE_FEATURES=SSL_ALPN > "require features" is something that will come with openssl 3.0, there will be no version anymore. anyway, I can install for example openssl-1.1.0 without APLN support. version is not very good indicator (and we try to use features in ifdef wherever possible) as for current failures, as a short term solution, I think we can merge patch and figure out "feature" approach later. > > -- > William Lallemand >
Re: [PATCH] skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
On Thu, May 28, 2020 at 09:32:25AM +0200, Willy Tarreau wrote: > On Thu, May 28, 2020 at 12:21:20AM +0200, Tim Düsterhus wrote: > > Ilya, > > > > Am 27.05.20 um 22:53 schrieb ???: > > > Hello, > > > > > > let us skip new test on CentOS6 > > > > > > > There definitely should be a smarter solution than "delete test" to skip > > tests that depend on OpenSSL's features. > > Ideally we'd need to add a REQUIRE_OPENSSL_VERSION directive I think. > We'd make a special case of openssl but it simply matches the reality > which is that we have a very tight relation with it. I wouldn't be > surprized if in the future we also have to add a kernel version test > for certain advanced features. > > > Or maybe we should just get rid of CentOS 6 tests, it will be end of > > life on November, 30th anyway. > > It depends. Extended support is still valid for 4 more years, however > I can't imagine users running a newer haproxy version on that old a > system. Those relying on old systems just do that because they can't > afford to have moving pieces in mission critical deployments. They'd > definitely not upgrade haproxy on such a system! > > So yes, I think it can make sense to remove RHEL6 from the tests if > it's too much of a burden to maintain. Or maybe we can adopt Ilya's > workaround until 2.2 is released, and completely drop it afterwards ? > I'm against it, because it allows us to check if HAProxy still builds with old version of OpenSSL, even if they are not supported anymore, and even if no new features are available. HAProxy is an toolbox which is useful sometimes in old environments. People could use an old version of HAProxy, of course, but honestly it's not that painful to check if 0.9.8 still build. In my opinion it's not Centos 6 the problem, but the fact that we don't check the features available in OpenSSL during the tests. And we will have more problems with that in the future if we use features available only in newer versions of OpenSSL. And we probably already remove manually some tests because of that. We could also have this problem with BoringSSL etc. What I'm seeing in the sourcecode is that we still have a lot of #ifdef with checks on the version and the library. Instead we could do a cleaner thing, set constants for SSL features depending on the lib and version, and checks these constants instead of the version in the code. The constant could then be reported like the build options in haproxy -vv, and use that in REQUIRE_OPTIONS or in a new variable like REQUIRE_FEATURES. For example we could have in the .vtc: REQUIRE_FEATURES=SSL_ALPN -- William Lallemand
Re: [PATCH] skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
On Thu, May 28, 2020 at 12:21:20AM +0200, Tim Düsterhus wrote: > Ilya, > > Am 27.05.20 um 22:53 schrieb ???: > > Hello, > > > > let us skip new test on CentOS6 > > > > There definitely should be a smarter solution than "delete test" to skip > tests that depend on OpenSSL's features. Ideally we'd need to add a REQUIRE_OPENSSL_VERSION directive I think. We'd make a special case of openssl but it simply matches the reality which is that we have a very tight relation with it. I wouldn't be surprized if in the future we also have to add a kernel version test for certain advanced features. > Or maybe we should just get rid of CentOS 6 tests, it will be end of > life on November, 30th anyway. It depends. Extended support is still valid for 4 more years, however I can't imagine users running a newer haproxy version on that old a system. Those relying on old systems just do that because they can't afford to have moving pieces in mission critical deployments. They'd definitely not upgrade haproxy on such a system! So yes, I think it can make sense to remove RHEL6 from the tests if it's too much of a burden to maintain. Or maybe we can adopt Ilya's workaround until 2.2 is released, and completely drop it afterwards ? Willy
Re: [PATCH] skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
There were bug reports if centos 6 is broken. Which means people actively use it On Thu, May 28, 2020, 3:21 AM Tim Düsterhus wrote: > Ilya, > > Am 27.05.20 um 22:53 schrieb Илья Шипицин: > > Hello, > > > > let us skip new test on CentOS6 > > > > There definitely should be a smarter solution than "delete test" to skip > tests that depend on OpenSSL's features. > > Or maybe we should just get rid of CentOS 6 tests, it will be end of > life on November, 30th anyway. > > Best regards > Tim Düsterhus >
Re: [PATCH] skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
Ilya, Am 27.05.20 um 22:53 schrieb Илья Шипицин: > Hello, > > let us skip new test on CentOS6 > There definitely should be a smarter solution than "delete test" to skip tests that depend on OpenSSL's features. Or maybe we should just get rid of CentOS 6 tests, it will be end of life on November, 30th anyway. Best regards Tim Düsterhus
[PATCH] skip reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
Hello,
let us skip new test on CentOS6
Cheers,
Ilya Shipitcin
From 4585b4f3b3f6dcbef071b36e7a589cd89757818e Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin
Date: Thu, 28 May 2020 01:50:57 +0500
Subject: [PATCH] CI: cirrus-ci: skip
reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc on CentOS 6
as reg-tests/connection/proxy_protocol_send_unique_id_alpn.vtc requires ALPN, which is not
supported on CentOS 6, let us skip that test
---
.cirrus.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 07e1bb285..4e2a3330f 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -27,5 +27,5 @@ centos_6_task:
- ./haproxy -vv
- ldd haproxy
# remove some reg-tests (CentOS 6 does not support alpn)
-- rm reg-tests/connection/proxy_protocol_random_fail.vtc reg-tests/checks/tcp-check-ssl.vtc
+- rm reg-tests/{connection/proxy_protocol_random_fail,checks/tcp-check-ssl,connection/proxy_protocol_send_unique_id_alpn}.vtc
- env VTEST_PROGRAM=../vtest/vtest make reg-tests || (for folder in /tmp/*regtest*/vtc.*; do cat $folder/INFO $folder/LOG; done && exit 1)
--
2.26.2
