From 564ee5775cae717673feb73bfc54303f19720f82 Mon Sep 17 00:00:00 2001 From: Lin Yang Date: Mon, 12 Jun 2023 15:19:01 +0800 Subject: [PATCH] fix: annotations of service takes precedence over configuration from secret (#278) * fix: service annotations Signed-off-by: Lin Yang * refactor: simplify computing service annotations Signed-off-by: Lin Yang * fix: secret existence Signed-off-by: Lin Yang --------- Signed-off-by: Lin Yang --- controllers/flb/service_controller.go | 142 ++++++++++++++------------ 1 file changed, 74 insertions(+), 68 deletions(-) diff --git a/controllers/flb/service_controller.go b/controllers/flb/service_controller.go index 710b46d0..980a95ed 100644 --- a/controllers/flb/service_controller.go +++ b/controllers/flb/service_controller.go @@ -83,16 +83,8 @@ type ServiceReconciler struct { Scheme *runtime.Scheme Recorder record.EventRecorder ControlPlaneConfigStore *config.Store - //httpClient *resty.Client - //flbUser string - //flbPassword string - //flbDefaultCluster string - //flbDefaultAddressPool string - //flbDefaultAlgo string - //token string - - settings map[string]*setting - cache map[types.NamespacedName]*corev1.Service + settings map[string]*setting + cache map[types.NamespacedName]*corev1.Service } type setting struct { @@ -326,94 +318,100 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{}, nil } - if svc.Annotations == nil { - svc.Annotations = make(map[string]string) - } - mc := r.ControlPlaneConfigStore.MeshConfig.GetConfig() - secret, err := r.K8sAPI.Client.CoreV1(). + + secrets, err := r.K8sAPI.Client.CoreV1(). Secrets(svc.Namespace). - Get(context.TODO(), mc.FLB.SecretName, metav1.GetOptions{}) + List(context.TODO(), metav1.ListOptions{ + FieldSelector: fmt.Sprintf("metadata.name=%s", mc.FLB.SecretName), + LabelSelector: labels.SelectorFromSet( + map[string]string{commons.FlbSecretLabel: "true"}, + ).String(), + }) if err != nil { + defer r.Recorder.Eventf(svc, corev1.EventTypeWarning, "GetSecretFailed", "Failed to get FLB secret %s/%s", svc.Namespace, mc.FLB.SecretName) + return ctrl.Result{}, err + } + + switch len(secrets.Items) { + case 0: if mc.FLB.StrictMode { - defer r.Recorder.Eventf(svc, corev1.EventTypeWarning, "GetSecretFailed", "Failed to get FLB secret: %s", err) + defer r.Recorder.Eventf(svc, corev1.EventTypeWarning, "GetSecretFailed", "In StrictMode, FLB secret %s/%s must exist", svc.Namespace, mc.FLB.SecretName) return ctrl.Result{}, err } else { - if !errors.IsNotFound(err) { - defer r.Recorder.Eventf(svc, corev1.EventTypeWarning, "GetSecretFailed", "Failed to get FLB secret: %s", err) - return ctrl.Result{}, err - } - if r.settings[svc.Namespace] == nil { defer r.Recorder.Eventf(svc, corev1.EventTypeNormal, "UseDefaultSecret", "FLB Secret %s/%s doesn't exist, using default ...", svc.Namespace, mc.FLB.SecretName) r.settings[svc.Namespace] = r.settings[flbDefaultSettingKey] - } else { - setting := r.settings[svc.Namespace] - if isSettingChanged(secret, setting, r.settings[flbDefaultSettingKey], mc) { - if svc.Namespace == config.GetFsmNamespace() { - r.settings[flbDefaultSettingKey] = newSetting(secret) - } - - r.settings[svc.Namespace] = newOverrideSetting(secret, r.settings[flbDefaultSettingKey]) - } } } - } - - if !secretHasRequiredLabel(secret) { - return ctrl.Result{}, fmt.Errorf("secret %s/%s doesn't have required label %s=true", svc.Namespace, mc.FLB.SecretName, commons.FlbSecretLabel) - } - - if r.settings[svc.Namespace] == nil { - if mc.FLB.StrictMode { - r.settings[svc.Namespace] = newSetting(secret) - } else { - r.settings[svc.Namespace] = newOverrideSetting(secret, r.settings[flbDefaultSettingKey]) - } - } else { - setting := r.settings[svc.Namespace] - if isSettingChanged(secret, setting, r.settings[flbDefaultSettingKey], mc) { - if svc.Namespace == config.GetFsmNamespace() { - r.settings[flbDefaultSettingKey] = newSetting(secret) - } + case 1: + secret := &secrets.Items[0] + if r.settings[svc.Namespace] == nil { if mc.FLB.StrictMode { r.settings[svc.Namespace] = newSetting(secret) } else { r.settings[svc.Namespace] = newOverrideSetting(secret, r.settings[flbDefaultSettingKey]) } + } else { + setting := r.settings[svc.Namespace] + if isSettingChanged(secret, setting, r.settings[flbDefaultSettingKey], mc) { + if svc.Namespace == config.GetFsmNamespace() { + r.settings[flbDefaultSettingKey] = newSetting(secret) + } + + if mc.FLB.StrictMode { + r.settings[svc.Namespace] = newSetting(secret) + } else { + r.settings[svc.Namespace] = newOverrideSetting(secret, r.settings[flbDefaultSettingKey]) + } + } } } - klog.V(5).Infof("Annotations of service %s/%s is %s", svc.Namespace, svc.Name, svc.Annotations) + klog.V(5).Infof("Annotations of service %s/%s is %v", svc.Namespace, svc.Name, svc.Annotations) + if newAnnotations := r.computeServiceAnnotations(svc); newAnnotations != nil { + svc.Annotations = newAnnotations + if err := r.Update(ctx, svc); err != nil { + klog.Errorf("Failed update annotations of service %s/%s: %s", svc.Namespace, svc.Name, err) + return ctrl.Result{}, err + } - setting := r.settings[svc.Namespace] - klog.V(5).Infof("Setting for Namespace %q: %v", svc.Namespace, setting) + klog.V(5).Infof("After updating, annotations of service %s/%s is %v", svc.Namespace, svc.Name, svc.Annotations) + } - svcCopy := svc.DeepCopy() - svcCopy.Annotations[commons.FlbClusterAnnotation] = setting.flbDefaultCluster - svcCopy.Annotations[commons.FlbAddressPoolAnnotation] = setting.flbDefaultAddressPool - svcCopy.Annotations[commons.FlbAlgoAnnotation] = getValidAlgo(setting.flbDefaultAlgo) + return r.createOrUpdateFlbEntry(ctx, svc) + } - if !reflect.DeepEqual(svc.GetAnnotations(), svcCopy.GetAnnotations()) { - klog.V(5).Infof("Annotation of Service %s/%s changed", svcCopy.Namespace, svcCopy.Name) + return ctrl.Result{}, nil +} - if err := r.Update(ctx, svcCopy); err != nil { - klog.Errorf("Failed update annotations of service %s/%s: %s", svcCopy.Namespace, svcCopy.Name, err) - return ctrl.Result{}, err - } +func (r *ServiceReconciler) computeServiceAnnotations(svc *corev1.Service) map[string]string { + setting := r.settings[svc.Namespace] + klog.V(5).Infof("Setting for Namespace %q: %v", svc.Namespace, setting) - klog.V(5).Infof("After updating, annotations of service %s/%s is %s", svcCopy.Namespace, svcCopy.Name, svcCopy.Annotations) - klog.V(5).Infof("Service %s/%s is updated successfully, requeue it for further processing", svcCopy.Namespace, svcCopy.Name) + svcCopy := svc.DeepCopy() + if svcCopy.Annotations == nil { + svcCopy.Annotations = make(map[string]string) + } - return ctrl.Result{Requeue: true}, nil + for key, value := range map[string]string{ + commons.FlbClusterAnnotation: setting.flbDefaultCluster, + commons.FlbAddressPoolAnnotation: setting.flbDefaultAddressPool, + commons.FlbAlgoAnnotation: getValidAlgo(setting.flbDefaultAlgo), + } { + v, ok := svcCopy.Annotations[key] + if !ok || v == "" { + svcCopy.Annotations[key] = value } + } - return r.createOrUpdateFlbEntry(ctx, svc) + if !reflect.DeepEqual(svc.GetAnnotations(), svcCopy.GetAnnotations()) { + return svcCopy.Annotations } - return ctrl.Result{}, nil + return nil } func isSettingChanged(secret *corev1.Secret, setting, defaultSetting *setting, mc *config.MeshConfig) bool { @@ -442,7 +440,15 @@ func secretHasRequiredLabel(secret *corev1.Secret) bool { return false } - return value == "true" + switch strings.ToLower(value) { + case "yes", "true", "1", "y", "t": + return true + case "no", "false", "0", "n", "f", "": + return false + default: + klog.Warningf("%s doesn't have a valid value: %q", commons.FlbSecretLabel, value) + return false + } } func (r *ServiceReconciler) deleteEntryFromFLB(ctx context.Context, svc *corev1.Service) (ctrl.Result, error) {