-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG] GSLB is not updated when Ingress has change #932
Comments
related to #932 The setup manager is quite complex. We are currently solving a bug related to the fact that changes in ingress also change the GSLB and can cause cycles #932. I am doing the necessary refactoring (code is better structured, but the functionality remains the same), to make it easier to make the necessary changes.. - split SetupWithManager and Reconciliation into two files - encapsulate createGslbFromIngress - simplify ingressHandler - extracting parse strategy func Signed-off-by: kuritka <kuritka@gmail.com>
related to #932 The setup manager is quite complex. We are currently solving a bug related to the fact that changes in ingress also change the GSLB and can cause cycles #932. I am doing the necessary refactoring (code is better structured, but the functionality remains the same), to make it easier to make the necessary changes.. - split SetupWithManager and Reconciliation into two files - encapsulate createGslbFromIngress - simplify ingressHandler - extracting parse strategy func Signed-off-by: kuritka <kuritka@gmail.com>
* Refactoring SetupManager related to #932 The setup manager is quite complex. We are currently solving a bug related to the fact that changes in ingress also change the GSLB and can cause cycles #932. I am doing the necessary refactoring (code is better structured, but the functionality remains the same), to make it easier to make the necessary changes.. - split SetupWithManager and Reconciliation into two files - encapsulate createGslbFromIngress - simplify ingressHandler - extracting parse strategy func Signed-off-by: kuritka <kuritka@gmail.com> * Update controllers/gslb_controller_setup.go Co-authored-by: Jirka Kremser <535866+jkremser@users.noreply.github.com> Signed-off-by: MichalK <kuritka@gmail.com> Signed-off-by: kuritka <kuritka@gmail.com> Signed-off-by: MichalK <kuritka@gmail.com> Co-authored-by: Jirka Kremser <535866+jkremser@users.noreply.github.com>
returning back on this task: BUG reproducing manuallyhaving ONLY simple podinfo app and k8gb deployed (using clean test environment https://github.com/kuritka/k8gb-demo-cncf-2022) than, by applying ingress puting test application under k8gb control (application is loadbalancable): apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
annotations:
x.y.io/ep: '[{"addresses":["1.2.3.4"],"port":80}]'
k8gb.io/strategy: roundRobin
name: gslb
namespace: demo
spec:
ingressClassName: nginx
rules:
- host: demo.cloud.example.com
http:
paths:
- backend:
service:
name: frontend-podinfo
port:
name: http
path: /
pathType: Prefix manually changing ingress command |
closes #932 related to #1018 - Fixed implementation In some cases, when the ingress was changed, reconciliation was triggered, which changed the annotations back to the values calculated with GSLB. I modified the merge function to filter which values should be synced and which should not. In fact, it is only `k8gb.io/strategy` and `k8gb.io/primary-geotag` that are changed on initialization. - utit tests update - terratest terratests are focused on repeated patching of annotations in ingress and examining ingress when patching annotations in GSLB Signed-off-by: kuritka <kuritka@gmail.com>
closes #932 related to #1018 - Fixed implementation In some cases, when the ingress was changed, reconciliation was triggered, which changed the annotations back to the values calculated with GSLB. I modified the merge function to filter which values should be synced and which should not. In fact, it is only `k8gb.io/strategy` and `k8gb.io/primary-geotag` that are changed on initialization. - utit tests update - terratest terratests are focused on repeated patching of annotations in ingress and examining ingress when patching annotations in GSLB Signed-off-by: kuritka <kuritka@gmail.com>
closes #932 related to #1018 - Implementation: In some cases, when the ingress was changed, reconciliation was triggered, which changed the annotations back to the values calculated with GSLB. I modified the merge function to filter which values should be synced and which should not. In fact, it is only `k8gb.io/strategy` and `k8gb.io/primary-geotag` that are changed on initialization. - Utit tests update - Terratests are focused on repeated patching of annotations in ingress and examining ingress when patching annotations in GSLB Signed-off-by: kuritka <kuritka@gmail.com>
related to #1019 This error arises because GLSB and ingress are two sources for k8gb. If you overwrite one of them, k8gb takes the data from the second one and overwrites the first one. Another consequence is that if you delete one source, for example, it can be regenerated from the first one. If there is an inconsistency, one, the source is modified by the other and vice versa. This can cause several error cycles at the beginning until the reconciliation is stopped. It also depends on whether you create an ingress with GSLB or vice versa. The best solution is to have only one source of truth (in our case it would be ingress, later gateway). I started a working demo where I removed this : k8gb-lite. This required refactoring the watcher, throwing out the GSLB and rewriting the terratest. I put terratests for FO, RR, WRR against three clusters and did manual tests at the same time, everything works as expected. |
We found a bug where GSLB does not update itself (specifically annotations section) when the ingress has changed.
The problem with the annotations was that the rancher kept changing the annotations and k8gb kept changing them back. They were stuck in a loop.
The bug is located here:
k8gb/controllers/gslb_controller.go
Lines 234 to 237 in 8820bba
The text was updated successfully, but these errors were encountered: