From e153b8a3df1be332862bfa1666ec8c4a0cfa71fe Mon Sep 17 00:00:00 2001 From: sopida-chotwanwirach Date: Wed, 13 Mar 2024 17:22:05 -0700 Subject: [PATCH 1/2] fix(gloo): Update reconciler to detect change in gloo upstream spec Signed-off-by: sopida-chotwanwirach --- pkg/router/gloo.go | 33 +++++++++++++++++++++++++++++++- test/gloo/test-canary.sh | 41 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/pkg/router/gloo.go b/pkg/router/gloo.go index 23658145..b8a41528 100644 --- a/pkg/router/gloo.go +++ b/pkg/router/gloo.go @@ -275,7 +275,8 @@ func (gr *GlooRouter) createFlaggerUpstream(canary *flaggerv1.Canary, upstreamNa if err != nil { return fmt.Errorf("service %s.%s get query error: %w", svcName, canary.Namespace, err) } - _, err = upstreamClient.Get(context.TODO(), upstreamName, metav1.GetOptions{}) + curUpstream, err := upstreamClient.Get(context.TODO(), upstreamName, metav1.GetOptions{}) + if errors.IsNotFound(err) { glooUpstreamWithConfig, err := gr.getGlooConfigUpstream(canary) if err != nil { @@ -288,10 +289,40 @@ func (gr *GlooRouter) createFlaggerUpstream(canary *flaggerv1.Canary, upstreamNa } } else if err != nil { return fmt.Errorf("upstream %s.%s get query error: %w", upstreamName, canary.Namespace, err) + } else { + return gr.syncUpstreamSpec(curUpstream, canary) } return nil } +func (gr *GlooRouter) syncUpstreamSpec(curUpstream *gloov1.Upstream, canary *flaggerv1.Canary) error { + glooUpstreamWithConfig, err := gr.getGlooConfigUpstream(canary) + if err != nil { + return err + } + + if glooUpstreamWithConfig == nil { + return nil + } + + glooUpstreamLB := glooUpstreamWithConfig.Spec.LoadBalancerConfig + loadBalancerDiff := cmp.Diff(glooUpstreamLB, curUpstream.Spec.LoadBalancerConfig) + + if loadBalancerDiff != "" { + gr.logger.Debugf("detect diff in upstream spec %s.%s %s", curUpstream.Name, canary.Namespace, loadBalancerDiff) + + cloneUpstream := curUpstream.DeepCopy() + cloneUpstream.Spec.LoadBalancerConfig = glooUpstreamLB + + _, err = gr.glooClient.GlooV1().Upstreams(canary.Namespace).Update(context.TODO(), cloneUpstream, metav1.UpdateOptions{}) + if err != nil { + return fmt.Errorf("upstream %s.%s spec update error: %w", curUpstream.Name, canary.Namespace, err) + } + } + + return nil +} + func (gr *GlooRouter) getGlooUpstreamKubeService(canary *flaggerv1.Canary, svc *corev1.Service, upstreamName string, glooUpstreamWithConfig *gloov1.Upstream) *gloov1.Upstream { upstreamSpec := gloov1.UpstreamSpec{} diff --git a/test/gloo/test-canary.sh b/test/gloo/test-canary.sh index f3afd257..047c58b9 100755 --- a/test/gloo/test-canary.sh +++ b/test/gloo/test-canary.sh @@ -117,6 +117,47 @@ done echo '✔ Canary initialization test passed' +echo '>>> Waiting for primary spec to be updated' + +# Update gloo upstream on slow start config which will trigger update on flagger upstreams +cat <>> Triggering canary deployment' kubectl -n test set image deployment/podinfo podinfod=ghcr.io/stefanprodan/podinfo:6.0.1 From e3a529e1c8744c87fc76daf363543e519ad21a8a Mon Sep 17 00:00:00 2001 From: sopida-chotwanwirach Date: Mon, 18 Mar 2024 19:36:45 -0700 Subject: [PATCH 2/2] switch to use patch Signed-off-by: sopida-chotwanwirach --- pkg/router/gloo.go | 17 ++++++++++++----- test/gloo/test-canary.sh | 21 +-------------------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/pkg/router/gloo.go b/pkg/router/gloo.go index b8a41528..4b3c1b3b 100644 --- a/pkg/router/gloo.go +++ b/pkg/router/gloo.go @@ -18,6 +18,7 @@ package router import ( "context" + "encoding/json" "fmt" corev1 "k8s.io/api/core/v1" @@ -30,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + types "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1" @@ -311,12 +313,17 @@ func (gr *GlooRouter) syncUpstreamSpec(curUpstream *gloov1.Upstream, canary *fla if loadBalancerDiff != "" { gr.logger.Debugf("detect diff in upstream spec %s.%s %s", curUpstream.Name, canary.Namespace, loadBalancerDiff) - cloneUpstream := curUpstream.DeepCopy() - cloneUpstream.Spec.LoadBalancerConfig = glooUpstreamLB - - _, err = gr.glooClient.GlooV1().Upstreams(canary.Namespace).Update(context.TODO(), cloneUpstream, metav1.UpdateOptions{}) + patchUpstream := gloov1.Upstream{} + patchUpstream.Spec = gloov1.UpstreamSpec{} + patchUpstream.Spec.LoadBalancerConfig = glooUpstreamLB + patchBytes, err := json.Marshal(patchUpstream) if err != nil { - return fmt.Errorf("upstream %s.%s spec update error: %w", curUpstream.Name, canary.Namespace, err) + return fmt.Errorf("unable to marshal patch upstream from %s.%s with error: %w", glooUpstreamWithConfig.Name, glooUpstreamWithConfig.Namespace, err) + } + + _, err = gr.glooClient.GlooV1().Upstreams(canary.Namespace).Patch(context.TODO(), curUpstream.Name, types.MergePatchType, patchBytes, metav1.PatchOptions{}) + if err != nil { + return fmt.Errorf("upstream %s.%s spec patch error: %w", curUpstream.Name, canary.Namespace, err) } } diff --git a/test/gloo/test-canary.sh b/test/gloo/test-canary.sh index 047c58b9..ac3e8b21 100755 --- a/test/gloo/test-canary.sh +++ b/test/gloo/test-canary.sh @@ -120,26 +120,7 @@ echo '✔ Canary initialization test passed' echo '>>> Waiting for primary spec to be updated' # Update gloo upstream on slow start config which will trigger update on flagger upstreams -cat <