Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: auto-upgrade flagd-proxy with OFO upgrades #596

Merged
merged 9 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions chart/open-feature-operator/README.md
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
59 changes: 33 additions & 26 deletions common/flagdproxy/flagdproxy.go
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
odubajDT marked this conversation as resolved.
Show resolved Hide resolved

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, 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) {
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 *appsV1.Deployment) bool {
return isDeployedByOFO(old) && !reflect.DeepEqual(old.Spec, new.Spec)
odubajDT marked this conversation as resolved.
Show resolved Hide resolved
}

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, 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, err
}

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

func isDeployedByOFO(d *appsV1.Deployment) bool {
val, ok := d.Labels["app.kubernetes.io/managed-by"]
return ok && val == ManagedByAnnotationValue
}
116 changes: 108 additions & 8 deletions common/flagdproxy/flagdproxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,66 @@ 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},
Labels: map[string]string{
"app.kubernetes.io/managed-by": ManagedByAnnotationValue,
},
},
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_ProxyExistsWithoutLabel(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 +201,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 +222,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 +234,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 +278,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 +329,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 +397,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 +422,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
}
4 changes: 2 additions & 2 deletions config/manager/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
images:
- name: controller
newName: docker.io/odubajdt/operator
newTag: proxy-update
newName: controller
newTag: latest
13 changes: 11 additions & 2 deletions controllers/core/featureflagsource/controller_test.go
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
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
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).
Loading