Fix ingress Annotations replacement loop #1652
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kuritka
requested review from
donovanmuller,
k0da,
ytsarev and
jkremser
as code owners
k0da
reviewed
Jul 10, 2024
Annotation implementation change, terratests covered, test change. When I create GSLB - not k8gb annotations are not copied to ingress. Annotations that are directly related to k8gb are now created in ingress. When I change an annotation in ingress, the annotation is not copied to the GSLB If I change an annotation that is related to k8gb in ingress, the GSLB as ource of truth returns the annotation When I change an annotation in ingress that is not related to k8gb, reconciliation is not triggered and the annotation remains in ingress and is ignored by k8gb When I create a GSLB from an ingress, the annotations are not copied to the GSLB. Signed-off-by: Michal Kuritka <kuritka@gmail.com>
kuritka
force-pushed
the
fix-annotation-4
branch
from
f671d22
to
a01d5aa
Compare
ytsarev
approved these changes
Jul 22, 2024
There was a problem hiding this comment.
This is great, I performed the e2e testing validating annotation propagation behaviour especially focusing on
If I change an annotation that is related to k8gb in ingress, the GSLB as source of truth returns the annotation to it's state
When I change an annotation in ingress that is not related to k8gb, reconciliation is not triggered and the annotation remains in ingress and is ignored by k8gb.
k -n test-gslb get ing test-gslb-failover -o yaml
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
annotations:
k8gb.io/primary-geotag: eu # this is getting reverted according to Gslb spec if changed manually on Ingress
k8gb.io/strategy: failover
annotation: test # this is staying intact
abaguas
added a commit
to abaguas/k8gb
that referenced
this pull request
Jul 30, 2024
The idea was to still trigger reconciliation cycles if resources changes by checking if there was a `generation` update. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata > generation: a sequence number representing a specific generation of the desired state. Set by the system and monotonically increasing, per-resource. May be compared, such as for RAW and WAW consistency. However, the change caused a degradation in `Endpoints` resources. These resources don't have generation because they don't have a status. As a result, a reconciliation was only triggered on creation and deletion, but not on subset update. This degradated also the behavior of the k8gb controller. The controller became too slow monitoring the health of applications since `Endpoints` updates no longer triggered a reconcile of the GSLB resource. This lead to slow failovers and failing e2e tests. The solution proposed in this PR is to always trigger the `endpointMapHandler` function when an `Endpoint` resource changes. This is the same behavior as before k8gb-io#1652. Signed-off-by: Andre Baptista Aguas <andre.aguas@protonmail.com>
abaguas
added a commit
to abaguas/k8gb
that referenced
this pull request
Jul 30, 2024
The idea was to still trigger reconciliation cycles if resources changes by checking if there was a `generation` update. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata > generation: a sequence number representing a specific generation of the desired state. Set by the system and monotonically increasing, per-resource. May be compared, such as for RAW and WAW consistency. However, the change caused a degradation in `Endpoints` resources. These resources don't have generation because they don't have a status. As a result, a reconciliation was only triggered on creation and deletion, but not on subset update. This degradated also the behavior of the k8gb controller. The controller became too slow monitoring the health of applications since `Endpoints` updates no longer triggered a reconcile of the GSLB resource. This lead to slow failovers and failing e2e tests. The solution proposed in this PR is to always trigger the `endpointMapHandler` function when an `Endpoint` resource changes. This is the same behavior as before k8gb-io#1652. Signed-off-by: Andre Baptista Aguas <andre.aguas@protonmail.com>
abaguas
added a commit
to abaguas/k8gb
that referenced
this pull request
Jul 30, 2024
The idea was to still trigger reconciliation cycles if resources changes by checking if there was a `generation` update. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata > generation: a sequence number representing a specific generation of the desired state. Set by the system and monotonically increasing, per-resource. May be compared, such as for RAW and WAW consistency. However, the change caused a degradation in `Endpoints` resources. These resources don't have generation because they don't have a status. As a result, a reconciliation was only triggered on creation and deletion, but not on subset update. This degradated also the behavior of the k8gb controller. The controller became too slow monitoring the health of applications since `Endpoints` updates no longer triggered a reconcile of the GSLB resource. This lead to slow failovers and failing e2e tests. The solution proposed in this PR is to always trigger the `endpointMapHandler` function when an `Endpoint` resource changes. This is the same behavior as before k8gb-io#1652. Signed-off-by: Andre Baptista Aguas <andre.aguas@protonmail.com>
abaguas
added a commit
to abaguas/k8gb
that referenced
this pull request
Jul 30, 2024
The idea was to still trigger reconciliation cycles if resources changes by checking if there was a `generation` update. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata > generation: a sequence number representing a specific generation of the desired state. Set by the system and monotonically increasing, per-resource. May be compared, such as for RAW and WAW consistency. However, the change caused a degradation in `Endpoints` resources. These resources don't have generation because they don't have a status. As a result, a reconciliation was only triggered on creation and deletion, but not on subset update. This degradated also the behavior of the k8gb controller. The controller became too slow monitoring the health of applications since `Endpoints` updates no longer triggered a reconcile of the GSLB resource. This lead to slow failovers and failing e2e tests. The solution proposed in this PR is to always trigger the `endpointMapHandler` function when an `Endpoint` resource changes. This is the same behavior as before k8gb-io#1652. Signed-off-by: Andre Baptista Aguas <andre.aguas@protonmail.com>
abaguas
added a commit
to abaguas/k8gb
that referenced
this pull request
Jul 31, 2024
The idea was to still trigger reconciliation cycles if resources changes by checking if there was a `generation` update. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata > generation: a sequence number representing a specific generation of the desired state. Set by the system and monotonically increasing, per-resource. May be compared, such as for RAW and WAW consistency. However, the change caused a degradation in `Endpoints` resources. These resources don't have generation because they don't have a status. As a result, a reconciliation was only triggered on creation and deletion, but not on subset update. This degradated also the behavior of the k8gb controller. The controller became too slow monitoring the health of applications since `Endpoints` updates no longer triggered a reconcile of the GSLB resource. This lead to slow failovers and failing e2e tests. The solution proposed in this PR is to always trigger the `endpointMapHandler` function when an `Endpoint` resource changes. This is the same behavior as before k8gb-io#1652. Signed-off-by: Andre Baptista Aguas <andre.aguas@protonmail.com>
abaguas
added a commit
to abaguas/k8gb
that referenced
this pull request
Jul 31, 2024
The idea was to still trigger reconciliation cycles if resources changes by checking if there was a `generation` update. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata > generation: a sequence number representing a specific generation of the desired state. Set by the system and monotonically increasing, per-resource. May be compared, such as for RAW and WAW consistency. However, the change caused a degradation in `Endpoints` resources. These resources don't have generation because they don't have a status. As a result, a reconciliation was only triggered on creation and deletion, but not on subset update. This degradated also the behavior of the k8gb controller. The controller became too slow monitoring the health of applications since `Endpoints` updates no longer triggered a reconcile of the GSLB resource. This lead to slow failovers and failing e2e tests. The solution proposed in this PR is to always trigger the `endpointMapHandler` function when an `Endpoint` resource changes. This is the same behavior as before k8gb-io#1652. Signed-off-by: Andre Baptista Aguas <andre.aguas@protonmail.com>
abaguas
added a commit
to abaguas/k8gb
that referenced
this pull request
Jul 31, 2024
The idea was to still trigger reconciliation cycles if resources changes by checking if there was a `generation` update. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata > generation: a sequence number representing a specific generation of the desired state. Set by the system and monotonically increasing, per-resource. May be compared, such as for RAW and WAW consistency. However, the change caused a degradation in `Endpoints` resources. These resources don't have generation because they don't have a status. As a result, a reconciliation was only triggered on creation and deletion, but not on subset update. This degradated also the behavior of the k8gb controller. The controller became too slow monitoring the health of applications since `Endpoints` updates no longer triggered a reconcile of the GSLB resource. This lead to slow failovers and failing e2e tests. The solution proposed in this PR is to always trigger the `endpointMapHandler` function when an `Endpoint` resource changes. This is the same behavior as before k8gb-io#1652. Signed-off-by: Andre Baptista Aguas <andre.aguas@protonmail.com>
abaguas
added a commit
to abaguas/k8gb
that referenced
this pull request
Jul 31, 2024
The idea was to still trigger reconciliation cycles if resources changes by checking if there was a `generation` update. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata > generation: a sequence number representing a specific generation of the desired state. Set by the system and monotonically increasing, per-resource. May be compared, such as for RAW and WAW consistency. However, the change caused a degradation in `Endpoints` resources. These resources don't have generation because they don't have a status. As a result, a reconciliation was only triggered on creation and deletion, but not on subset update. This degradated also the behavior of the k8gb controller. The controller became too slow monitoring the health of applications since `Endpoints` updates no longer triggered a reconcile of the GSLB resource. This lead to slow failovers and failing e2e tests. The solution proposed in this PR is to always trigger the `endpointMapHandler` function when an `Endpoint` resource changes. This is the same behavior as before k8gb-io#1652. Signed-off-by: Andre Baptista Aguas <andre.aguas@protonmail.com>
abaguas
added a commit
to abaguas/k8gb
that referenced
this pull request
Jul 31, 2024
The idea was to still trigger reconciliation cycles if resources changes by checking if there was a `generation` update. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata > generation: a sequence number representing a specific generation of the desired state. Set by the system and monotonically increasing, per-resource. May be compared, such as for RAW and WAW consistency. However, the change caused a degradation in `Endpoints` resources. These resources don't have generation because they don't have a status. As a result, a reconciliation was only triggered on creation and deletion, but not on subset update. This degradated also the behavior of the k8gb controller. The controller became too slow monitoring the health of applications since `Endpoints` updates no longer triggered a reconcile of the GSLB resource. This lead to slow failovers and failing e2e tests. The solution proposed in this PR is to always trigger the `endpointMapHandler` function when an `Endpoint` resource changes. This is the same behavior as before k8gb-io#1652. Signed-off-by: Andre Baptista Aguas <andre.aguas@protonmail.com>
ytsarev
pushed a commit
that referenced
this pull request
Jul 31, 2024
The idea was to still trigger reconciliation cycles if resources changes by checking if there was a `generation` update. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata > generation: a sequence number representing a specific generation of the desired state. Set by the system and monotonically increasing, per-resource. May be compared, such as for RAW and WAW consistency. However, the change caused a degradation in `Endpoints` resources. These resources don't have generation because they don't have a status. As a result, a reconciliation was only triggered on creation and deletion, but not on subset update. This degradated also the behavior of the k8gb controller. The controller became too slow monitoring the health of applications since `Endpoints` updates no longer triggered a reconcile of the GSLB resource. This lead to slow failovers and failing e2e tests. The solution proposed in this PR is to always trigger the `endpointMapHandler` function when an `Endpoint` resource changes. This is the same behavior as before #1652. Signed-off-by: Andre Baptista Aguas <andre.aguas@protonmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Annotation implementation change, terratests covered, test change.
closes #932
closes #1019
related to #1018
When I create GSLB - not k8gb annotations are not copied to ingress. Annotations that are directly related to k8gb are now created in ingress.
When I modify an annotatiosn in ingress, the annotations are not copied to the GSLB
If I change an annotation that is related to k8gb in ingress, the GSLB as source of truth returns the annotation to it's state
When I change an annotation in ingress that is not related to k8gb, reconciliation is not triggered and the annotation remains in ingress and is ignored by k8gb.
When I create a GSLB from an ingress, the annotations are not copied to the GSLB.
I simplify WeightRoundRobin terratests from waiting cycles, the waiting cycles are already implemented in WaitForApp.