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

chore: split controllers to separate packages + cover them with unit tests #404

Merged
merged 9 commits into from
Mar 27, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 1 addition & 7 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,7 @@ vet: ## Run go vet against code.

.PHONY: component-test
component-test: manifests generate fmt vet envtest ## Run tests.
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./controllers/... -coverprofile cover-controllers.out
odubajDT marked this conversation as resolved.
Show resolved Hide resolved
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./webhooks/... -coverprofile cover-webhooks.out
sed -i '/mode: set/d' "cover-controllers.out"
sed -i '/mode: set/d' "cover-webhooks.out"
echo "mode: set" > cover.out
cat cover-controllers.out cover-webhooks.out >> cover.out
rm cover-controllers.out cover-webhooks.out
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./webhooks/... -coverprofile cover.out

.PHONY: unit-test
unit-test: manifests fmt vet generate envtest ## Run tests.
Expand Down
4 changes: 4 additions & 0 deletions apis/core/v1alpha1/featureflagconfiguration_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,3 +145,7 @@ func FeatureFlagConfigurationId(namespace, name string) string {
func FeatureFlagConfigurationConfigMapKey(namespace, name string) string {
return fmt.Sprintf("%s.flagd.json", FeatureFlagConfigurationId(namespace, name))
}

func (p *FeatureFlagServiceProvider) IsSet() bool {
return p != nil && p.Name != ""
}
42 changes: 42 additions & 0 deletions controllers/common/common.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package common

import (
"fmt"
"time"

appsV1 "k8s.io/api/apps/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
)

const (
CrdName = "FeatureFlagConfiguration"
ReconcileErrorInterval = 10 * time.Second
ReconcileSuccessInterval = 120 * time.Second
FinalizerName = "featureflagconfiguration.core.openfeature.dev/finalizer"
OpenFeatureAnnotationPath = "spec.template.metadata.annotations.openfeature.dev/openfeature.dev"
FlagSourceConfigurationAnnotation = "flagsourceconfiguration"
OpenFeatureAnnotationRoot = "openfeature.dev"
)

func FlagSourceConfigurationIndex(o client.Object) []string {
deployment, ok := o.(*appsV1.Deployment)
if !ok {
return []string{
"false",
}
}

if deployment.Spec.Template.ObjectMeta.Annotations == nil {
return []string{
"false",
}
}
if _, ok := deployment.Spec.Template.ObjectMeta.Annotations[fmt.Sprintf("openfeature.dev/%s", FlagSourceConfigurationAnnotation)]; ok {
return []string{
"true",
}
}
return []string{
"false",
}
}
69 changes: 69 additions & 0 deletions controllers/common/common_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package common

import (
"fmt"
"testing"

"github.com/stretchr/testify/require"
appsV1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
)

func TestFlagSourceConfigurationIndex(t *testing.T) {
tests := []struct {
name string
obj client.Object
out []string
}{
{
name: "non-deployment object",
obj: &appsV1.DaemonSet{},
out: []string{"false"},
},
{
name: "no annotations",
obj: &appsV1.Deployment{},
out: []string{"false"},
},
{
name: "not existing right annotation",
obj: &appsV1.Deployment{
Spec: appsV1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"silly": "some",
},
},
},
},
},
out: []string{"false"},
},
{
name: "existing annotation",
obj: &appsV1.Deployment{
Spec: appsV1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
fmt.Sprintf("openfeature.dev/%s", FlagSourceConfigurationAnnotation): "true",
},
},
},
},
},
out: []string{"true"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
out := FlagSourceConfigurationIndex(tt.obj)
require.Equal(t, tt.out, out)
})

}
}
100 changes: 0 additions & 100 deletions controllers/controllers_test.go

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,23 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package controllers
package featureflagconfiguration

import (
"context"
"time"

"github.com/go-logr/logr"
"github.com/open-feature/open-feature-operator/controllers/common"
"github.com/open-feature/open-feature-operator/pkg/utils"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/predicate"

"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"

corev1alpha1 "github.com/open-feature/open-feature-operator/apis/core/v1alpha1"
)
Expand All @@ -41,8 +41,6 @@ type FeatureFlagConfigurationReconciler struct {

// Scheme contains the scheme of this controller
Scheme *runtime.Scheme
// Recorder contains the Recorder of this controller
Recorder record.EventRecorder
// ReqLogger contains the Logger of this controller
Log logr.Logger
}
Expand All @@ -61,42 +59,36 @@ type FeatureFlagConfigurationReconciler struct {
// For more details, check Reconcile and its Result here:
// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.11.0/pkg/reconcile

const (
crdName = "FeatureFlagConfiguration"
reconcileErrorInterval = 10 * time.Second
reconcileSuccessInterval = 120 * time.Second
finalizerName = "featureflagconfiguration.core.openfeature.dev/finalizer"
)
const CrdName = "FeatureFlagConfiguration"

func (r *FeatureFlagConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
r.Log = log.FromContext(ctx)
r.Log.Info("Reconciling" + crdName)
r.Log.Info("Reconciling" + CrdName)

ffconf := &corev1alpha1.FeatureFlagConfiguration{}
if err := r.Client.Get(ctx, req.NamespacedName, ffconf); err != nil {
if errors.IsNotFound(err) {
// taking down all associated K8s resources is handled by K8s
r.Log.Info(crdName + " resource not found. Ignoring since object must be deleted")
r.Log.Info(CrdName + " resource not found. Ignoring since object must be deleted")
return r.finishReconcile(nil, false)
}
r.Log.Error(err, "Failed to get the "+crdName)
r.Log.Error(err, "Failed to get the "+CrdName)
return r.finishReconcile(err, false)
}

if ffconf.ObjectMeta.DeletionTimestamp.IsZero() {
// The object is not being deleted, so if it does not have our finalizer,
// then lets add the finalizer and update the object. This is equivalent
// registering our finalizer.
if !utils.ContainsString(ffconf.GetFinalizers(), finalizerName) {
controllerutil.AddFinalizer(ffconf, finalizerName)
if !utils.ContainsString(ffconf.GetFinalizers(), common.FinalizerName) {
controllerutil.AddFinalizer(ffconf, common.FinalizerName)
if err := r.Update(ctx, ffconf); err != nil {
return r.finishReconcile(err, false)
}
}
} else {
// The object is being deleted
if utils.ContainsString(ffconf.GetFinalizers(), finalizerName) {
controllerutil.RemoveFinalizer(ffconf, finalizerName)
if utils.ContainsString(ffconf.GetFinalizers(), common.FinalizerName) {
controllerutil.RemoveFinalizer(ffconf, common.FinalizerName)
if err := r.Update(ctx, ffconf); err != nil {
return ctrl.Result{}, err
}
Expand All @@ -106,7 +98,7 @@ func (r *FeatureFlagConfigurationReconciler) Reconcile(ctx context.Context, req
}

// Check the provider on the FeatureFlagConfiguration
if ffconf.Spec.ServiceProvider == nil {
if !ffconf.Spec.ServiceProvider.IsSet() {
r.Log.Info("No service provider specified for FeatureFlagConfiguration, using FlagD")
ffconf.Spec.ServiceProvider = &corev1alpha1.FeatureFlagServiceProvider{
Name: "flagd",
Expand Down Expand Up @@ -161,28 +153,20 @@ func (r *FeatureFlagConfigurationReconciler) Reconcile(ctx context.Context, req
return r.finishReconcile(nil, false)
}

// SetupWithManager sets up the controller with the Manager.
func (r *FeatureFlagConfigurationReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&corev1alpha1.FeatureFlagConfiguration{}).
Owns(&corev1.ConfigMap{}).
Complete(r)
}

func (r *FeatureFlagConfigurationReconciler) finishReconcile(err error, requeueImmediate bool) (ctrl.Result, error) {
if err != nil {
interval := reconcileErrorInterval
interval := common.ReconcileErrorInterval
if requeueImmediate {
interval = 0
}
r.Log.Error(err, "Finished Reconciling "+crdName+" with error: %w")
r.Log.Error(err, "Finished Reconciling "+CrdName+" with error: %w")
return ctrl.Result{Requeue: true, RequeueAfter: interval}, err
}
interval := reconcileSuccessInterval
interval := common.ReconcileSuccessInterval
if requeueImmediate {
interval = 0
}
r.Log.Info("Finished Reconciling " + crdName)
r.Log.Info("Finished Reconciling " + CrdName)
return ctrl.Result{Requeue: true, RequeueAfter: interval}, nil
}

Expand All @@ -194,3 +178,11 @@ func (r *FeatureFlagConfigurationReconciler) featureFlagResourceIsOwner(ff *core
}
return false
}

// SetupWithManager sets up the controller with the Manager.
func (r *FeatureFlagConfigurationReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&corev1alpha1.FeatureFlagConfiguration{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
Owns(&corev1.ConfigMap{}).
Complete(r)
}
Loading