Skip to content

Commit

Permalink
fix
Browse files Browse the repository at this point in the history
Signed-off-by: Gidi233 <[email protected]>
  • Loading branch information
Gidi233 committed Sep 11, 2024
1 parent 0a31983 commit a7a9cf9
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 43 deletions.
98 changes: 63 additions & 35 deletions pkg/fleet-manager/application/rollout_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,7 @@ func (a *ApplicationManager) syncRolloutPolicyForCluster(ctx context.Context,
}
result, err := controllerutil.CreateOrUpdate(ctx, fleetClusterClient, ingress, func() error {
ingress.SetAnnotations(annotation)
renderIngress(ingress, rolloutPolicy)
return nil
return renderNginxIngress(ingress, rolloutPolicy)
})
if err != nil {
return ctrl.Result{}, errors.Wrapf(err, "failed to operate ingress")
Expand Down Expand Up @@ -357,11 +356,16 @@ func enableSidecarInjection(ctx context.Context, kubeClient client.Client, names
var newNs client.Object
switch provider {
case fleetapi.Kuma:
newNs = addAnnotations(ns, map[string]string{
newNs, err = addAnnotations(ns, map[string]string{
kumaInject: "enabled",
})
if err != nil {
return err
}
case fleetapi.Istio:
newNs = addLabels(ns, istioInject, "enabled")
if newNs, err = addLabels(ns, map[string]string{istioInject: "enabled"}); err != nil {
return err
}
}
if updateErr := kubeClient.Update(ctx, newNs); updateErr != nil {
return errors.Wrapf(updateErr, "failed to update namespace %s", namespacedName.Namespace)
Expand Down Expand Up @@ -462,21 +466,13 @@ func deleteIngressCreatedByKurator(ctx context.Context, kubeClient client.Client
labels := ingress.GetLabels()
if set := sets.New(strings.Split(labels[ingressLabelKey], ",")...); set.Contains(rollout.ServiceName) {
if set.Len() == 1 {
if deleteErr := kubeClient.Delete(ctx, ingress); deleteErr != nil && !apierrors.IsNotFound(deleteErr) {
if deleteErr := kubeClient.Delete(ctx, ingress); deleteErr != nil {
return errors.Wrapf(deleteErr, "failed to Delete ingress %s in %s", namespaceName.Name, namespaceName.Namespace)
}
} else {
newRules := make([]ingressv1.IngressRule, 0)
for _, rule := range ingress.Spec.Rules {
if rule.Host != rollout.RolloutPolicy.TrafficRouting.Host {
newRules = append(newRules, rule)
}
}
ingress.Spec.Rules = newRules
labels[ingressLabelKey] = strings.Join(set.Delete(rollout.ServiceName).UnsortedList(), ",")
ingress.SetLabels(labels)
if err := kubeClient.Update(ctx, ingress); err != nil {
return errors.Wrapf(err, "failed to Update ingress %s in %s", namespaceName.Name, namespaceName.Namespace)
// There are still other canaries using this ingress
if err := updateIngressRulesAndLabels(ctx, kubeClient, ingress, rollout, namespaceName, set); err != nil {
return err
}
}
}
Expand All @@ -485,17 +481,38 @@ func deleteIngressCreatedByKurator(ctx context.Context, kubeClient client.Client
return nil
}

func updateIngressRulesAndLabels(ctx context.Context, kubeClient client.Client, ingress *ingressv1.Ingress, rollout *applicationapi.RolloutConfig, namespaceName types.NamespacedName, set sets.Set[string]) error {
newRules := make([]ingressv1.IngressRule, 0)
for _, rule := range ingress.Spec.Rules {
if rule.Host != rollout.RolloutPolicy.TrafficRouting.Host {
newRules = append(newRules, rule)
}
}
ingress.Spec.Rules = newRules

labels := ingress.GetLabels()
labels[ingressLabelKey] = strings.Join(set.Delete(rollout.ServiceName).UnsortedList(), ",")
ingress.SetLabels(labels)

if err := kubeClient.Update(ctx, ingress); err != nil {
return errors.Wrapf(err, "failed to Update ingress %s in %s", namespaceName.Name, namespaceName.Namespace)
}
return nil
}

// create/update ingress configuration
func renderIngress(ingress *ingressv1.Ingress, rollout *applicationapi.RolloutConfig) {
func renderNginxIngress(ingress *ingressv1.Ingress, rollout *applicationapi.RolloutConfig) error {
if labels := ingress.GetLabels(); labels == nil || labels[ingressLabelKey] == "" {
ingress.SetLabels(map[string]string{ingressLabelKey: rollout.ServiceName})
} else {
labels[ingressLabelKey] = strings.Join(sets.New(strings.Split(labels[ingressLabelKey], ",")...).Insert(rollout.ServiceName).UnsortedList(), ",")
ingress.SetLabels(labels)
}
addAnnotations(ingress, map[string]string{
if _, err := addAnnotations(ingress, map[string]string{
ingressAnnotationKey: ingressAnnotationValue,
})
}); err != nil {
return err
}
Prefix := ingressv1.PathTypePrefix
rule := ingressv1.IngressRule{
Host: rollout.RolloutPolicy.TrafficRouting.Host,
Expand All @@ -517,6 +534,7 @@ func renderIngress(ingress *ingressv1.Ingress, rollout *applicationapi.RolloutCo
},
}
ingress.Spec.Rules = append(ingress.Spec.Rules, rule)
return nil
}

// create/update canary configuration
Expand Down Expand Up @@ -691,31 +709,41 @@ func generateWebhookUrl(name, namespace string) string {
return url
}

func addLabels(obj client.Object, key, value string) client.Object {
labels := obj.GetLabels()
// prevent nil pointer panic
if labels == nil {
obj.SetLabels(map[string]string{
key: value,
})
return obj
}
labels[key] = value
obj.SetLabels(labels)
return obj
func addLabels(obj client.Object, labels map[string]string) (client.Object, error) {
existingLabels := obj.GetLabels()
if existingLabels == nil {
obj.SetLabels(labels)
} else {
for k, v := range labels {
if value, exists := existingLabels[k]; exists {
if value != v {
return nil, errors.New("label key conflict")
}
} else {
existingLabels[k] = v
}
}
obj.SetLabels(existingLabels)
}
return obj, nil
}

func addAnnotations(obj client.Object, ann map[string]string) client.Object {
func addAnnotations(obj client.Object, ann map[string]string) (client.Object, error) {
annotations := obj.GetAnnotations()
if annotations == nil {
obj.SetAnnotations(ann)
} else {
for k, v := range ann {
annotations[k] = v
if value, exists := annotations[k]; exists {
if value != v {
return nil, errors.New("annotation key conflict")
}
} else {
annotations[k] = v
}
}
obj.SetAnnotations(annotations)
}
return obj
return obj, nil
}

func mergeMap(map1, map2 map[string]*applicationapi.RolloutStatus) map[string]*applicationapi.RolloutStatus {
Expand Down
15 changes: 7 additions & 8 deletions pkg/fleet-manager/application/rollout_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -595,8 +595,7 @@ func Test_renderCanaryAnalysis(t *testing.T) {
func Test_addLables(t *testing.T) {
type args struct {
obj client.Object
key string
value string
label map[string]string
}
tests := []struct {
name string
Expand All @@ -618,8 +617,9 @@ func Test_addLables(t *testing.T) {
},
},
},
key: "istio-injection",
value: "ebabled",
label: map[string]string{
"istio-injection": "enabled",
},
},
want: &corev1.Namespace{
TypeMeta: metav1.TypeMeta{
Expand All @@ -630,7 +630,7 @@ func Test_addLables(t *testing.T) {
Name: "webapp",
Labels: map[string]string{
"xxx": "abc",
"istio-injection": "ebabled",
"istio-injection": "enabled",
},
},
},
Expand All @@ -647,8 +647,7 @@ func Test_addLables(t *testing.T) {
Name: "webapp",
},
},
key: "XXX",
value: "abc",
label: map[string]string{"XXX": "abc"},
},
want: &corev1.Namespace{
TypeMeta: metav1.TypeMeta{
Expand All @@ -666,7 +665,7 @@ func Test_addLables(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := addLabels(tt.args.obj, tt.args.key, tt.args.value); !reflect.DeepEqual(got, tt.want) {
if got, _ := addLabels(tt.args.obj, tt.args.label); !reflect.DeepEqual(got, tt.want) {
t.Errorf("addLablesOrAnnotaions() = %v, want %v", got, tt.want)
}
})
Expand Down

0 comments on commit a7a9cf9

Please sign in to comment.