Skip to content

Commit

Permalink
check ownerRef before updating proxy Deployment
Browse files Browse the repository at this point in the history 
Signed-off-by: odubajDT <[email protected]>
  • Loading branch information
@odubajDT
odubajDT committed Jan 19, 2024
1 parent 995bac6 commit 0c66bf7
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 40 deletions.
4 changes: 2 additions & 2 deletions chart/open-feature-operator/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ helm upgrade --install open-feature-operator openfeature/open-feature-operator
```

> [!NOTE]
> If you have used `flagd-proxy` provider and upgrading to OFO version `v0.5.4` or higher,
> If you upgrade to OFO `v0.5.4` or higher while using a `flagd-proxy` provider, the instance of
`flagd-proxy` will be automatically upgraded to the latest supported version by the `open-feature-operator`.
This upgrade will also consider your current `FeatureFlagSource` configuration and adapt
The upgrade of `flagd-proxy` will also consider your current `FeatureFlagSource` configuration and adapt
the `flagd-proxy` Deployment accordingly.
If you are upgrading OFO to `v0.5.3` or lower, `flagd-proxy` (if present) won't be upgraded automatically.

Expand Down
57 changes: 32 additions & 25 deletions common/flagdproxy/flagdproxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type FlagdProxyHandler struct {
}

type CreateUpdateFunc func(ctx context.Context, obj client.Object) error

type FlagdProxyConfiguration struct {
Port int
ManagementPort int
Expand Down Expand Up @@ -78,41 +79,44 @@ func (f *FlagdProxyHandler) HandleFlagdProxy(ctx context.Context) error {
return err
}

ownerReferences := f.getOwnerReferences(ctx)
newDeployment := f.newFlagdProxyManifest(ownerReferences)
newService := f.newFlagdProxyServiceManifest(ownerReferences)
ownerReference, operatorDeployment, 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 it's the right version
if !f.isFlagdProxyUpToDate(deployment, newDeployment) {
f.Log.Info("flagd-proxy Deployment changed, updating")
// flagd-proxy exists, need to check if we should update it
if f.shouldUpdateFlagdProxy(deployment, newDeployment, operatorDeployment) {
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, ff CreateUpdateFunc, deployment *appsV1.Deployment, service *corev1.Service) error {
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 := ff(ctx, deployment); err != nil && !errors.IsAlreadyExists(err) {
if err := createUpdateFunc(ctx, deployment); err != nil && !errors.IsAlreadyExists(err) {
return err
}
f.Log.Info("deploying the flagd-proxy service")
if err := ff(ctx, service); err != nil && !errors.IsAlreadyExists(err) {
if err := createUpdateFunc(ctx, service); err != nil && !errors.IsAlreadyExists(err) {
return err
}
return nil
}

func (f *FlagdProxyHandler) newFlagdProxyServiceManifest(ownerReferences []metav1.OwnerReference) *corev1.Service {
func (f *FlagdProxyHandler) newFlagdProxyServiceManifest(ownerReference *metav1.OwnerReference) *corev1.Service {
return &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: FlagdProxyServiceName,
Namespace: f.config.Namespace,
OwnerReferences: ownerReferences,
OwnerReferences: []metav1.OwnerReference{*ownerReference},
},
Spec: corev1.ServiceSpec{
Selector: map[string]string{
Expand All @@ -130,7 +134,7 @@ func (f *FlagdProxyHandler) newFlagdProxyServiceManifest(ownerReferences []metav
}
}

func (f *FlagdProxyHandler) newFlagdProxyManifest(ownerReferences []metav1.OwnerReference) *appsV1.Deployment {
func (f *FlagdProxyHandler) newFlagdProxyManifest(ownerReference *metav1.OwnerReference) *appsV1.Deployment {
replicas := int32(1)
args := []string{
"start",
Expand All @@ -149,7 +153,7 @@ func (f *FlagdProxyHandler) newFlagdProxyManifest(ownerReferences []metav1.Owner
"app.kubernetes.io/managed-by": ManagedByAnnotationValue,
"app.kubernetes.io/version": f.config.Tag,
},
OwnerReferences: ownerReferences,
OwnerReferences: []metav1.OwnerReference{*ownerReference},
},
Spec: appsV1.DeploymentSpec{
Replicas: &replicas,
Expand Down Expand Up @@ -206,32 +210,35 @@ func (f *FlagdProxyHandler) doesFlagdProxyExist(ctx context.Context) (bool, *app
return true, d, nil
}

func (f *FlagdProxyHandler) isFlagdProxyUpToDate(old, new *appsV1.Deployment) bool {
return reflect.DeepEqual(old.Spec, new.Spec)
func (f *FlagdProxyHandler) shouldUpdateFlagdProxy(old, new, operatorDeployment *appsV1.Deployment) bool {
return reflect.DeepEqual(old.OwnerReferences, []metav1.OwnerReference{*generateOwnerRef(operatorDeployment)}) &&
!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 to create owner reference: %w", err)
return nil, fmt.Errorf("unable to fetch operator deployment: %w", err)
}
return d, nil

}

func (f *FlagdProxyHandler) getOwnerReferences(ctx context.Context) []metav1.OwnerReference {
func (f *FlagdProxyHandler) getOwnerReference(ctx context.Context) (*metav1.OwnerReference, *appsV1.Deployment, error) {
operatorDeployment, err := f.getOperatorDeployment(ctx)
if err != nil {
f.Log.Error(err, "unable to create owner reference for open-feature-operator")
return []metav1.OwnerReference{}
return nil, nil, err
}

return []metav1.OwnerReference{
{
UID: operatorDeployment.GetUID(),
Name: operatorDeployment.GetName(),
APIVersion: operatorDeployment.APIVersion,
Kind: operatorDeployment.Kind,
},
return generateOwnerRef(operatorDeployment), operatorDeployment, nil
}

func generateOwnerRef(operatorDeployment *appsV1.Deployment) *metav1.OwnerReference {
return &metav1.OwnerReference{
UID: operatorDeployment.GetUID(),
Name: operatorDeployment.GetName(),
APIVersion: operatorDeployment.APIVersion,
Kind: operatorDeployment.Kind,
}
}
113 changes: 105 additions & 8 deletions common/flagdproxy/flagdproxy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,63 @@ func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithBadVersion(t *testing

require.NotNil(t, kpConfig)

fakeClient := fake.NewClientBuilder().WithObjects(&v1.Deployment{
fakeClient := fake.NewClientBuilder().WithObjects(createOFOTestDeployment(env.PodNamespace)).Build()

ownerRef, err := getTestOFODeploymentOwnerRef(fakeClient, env.PodNamespace)
require.Nil(t, err)

proxyDeployment := &v1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: kpConfig.Namespace,
Name: FlagdProxyDeploymentName,
OwnerReferences: []metav1.OwnerReference{*ownerRef},
},
Spec: v1.DeploymentSpec{
Template: v12.PodTemplateSpec{
Spec: v12.PodSpec{
Containers: []v12.Container{
{
Name: "my-container",
},
},
},
},
},
}

err = fakeClient.Create(context.TODO(), proxyDeployment)
require.Nil(t, err)

ph := NewFlagdProxyHandler(kpConfig, fakeClient, testr.New(t))

require.NotNil(t, ph)

err = ph.HandleFlagdProxy(context.Background())

require.Nil(t, err)

deployment := &v1.Deployment{}
err = fakeClient.Get(context.Background(), client.ObjectKey{
Namespace: env.PodNamespace,
Name: FlagdProxyDeploymentName,
}, deployment)

require.Nil(t, err)
require.NotNil(t, deployment)

// verify that the existing deployment has been changed
require.Equal(t, "flagd-proxy", deployment.Spec.Template.Spec.Containers[0].Name)
}

func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithDifferentOwnerRef(t *testing.T) {
env := types.EnvConfig{
PodNamespace: "ns",
}
kpConfig := NewFlagdProxyConfiguration(env)

require.NotNil(t, kpConfig)

proxyDeployment := &v1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: kpConfig.Namespace,
Name: FlagdProxyDeploymentName,
Expand All @@ -142,7 +198,9 @@ func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithBadVersion(t *testing
},
},
},
}).Build()
}

fakeClient := fake.NewClientBuilder().WithObjects(createOFOTestDeployment(env.PodNamespace), proxyDeployment).Build()

ph := NewFlagdProxyHandler(kpConfig, fakeClient, testr.New(t))

Expand All @@ -161,8 +219,8 @@ func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithBadVersion(t *testing
require.Nil(t, err)
require.NotNil(t, deployment)

// verify that the existing deployment has been changed
require.Equal(t, "flagd-proxy", deployment.Spec.Template.Spec.Containers[0].Name)
// verify that the existing deployment has not been changed
require.Equal(t, "my-container", deployment.Spec.Template.Spec.Containers[0].Name)
}

func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithNewestVersion(t *testing.T) {
Expand All @@ -173,15 +231,18 @@ func TestFlagdProxyHandler_HandleFlagdProxy_ProxyExistsWithNewestVersion(t *test

require.NotNil(t, kpConfig)

fakeClient := fake.NewClientBuilder().WithObjects().Build()
fakeClient := fake.NewClientBuilder().WithObjects(createOFOTestDeployment(env.PodNamespace)).Build()

ph := NewFlagdProxyHandler(kpConfig, fakeClient, testr.New(t))

require.NotNil(t, ph)

proxy := ph.newFlagdProxyManifest([]metav1.OwnerReference{})
ownerRef, err := getTestOFODeploymentOwnerRef(fakeClient, env.PodNamespace)
require.Nil(t, err)

proxy := ph.newFlagdProxyManifest(ownerRef)

err := fakeClient.Create(context.TODO(), proxy)
err = fakeClient.Create(context.TODO(), proxy)
require.Nil(t, err)

err = ph.HandleFlagdProxy(context.Background())
Expand Down Expand Up @@ -214,7 +275,7 @@ func TestFlagdProxyHandler_HandleFlagdProxy_CreateProxy(t *testing.T) {

require.NotNil(t, kpConfig)

fakeClient := fake.NewClientBuilder().Build()
fakeClient := fake.NewClientBuilder().WithObjects(createOFOTestDeployment(env.PodNamespace)).Build()

ph := NewFlagdProxyHandler(kpConfig, fakeClient, testr.New(t))

Expand Down Expand Up @@ -265,6 +326,13 @@ func TestFlagdProxyHandler_HandleFlagdProxy_CreateProxy(t *testing.T) {
"app.kubernetes.io/version": "tag",
},
ResourceVersion: "1",
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "apps/v1",
Kind: "Deployment",
Name: operatorDeploymentName,
},
},
},
Spec: appsV1.DeploymentSpec{
Replicas: &replicas,
Expand Down Expand Up @@ -326,6 +394,13 @@ func TestFlagdProxyHandler_HandleFlagdProxy_CreateProxy(t *testing.T) {
Name: FlagdProxyServiceName,
Namespace: "ns",
ResourceVersion: "1",
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "apps/v1",
Kind: "Deployment",
Name: operatorDeploymentName,
},
},
},
Spec: corev1.ServiceSpec{
Selector: map[string]string{
Expand All @@ -344,3 +419,25 @@ func TestFlagdProxyHandler_HandleFlagdProxy_CreateProxy(t *testing.T) {

require.Equal(t, expectedService, service)
}

func createOFOTestDeployment(ns string) *v1.Deployment {
return &v1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns,
Name: operatorDeploymentName,
},
}
}

func getTestOFODeploymentOwnerRef(c client.Client, ns string) (*metav1.OwnerReference, error) {
d := &appsV1.Deployment{}
if err := c.Get(context.TODO(), client.ObjectKey{Name: operatorDeploymentName, Namespace: ns}, d); err != nil {
return nil, err
}
return &metav1.OwnerReference{
UID: d.GetUID(),
Name: d.GetName(),
APIVersion: d.APIVersion,
Kind: d.Kind,
}, nil
}
13 changes: 11 additions & 2 deletions controllers/core/featureflagsource/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ func TestFeatureFlagSourceReconciler_Reconcile(t *testing.T) {
// setting up fake k8s client
var fakeClient client.Client
if tt.deployment != nil {
fakeClient = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tt.fsConfig, tt.deployment).WithIndex(&appsv1.Deployment{}, fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationPath, common.FeatureFlagSourceAnnotation), common.FeatureFlagSourceIndex).Build()
fakeClient = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(createOFOTestDeployment(testNamespace), tt.fsConfig, tt.deployment).WithIndex(&appsv1.Deployment{}, fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationPath, common.FeatureFlagSourceAnnotation), common.FeatureFlagSourceIndex).Build()
} else {
fakeClient = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tt.fsConfig).WithIndex(&appsv1.Deployment{}, fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationPath, common.FeatureFlagSourceAnnotation), common.FeatureFlagSourceIndex).Build()
fakeClient = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(createOFOTestDeployment(testNamespace), tt.fsConfig).WithIndex(&appsv1.Deployment{}, fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationPath, common.FeatureFlagSourceAnnotation), common.FeatureFlagSourceIndex).Build()
}
kpConfig := flagdproxy.NewFlagdProxyConfiguration(commontypes.EnvConfig{
FlagdProxyImage: "ghcr.io/open-feature/flagd-proxy",
Expand Down Expand Up @@ -210,3 +210,12 @@ func createTestFSConfig(fsConfigName string, testNamespace string, rollout bool,

return fsConfig
}

func createOFOTestDeployment(ns string) *appsv1.Deployment {
return &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns,
Name: "open-feature-operator-controller-manager",
},
}
}
4 changes: 2 additions & 2 deletions docs/installation.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ helm upgrade --install openfeature openfeature/open-feature-operator
```

> [!NOTE]
> If you have used `flagd-proxy` provider and upgrading to OFO version `v0.5.4` or higher,
> If you upgrade to OFO `v0.5.4` or higher while using a `flagd-proxy` provider, the instance of
`flagd-proxy` will be automatically upgraded to the latest supported version by the `open-feature-operator`.
This upgrade will also consider your current `FeatureFlagSource` configuration and adapt
The upgrade of `flagd-proxy` will also consider your current `FeatureFlagSource` configuration and adapt
the `flagd-proxy` Deployment accordingly.
If you are upgrading OFO to `v0.5.3` or lower, `flagd-proxy` (if present) won't be upgraded automatically.

Expand Down
2 changes: 1 addition & 1 deletion docs/v1beta_migration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,6 @@ If you have used `flagd-proxy` provider, then you have to upgrade the image used
For this, please edit the deployment of `flagd-proxy` to version [v0.3.1](https://github.com/open-feature/flagd/pkgs/container/flagd-proxy/152333134?tag=v0.3.1) or above.

> [!NOTE]
> Since OFO version `v0.5.4`, `flagd-proxy` pod (if present) will be upgraded automatically to the
> Since OFO version `v0.5.4`, the instance of `flagd-proxy` pod (if used) will be upgraded automatically to the
to the latest supported version by `open-feature-operator`.
For more information see the [upgrade section](./installation.md#upgrading).

0 comments on commit 0c66bf7

Please sign in to comment.