Re: [RFC] Add weights to kubernetes-ingress
Hi David, On 18/03/2020 18:21, David Spitzer-Dulagan wrote: > > Hi, > > If this isn't the right place please let me know - after reading the > "contributing" page that was linked in the readme of > kubernetes-ingress I thought that this mailing list might be my best > bet to ask for your input. > Thanks for you interest in the project. This ML is the right place to contribute to the HAProxy software, but for the ingress controller better do this by creating an issue in the github project. It isn't your fault anyway, we have updated the contribute/discussion sections of the README in order to put clear information. > I wanted to know if the following feature would be something you'd be > interested in seeing in the official kubernetes-ingress controller: > The ability to add an annotation to a pod that would translate into a > weight in the backend wherever that pod is used. > We'd be interested to know more about the use case. In a standard use case, pods are ephemeral and they are not expected to carry specific information. Thus they can be easily replaceable, scalable, etc So typical questions: - What kind of pods are these? static pods or created via Service ? - How they are configured (annotation part in this case)? So don't hesitate to give us more context about this in the github issue. > Our company currently has the need for setting weights for specific > pods and there is no way that I could find to achieve that with the > current kubernetes-ingress controller. > > The implementation would be as follows: > > * Cache all Pod objects and subscribe to changes on all namespaces > similar to the other observed k8s objects in the controller > * Extend EndpointIP in types.go to include the targetRef property of > the k8s object > * handleEndpointIP in controller.go would be extended to look up the > pod from the cache via the targetRef property and set the weight > if found > * Changes to the weight in the Pod would trigger a reload as well > > I've started working on a patch for kubernetes-ingress to implement > that. I'd love your thoughts on this. > > David > > The general idea makes sense, here are some notes: - Caching pods objects, tracking them and syncing the corresponding configuration is an important amount of work which for now seems to be only useful for an edge case. That's why we would probably prefer having CLI arg to activate this. - No need for reload since we can set server weight via the Runtime API (for example we do this already to change a server address via c.NativeAPI.Runtime.SetServerAddr) My coworkers will probable have other thoughts to add once the github issue is created. -- Moemen MHEDHBI
Re: [PATCH] CLEANUP: h2: Help static analyzers understand the list's end marker
Willy, Am 19.03.20 um 15:55 schrieb Willy Tarreau: > Actually I'm pretty sure that I did it this way precisely for performance > reasons: avoid repeatedly checking a pointer for half of the headers which > are pseudo headers (method, scheme, authority, path just for the request). > > It's perfectly possible that the difference is negligible though, but if > it's not, I'm sorry but I'll favor performance over static analysers' > own pleasure. So this one will definitely deserve a test. Yes, the performance <-> static analyzer trade-off not preferring the static analyzers is acceptable to me. Best regards Tim Düsterhus
[PATCH 1/2] BUG/MINOR: ssl: Do not free garbage pointers on memory allocation failure
In `ckch_inst_sni_ctx_to_sni_filters` use `calloc()` to allocate the filter
array. When the function fails to allocate memory for a single entry the
whole array will be `free()`d using free_sni_filters(). With the previous
`malloc()` the pointers for entries after the failing allocation could
possibly be a garbage value.
This bug was introduced in commit 38df1c8006a2adf97f4ad5a183f80cfdcba3da8a,
which is 2.2+. No backport needed.
---
src/ssl_sock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 73375bcf9..3d32ced7f 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -3939,7 +3939,7 @@ static int ckch_inst_sni_ctx_to_sni_filters(const struct
ckch_inst *ckchi, char
if (!tmp_fcount)
goto end;
- tmp_filter = malloc(sizeof(*tmp_filter) * tmp_fcount);
+ tmp_filter = calloc(tmp_fcount, sizeof(*tmp_filter));
if (!tmp_filter) {
errcode |= ERR_FATAL|ERR_ALERT;
goto error;
--
2.25.2
[PATCH 2/2] BUG/MINOR: ssl: Correctly add the 1 for the sentinel to the number of elements
William, I hope I correctly understood the purpose of that `+ 1` there. The issue was found using a static analyzer that complained that `fcount` could be zero, leading to a 0 byte allocation. If this fix is incorrect then the function must be adjusted to check for `fcount == 0` and do something sane. Best regards Tim Düsterhus Apply with `git am --scissors` to automatically cut the commit message. -- >8 -- In `crtlist_dup_filters()` add the `1` to the number of elements instead of the size of a single element. This bug was introduced in commit 2954c478ebab019b814b97cbaec4653af7f03f34, which is 2.2+. No backport needed. --- src/ssl_sock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 3d32ced7f..82b5cba4d 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -4656,7 +4656,7 @@ static char **crtlist_dup_filters(char **args, int fcount) char **dst; int i; - dst = calloc(fcount, sizeof(*dst) + 1); + dst = calloc(fcount + 1, sizeof(*dst)); if (!dst) return NULL; -- 2.25.2
Re: [PATCH] CLEANUP: h2: Help static analyzers understand the list's end marker
Hi Tim,
On Thu, Mar 19, 2020 at 03:15:24PM +0100, Tim Duesterhus wrote:
> Willy,
>
> I know you dislike adjusting code to please static analyzers, but I'd argue
> that using the new IST_NULL + isttest() combination is easier to understand
> for humans as well. A simple .ptr == NULL check might also be slightly faster
> compared to isteq() with an empty string?
>
> I have verified that reg-tests pass, but as this is deep within the internals
> please check this carefully.
>
> Best regards
> Tim Düsterhus
>
> Apply with `git am --scissors` to automatically cut the commit message.
>
> -- >8 --
> Clang Static Analyzer (scan-build) was having a hard time to understand that
> `hpack_encode_header` would never be called with a garbage value for `v`.
>
> It failed to detect that `if (isteq(n, ist(""))) break;` would exist the
> loop in all cases. By setting `n` to `IST_NULL` and checking with
> `isttest()` it no longer complains.
Actually I'm pretty sure that I did it this way precisely for performance
reasons: avoid repeatedly checking a pointer for half of the headers which
are pseudo headers (method, scheme, authority, path just for the request).
It's perfectly possible that the difference is negligible though, but if
it's not, I'm sorry but I'll favor performance over static analysers'
own pleasure. So this one will definitely deserve a test.
Thanks,
Willy
[PATCH] CLEANUP: h2: Help static analyzers understand the list's end marker
Willy,
I know you dislike adjusting code to please static analyzers, but I'd argue
that using the new IST_NULL + isttest() combination is easier to understand
for humans as well. A simple .ptr == NULL check might also be slightly faster
compared to isteq() with an empty string?
I have verified that reg-tests pass, but as this is deep within the internals
please check this carefully.
Best regards
Tim Düsterhus
Apply with `git am --scissors` to automatically cut the commit message.
-- >8 --
Clang Static Analyzer (scan-build) was having a hard time to understand that
`hpack_encode_header` would never be called with a garbage value for `v`.
It failed to detect that `if (isteq(n, ist(""))) break;` would exist the
loop in all cases. By setting `n` to `IST_NULL` and checking with
`isttest()` it no longer complains.
The check must be moved to the beginning of the loop to prevent a NULL
pointer derefence for the pseudo header skip.
---
src/mux_h2.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/mux_h2.c b/src/mux_h2.c
index 013ef86f8..b353c4d7c 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -4617,7 +4617,7 @@ static size_t h2s_frt_make_resp_headers(struct h2s *h2s,
struct htx *htx)
}
/* marker for end of headers */
- list[hdr].n = ist("");
+ list[hdr].n = IST_NULL;
if (h2s->status == 204 || h2s->status == 304) {
/* no contents, claim c-len is present and set to zero */
@@ -4660,6 +4660,9 @@ static size_t h2s_frt_make_resp_headers(struct h2s *h2s,
struct htx *htx)
/* encode all headers, stop at empty name */
for (hdr = 0; hdr < sizeof(list)/sizeof(list[0]); hdr++) {
+ if (!isttest(list[hdr].n))
+ break; // end
+
/* these ones do not exist in H2 and must be dropped. */
if (isteq(list[hdr].n, ist("connection")) ||
isteq(list[hdr].n, ist("proxy-connection")) ||
@@ -4672,9 +4675,6 @@ static size_t h2s_frt_make_resp_headers(struct h2s *h2s,
struct htx *htx)
if (*(list[hdr].n.ptr) == ':')
continue;
- if (isteq(list[hdr].n, ist("")))
- break; // end
-
if (!hpack_encode_header(, list[hdr].n, list[hdr].v)) {
/* output full */
if (b_space_wraps(mbuf))
@@ -4870,7 +4870,7 @@ static size_t h2s_bck_make_req_headers(struct h2s *h2s,
struct htx *htx)
}
/* marker for end of headers */
- list[hdr].n = ist("");
+ list[hdr].n = IST_NULL;
mbuf = br_tail(h2c->mbuf);
retry:
@@ -5007,6 +5007,9 @@ static size_t h2s_bck_make_req_headers(struct h2s *h2s,
struct htx *htx)
struct ist n = list[hdr].n;
struct ist v = list[hdr].v;
+ if (!isttest(n))
+ break; // end
+
/* these ones do not exist in H2 and must be dropped. */
if (isteq(n, ist("connection")) ||
(auth.len && isteq(n, ist("host"))) ||
@@ -5030,9 +5033,6 @@ static size_t h2s_bck_make_req_headers(struct h2s *h2s,
struct htx *htx)
if (*(n.ptr) == ':')
continue;
- if (isteq(n, ist("")))
- break; // end
-
if (!hpack_encode_header(, n, v)) {
/* output full */
if (b_space_wraps(mbuf))
--
2.25.2
Re: More Traffic for haproxy.com
Hi haproxy.com Online Promotion has never been that critical as it’s now until you move ahead in a right direction. Every search engine now emphasis on user-experience, however going over your website what I can see optimization methods are quite ignorant. 1. Every search engines count *Error* free websites. 2. Strategic optimization for *relevant links *been ignored. 3. Proper *Keyword selection* and ranking. 4. Your *Social Media* presence seems lost. As a company we talk and deliver with guarantee, again digital marketing strategy is an open ended approach where you need to do proper A/B testing to get your ROI. I guess, now you are wondering how to and where to start. It’s easy! You can reply me back for a *free website analysis report* or contact me on below mentioned numbers to discuss further. *Best Regards,* *Robert Wesley* *Digital Marketing Consultant* -- [image: beacon]
Re: Segfault on 2.1.3
Le 17/03/2020 à 16:41, Sean Reifschneider a écrit :
The only place tcp-request appears in my config is in relation to rate-limiting,
which we have set up to track but not enforce. Here are the associated rules:
frontend main
[...]
acl rate_whitelist src 10.0.0.1
acl rate_whitelist src 10.0.1.1
acl rate_whitelist src 10.0.1.2
acl rate_whitelist src 10.0.1.3
acl rate_whitelist src 10.0.1.4
stick-table type ip size 200k expire 60s store gpc0
tcp-request connection track-sc0 src if ! rate_whitelist
#use_backend throttled if { sc0_get_gpc0 gt 0 }
backend www
[...]
stick-table type ip size 200k expire 1m store http_req_rate(30s)
acl abuse_req_rate sc1_http_req_rate gt 1000
acl mark_as_abuser sc0_inc_gpc0(main) gt 0
tcp-request content track-sc1 src
tcp-request content reject if abuse_req_rate mark_as_abuser
Here's a pastebin of the full config: https://paste.ubuntu.com/p/nM6xq4Vp2z/
Ok, so the failing ACL is rate_whitelist. But there is nothing strange here. And
your configuration is pretty clean. It is probably a side effect of another bug.
Without a core file it will be hard to investigate.
--
Christopher Faulet
[RFC] Add weights to kubernetes-ingress
Hi, If this isn't the right place please let me know - after reading the "contributing" page that was linked in the readme of kubernetes-ingress I thought that this mailing list might be my best bet to ask for your input. I wanted to know if the following feature would be something you'd be interested in seeing in the official kubernetes-ingress controller: The ability to add an annotation to a pod that would translate into a weight in the backend wherever that pod is used. Our company currently has the need for setting weights for specific pods and there is no way that I could find to achieve that with the current kubernetes-ingress controller. The implementation would be as follows: * Cache all Pod objects and subscribe to changes on all namespaces similar to the other observed k8s objects in the controller * Extend EndpointIP in types.go to include the targetRef property of the k8s object * handleEndpointIP in controller.go would be extended to look up the pod from the cache via the targetRef property and set the weight if found * Changes to the weight in the Pod would trigger a reload as well I've started working on a patch for kubernetes-ingress to implement that. I'd love your thoughts on this. David
Re: [PATCH] fix errored ARM64 builds in travis-ci
well, there are several topics on travis-ci forum related to "output on ARM64 got truncated in the mid of ..." Let us disable ARM64 travis-ci builds for few months. Martin, I'll play with hosted github runner in order to find a way how we can limit its builds to allowed only. ср, 18 мар. 2020 г. в 18:57, Martin Grigorov : > > Current master's build passed the problematic point in my TravisCI > project: https://travis-ci.org/github/martin-g/haproxy/jobs/663953359 > Note: I use TravisCI .org while HAProxy's official project is at .com: > https://travis-ci.com/github/haproxy/haproxy > I also think this is a problem on TravisCI's end. > > Martin > > On Wed, Mar 18, 2020 at 3:43 PM Илья Шипицин wrote: > >> I will disable PR builds. >> >> On Wed, Mar 18, 2020, 6:27 PM Willy Tarreau wrote: >> >>> On Wed, Mar 18, 2020 at 06:21:15PM +0500, ??? wrote: >>> > let us calm down a bit :) >>> >>> Agreed, especially since the build on PRs already happens and already >>> adds noise. >>> >>> > yes, I still believe it is because of buffering. I might have missed >>> > something. >>> > unless I will repair it, I'll drop arm64 support on travis (and we will >>> > switch to self hosted github action runner) >>> >>> OK. >>> >>> Willy >>> >>
[PATCH] temporarily disable travis-ci arm64 builds
Hello, due to arm64 instability, let us disable arm64 builds until this is resolved. Cheers, Ilya Shipitcin From b39b8a6e4ebcae5a28f497170d4ae8e6b208023f Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Wed, 18 Mar 2020 23:37:12 +0500 Subject: [PATCH] CI: temporarily disable unstable travis arm64 builds --- .travis.yml | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index a1e71e35d..f500e02d3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,11 +42,13 @@ matrix: if: type == push compiler: clang env: TARGET=linux-glibc OPENSSL_VERSION=1.1.1d CC=clang-9 - - os: linux -arch: arm64 -if: type == push -compiler: clang -env: TARGET=linux-glibc OPENSSL_VERSION=1.1.1d CC=clang-9 +## +## temporarily disabled, until arm64 runners become stable +# - os: linux +#arch: arm64 +#if: type == push +#compiler: clang +#env: TARGET=linux-glibc OPENSSL_VERSION=1.1.1d CC=clang-9 - os: linux arch: s390x if: type == push -- 2.24.1
