From 7f5a9940481ac687851f53a2bd7788ab1f2a87dd Mon Sep 17 00:00:00 2001 From: Matthias Riegler Date: Wed, 9 Oct 2024 11:19:17 +0200 Subject: [PATCH 1/6] feat: add flagd-proxy HA configuration Signed-off-by: Matthias Riegler --- common/flagdproxy/flagdproxy.go | 205 ++++++++---- common/flagdproxy/flagdproxy_test.go | 474 ++++++++++++++++----------- common/types/envconfig.go | 1 + docs/flagd_proxy.md | 1 + go.mod | 4 +- go.sum | 25 +- 6 files changed, 421 insertions(+), 289 deletions(-) diff --git a/common/flagdproxy/flagdproxy.go b/common/flagdproxy/flagdproxy.go index 28131dd45..6850bb60c 100644 --- a/common/flagdproxy/flagdproxy.go +++ b/common/flagdproxy/flagdproxy.go @@ -11,16 +11,19 @@ import ( "golang.org/x/exp/maps" appsV1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" ) const ( - FlagdProxyDeploymentName = "flagd-proxy" - FlagdProxyServiceAccountName = "open-feature-operator-flagd-proxy" - FlagdProxyServiceName = "flagd-proxy-svc" + FlagdProxyDeploymentName = "flagd-proxy" + FlagdProxyServiceAccountName = "open-feature-operator-flagd-proxy" + FlagdProxyServiceName = "flagd-proxy-svc" + FlagdProxyPodDisruptionBudgetName = "flagd-proxy-pdb" ) type FlagdProxyHandler struct { @@ -37,6 +40,7 @@ type FlagdProxyConfiguration struct { DebugLogging bool Image string Tag string + Replicas int Namespace string OperatorDeploymentName string ImagePullSecrets []string @@ -53,6 +57,7 @@ func NewFlagdProxyConfiguration(env types.EnvConfig, imagePullSecrets []string, Port: env.FlagdProxyPort, ManagementPort: env.FlagdProxyManagementPort, DebugLogging: env.FlagdProxyDebugLogging, + Replicas: env.FlagdProxyReplicaCount, ImagePullSecrets: imagePullSecrets, Labels: labels, Annotations: annotations, @@ -71,58 +76,99 @@ func (f *FlagdProxyHandler) Config() *FlagdProxyConfiguration { return f.config } -func (f *FlagdProxyHandler) createObject(ctx context.Context, obj client.Object) error { - return f.Client.Create(ctx, obj) +func specDiffers(a, b client.Object) (bool, error) { + if a == nil || b == nil { + return false, fmt.Errorf("object is nil") + } + + // Compare only spec based on the object type + switch a.(type) { + case *corev1.Service: + return !reflect.DeepEqual(a.(*corev1.Service).Spec, b.(*corev1.Service).Spec), nil + case *appsV1.Deployment: + return !reflect.DeepEqual(a.(*appsV1.Deployment).Spec, b.(*appsV1.Deployment).Spec), nil + case *policyv1.PodDisruptionBudget: + return !reflect.DeepEqual(a.(*policyv1.PodDisruptionBudget).Spec, b.(*policyv1.PodDisruptionBudget).Spec), nil + default: + return false, fmt.Errorf("unsupported object type") + } } -func (f *FlagdProxyHandler) updateObject(ctx context.Context, obj client.Object) error { - return f.Client.Update(ctx, obj) +// ensureFlagdProxyResource ensures that the given object is reconciled in the cluster. If the object does not exist, it will be created. +func (f *FlagdProxyHandler) ensureFlagdProxyResource(ctx context.Context, obj client.Object) error { + if obj == nil { + return fmt.Errorf("object is nil") + } + + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + var old = obj.DeepCopyObject().(client.Object) + + // Try to get the existing object + err := f.Client.Get(ctx, client.ObjectKey{Name: old.GetName(), Namespace: old.GetNamespace()}, old) + notFound := errors.IsNotFound(err) + if err != nil && !notFound { + return err + } + + // If the object exists but is not managed by OFO, return an error + if !notFound && !common.IsManagedByOFO(old) { + return fmt.Errorf("%s not managed by OFO", obj.GetName()) + } + + // If the object is not found, we will create it + if notFound { + return f.Client.Create(ctx, obj) + } + + // If the object is found, update if necessary + needsUpdate, err := specDiffers(obj, old) + if err != nil { + return err + } + + if needsUpdate { + obj.SetResourceVersion(old.GetResourceVersion()) + return f.Client.Update(ctx, obj) + } + + return nil + }) } +// HandleFlagdProxy ensures flagd-proxy kubernetes components are configured properly func (f *FlagdProxyHandler) HandleFlagdProxy(ctx context.Context) error { - exists, deployment, err := f.doesFlagdProxyExist(ctx) - if err != nil { - return err - } + var err error - ownerReference, err := f.getOwnerReference(ctx) + ownerRef, err := f.getOwnerReference(ctx) if err != nil { return err } - newDeployment := f.newFlagdProxyManifest(ownerReference) - newService := f.newFlagdProxyServiceManifest(ownerReference) - - if !exists { - f.Log.Info("flagd-proxy Deployment does not exist, creating") - return f.deployFlagdProxy(ctx, f.createObject, newDeployment, newService) - } - // flagd-proxy exists, need to check if we should update it - if f.shouldUpdateFlagdProxy(deployment, newDeployment) { - f.Log.Info("flagd-proxy Deployment out of sync, updating") - return f.deployFlagdProxy(ctx, f.updateObject, newDeployment, newService) - } - f.Log.Info("flagd-proxy Deployment up-to-date") - return nil -} -func (f *FlagdProxyHandler) deployFlagdProxy(ctx context.Context, createUpdateFunc CreateUpdateFunc, deployment *appsV1.Deployment, service *corev1.Service) error { - f.Log.Info("deploying the flagd-proxy") - if err := createUpdateFunc(ctx, deployment); err != nil && !errors.IsAlreadyExists(err) { + if err = f.ensureFlagdProxyResource(ctx, f.newFlagdProxyDeployment(ownerRef)); err != nil { return err } - f.Log.Info("deploying the flagd-proxy service") - if err := createUpdateFunc(ctx, service); err != nil && !errors.IsAlreadyExists(err) { + + if err = f.ensureFlagdProxyResource(ctx, f.newFlagdProxyService(ownerRef)); err != nil { return err } - return nil + + err = f.ensureFlagdProxyResource(ctx, f.newFlagdProxyPodDisruptionBudget(ownerRef)) + return err } -func (f *FlagdProxyHandler) newFlagdProxyServiceManifest(ownerReference *metav1.OwnerReference) *corev1.Service { +func (f *FlagdProxyHandler) newFlagdProxyService(ownerReference *metav1.OwnerReference) *corev1.Service { return &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: "v1", + }, ObjectMeta: metav1.ObjectMeta{ Name: FlagdProxyServiceName, Namespace: f.config.Namespace, OwnerReferences: []metav1.OwnerReference{*ownerReference}, + Labels: map[string]string{ + common.ManagedByAnnotationKey: common.ManagedByAnnotationValue, + }, }, Spec: corev1.ServiceSpec{ Selector: map[string]string{ @@ -140,8 +186,41 @@ func (f *FlagdProxyHandler) newFlagdProxyServiceManifest(ownerReference *metav1. } } -func (f *FlagdProxyHandler) newFlagdProxyManifest(ownerReference *metav1.OwnerReference) *appsV1.Deployment { - replicas := int32(1) +func (f *FlagdProxyHandler) newFlagdProxyPodDisruptionBudget(ownerReference *metav1.OwnerReference) *policyv1.PodDisruptionBudget { + + // Only require pods to be available if there is >1 replica configured (HA setup) + minReplicas := intstr.FromInt(0) + if f.config.Replicas > 1 { + minReplicas = intstr.FromInt(f.config.Replicas / 2) + } + + return &policyv1.PodDisruptionBudget{ + TypeMeta: metav1.TypeMeta{ + Kind: "PodDisruptionBudget", + APIVersion: "policy/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: FlagdProxyPodDisruptionBudgetName, + Namespace: f.config.Namespace, + OwnerReferences: []metav1.OwnerReference{*ownerReference}, + Labels: map[string]string{ + common.ManagedByAnnotationKey: common.ManagedByAnnotationValue, + }, + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: &minReplicas, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app.kubernetes.io/name": FlagdProxyDeploymentName, + common.ManagedByAnnotationKey: common.ManagedByAnnotationValue, + }, + }, + }, + } +} + +func (f *FlagdProxyHandler) newFlagdProxyDeployment(ownerReference *metav1.OwnerReference) *appsV1.Deployment { + replicas := int32(f.config.Replicas) args := []string{ "start", "--management-port", @@ -157,10 +236,10 @@ func (f *FlagdProxyHandler) newFlagdProxyManifest(ownerReference *metav1.OwnerRe }) } flagdLabels := map[string]string{ - "app": FlagdProxyDeploymentName, - "app.kubernetes.io/name": FlagdProxyDeploymentName, - "app.kubernetes.io/managed-by": common.ManagedByAnnotationValue, - "app.kubernetes.io/version": f.config.Tag, + "app": FlagdProxyDeploymentName, + "app.kubernetes.io/name": FlagdProxyDeploymentName, + common.ManagedByAnnotationKey: common.ManagedByAnnotationValue, + "app.kubernetes.io/version": f.config.Tag, } if len(f.config.Labels) > 0 { maps.Copy(flagdLabels, f.config.Labels) @@ -173,13 +252,17 @@ func (f *FlagdProxyHandler) newFlagdProxyManifest(ownerReference *metav1.OwnerRe } return &appsV1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: "apps/v1", + }, ObjectMeta: metav1.ObjectMeta{ Name: FlagdProxyDeploymentName, Namespace: f.config.Namespace, Labels: map[string]string{ - "app": FlagdProxyDeploymentName, - "app.kubernetes.io/managed-by": common.ManagedByAnnotationValue, - "app.kubernetes.io/version": f.config.Tag, + "app": FlagdProxyDeploymentName, + common.ManagedByAnnotationKey: common.ManagedByAnnotationValue, + "app.kubernetes.io/version": f.config.Tag, }, OwnerReferences: []metav1.OwnerReference{*ownerReference}, }, @@ -215,41 +298,31 @@ func (f *FlagdProxyHandler) newFlagdProxyManifest(ownerReference *metav1.OwnerRe Args: args, }, }, + TopologySpreadConstraints: []corev1.TopologySpreadConstraint{ + { + MaxSkew: 1, + TopologyKey: "kubernetes.io/hostname", + WhenUnsatisfiable: corev1.DoNotSchedule, + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app.kubernetes.io/name": FlagdProxyDeploymentName, + common.ManagedByAnnotationKey: common.ManagedByAnnotationValue, + }, + }, + }, + }, }, }, }, } } -func (f *FlagdProxyHandler) doesFlagdProxyExist(ctx context.Context) (bool, *appsV1.Deployment, error) { - d := &appsV1.Deployment{} - err := f.Client.Get(ctx, client.ObjectKey{Name: FlagdProxyDeploymentName, Namespace: f.config.Namespace}, d) - if err != nil { - if errors.IsNotFound(err) { - // does not exist, is not ready, no error - return false, nil, nil - } - // does not exist, is not ready, is in error - return false, nil, err - } - return true, d, nil -} - -func (f *FlagdProxyHandler) shouldUpdateFlagdProxy(old, new *appsV1.Deployment) bool { - if !common.IsManagedByOFO(old) { - f.Log.Info("flagd-proxy Deployment not managed by OFO") - return false - } - return !reflect.DeepEqual(old.Spec, new.Spec) -} - func (f *FlagdProxyHandler) getOperatorDeployment(ctx context.Context) (*appsV1.Deployment, error) { d := &appsV1.Deployment{} if err := f.Client.Get(ctx, client.ObjectKey{Name: f.config.OperatorDeploymentName, Namespace: f.config.Namespace}, d); err != nil { return nil, fmt.Errorf("unable to fetch operator deployment: %w", err) } return d, nil - } func (f *FlagdProxyHandler) getOwnerReference(ctx context.Context) (*metav1.OwnerReference, error) { diff --git a/common/flagdproxy/flagdproxy_test.go b/common/flagdproxy/flagdproxy_test.go index 8ab7d223c..7d619f822 100644 --- a/common/flagdproxy/flagdproxy_test.go +++ b/common/flagdproxy/flagdproxy_test.go @@ -9,33 +9,202 @@ import ( "github.com/open-feature/open-feature-operator/common" "github.com/open-feature/open-feature-operator/common/types" "github.com/stretchr/testify/require" - appsV1 "k8s.io/api/apps/v1" - v1 "k8s.io/api/apps/v1" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - v12 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -var pullSecrets = []string{"test-pullSecret"} +var ( + pullSecrets = []string{"test-pullSecret"} -var labels = map[string]string{ - "label1": "labelValue1", - "label2": "labelValue2", -} + labels = map[string]string{ + "label1": "labelValue1", + "label2": "labelValue2", + } -var annotations = map[string]string{ - "annotation1": "annotationValue1", - "annotation2": "annotationValue2", -} + annotations = map[string]string{ + "annotation1": "annotationValue1", + "annotation2": "annotationValue2", + } + + testPort = 88 + testManagementPort = 90 + testReplicaCount = 1 + testDebugLogging = true + testNamespace = "ns" + testImage = "image" + testTag = "tag" + + expectedDeploymentReplicas = int32(testReplicaCount) + expectedDeployment = &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: "apps/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: FlagdProxyDeploymentName, + Namespace: "ns", + Labels: map[string]string{ + "app": FlagdProxyDeploymentName, + "app.kubernetes.io/managed-by": common.ManagedByAnnotationValue, + "app.kubernetes.io/version": testTag, + }, + ResourceVersion: "1", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "apps/v1", + Kind: "Deployment", + Name: common.OperatorDeploymentName, + }, + }, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: &expectedDeploymentReplicas, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": FlagdProxyDeploymentName, + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": FlagdProxyDeploymentName, + "app.kubernetes.io/name": FlagdProxyDeploymentName, + "app.kubernetes.io/managed-by": common.ManagedByAnnotationValue, + "app.kubernetes.io/version": "tag", + "label1": "labelValue1", + "label2": "labelValue2", + }, + Annotations: annotations, + }, + Spec: corev1.PodSpec{ + ServiceAccountName: FlagdProxyServiceAccountName, + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: pullSecrets[0]}, + }, + Containers: []corev1.Container{ + { + Image: fmt.Sprintf("%s:%s", testImage, testTag), + Name: FlagdProxyDeploymentName, + Ports: []corev1.ContainerPort{ + { + Name: "port", + ContainerPort: int32(testPort), + }, + { + Name: "management-port", + ContainerPort: int32(testManagementPort), + }, + }, + Args: []string{"start", "--management-port", fmt.Sprint(testManagementPort), "--debug"}, + }, + }, + TopologySpreadConstraints: []corev1.TopologySpreadConstraint{ + { + MaxSkew: 1, + TopologyKey: "kubernetes.io/hostname", + WhenUnsatisfiable: corev1.DoNotSchedule, + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app.kubernetes.io/name": FlagdProxyDeploymentName, + common.ManagedByAnnotationKey: common.ManagedByAnnotationValue, + }, + }, + }, + }, + }, + }, + }, + } + expectedService = &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Service", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: FlagdProxyServiceName, + Namespace: testNamespace, + ResourceVersion: "1", + Labels: map[string]string{ + common.ManagedByAnnotationKey: common.ManagedByAnnotationValue, + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "apps/v1", + Kind: "Deployment", + Name: common.OperatorDeploymentName, + }, + }, + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "app.kubernetes.io/name": FlagdProxyDeploymentName, + "app.kubernetes.io/managed-by": common.ManagedByAnnotationValue, + }, + Ports: []corev1.ServicePort{ + { + Name: "flagd-proxy", + Port: int32(testPort), + TargetPort: intstr.FromInt(testPort), + }, + }, + }, + } + + expectedPDBminAvailable = intstr.FromInt(0) + expectedPDB = &policyv1.PodDisruptionBudget{ + TypeMeta: metav1.TypeMeta{ + Kind: "PodDisruptionBudget", + APIVersion: "policy/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: FlagdProxyPodDisruptionBudgetName, + Namespace: "ns", + ResourceVersion: "1", + Labels: map[string]string{ + common.ManagedByAnnotationKey: common.ManagedByAnnotationValue, + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "apps/v1", + Kind: "Deployment", + Name: common.OperatorDeploymentName, + }, + }, + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: &expectedPDBminAvailable, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app.kubernetes.io/name": FlagdProxyDeploymentName, + common.ManagedByAnnotationKey: common.ManagedByAnnotationValue, + }, + }, + }, + } + + testEnvConfig = types.EnvConfig{ + PodNamespace: testNamespace, + FlagdProxyImage: testImage, + FlagdProxyTag: testTag, + FlagdProxyPort: testPort, + FlagdProxyManagementPort: testManagementPort, + FlagdProxyReplicaCount: testReplicaCount, + FlagdProxyDebugLogging: true, + } +) func TestNewFlagdProxyConfiguration(t *testing.T) { kpConfig := NewFlagdProxyConfiguration(types.EnvConfig{ FlagdProxyPort: 8015, FlagdProxyManagementPort: 8016, + FlagdProxyReplicaCount: 123, }, pullSecrets, labels, annotations) require.NotNil(t, kpConfig) @@ -45,6 +214,7 @@ func TestNewFlagdProxyConfiguration(t *testing.T) { DebugLogging: false, OperatorDeploymentName: common.OperatorDeploymentName, ImagePullSecrets: pullSecrets, + Replicas: 123, Labels: labels, Annotations: annotations, }, kpConfig) @@ -91,53 +261,6 @@ func TestNewFlagdProxyHandler(t *testing.T) { require.Equal(t, kpConfig, ph.Config()) } -func TestDoesFlagdProxyExist(t *testing.T) { - env := types.EnvConfig{ - PodNamespace: "ns", - } - - deployment := &v1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "ns", - Name: FlagdProxyDeploymentName, - }, - Spec: v1.DeploymentSpec{ - Template: v12.PodTemplateSpec{ - Spec: v12.PodSpec{ - Containers: []v12.Container{ - { - Name: "my-container", - }, - }, - }, - }, - }, - } - - kpConfig := NewFlagdProxyConfiguration(env, pullSecrets, labels, annotations) - - require.NotNil(t, kpConfig) - - fakeClient := fake.NewClientBuilder().WithObjects().Build() - - ph := NewFlagdProxyHandler(kpConfig, fakeClient, testr.New(t)) - - require.NotNil(t, ph) - - res, d, err := ph.doesFlagdProxyExist(context.TODO()) - require.Nil(t, err) - require.Nil(t, d) - require.False(t, res) - - err = fakeClient.Create(context.TODO(), deployment) - require.Nil(t, err) - - res, d, err = ph.doesFlagdProxyExist(context.TODO()) - require.Nil(t, err) - require.NotNil(t, d) - require.True(t, res) -} - func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithBadVersion(t *testing.T) { env := types.EnvConfig{ PodNamespace: "ns", @@ -151,7 +274,7 @@ func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithBadVersion(t *testing ownerRef, err := getTestOFODeploymentOwnerRef(fakeClient, env.PodNamespace) require.Nil(t, err) - proxyDeployment := &v1.Deployment{ + proxyDeployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Namespace: kpConfig.Namespace, Name: FlagdProxyDeploymentName, @@ -160,10 +283,10 @@ func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithBadVersion(t *testing "app.kubernetes.io/managed-by": common.ManagedByAnnotationValue, }, }, - Spec: v1.DeploymentSpec{ - Template: v12.PodTemplateSpec{ - Spec: v12.PodSpec{ - Containers: []v12.Container{ + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ { Name: "my-container", }, @@ -184,7 +307,7 @@ func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithBadVersion(t *testing require.Nil(t, err) - deployment := &v1.Deployment{} + deployment := &appsv1.Deployment{} err = fakeClient.Get(context.Background(), client.ObjectKey{ Namespace: env.PodNamespace, Name: FlagdProxyDeploymentName, @@ -205,15 +328,15 @@ func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithoutLabel(t *testing.T require.NotNil(t, kpConfig) - proxyDeployment := &v1.Deployment{ + proxyDeployment := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Namespace: kpConfig.Namespace, Name: FlagdProxyDeploymentName, }, - Spec: v1.DeploymentSpec{ - Template: v12.PodTemplateSpec{ - Spec: v12.PodSpec{ - Containers: []v12.Container{ + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ { Name: "my-container", }, @@ -231,9 +354,9 @@ func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithoutLabel(t *testing.T err := ph.HandleFlagdProxy(context.Background()) - require.Nil(t, err) + require.ErrorContains(t, err, "not managed by OFO") - deployment := &v1.Deployment{} + deployment := &appsv1.Deployment{} err = fakeClient.Get(context.Background(), client.ObjectKey{ Namespace: env.PodNamespace, Name: FlagdProxyDeploymentName, @@ -263,7 +386,7 @@ func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithNewestVersion(t *test ownerRef, err := getTestOFODeploymentOwnerRef(fakeClient, env.PodNamespace) require.Nil(t, err) - proxy := ph.newFlagdProxyManifest(ownerRef) + proxy := ph.newFlagdProxyDeployment(ownerRef) err = fakeClient.Create(context.TODO(), proxy) require.Nil(t, err) @@ -272,7 +395,7 @@ func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithNewestVersion(t *test require.Nil(t, err) - deployment := &v1.Deployment{} + deployment := &appsv1.Deployment{} err = fakeClient.Get(context.Background(), client.ObjectKey{ Namespace: env.PodNamespace, Name: FlagdProxyDeploymentName, @@ -286,28 +409,20 @@ func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithNewestVersion(t *test } func TestFlagdProxyHandler_HandleFlagdProxy_CreateProxy(t *testing.T) { - env := types.EnvConfig{ - PodNamespace: "ns", - FlagdProxyImage: "image", - FlagdProxyTag: "tag", - FlagdProxyPort: 88, - FlagdProxyManagementPort: 90, - FlagdProxyDebugLogging: true, - } - kpConfig := NewFlagdProxyConfiguration(env, pullSecrets, labels, annotations) + kpConfig := NewFlagdProxyConfiguration(testEnvConfig, pullSecrets, labels, annotations) require.NotNil(t, kpConfig) - fakeClient := fake.NewClientBuilder().WithObjects(createOFOTestDeployment(env.PodNamespace)).Build() + fakeClient := fake.NewClientBuilder().WithObjects(createOFOTestDeployment(testNamespace)).Build() ph := NewFlagdProxyHandler(kpConfig, fakeClient, testr.New(t)) require.NotNil(t, ph) // proxy does not exist - deployment := &v1.Deployment{} + deployment := &appsv1.Deployment{} err := fakeClient.Get(context.Background(), client.ObjectKey{ - Namespace: env.PodNamespace, + Namespace: testNamespace, Name: FlagdProxyDeploymentName, }, deployment) @@ -318,139 +433,98 @@ func TestFlagdProxyHandler_HandleFlagdProxy_CreateProxy(t *testing.T) { require.Nil(t, err) // proxy should exist - deployment = &v1.Deployment{} + deployment = &appsv1.Deployment{} err = fakeClient.Get(context.Background(), client.ObjectKey{ - Namespace: env.PodNamespace, + Namespace: testNamespace, Name: FlagdProxyDeploymentName, }, deployment) require.Nil(t, err) require.NotNil(t, deployment) - replicas := int32(1) - args := []string{ - "start", - "--management-port", - fmt.Sprintf("%d", 90), - "--debug", - } - - expectedDeployment := &appsV1.Deployment{ - TypeMeta: metav1.TypeMeta{ - Kind: "Deployment", - APIVersion: "apps/v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: FlagdProxyDeploymentName, - Namespace: "ns", - Labels: map[string]string{ - "app": FlagdProxyDeploymentName, - "app.kubernetes.io/managed-by": common.ManagedByAnnotationValue, - "app.kubernetes.io/version": "tag", - }, - ResourceVersion: "1", - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: "apps/v1", - Kind: "Deployment", - Name: common.OperatorDeploymentName, - }, - }, - }, - Spec: appsV1.DeploymentSpec{ - Replicas: &replicas, - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": FlagdProxyDeploymentName, - }, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "app": FlagdProxyDeploymentName, - "app.kubernetes.io/name": FlagdProxyDeploymentName, - "app.kubernetes.io/managed-by": common.ManagedByAnnotationValue, - "app.kubernetes.io/version": "tag", - "label1": "labelValue1", - "label2": "labelValue2", - }, - Annotations: annotations, - }, - Spec: corev1.PodSpec{ - ServiceAccountName: FlagdProxyServiceAccountName, - ImagePullSecrets: []corev1.LocalObjectReference{ - {Name: pullSecrets[0]}, - }, - Containers: []corev1.Container{ - { - Image: "image:tag", - Name: FlagdProxyDeploymentName, - Ports: []corev1.ContainerPort{ - { - Name: "port", - ContainerPort: int32(88), - }, - { - Name: "management-port", - ContainerPort: int32(90), - }, - }, - Args: args, - }, - }, - }, - }, - }, - } - require.Equal(t, expectedDeployment, deployment) service := &corev1.Service{} err = fakeClient.Get(context.Background(), client.ObjectKey{ - Namespace: env.PodNamespace, + Namespace: testNamespace, Name: FlagdProxyServiceName, }, service) require.Nil(t, err) require.NotNil(t, service) - expectedService := &corev1.Service{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "Service", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: FlagdProxyServiceName, - Namespace: "ns", - ResourceVersion: "1", - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: "apps/v1", - Kind: "Deployment", - Name: common.OperatorDeploymentName, - }, - }, - }, - Spec: corev1.ServiceSpec{ - Selector: map[string]string{ - "app.kubernetes.io/name": FlagdProxyDeploymentName, - "app.kubernetes.io/managed-by": common.ManagedByAnnotationValue, - }, - Ports: []corev1.ServicePort{ - { - Name: "flagd-proxy", - Port: int32(88), - TargetPort: intstr.FromInt(88), - }, - }, - }, - } - require.Equal(t, expectedService, service) + + pdb := &policyv1.PodDisruptionBudget{} + err = fakeClient.Get(context.Background(), client.ObjectKey{ + Namespace: testNamespace, + Name: FlagdProxyPodDisruptionBudgetName, + }, pdb) + require.Nil(t, err) + require.NotNil(t, pdb) + + require.Equal(t, expectedPDB, pdb) +} + +func TestFlagdProxyHandler_HandleFlagdProxy_UpdateAllComponents(t *testing.T) { + kpConfig := NewFlagdProxyConfiguration(testEnvConfig, pullSecrets, labels, annotations) + require.NotNil(t, kpConfig) + + fakeClient := fake.NewClientBuilder().WithObjects(createOFOTestDeployment(testNamespace)).Build() + + ph := NewFlagdProxyHandler(kpConfig, fakeClient, testr.New(t)) + require.NotNil(t, ph) + + // Seed with slightly different values + deploy := expectedDeployment.DeepCopy() + deploy.ResourceVersion = "" + deploy.Spec.Replicas = pointer.Int32(100000) + require.Nil(t, fakeClient.Create(context.Background(), deploy)) + + svc := expectedService.DeepCopy() + svc.ResourceVersion = "" + svc.Spec.Ports[0].Port = 100000 + require.Nil(t, fakeClient.Create(context.Background(), svc)) + + pdb := expectedPDB.DeepCopy() + pdb.ResourceVersion = "" + minAvailable := intstr.FromInt(100000) + pdb.Spec.MinAvailable = &minAvailable + require.Nil(t, fakeClient.Create(context.Background(), pdb)) + + // Run + err := ph.HandleFlagdProxy(context.Background()) + require.Nil(t, err) + + // Get the updated resources + require.Nil(t, fakeClient.Get(context.Background(), client.ObjectKey{ + Namespace: testNamespace, + Name: FlagdProxyDeploymentName, + }, deploy)) + updatedExpectedDeployment := expectedDeployment.DeepCopy() + updatedExpectedDeployment.ResourceVersion = "2" + require.Equal(t, updatedExpectedDeployment, deploy) + + require.Nil(t, fakeClient.Get(context.Background(), client.ObjectKey{ + Namespace: testNamespace, + Name: FlagdProxyServiceName, + }, svc)) + updatedExpectedService := expectedService.DeepCopy() + updatedExpectedService.ResourceVersion = "2" + require.Equal(t, updatedExpectedService, svc) + + // pdb := expectedPDB.DeepCopy() + require.Nil(t, fakeClient.Get(context.Background(), client.ObjectKey{ + Namespace: testNamespace, + Name: FlagdProxyPodDisruptionBudgetName, + }, pdb)) + updatedExpectedPDB := expectedPDB.DeepCopy() + updatedExpectedPDB.ResourceVersion = "2" + require.Equal(t, updatedExpectedPDB, pdb) } -func createOFOTestDeployment(ns string) *v1.Deployment { - return &v1.Deployment{ +func createOFOTestDeployment(ns string) *appsv1.Deployment { + return &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns, Name: common.OperatorDeploymentName, @@ -459,7 +533,7 @@ func createOFOTestDeployment(ns string) *v1.Deployment { } func getTestOFODeploymentOwnerRef(c client.Client, ns string) (*metav1.OwnerReference, error) { - d := &appsV1.Deployment{} + d := &appsv1.Deployment{} if err := c.Get(context.TODO(), client.ObjectKey{Name: common.OperatorDeploymentName, Namespace: ns}, d); err != nil { return nil, err } diff --git a/common/types/envconfig.go b/common/types/envconfig.go index 09aa88ffa..c2c4231fa 100644 --- a/common/types/envconfig.go +++ b/common/types/envconfig.go @@ -4,6 +4,7 @@ type EnvConfig struct { PodNamespace string `envconfig:"POD_NAMESPACE" default:"open-feature-operator-system"` FlagdProxyImage string `envconfig:"FLAGD_PROXY_IMAGE" default:"ghcr.io/open-feature/flagd-proxy"` FlagsValidationEnabled bool `envconfig:"FLAGS_VALIDATION_ENABLED" default:"true"` + FlagdProxyReplicaCount int `envconfig:"FLAGD_PROXY_REPLICA_COUNT" default:"1"` // renovate: datasource=github-tags depName=open-feature/flagd/flagd-proxy FlagdProxyTag string `envconfig:"FLAGD_PROXY_TAG" default:"v0.6.4"` FlagdProxyPort int `envconfig:"FLAGD_PROXY_PORT" default:"8015"` diff --git a/docs/flagd_proxy.md b/docs/flagd_proxy.md index 2bca89005..b88f565d4 100644 --- a/docs/flagd_proxy.md +++ b/docs/flagd_proxy.md @@ -54,6 +54,7 @@ The current implementation of the `flagd-proxy` allows for a set of basic config |---------------------------|-----------------------------------------------------------------------------------------------| | FLAGD_PROXY_IMAGE | Allows for the default flagd-proxy image to be overwritten | | FLAGD_PROXY_TAG | Allows for the default flagd-proxy tag to be overwritten | +| FLAGD_PROXY_Replicas | Allows to configure the number of replicas for the flagd-proxy deployment. | | FLAGD_PROXY_PORT | Allows the default port of `8015` to eb overwritten | | FLAGD_PROXY_METRICS_PORT | Allows the default metrics port of `8016` to be overwritten | | FLAGD_PROXY_DEBUG_LOGGING | Defaults to `"false"`, allows for the `--debug` flag to be set on the `flagd-proxy` container | diff --git a/go.mod b/go.mod index 4e73b245d..9165ba3e0 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/go-logr/logr v1.4.2 github.com/golang/mock v1.4.4 github.com/kelseyhightower/envconfig v1.4.0 - github.com/open-feature/open-feature-operator/apis v0.2.41-0.20240506125212-c4831a3cdc00 + github.com/open-feature/open-feature-operator/apis v0.2.44 github.com/stretchr/testify v1.9.0 go.uber.org/zap v1.27.0 golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 @@ -43,7 +43,7 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect - github.com/open-feature/flagd-schemas v0.2.9-0.20240408192555-ea4f119d2bd7 // indirect + github.com/open-feature/flagd-schemas v0.2.9-0.20240708163558-2aa89b314322 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_golang v1.16.0 // indirect diff --git a/go.sum b/go.sum index 5d233e9fd..26c42bd15 100644 --- a/go.sum +++ b/go.sum @@ -84,10 +84,10 @@ github.com/onsi/ginkgo/v2 v2.11.0 h1:WgqUCUt/lT6yXoQ8Wef0fsNn5cAuMK7+KT9UFRz2tcU github.com/onsi/ginkgo/v2 v2.11.0/go.mod h1:ZhrRA5XmEE3x3rhlzamx/JJvujdZoJ2uvgI7kR0iZvM= github.com/onsi/gomega v1.27.10 h1:naR28SdDFlqrG6kScpT8VWpu1xWY5nJRCF3XaYyBjhI= github.com/onsi/gomega v1.27.10/go.mod h1:RsS8tutOdbdgzbPtzzATp12yT7kM5I5aElG3evPbQ0M= -github.com/open-feature/flagd-schemas v0.2.9-0.20240408192555-ea4f119d2bd7 h1:oP+BH8RiNEmSWTffKEXz2ciwen7wbvyX0fESx0aoJ80= -github.com/open-feature/flagd-schemas v0.2.9-0.20240408192555-ea4f119d2bd7/go.mod h1:WKtwo1eW9/K6D+4HfgTXWBqCDzpvMhDa5eRxW7R5B2U= -github.com/open-feature/open-feature-operator/apis v0.2.41-0.20240506125212-c4831a3cdc00 h1:EsG43eil7L+4c6BXMGPMRm9qYVz0J1+sjukz5xw+vTM= -github.com/open-feature/open-feature-operator/apis v0.2.41-0.20240506125212-c4831a3cdc00/go.mod h1:BLFkIDsj+coW30XxILSY2X++vVniRTyWozzONuVKh5U= +github.com/open-feature/flagd-schemas v0.2.9-0.20240708163558-2aa89b314322 h1:5zbNHqcZAc9jlhSrC0onuVL2RPpvYcDaNvW2wOZBfUY= +github.com/open-feature/flagd-schemas v0.2.9-0.20240708163558-2aa89b314322/go.mod h1:WKtwo1eW9/K6D+4HfgTXWBqCDzpvMhDa5eRxW7R5B2U= +github.com/open-feature/open-feature-operator/apis v0.2.44 h1:0r4Z+RnJltuHdRBv79NFgAckhna6/M3Wcec6gzNX5vI= +github.com/open-feature/open-feature-operator/apis v0.2.44/go.mod h1:xB2uLzvUkbydieX7q6/NqannBz3bt/e5BS2DeOyyw4Q= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -113,8 +113,6 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= -github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c= @@ -139,10 +137,7 @@ go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M= golang.org/x/crypto v0.25.0/go.mod h1:T+wALwcMOSE0kXgUAnPAHqTLW+XHgcELELW8VaDgm/M= -golang.org/x/exp v0.0.0-20240707233637-46b078467d37 h1:uLDX+AfeFCct3a2C7uIWBKMJIR3CJMhcgfrUAqjRK6w= -golang.org/x/exp v0.0.0-20240707233637-46b078467d37/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY= golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0LeHDbnYEryqj5Q1ug8= golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY= golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= @@ -150,12 +145,9 @@ golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= -golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.15.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= -golang.org/x/net v0.24.0 h1:1PcaxkF854Fu3+lvBIx5SYn9wRlBzzcnHZSiaFFAb0w= -golang.org/x/net v0.24.0/go.mod h1:2Q7sJY5mzlzWjKtYUEXSlBWCdyaioyXzRB2RtU8KVE8= golang.org/x/net v0.27.0 h1:5K3Njcw06/l2y9vpGCSdcxWOYHOUk3dVNGDXN+FvAys= golang.org/x/net v0.27.0/go.mod h1:dDi0PyhWNoiUOrAS8uXv/vnScO4wnHQO4mj9fn/RytE= golang.org/x/oauth2 v0.8.0 h1:6dkIjl3j3LtZ/O3sTgZTMsLKSftL/B8Zgq4huOIIUu8= @@ -166,7 +158,6 @@ golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= @@ -176,25 +167,18 @@ golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o= -golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/telemetry v0.0.0-20240228155512-f48c80bd79b2/go.mod h1:TeRTkGYfJXctD9OcfyVLyj2J3IxLnKwHJR8f4D8a3YE= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.19.0 h1:+ThwsDv+tYfnJFhF4L8jITxu1tdTWRTZpdsWgEgjL6Q= -golang.org/x/term v0.19.0/go.mod h1:2CuTdWZ7KHSQwUzKva0cbMg6q2DMI3Mmxp+gKJbskEk= golang.org/x/term v0.22.0 h1:BbsgPEJULsl2fV/AT3v15Mjva5yXKQDyKf+TbDz7QJk= golang.org/x/term v0.22.0/go.mod h1:F3qCibpT5AMpCRfhfT53vVJwhLtIVHhB9XDjfFvnMI4= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= -golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= -golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4= @@ -207,7 +191,6 @@ golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roY golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d/go.mod h1:aiJjzUbINMkxbQROHiO6hDPo2LHcIPhhQsa9DLh0yGk= golang.org/x/tools v0.23.0 h1:SGsXPZ+2l4JsgaCKkx+FQ9YZ5XEtA1GZYuoDjenLjvg= From fbe8ec0601174de7700aeeb4b1e20ce081f81097 Mon Sep 17 00:00:00 2001 From: Matthias Riegler Date: Mon, 14 Oct 2024 14:59:32 +0200 Subject: [PATCH 2/6] chore: fix docs Signed-off-by: Matthias Riegler --- docs/flagd_proxy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/flagd_proxy.md b/docs/flagd_proxy.md index b88f565d4..b9c179456 100644 --- a/docs/flagd_proxy.md +++ b/docs/flagd_proxy.md @@ -54,7 +54,7 @@ The current implementation of the `flagd-proxy` allows for a set of basic config |---------------------------|-----------------------------------------------------------------------------------------------| | FLAGD_PROXY_IMAGE | Allows for the default flagd-proxy image to be overwritten | | FLAGD_PROXY_TAG | Allows for the default flagd-proxy tag to be overwritten | -| FLAGD_PROXY_Replicas | Allows to configure the number of replicas for the flagd-proxy deployment. | +| FLAGD_PROXY_REPLICA_COUNT | Allows to configure the number of replicas for the flagd-proxy deployment. | | FLAGD_PROXY_PORT | Allows the default port of `8015` to eb overwritten | | FLAGD_PROXY_METRICS_PORT | Allows the default metrics port of `8016` to be overwritten | | FLAGD_PROXY_DEBUG_LOGGING | Defaults to `"false"`, allows for the `--debug` flag to be set on the `flagd-proxy` container | From 85d39788cd0f50c8a33518910881b08cb256590c Mon Sep 17 00:00:00 2001 From: Matthias Riegler Date: Mon, 14 Oct 2024 16:08:17 +0200 Subject: [PATCH 3/6] chore: address golangci Signed-off-by: Matthias Riegler --- common/flagdproxy/flagdproxy_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/common/flagdproxy/flagdproxy_test.go b/common/flagdproxy/flagdproxy_test.go index 7d619f822..e4ccfd10e 100644 --- a/common/flagdproxy/flagdproxy_test.go +++ b/common/flagdproxy/flagdproxy_test.go @@ -40,6 +40,16 @@ var ( testImage = "image" testTag = "tag" + testEnvConfig = types.EnvConfig{ + PodNamespace: testNamespace, + FlagdProxyImage: testImage, + FlagdProxyTag: testTag, + FlagdProxyPort: testPort, + FlagdProxyManagementPort: testManagementPort, + FlagdProxyReplicaCount: testReplicaCount, + FlagdProxyDebugLogging: testDebugLogging, + } + expectedDeploymentReplicas = int32(testReplicaCount) expectedDeployment = &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ @@ -187,16 +197,6 @@ var ( }, }, } - - testEnvConfig = types.EnvConfig{ - PodNamespace: testNamespace, - FlagdProxyImage: testImage, - FlagdProxyTag: testTag, - FlagdProxyPort: testPort, - FlagdProxyManagementPort: testManagementPort, - FlagdProxyReplicaCount: testReplicaCount, - FlagdProxyDebugLogging: true, - } ) func TestNewFlagdProxyConfiguration(t *testing.T) { From dce4f1748d967a30a0da57bade985a8e9b21f5e7 Mon Sep 17 00:00:00 2001 From: Matthias Riegler Date: Mon, 21 Oct 2024 10:18:35 +0200 Subject: [PATCH 4/6] chore: add logging within the retry loop Signed-off-by: Matthias Riegler --- common/flagdproxy/flagdproxy.go | 1 + 1 file changed, 1 insertion(+) diff --git a/common/flagdproxy/flagdproxy.go b/common/flagdproxy/flagdproxy.go index 6850bb60c..5077fde93 100644 --- a/common/flagdproxy/flagdproxy.go +++ b/common/flagdproxy/flagdproxy.go @@ -102,6 +102,7 @@ func (f *FlagdProxyHandler) ensureFlagdProxyResource(ctx context.Context, obj cl return retry.RetryOnConflict(retry.DefaultRetry, func() error { var old = obj.DeepCopyObject().(client.Object) + f.Log.Info("Ensuring object exists", "name", obj.GetName(), "namespace", obj.GetNamespace()) // Try to get the existing object err := f.Client.Get(ctx, client.ObjectKey{Name: old.GetName(), Namespace: old.GetNamespace()}, old) From b2d5090a6741402a90c01e60b29f56831b7694e4 Mon Sep 17 00:00:00 2001 From: Matthias Riegler Date: Mon, 21 Oct 2024 11:18:04 +0200 Subject: [PATCH 5/6] feat: exponential backoff for flagd-proxy reconciliation Signed-off-by: Matthias Riegler --- common/utils/utils.go | 22 +++++++++ common/utils/utils_test.go | 45 +++++++++++++++++++ .../core/featureflagsource/controller.go | 27 ++++++++--- .../core/featureflagsource/controller_test.go | 10 +++-- controllers/core/flagd/controller_test.go | 2 +- main.go | 6 +++ 6 files changed, 100 insertions(+), 12 deletions(-) diff --git a/common/utils/utils.go b/common/utils/utils.go index 52515c4b4..f66c9a46e 100644 --- a/common/utils/utils.go +++ b/common/utils/utils.go @@ -3,6 +3,8 @@ package utils import ( "fmt" "strings" + "sync/atomic" + "time" ) func TrueVal() *bool { @@ -41,3 +43,23 @@ func FeatureFlagId(namespace, name string) string { func FeatureFlagConfigMapKey(namespace, name string) string { return fmt.Sprintf("%s.flagd.json", FeatureFlagId(namespace, name)) } + +type ExponentialBackoff struct { + StartDelay time.Duration + MaxDelay time.Duration + counter int64 +} + +func (e *ExponentialBackoff) Next() time.Duration { + val := atomic.AddInt64(&e.counter, 1) + + delay := e.StartDelay * (1 << (val - 1)) + if delay > e.MaxDelay { + delay = e.MaxDelay + } + return delay +} + +func (e *ExponentialBackoff) Reset() { + e.counter = 0 +} diff --git a/common/utils/utils_test.go b/common/utils/utils_test.go index 5f7517321..51d368d4d 100644 --- a/common/utils/utils_test.go +++ b/common/utils/utils_test.go @@ -2,6 +2,7 @@ package utils import ( "testing" + "time" "github.com/stretchr/testify/require" ) @@ -39,3 +40,47 @@ func Test_ParseAnnotations(t *testing.T) { require.Equal(t, "default", s1) require.Equal(t, "anno", s2) } + +func TestExponentialBackoff_Next(t *testing.T) { + tests := []struct { + name string + startDelay time.Duration + maxDelay time.Duration + steps int + expected time.Duration + }{ + {name: "basic backoff", startDelay: 1 * time.Second, maxDelay: 16 * time.Second, steps: 3, expected: 4 * time.Second}, + {name: "max delay reached", startDelay: 1 * time.Second, maxDelay: 5 * time.Second, steps: 10, expected: 5 * time.Second}, + {name: "single step", startDelay: 500 * time.Millisecond, maxDelay: 10 * time.Second, steps: 1, expected: 500 * time.Millisecond}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + backoff := &ExponentialBackoff{StartDelay: tt.startDelay, MaxDelay: tt.maxDelay} + var result time.Duration + for i := 0; i < tt.steps; i++ { + result = backoff.Next() + } + if result != tt.expected { + t.Errorf("Expected delay after %d steps to be %v; got %v", tt.steps, tt.expected, result) + } + }) + } +} + +func TestExponentialBackoff_Reset(t *testing.T) { + backoff := &ExponentialBackoff{StartDelay: 1 * time.Second, MaxDelay: 10 * time.Second} + + // Increment the backoff a few times + backoff.Next() + backoff.Next() + + // Reset and check the counter + backoff.Reset() + if backoff.counter != 0 { + t.Errorf("Expected counter to be reset to 0; got %d", backoff.counter) + } + if backoff.Next() != 1*time.Second { + t.Errorf("Expected delay after reset to be %v; got %v", 1*time.Second, backoff.Next()) + } +} diff --git a/controllers/core/featureflagsource/controller.go b/controllers/core/featureflagsource/controller.go index b81e7341c..98857db29 100644 --- a/controllers/core/featureflagsource/controller.go +++ b/controllers/core/featureflagsource/controller.go @@ -26,6 +26,7 @@ import ( api "github.com/open-feature/open-feature-operator/apis/core/v1beta1" "github.com/open-feature/open-feature-operator/common" "github.com/open-feature/open-feature-operator/common/flagdproxy" + "github.com/open-feature/open-feature-operator/common/utils" appsV1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -33,6 +34,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" ) // FeatureFlagSourceReconciler reconciles a FeatureFlagSource object @@ -40,8 +42,11 @@ type FeatureFlagSourceReconciler struct { client.Client Scheme *runtime.Scheme // ReqLogger contains the Logger of this controller - Log logr.Logger - FlagdProxy *flagdproxy.FlagdProxyHandler + Log logr.Logger + + // FlagdProxy is the handler for the flagd-proxy deployment + FlagdProxy *flagdproxy.FlagdProxyHandler + FlagdProxyBackoff *utils.ExponentialBackoff } // renovate: datasource=github-tags depName=open-feature/flagd/flagd-proxy @@ -73,13 +78,21 @@ func (r *FeatureFlagSourceReconciler) Reconcile(ctx context.Context, req ctrl.Re return r.finishReconcile(err, false) } + needsFlagdProxy := false for _, source := range fsConfig.Spec.Sources { if source.Provider.IsFlagdProxy() { - r.Log.Info(fmt.Sprintf("featureflagsource %s uses flagd-proxy, checking deployment", req.NamespacedName)) - if err := r.FlagdProxy.HandleFlagdProxy(ctx); err != nil { - r.Log.Error(err, "error handling the flagd-proxy deployment") - } - break + r.Log.Info(fmt.Sprintf("featureflagsource %s requires flagd-proxy", req.NamespacedName)) + needsFlagdProxy = true + } + } + + if needsFlagdProxy { + r.Log.Info(fmt.Sprintf("featureflagsource %s uses flagd-proxy, checking deployment", req.NamespacedName)) + if err := r.FlagdProxy.HandleFlagdProxy(ctx); err != nil { + r.Log.Error(err, "error handling the flagd-proxy deployment") + return reconcile.Result{RequeueAfter: r.FlagdProxyBackoff.Next()}, err + } else { + r.FlagdProxyBackoff.Reset() } } diff --git a/controllers/core/featureflagsource/controller_test.go b/controllers/core/featureflagsource/controller_test.go index 6f2613b66..d88451b78 100644 --- a/controllers/core/featureflagsource/controller_test.go +++ b/controllers/core/featureflagsource/controller_test.go @@ -11,6 +11,7 @@ import ( "github.com/open-feature/open-feature-operator/common" "github.com/open-feature/open-feature-operator/common/flagdproxy" commontypes "github.com/open-feature/open-feature-operator/common/types" + "github.com/open-feature/open-feature-operator/common/utils" "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -113,10 +114,11 @@ func TestFeatureFlagSourceReconciler_Reconcile(t *testing.T) { ) r := &FeatureFlagSourceReconciler{ - Client: fakeClient, - Log: ctrl.Log.WithName("featureflagsource-controller"), - Scheme: fakeClient.Scheme(), - FlagdProxy: kph, + Client: fakeClient, + Log: ctrl.Log.WithName("featureflagsource-controller"), + Scheme: fakeClient.Scheme(), + FlagdProxy: kph, + FlagdProxyBackoff: &utils.ExponentialBackoff{StartDelay: time.Duration(0), MaxDelay: time.Duration(0)}, } if tt.deployment != nil { diff --git a/controllers/core/flagd/controller_test.go b/controllers/core/flagd/controller_test.go index cf81fb8ef..1d2de5c16 100644 --- a/controllers/core/flagd/controller_test.go +++ b/controllers/core/flagd/controller_test.go @@ -9,7 +9,7 @@ import ( "github.com/golang/mock/gomock" api "github.com/open-feature/open-feature-operator/apis/core/v1beta1" - "github.com/open-feature/open-feature-operator/controllers/core/flagd/common" + resources "github.com/open-feature/open-feature-operator/controllers/core/flagd/common" commonmock "github.com/open-feature/open-feature-operator/controllers/core/flagd/mock" resourcemock "github.com/open-feature/open-feature-operator/controllers/core/flagd/resources/mock" "github.com/stretchr/testify/require" diff --git a/main.go b/main.go index c3902e062..45e8fe025 100644 --- a/main.go +++ b/main.go @@ -23,6 +23,7 @@ import ( "log" "os" "strings" + "time" "github.com/kelseyhightower/envconfig" corev1beta1 "github.com/open-feature/open-feature-operator/apis/core/v1beta1" @@ -30,6 +31,7 @@ import ( "github.com/open-feature/open-feature-operator/common/flagdinjector" "github.com/open-feature/open-feature-operator/common/flagdproxy" "github.com/open-feature/open-feature-operator/common/types" + "github.com/open-feature/open-feature-operator/common/utils" "github.com/open-feature/open-feature-operator/controllers/core/featureflagsource" "github.com/open-feature/open-feature-operator/controllers/core/flagd" flagdresources "github.com/open-feature/open-feature-operator/controllers/core/flagd/resources" @@ -228,6 +230,10 @@ func main() { Scheme: mgr.GetScheme(), Log: ctrl.Log.WithName("FeatureFlagSource Controller"), FlagdProxy: kph, + FlagdProxyBackoff: &utils.ExponentialBackoff{ + StartDelay: time.Second, + MaxDelay: time.Minute, + }, } if err = flagSourceController.SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "FeatureFlagSource") From 2f6f42c72ceb2041617f052777e53e69c83ed44a Mon Sep 17 00:00:00 2001 From: Matthias Riegler Date: Mon, 21 Oct 2024 11:36:00 +0200 Subject: [PATCH 6/6] chore: CR Signed-off-by: Matthias Riegler --- common/flagdproxy/flagdproxy.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/common/flagdproxy/flagdproxy.go b/common/flagdproxy/flagdproxy.go index 5077fde93..c96f8e269 100644 --- a/common/flagdproxy/flagdproxy.go +++ b/common/flagdproxy/flagdproxy.go @@ -111,15 +111,14 @@ func (f *FlagdProxyHandler) ensureFlagdProxyResource(ctx context.Context, obj cl return err } - // If the object exists but is not managed by OFO, return an error - if !notFound && !common.IsManagedByOFO(old) { - return fmt.Errorf("%s not managed by OFO", obj.GetName()) - } - // If the object is not found, we will create it if notFound { return f.Client.Create(ctx, obj) } + // If the object exists but is not managed by OFO, return an error + if !common.IsManagedByOFO(old) { + return fmt.Errorf("%s not managed by OFO", obj.GetName()) + } // If the object is found, update if necessary needsUpdate, err := specDiffers(obj, old)