From 72eb94702e327fb807020a071a323e2098ec1076 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 17 Mar 2023 09:38:30 +0100 Subject: [PATCH 1/9] chore: move controllers to separate packages Signed-off-by: odubajDT --- controllers/common/common.go | 36 ++++++++++++++ .../featureflagconfiguration/controller.go} | 49 ++++++++----------- .../flagsourceconfiguration/controller.go} | 38 +++----------- .../controllers_test.go | 7 +-- .../flagsourceconfiguration}/suite_test.go | 7 +-- main.go | 13 +++-- 6 files changed, 81 insertions(+), 69 deletions(-) create mode 100644 controllers/common/common.go rename controllers/{featureflagconfiguration_controller.go => core/featureflagconfiguration/controller.go} (87%) rename controllers/{flagsourceconfiguration_controller.go => core/flagsourceconfiguration/controller.go} (81%) rename controllers/{ => core/flagsourceconfiguration}/controllers_test.go (91%) rename controllers/{ => core/flagsourceconfiguration}/suite_test.go (93%) diff --git a/controllers/common/common.go b/controllers/common/common.go new file mode 100644 index 000000000..e506877eb --- /dev/null +++ b/controllers/common/common.go @@ -0,0 +1,36 @@ +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 := o.(*appsV1.Deployment) + 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", + } +} diff --git a/controllers/featureflagconfiguration_controller.go b/controllers/core/featureflagconfiguration/controller.go similarity index 87% rename from controllers/featureflagconfiguration_controller.go rename to controllers/core/featureflagconfiguration/controller.go index ba337886b..efca5b930 100644 --- a/controllers/featureflagconfiguration_controller.go +++ b/controllers/core/featureflagconfiguration/controller.go @@ -14,13 +14,13 @@ 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" @@ -61,25 +61,18 @@ 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" -) - 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" + common.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(common.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 "+common.CrdName) return r.finishReconcile(err, false) } @@ -87,16 +80,16 @@ func (r *FeatureFlagConfigurationReconciler) Reconcile(ctx context.Context, req // 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 } @@ -161,28 +154,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 "+common.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 " + common.CrdName) return ctrl.Result{Requeue: true, RequeueAfter: interval}, nil } @@ -194,3 +179,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{}). + Owns(&corev1.ConfigMap{}). + Complete(r) +} diff --git a/controllers/flagsourceconfiguration_controller.go b/controllers/core/flagsourceconfiguration/controller.go similarity index 81% rename from controllers/flagsourceconfiguration_controller.go rename to controllers/core/flagsourceconfiguration/controller.go index d0b691afe..9a883bfce 100644 --- a/controllers/flagsourceconfiguration_controller.go +++ b/controllers/core/flagsourceconfiguration/controller.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package controllers +package flagsourceconfiguration import ( "context" @@ -22,6 +22,7 @@ import ( "strings" "time" + "github.com/open-feature/open-feature-operator/controllers/common" appsV1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -34,12 +35,6 @@ import ( corev1alpha1 "github.com/open-feature/open-feature-operator/apis/core/v1alpha1" ) -const ( - OpenFeatureAnnotationPath = "spec.template.metadata.annotations.openfeature.dev/openfeature.dev" - FlagSourceConfigurationAnnotation = "flagsourceconfiguration" - OpenFeatureAnnotationRoot = "openfeature.dev" -) - // FlagSourceConfigurationReconciler reconciles a FlagSourceConfiguration object type FlagSourceConfigurationReconciler struct { client.Client @@ -82,9 +77,9 @@ func (r *FlagSourceConfigurationReconciler) Reconcile(ctx context.Context, req c // and our resource exists within the cluster deployList := &appsV1.DeploymentList{} if err := r.Client.List(ctx, deployList, client.MatchingFields{ - fmt.Sprintf("%s/%s", OpenFeatureAnnotationPath, FlagSourceConfigurationAnnotation): "true", + fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationPath, common.FlagSourceConfigurationAnnotation): "true", }); err != nil { - r.Log.Error(err, fmt.Sprintf("Failed to get the pods with annotation %s/%s", OpenFeatureAnnotationPath, FlagSourceConfigurationAnnotation)) + r.Log.Error(err, fmt.Sprintf("Failed to get the pods with annotation %s/%s", common.OpenFeatureAnnotationPath, common.FlagSourceConfigurationAnnotation)) return r.finishReconcile(err, false) } @@ -92,11 +87,11 @@ func (r *FlagSourceConfigurationReconciler) Reconcile(ctx context.Context, req c // and trigger a restart for any which have our resource listed as a configuration for _, deployment := range deployList.Items { annotations := deployment.Spec.Template.Annotations - annotation, ok := annotations[fmt.Sprintf("%s/%s", OpenFeatureAnnotationRoot, FlagSourceConfigurationAnnotation)] + annotation, ok := annotations[fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationRoot, common.FlagSourceConfigurationAnnotation)] if !ok { continue } - if isUsingConfiguration(fsConfig.Namespace, fsConfig.Name, deployment.Namespace, annotation) { + if r.isUsingConfiguration(fsConfig.Namespace, fsConfig.Name, deployment.Namespace, annotation) { r.Log.Info(fmt.Sprintf("restarting deployment %s/%s", deployment.Namespace, deployment.Name)) deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"] = time.Now().Format(time.RFC3339) if err := r.Client.Update(ctx, &deployment); err != nil { @@ -109,7 +104,7 @@ func (r *FlagSourceConfigurationReconciler) Reconcile(ctx context.Context, req c return r.finishReconcile(nil, false) } -func isUsingConfiguration(namespace string, name string, deploymentNamespace string, annotation string) bool { +func (r *FlagSourceConfigurationReconciler) isUsingConfiguration(namespace string, name string, deploymentNamespace string, annotation string) bool { s := strings.Split(annotation, ",") // parse annotation list for _, target := range s { ss := strings.Split(strings.TrimSpace(target), "/") @@ -125,7 +120,7 @@ func isUsingConfiguration(namespace string, name string, deploymentNamespace str func (r *FlagSourceConfigurationReconciler) finishReconcile(err error, requeueImmediate bool) (ctrl.Result, error) { if err != nil { - interval := reconcileErrorInterval + interval := common.ReconcileErrorInterval if requeueImmediate { interval = 0 } @@ -136,23 +131,6 @@ func (r *FlagSourceConfigurationReconciler) finishReconcile(err error, requeueIm return ctrl.Result{Requeue: false}, nil } -func FlagSourceConfigurationIndex(o client.Object) []string { - deployment := o.(*appsV1.Deployment) - 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", - } -} - // SetupWithManager sets up the controller with the Manager. func (r *FlagSourceConfigurationReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/controllers/controllers_test.go b/controllers/core/flagsourceconfiguration/controllers_test.go similarity index 91% rename from controllers/controllers_test.go rename to controllers/core/flagsourceconfiguration/controllers_test.go index aeea656ed..a2b8fd97c 100644 --- a/controllers/controllers_test.go +++ b/controllers/core/flagsourceconfiguration/controllers_test.go @@ -1,4 +1,4 @@ -package controllers +package flagsourceconfiguration import ( "fmt" @@ -6,6 +6,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/open-feature/open-feature-operator/apis/core/v1alpha1" + "github.com/open-feature/open-feature-operator/controllers/common" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -23,8 +24,8 @@ func createTestDeployment() { deploy.Name = "test-deploy" deploy.Namespace = testNamespace deploy.Spec.Template.ObjectMeta.Annotations = map[string]string{} - deploy.Spec.Template.ObjectMeta.Annotations[fmt.Sprintf("%s/%s", OpenFeatureAnnotationRoot, "enabled")] = "true" - deploy.Spec.Template.ObjectMeta.Annotations[fmt.Sprintf("%s/%s", OpenFeatureAnnotationRoot, FlagSourceConfigurationAnnotation)] = fmt.Sprintf("%s/%s", testNamespace, fsConfigName) + deploy.Spec.Template.ObjectMeta.Annotations[fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationRoot, "enabled")] = "true" + deploy.Spec.Template.ObjectMeta.Annotations[fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationRoot, common.FlagSourceConfigurationAnnotation)] = fmt.Sprintf("%s/%s", testNamespace, fsConfigName) deploy.Spec.Selector = &metav1.LabelSelector{ MatchLabels: map[string]string{ "app": "test", diff --git a/controllers/suite_test.go b/controllers/core/flagsourceconfiguration/suite_test.go similarity index 93% rename from controllers/suite_test.go rename to controllers/core/flagsourceconfiguration/suite_test.go index 6c697a69e..1d4507707 100644 --- a/controllers/suite_test.go +++ b/controllers/core/flagsourceconfiguration/suite_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package controllers +package flagsourceconfiguration import ( "context" @@ -25,6 +25,7 @@ import ( corev1alpha1 "github.com/open-feature/open-feature-operator/apis/core/v1alpha1" corev1alpha2 "github.com/open-feature/open-feature-operator/apis/core/v1alpha2" corev1alpha3 "github.com/open-feature/open-feature-operator/apis/core/v1alpha3" + "github.com/open-feature/open-feature-operator/controllers/common" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -105,8 +106,8 @@ var _ = BeforeSuite(func() { err = mgr.GetFieldIndexer().IndexField( context.Background(), &appsV1.Deployment{}, - fmt.Sprintf("%s/%s", OpenFeatureAnnotationPath, FlagSourceConfigurationAnnotation), - FlagSourceConfigurationIndex, + fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationPath, common.FlagSourceConfigurationAnnotation), + common.FlagSourceConfigurationIndex, ) Expect(err).ToNot(HaveOccurred()) diff --git a/main.go b/main.go index f4d7f7d5d..ee7f2c56d 100644 --- a/main.go +++ b/main.go @@ -27,6 +27,8 @@ import ( // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. + "github.com/open-feature/open-feature-operator/controllers/common" + controllercommon "github.com/open-feature/open-feature-operator/controllers/common" "go.uber.org/zap/zapcore" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -40,7 +42,8 @@ import ( corev1alpha1 "github.com/open-feature/open-feature-operator/apis/core/v1alpha1" corev1alpha2 "github.com/open-feature/open-feature-operator/apis/core/v1alpha2" corev1alpha3 "github.com/open-feature/open-feature-operator/apis/core/v1alpha3" - "github.com/open-feature/open-feature-operator/controllers" + "github.com/open-feature/open-feature-operator/controllers/core/featureflagconfiguration" + "github.com/open-feature/open-feature-operator/controllers/core/flagsourceconfiguration" webhooks "github.com/open-feature/open-feature-operator/webhooks" appsV1 "k8s.io/api/apps/v1" //+kubebuilder:scaffold:imports @@ -196,8 +199,8 @@ func main() { if err := mgr.GetFieldIndexer().IndexField( context.Background(), &appsV1.Deployment{}, - fmt.Sprintf("%s/%s", controllers.OpenFeatureAnnotationPath, controllers.FlagSourceConfigurationAnnotation), - controllers.FlagSourceConfigurationIndex, + fmt.Sprintf("%s/%s", controllercommon.OpenFeatureAnnotationPath, common.FlagSourceConfigurationAnnotation), + common.FlagSourceConfigurationIndex, ); err != nil { setupLog.Error( err, @@ -208,7 +211,7 @@ func main() { os.Exit(1) } - if err = (&controllers.FeatureFlagConfigurationReconciler{ + if err = (&featureflagconfiguration.FeatureFlagConfigurationReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), }).SetupWithManager(mgr); err != nil { @@ -221,7 +224,7 @@ func main() { os.Exit(1) } - if err = (&controllers.FlagSourceConfigurationReconciler{ + if err = (&flagsourceconfiguration.FlagSourceConfigurationReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), }).SetupWithManager(mgr); err != nil { From 66e009cb71a36f5f360ae518c22cb11f057cc678 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 17 Mar 2023 10:06:57 +0100 Subject: [PATCH 2/9] fix tests Signed-off-by: odubajDT --- controllers/core/flagsourceconfiguration/suite_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/core/flagsourceconfiguration/suite_test.go b/controllers/core/flagsourceconfiguration/suite_test.go index 1d4507707..2d5462316 100644 --- a/controllers/core/flagsourceconfiguration/suite_test.go +++ b/controllers/core/flagsourceconfiguration/suite_test.go @@ -78,7 +78,7 @@ var _ = BeforeSuite(func() { Expect(err).ToNot(HaveOccurred()) testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")}, + CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")}, ErrorIfCRDPathMissing: true, Scheme: scheme, CRDInstallOptions: envtest.CRDInstallOptions{Scheme: scheme}, From 99829526d771adfe92d7df0eee48a0c405fc905b Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 17 Mar 2023 11:34:17 +0100 Subject: [PATCH 3/9] migrate ginkgo test to standard unit test Signed-off-by: odubajDT --- Makefile | 8 +- .../flagsourceconfiguration/controller.go | 3 +- .../controller_test.go | 126 +++++++++++++++++ .../controllers_test.go | 101 ------------- .../flagsourceconfiguration/suite_test.go | 133 ------------------ 5 files changed, 128 insertions(+), 243 deletions(-) create mode 100644 controllers/core/flagsourceconfiguration/controller_test.go delete mode 100644 controllers/core/flagsourceconfiguration/controllers_test.go delete mode 100644 controllers/core/flagsourceconfiguration/suite_test.go diff --git a/Makefile b/Makefile index 217dba7da..91dfbbeb4 100644 --- a/Makefile +++ b/Makefile @@ -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 - 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. diff --git a/controllers/core/flagsourceconfiguration/controller.go b/controllers/core/flagsourceconfiguration/controller.go index 9a883bfce..0c8c23eb5 100644 --- a/controllers/core/flagsourceconfiguration/controller.go +++ b/controllers/core/flagsourceconfiguration/controller.go @@ -28,7 +28,6 @@ import ( "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" "sigs.k8s.io/controller-runtime/pkg/predicate" "github.com/go-logr/logr" @@ -54,7 +53,7 @@ type FlagSourceConfigurationReconciler struct { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.13.0/pkg/reconcile func (r *FlagSourceConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - r.Log = log.FromContext(ctx) + r.Log.Info("Searching for FlagSourceConfiguration") // Fetch the FlagSourceConfiguration from the cache fsConfig := &corev1alpha1.FlagSourceConfiguration{} diff --git a/controllers/core/flagsourceconfiguration/controller_test.go b/controllers/core/flagsourceconfiguration/controller_test.go new file mode 100644 index 000000000..d5aa48431 --- /dev/null +++ b/controllers/core/flagsourceconfiguration/controller_test.go @@ -0,0 +1,126 @@ +package flagsourceconfiguration + +import ( + "context" + "fmt" + "testing" + + "github.com/open-feature/open-feature-operator/apis/core/v1alpha1" + "github.com/open-feature/open-feature-operator/controllers/common" + "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" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { + const ( + testNamespace = "test-namespace" + fsConfigName = "test-config" + deploymentName = "test-deploy" + ) + + deployment := createTestDeployment(fsConfigName, testNamespace, deploymentName) + fsConfig := createTestFSConfig(fsConfigName, testNamespace, deploymentName) + + err := v1alpha1.AddToScheme(scheme.Scheme) + require.Nil(t, err) + + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(deployment, fsConfig).WithIndex(&appsv1.Deployment{}, fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationPath, common.FlagSourceConfigurationAnnotation), common.FlagSourceConfigurationIndex).Build() + + r := &FlagSourceConfigurationReconciler{ + Client: fakeClient, + Log: ctrl.Log.WithName("flagsourceconfiguration-controller"), + Scheme: fakeClient.Scheme(), + } + + req := ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: testNamespace, + Name: fsConfigName, + }, + } + + ctx := context.TODO() + + deployment2 := &appsv1.Deployment{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment2) + require.Nil(t, err) + restartAt := deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"] + require.Equal(t, "", restartAt) + + r.Reconcile(ctx, req) + + err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment) + require.Nil(t, err) + + require.NotEqual(t, restartAt, deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"]) + +} + +func createTestDeployment(fsConfigName string, testNamespace string, deploymentName string) *appsv1.Deployment { + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: deploymentName, + Namespace: testNamespace, + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationPath, common.FlagSourceConfigurationAnnotation): "true", + fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationRoot, common.FlagSourceConfigurationAnnotation): fmt.Sprintf("%s/%s", testNamespace, fsConfigName), + }, + Labels: map[string]string{ + "app": "test", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test", + Image: "busybox", + Args: []string{ + "sleep", + "1000", + }, + }, + }, + }, + }, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "test", + }, + }, + }, + } + + return deployment +} + +func createTestFSConfig(fsConfigName string, testNamespace string, deploymentName string) *v1alpha1.FlagSourceConfiguration { + rolloutOnChange := true + fsConfig := &v1alpha1.FlagSourceConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: fsConfigName, + Namespace: testNamespace, + }, + Spec: v1alpha1.FlagSourceConfigurationSpec{ + Image: deploymentName, + Sources: []v1alpha1.Source{ + { + Source: "not-real.com", + Provider: "http", + }, + }, + RolloutOnChange: &rolloutOnChange, + }, + } + + return fsConfig +} diff --git a/controllers/core/flagsourceconfiguration/controllers_test.go b/controllers/core/flagsourceconfiguration/controllers_test.go deleted file mode 100644 index a2b8fd97c..000000000 --- a/controllers/core/flagsourceconfiguration/controllers_test.go +++ /dev/null @@ -1,101 +0,0 @@ -package flagsourceconfiguration - -import ( - "fmt" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/open-feature/open-feature-operator/apis/core/v1alpha1" - "github.com/open-feature/open-feature-operator/controllers/common" - 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" -) - -const ( - testNamespace = "test-namespace" - fsConfigName = "test-config" - deploymentName = "test-deploy" -) - -func createTestDeployment() { - deploy := &appsv1.Deployment{} - deploy.Name = "test-deploy" - deploy.Namespace = testNamespace - deploy.Spec.Template.ObjectMeta.Annotations = map[string]string{} - deploy.Spec.Template.ObjectMeta.Annotations[fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationRoot, "enabled")] = "true" - deploy.Spec.Template.ObjectMeta.Annotations[fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationRoot, common.FlagSourceConfigurationAnnotation)] = fmt.Sprintf("%s/%s", testNamespace, fsConfigName) - deploy.Spec.Selector = &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "test", - }, - } - deploy.Spec.Template.ObjectMeta.Labels = map[string]string{ - "app": "test", - } - deploy.Spec.Template.Spec.Containers = []corev1.Container{ - { - Name: "test", - Image: "busybox", - Args: []string{ - "sleep", - "1000", - }, - }, - } - err := k8sClient.Create(testCtx, deploy) - Expect(err).ShouldNot(HaveOccurred()) -} - -func createTestFSConfig() *v1alpha1.FlagSourceConfiguration { - fsConfig := &v1alpha1.FlagSourceConfiguration{} - rolloutOnChange := true - fsConfig.Namespace = testNamespace - fsConfig.Name = fsConfigName - fsConfig.Spec.Image = deploymentName - fsConfig.Spec.Sources = []v1alpha1.Source{ - { - Source: "not-real.com", - Provider: "http", - }, - } - fsConfig.Spec.RolloutOnChange = &rolloutOnChange - err := k8sClient.Create(testCtx, fsConfig) - Expect(err).ShouldNot(HaveOccurred()) - return fsConfig -} - -func setup() { - ns := &corev1.Namespace{} - ns.Name = testNamespace - err := k8sClient.Create(testCtx, ns) - Expect(err).ShouldNot(HaveOccurred()) -} - -var _ = Describe("flagsourceconfiguration controller tests", func() { - - It("should restart annotated deployments", func() { - config := createTestFSConfig() - createTestDeployment() - - // get deployment + set var equal to restartedAt annotation value - deployment := &appsv1.Deployment{} - err := k8sClient.Get(testCtx, client.ObjectKey{Name: deploymentName, Namespace: testNamespace}, deployment) - Expect(err).ShouldNot(HaveOccurred()) - restartAt := deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"] - - // update the fsconfig - config.Spec.Image = "image-2" - err = k8sClient.Update(testCtx, config) - Expect(err).ShouldNot(HaveOccurred()) - - // fetch deployment and test if it has been updated - Eventually(func(g Gomega) { - err := k8sClient.Get(testCtx, client.ObjectKey{Name: deploymentName, Namespace: testNamespace}, deployment) - g.Expect(err).To(BeNil()) - g.Expect(deployment).To(Not(BeNil())) - g.Expect(deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"]).NotTo(BeEquivalentTo(restartAt)) - }, "30s").Should(Succeed()) - }) -}) diff --git a/controllers/core/flagsourceconfiguration/suite_test.go b/controllers/core/flagsourceconfiguration/suite_test.go deleted file mode 100644 index 2d5462316..000000000 --- a/controllers/core/flagsourceconfiguration/suite_test.go +++ /dev/null @@ -1,133 +0,0 @@ -/* -Copyright 2022. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package flagsourceconfiguration - -import ( - "context" - "fmt" - "path/filepath" - "testing" - - corev1alpha1 "github.com/open-feature/open-feature-operator/apis/core/v1alpha1" - corev1alpha2 "github.com/open-feature/open-feature-operator/apis/core/v1alpha2" - corev1alpha3 "github.com/open-feature/open-feature-operator/apis/core/v1alpha3" - "github.com/open-feature/open-feature-operator/controllers/common" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - appsV1 "k8s.io/api/apps/v1" - "k8s.io/apimachinery/pkg/runtime" - clientgoscheme "k8s.io/client-go/kubernetes/scheme" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/envtest" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/log/zap" - //+kubebuilder:scaffold:imports -) - -// These tests use Ginkgo (BDD-style Go testing framework). Refer to -// http://onsi.github.io/ginkgo/ to learn more about Ginkgo. - -var ( - k8sClient client.Client - testEnv *envtest.Environment - testCtx, testCancel = context.WithCancel(context.Background()) -) - -func TestAPIs(t *testing.T) { - if testing.Short() { - t.Skip() - } - - RegisterFailHandler(Fail) - - RunSpecs(t, "Controller Suite") -} - -var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) - - By("bootstrapping test environment") - - scheme := runtime.NewScheme() - err := clientgoscheme.AddToScheme(scheme) - Expect(err).ToNot(HaveOccurred()) - - err = corev1alpha1.AddToScheme(scheme) - Expect(err).ToNot(HaveOccurred()) - - err = corev1alpha2.AddToScheme(scheme) - Expect(err).ToNot(HaveOccurred()) - - err = corev1alpha3.AddToScheme(scheme) - Expect(err).ToNot(HaveOccurred()) - - testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")}, - ErrorIfCRDPathMissing: true, - Scheme: scheme, - CRDInstallOptions: envtest.CRDInstallOptions{Scheme: scheme}, - } - - cfg, err := testEnv.Start() - Expect(err).NotTo(HaveOccurred()) - Expect(cfg).NotTo(BeNil()) - - //+kubebuilder:scaffold:scheme - - k8sClient, err = client.New(cfg, client.Options{Scheme: scheme}) - Expect(err).NotTo(HaveOccurred()) - Expect(k8sClient).NotTo(BeNil()) - - mgr, err := ctrl.NewManager(cfg, ctrl.Options{ - Scheme: scheme, - LeaderElection: false, - Host: testEnv.WebhookInstallOptions.LocalServingHost, - Port: testEnv.WebhookInstallOptions.LocalServingPort, - CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir, - }) - Expect(err).ToNot(HaveOccurred()) - - err = mgr.GetFieldIndexer().IndexField( - context.Background(), - &appsV1.Deployment{}, - fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationPath, common.FlagSourceConfigurationAnnotation), - common.FlagSourceConfigurationIndex, - ) - Expect(err).ToNot(HaveOccurred()) - - err = (&FlagSourceConfigurationReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - }).SetupWithManager(mgr) - Expect(err).ToNot(HaveOccurred()) - - go func() { - err := mgr.Start(testCtx) - Expect(err).ToNot(HaveOccurred()) - }() - - setup() -}) - -var _ = AfterSuite(func() { - By("tearing down the test environment") - testCancel() - err := testEnv.Stop() - Expect(err).NotTo(HaveOccurred()) -}) From 1b26b0b3084ea5698530346f5d3fac77a3976801 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 17 Mar 2023 11:57:24 +0100 Subject: [PATCH 4/9] adding unit tests + polishing Signed-off-by: odubajDT --- controllers/common/common.go | 8 ++- controllers/common/common_test.go | 69 +++++++++++++++++++ .../controller_test.go | 3 +- 3 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 controllers/common/common_test.go diff --git a/controllers/common/common.go b/controllers/common/common.go index e506877eb..a4c7f5927 100644 --- a/controllers/common/common.go +++ b/controllers/common/common.go @@ -19,7 +19,13 @@ const ( ) func FlagSourceConfigurationIndex(o client.Object) []string { - deployment := o.(*appsV1.Deployment) + deployment, ok := o.(*appsV1.Deployment) + if !ok { + return []string{ + "false", + } + } + if deployment.Spec.Template.ObjectMeta.Annotations == nil { return []string{ "false", diff --git a/controllers/common/common_test.go b/controllers/common/common_test.go new file mode 100644 index 000000000..1e93c5860 --- /dev/null +++ b/controllers/common/common_test.go @@ -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) + }) + + } +} diff --git a/controllers/core/flagsourceconfiguration/controller_test.go b/controllers/core/flagsourceconfiguration/controller_test.go index d5aa48431..d9289fd59 100644 --- a/controllers/core/flagsourceconfiguration/controller_test.go +++ b/controllers/core/flagsourceconfiguration/controller_test.go @@ -53,7 +53,8 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { restartAt := deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"] require.Equal(t, "", restartAt) - r.Reconcile(ctx, req) + _, err = r.Reconcile(ctx, req) + require.Nil(t, err) err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment) require.Nil(t, err) From 8437f6caf57b15566190ccc985b5794e5a9c0ae4 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Tue, 21 Mar 2023 07:40:34 +0100 Subject: [PATCH 5/9] additional unit tests for controllers Signed-off-by: odubajDT --- .../featureflagconfiguration/controller.go | 21 ++- .../controller_test.go | 168 ++++++++++++++++++ .../flagsourceconfiguration/controller.go | 3 +- .../controller_test.go | 93 +++++++--- main.go | 7 +- 5 files changed, 252 insertions(+), 40 deletions(-) create mode 100644 controllers/core/featureflagconfiguration/controller_test.go diff --git a/controllers/core/featureflagconfiguration/controller.go b/controllers/core/featureflagconfiguration/controller.go index efca5b930..3c10ec0da 100644 --- a/controllers/core/featureflagconfiguration/controller.go +++ b/controllers/core/featureflagconfiguration/controller.go @@ -24,13 +24,13 @@ import ( "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" ) @@ -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 } @@ -61,18 +59,19 @@ 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" + func (r *FeatureFlagConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - r.Log = log.FromContext(ctx) - r.Log.Info("Reconciling" + common.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(common.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 "+common.CrdName) + r.Log.Error(err, "Failed to get the "+CrdName) return r.finishReconcile(err, false) } @@ -160,14 +159,14 @@ func (r *FeatureFlagConfigurationReconciler) finishReconcile(err error, requeueI if requeueImmediate { interval = 0 } - r.Log.Error(err, "Finished Reconciling "+common.CrdName+" with error: %w") + r.Log.Error(err, "Finished Reconciling "+CrdName+" with error: %w") return ctrl.Result{Requeue: true, RequeueAfter: interval}, err } interval := common.ReconcileSuccessInterval if requeueImmediate { interval = 0 } - r.Log.Info("Finished Reconciling " + common.CrdName) + r.Log.Info("Finished Reconciling " + CrdName) return ctrl.Result{Requeue: true, RequeueAfter: interval}, nil } @@ -183,7 +182,7 @@ func (r *FeatureFlagConfigurationReconciler) featureFlagResourceIsOwner(ff *core // SetupWithManager sets up the controller with the Manager. func (r *FeatureFlagConfigurationReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&corev1alpha1.FeatureFlagConfiguration{}). + For(&corev1alpha1.FeatureFlagConfiguration{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Owns(&corev1.ConfigMap{}). Complete(r) } diff --git a/controllers/core/featureflagconfiguration/controller_test.go b/controllers/core/featureflagconfiguration/controller_test.go new file mode 100644 index 000000000..854a0f455 --- /dev/null +++ b/controllers/core/featureflagconfiguration/controller_test.go @@ -0,0 +1,168 @@ +package featureflagconfiguration + +import ( + "context" + "testing" + + "github.com/open-feature/open-feature-operator/apis/core/v1alpha1" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { + const ( + testNamespace = "test-namespace" + ffConfigName = "test-config" + cmName = "test-cm" + ) + + tests := []struct { + name string + ffConfig *v1alpha1.FeatureFlagConfiguration + cm *corev1.ConfigMap + wantProvider string + wantCM *corev1.ConfigMap + cmDeleted bool + }{ + { + name: "no provider set + no owner set -> ffconfig and cm will be updated", + cm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: cmName, + Namespace: testNamespace, + Annotations: map[string]string{ + "openfeature.dev/featureflagconfiguration": ffConfigName, + }, + }, + }, + ffConfig: createTestFFConfig(ffConfigName, testNamespace, cmName, ""), + wantProvider: "flagd", + wantCM: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: cmName, + Namespace: testNamespace, + Annotations: map[string]string{ + "openfeature.dev/featureflagconfiguration": ffConfigName, + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "core.openfeature.dev/v1alpha1", + Kind: "FeatureFlagConfiguration", + Name: ffConfigName, + }, + }, + }, + Data: map[string]string{ + v1alpha1.FeatureFlagConfigurationConfigMapKey(testNamespace, cmName): "spec", + }, + }, + cmDeleted: false, + }, + { + name: "one owner ref set -> cm will be deleted", + cm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: cmName, + Namespace: testNamespace, + Annotations: map[string]string{ + "openfeature.dev/featureflagconfiguration": ffConfigName, + }, + }, + }, + ffConfig: createTestFFConfig(ffConfigName, testNamespace, cmName, ""), + wantProvider: "flagd", + wantCM: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: cmName, + Namespace: testNamespace, + Annotations: map[string]string{ + "openfeature.dev/featureflagconfiguration": ffConfigName, + }, + }, + }, + cmDeleted: true, + }, + } + + err := v1alpha1.AddToScheme(scheme.Scheme) + require.Nil(t, err) + req := ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: testNamespace, + Name: ffConfigName, + }, + } + + ctx := context.TODO() + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tt.ffConfig, tt.cm).Build() + + r := &FeatureFlagConfigurationReconciler{ + Client: fakeClient, + Log: ctrl.Log.WithName("featureflagconfiguration-controller"), + Scheme: fakeClient.Scheme(), + } + + if tt.cmDeleted { + ffConfig2 := &v1alpha1.FeatureFlagConfiguration{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: ffConfigName, Namespace: testNamespace}, ffConfig2) + require.Nil(t, err) + + cm2 := &corev1.ConfigMap{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: cmName, Namespace: testNamespace}, cm2) + require.Nil(t, err) + + cm2.OwnerReferences = append(cm2.OwnerReferences, v1alpha1.GetFfReference(ffConfig2)) + err := r.Client.Update(ctx, cm2) + require.Nil(t, err) + } + + _, err = r.Reconcile(ctx, req) + require.Nil(t, err) + + ffConfig2 := &v1alpha1.FeatureFlagConfiguration{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: ffConfigName, Namespace: testNamespace}, ffConfig2) + require.Nil(t, err) + + require.Equal(t, tt.wantProvider, ffConfig2.Spec.ServiceProvider.Name) + + cm2 := &corev1.ConfigMap{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: cmName, Namespace: testNamespace}, cm2) + + if !tt.cmDeleted { + require.Nil(t, err) + require.Equal(t, tt.wantCM.Data, cm2.Data) + require.Len(t, cm2.OwnerReferences, len(tt.wantCM.OwnerReferences)) + require.Equal(t, tt.wantCM.OwnerReferences[0].APIVersion, cm2.OwnerReferences[0].APIVersion) + require.Equal(t, tt.wantCM.OwnerReferences[0].Name, cm2.OwnerReferences[0].Name) + require.Equal(t, tt.wantCM.OwnerReferences[0].Kind, cm2.OwnerReferences[0].Kind) + } else { + require.NotNil(t, err) + } + }) + } +} + +func createTestFFConfig(ffConfigName string, testNamespace string, cmName string, provider string) *v1alpha1.FeatureFlagConfiguration { + fsConfig := &v1alpha1.FeatureFlagConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: ffConfigName, + Namespace: testNamespace, + }, + Spec: v1alpha1.FeatureFlagConfigurationSpec{ + ServiceProvider: &v1alpha1.FeatureFlagServiceProvider{ + Name: provider, + }, + FeatureFlagSpec: "spec", + }, + } + + return fsConfig +} diff --git a/controllers/core/flagsourceconfiguration/controller.go b/controllers/core/flagsourceconfiguration/controller.go index 0c8c23eb5..caa90e820 100644 --- a/controllers/core/flagsourceconfiguration/controller.go +++ b/controllers/core/flagsourceconfiguration/controller.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -133,7 +134,7 @@ func (r *FlagSourceConfigurationReconciler) finishReconcile(err error, requeueIm // SetupWithManager sets up the controller with the Manager. func (r *FlagSourceConfigurationReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&corev1alpha1.FlagSourceConfiguration{}). + For(&corev1alpha1.FlagSourceConfiguration{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). // we are only interested in update events for this reconciliation loop WithEventFilter(predicate.GenerationChangedPredicate{}). Complete(r) diff --git a/controllers/core/flagsourceconfiguration/controller_test.go b/controllers/core/flagsourceconfiguration/controller_test.go index d9289fd59..c308d5c5a 100644 --- a/controllers/core/flagsourceconfiguration/controller_test.go +++ b/controllers/core/flagsourceconfiguration/controller_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "testing" + "time" "github.com/open-feature/open-feature-operator/apis/core/v1alpha1" "github.com/open-feature/open-feature-operator/controllers/common" @@ -14,6 +15,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -24,20 +26,39 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { deploymentName = "test-deploy" ) - deployment := createTestDeployment(fsConfigName, testNamespace, deploymentName) - fsConfig := createTestFSConfig(fsConfigName, testNamespace, deploymentName) + tests := []struct { + name string + fsConfig *v1alpha1.FlagSourceConfiguration + deployment *appsv1.Deployment + restarted1 string + restarted2 string + }{ + { + name: "deployment gets restarted with rollout", + fsConfig: createTestFSConfig(fsConfigName, testNamespace, deploymentName, true), + deployment: createTestDeployment(fsConfigName, testNamespace, deploymentName), + restarted1: "", + restarted2: time.Now().Format(time.RFC3339), + }, + { + name: "deployment without rollout", + fsConfig: createTestFSConfig(fsConfigName, testNamespace, deploymentName, false), + deployment: createTestDeployment(fsConfigName, testNamespace, deploymentName), + restarted1: "", + restarted2: "", + }, + { + name: "no deployment", + fsConfig: createTestFSConfig(fsConfigName, testNamespace, deploymentName, true), + deployment: nil, + restarted1: "", + restarted2: "", + }, + } err := v1alpha1.AddToScheme(scheme.Scheme) require.Nil(t, err) - fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(deployment, fsConfig).WithIndex(&appsv1.Deployment{}, fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationPath, common.FlagSourceConfigurationAnnotation), common.FlagSourceConfigurationIndex).Build() - - r := &FlagSourceConfigurationReconciler{ - Client: fakeClient, - Log: ctrl.Log.WithName("flagsourceconfiguration-controller"), - Scheme: fakeClient.Scheme(), - } - req := ctrl.Request{ NamespacedName: types.NamespacedName{ Namespace: testNamespace, @@ -47,19 +68,42 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { ctx := context.TODO() - deployment2 := &appsv1.Deployment{} - err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment2) - require.Nil(t, err) - restartAt := deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"] - require.Equal(t, "", restartAt) - - _, err = r.Reconcile(ctx, req) - require.Nil(t, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + 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.FlagSourceConfigurationAnnotation), common.FlagSourceConfigurationIndex).Build() + } else { + fakeClient = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tt.fsConfig).WithIndex(&appsv1.Deployment{}, fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationPath, common.FlagSourceConfigurationAnnotation), common.FlagSourceConfigurationIndex).Build() + } + + r := &FlagSourceConfigurationReconciler{ + Client: fakeClient, + Log: ctrl.Log.WithName("flagsourceconfiguration-controller"), + Scheme: fakeClient.Scheme(), + } + + if tt.deployment != nil { + deployment2 := &appsv1.Deployment{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment2) + require.Nil(t, err) + restartAt := deployment2.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"] + require.Equal(t, tt.restarted1, restartAt) + } + + _, err = r.Reconcile(ctx, req) + require.Nil(t, err) + + if tt.deployment != nil { + deployment2 := &appsv1.Deployment{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment2) + require.Nil(t, err) + + require.Equal(t, tt.restarted2, deployment2.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"]) + } + }) - err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment) - require.Nil(t, err) - - require.NotEqual(t, restartAt, deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"]) + } } @@ -104,8 +148,7 @@ func createTestDeployment(fsConfigName string, testNamespace string, deploymentN return deployment } -func createTestFSConfig(fsConfigName string, testNamespace string, deploymentName string) *v1alpha1.FlagSourceConfiguration { - rolloutOnChange := true +func createTestFSConfig(fsConfigName string, testNamespace string, deploymentName string, rollout bool) *v1alpha1.FlagSourceConfiguration { fsConfig := &v1alpha1.FlagSourceConfiguration{ ObjectMeta: metav1.ObjectMeta{ Name: fsConfigName, @@ -119,7 +162,7 @@ func createTestFSConfig(fsConfigName string, testNamespace string, deploymentNam Provider: "http", }, }, - RolloutOnChange: &rolloutOnChange, + RolloutOnChange: &rollout, }, } diff --git a/main.go b/main.go index ee7f2c56d..61bb93fcc 100644 --- a/main.go +++ b/main.go @@ -27,7 +27,6 @@ import ( // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. - "github.com/open-feature/open-feature-operator/controllers/common" controllercommon "github.com/open-feature/open-feature-operator/controllers/common" "go.uber.org/zap/zapcore" "k8s.io/apimachinery/pkg/runtime" @@ -199,8 +198,8 @@ func main() { if err := mgr.GetFieldIndexer().IndexField( context.Background(), &appsV1.Deployment{}, - fmt.Sprintf("%s/%s", controllercommon.OpenFeatureAnnotationPath, common.FlagSourceConfigurationAnnotation), - common.FlagSourceConfigurationIndex, + fmt.Sprintf("%s/%s", controllercommon.OpenFeatureAnnotationPath, controllercommon.FlagSourceConfigurationAnnotation), + controllercommon.FlagSourceConfigurationIndex, ); err != nil { setupLog.Error( err, @@ -214,6 +213,7 @@ func main() { if err = (&featureflagconfiguration.FeatureFlagConfigurationReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), + Log: ctrl.Log.WithName("FeatureFlagConfiguration Controller"), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "FeatureFlagConfiguration") os.Exit(1) @@ -227,6 +227,7 @@ func main() { if err = (&flagsourceconfiguration.FlagSourceConfigurationReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), + Log: ctrl.Log.WithName("FlagSourceConfiguration Controller"), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "FlagSourceConfiguration") os.Exit(1) From 6cbe37c69695084407241c51c2841504ad43222c Mon Sep 17 00:00:00 2001 From: odubajDT Date: Tue, 21 Mar 2023 07:52:34 +0100 Subject: [PATCH 6/9] minor fix in unit tests Signed-off-by: odubajDT --- apis/core/v1alpha1/featureflagconfiguration_types.go | 7 +++++++ controllers/core/featureflagconfiguration/controller.go | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/apis/core/v1alpha1/featureflagconfiguration_types.go b/apis/core/v1alpha1/featureflagconfiguration_types.go index 856f99f1f..670aa3a85 100644 --- a/apis/core/v1alpha1/featureflagconfiguration_types.go +++ b/apis/core/v1alpha1/featureflagconfiguration_types.go @@ -145,3 +145,10 @@ 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 { + if p == nil || p.Name == "" { + return false + } + return true +} diff --git a/controllers/core/featureflagconfiguration/controller.go b/controllers/core/featureflagconfiguration/controller.go index 3c10ec0da..713c78bdd 100644 --- a/controllers/core/featureflagconfiguration/controller.go +++ b/controllers/core/featureflagconfiguration/controller.go @@ -98,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", From 065f4442e5d98d1cc3df086193807bd0db66c765 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Thu, 23 Mar 2023 15:03:52 +0100 Subject: [PATCH 7/9] rename variables Signed-off-by: odubajDT --- .../controller_test.go | 32 +++++++++---------- .../controller_test.go | 12 +++---- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/controllers/core/featureflagconfiguration/controller_test.go b/controllers/core/featureflagconfiguration/controller_test.go index 854a0f455..3f5e96921 100644 --- a/controllers/core/featureflagconfiguration/controller_test.go +++ b/controllers/core/featureflagconfiguration/controller_test.go @@ -111,38 +111,38 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { } if tt.cmDeleted { - ffConfig2 := &v1alpha1.FeatureFlagConfiguration{} - err = fakeClient.Get(ctx, types.NamespacedName{Name: ffConfigName, Namespace: testNamespace}, ffConfig2) + ffConfig := &v1alpha1.FeatureFlagConfiguration{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: ffConfigName, Namespace: testNamespace}, ffConfig) require.Nil(t, err) - cm2 := &corev1.ConfigMap{} - err = fakeClient.Get(ctx, types.NamespacedName{Name: cmName, Namespace: testNamespace}, cm2) + cm := &corev1.ConfigMap{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: cmName, Namespace: testNamespace}, cm) require.Nil(t, err) - cm2.OwnerReferences = append(cm2.OwnerReferences, v1alpha1.GetFfReference(ffConfig2)) - err := r.Client.Update(ctx, cm2) + cm.OwnerReferences = append(cm.OwnerReferences, v1alpha1.GetFfReference(ffConfig)) + err := r.Client.Update(ctx, cm) require.Nil(t, err) } _, err = r.Reconcile(ctx, req) require.Nil(t, err) - ffConfig2 := &v1alpha1.FeatureFlagConfiguration{} - err = fakeClient.Get(ctx, types.NamespacedName{Name: ffConfigName, Namespace: testNamespace}, ffConfig2) + ffConfig := &v1alpha1.FeatureFlagConfiguration{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: ffConfigName, Namespace: testNamespace}, ffConfig) require.Nil(t, err) - require.Equal(t, tt.wantProvider, ffConfig2.Spec.ServiceProvider.Name) + require.Equal(t, tt.wantProvider, ffConfig.Spec.ServiceProvider.Name) - cm2 := &corev1.ConfigMap{} - err = fakeClient.Get(ctx, types.NamespacedName{Name: cmName, Namespace: testNamespace}, cm2) + cm := &corev1.ConfigMap{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: cmName, Namespace: testNamespace}, cm) if !tt.cmDeleted { require.Nil(t, err) - require.Equal(t, tt.wantCM.Data, cm2.Data) - require.Len(t, cm2.OwnerReferences, len(tt.wantCM.OwnerReferences)) - require.Equal(t, tt.wantCM.OwnerReferences[0].APIVersion, cm2.OwnerReferences[0].APIVersion) - require.Equal(t, tt.wantCM.OwnerReferences[0].Name, cm2.OwnerReferences[0].Name) - require.Equal(t, tt.wantCM.OwnerReferences[0].Kind, cm2.OwnerReferences[0].Kind) + require.Equal(t, tt.wantCM.Data, cm.Data) + require.Len(t, cm.OwnerReferences, len(tt.wantCM.OwnerReferences)) + require.Equal(t, tt.wantCM.OwnerReferences[0].APIVersion, cm.OwnerReferences[0].APIVersion) + require.Equal(t, tt.wantCM.OwnerReferences[0].Name, cm.OwnerReferences[0].Name) + require.Equal(t, tt.wantCM.OwnerReferences[0].Kind, cm.OwnerReferences[0].Kind) } else { require.NotNil(t, err) } diff --git a/controllers/core/flagsourceconfiguration/controller_test.go b/controllers/core/flagsourceconfiguration/controller_test.go index c308d5c5a..677293da8 100644 --- a/controllers/core/flagsourceconfiguration/controller_test.go +++ b/controllers/core/flagsourceconfiguration/controller_test.go @@ -84,10 +84,10 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { } if tt.deployment != nil { - deployment2 := &appsv1.Deployment{} - err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment2) + deployment := &appsv1.Deployment{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment) require.Nil(t, err) - restartAt := deployment2.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"] + restartAt := deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"] require.Equal(t, tt.restarted1, restartAt) } @@ -95,11 +95,11 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { require.Nil(t, err) if tt.deployment != nil { - deployment2 := &appsv1.Deployment{} - err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment2) + deployment := &appsv1.Deployment{} + err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment) require.Nil(t, err) - require.Equal(t, tt.restarted2, deployment2.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"]) + require.Equal(t, tt.restarted2, deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"]) } }) From 2ea30e6aa5a48156decfcf9c799cf27ace5bad7b Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 24 Mar 2023 19:21:59 +0100 Subject: [PATCH 8/9] fixed minor issue Signed-off-by: odubajDT --- apis/core/v1alpha1/featureflagconfiguration_types.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/apis/core/v1alpha1/featureflagconfiguration_types.go b/apis/core/v1alpha1/featureflagconfiguration_types.go index 670aa3a85..8c92c8a9a 100644 --- a/apis/core/v1alpha1/featureflagconfiguration_types.go +++ b/apis/core/v1alpha1/featureflagconfiguration_types.go @@ -147,8 +147,5 @@ func FeatureFlagConfigurationConfigMapKey(namespace, name string) string { } func (p *FeatureFlagServiceProvider) IsSet() bool { - if p == nil || p.Name == "" { - return false - } - return true + return p != nil && p.Name != "" } From 155c5bb8580d3c4813d8d0e4b7f47d4a35884390 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 24 Mar 2023 19:48:19 +0100 Subject: [PATCH 9/9] set up explanation comments for tests Signed-off-by: odubajDT --- .../controller_test.go | 6 +++ .../controller_test.go | 48 ++++++++++--------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/controllers/core/featureflagconfiguration/controller_test.go b/controllers/core/featureflagconfiguration/controller_test.go index 3f5e96921..f32f53c5a 100644 --- a/controllers/core/featureflagconfiguration/controller_test.go +++ b/controllers/core/featureflagconfiguration/controller_test.go @@ -102,6 +102,7 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + // set up k8s fake client fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tt.ffConfig, tt.cm).Build() r := &FeatureFlagConfigurationReconciler{ @@ -111,6 +112,7 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { } if tt.cmDeleted { + // if configMap should be deleted, we need to set ffConfig as the only OwnerRef before executing the Reconcile function ffConfig := &v1alpha1.FeatureFlagConfiguration{} err = fakeClient.Get(ctx, types.NamespacedName{Name: ffConfigName, Namespace: testNamespace}, ffConfig) require.Nil(t, err) @@ -124,6 +126,7 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { require.Nil(t, err) } + // reconcile _, err = r.Reconcile(ctx, req) require.Nil(t, err) @@ -131,12 +134,14 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { err = fakeClient.Get(ctx, types.NamespacedName{Name: ffConfigName, Namespace: testNamespace}, ffConfig) require.Nil(t, err) + // check that the provider name is set correctly require.Equal(t, tt.wantProvider, ffConfig.Spec.ServiceProvider.Name) cm := &corev1.ConfigMap{} err = fakeClient.Get(ctx, types.NamespacedName{Name: cmName, Namespace: testNamespace}, cm) if !tt.cmDeleted { + // if configMap should not be deleted, check the correct values require.Nil(t, err) require.Equal(t, tt.wantCM.Data, cm.Data) require.Len(t, cm.OwnerReferences, len(tt.wantCM.OwnerReferences)) @@ -144,6 +149,7 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { require.Equal(t, tt.wantCM.OwnerReferences[0].Name, cm.OwnerReferences[0].Name) require.Equal(t, tt.wantCM.OwnerReferences[0].Kind, cm.OwnerReferences[0].Kind) } else { + // if configMap should be deleted, we expect error require.NotNil(t, err) } }) diff --git a/controllers/core/flagsourceconfiguration/controller_test.go b/controllers/core/flagsourceconfiguration/controller_test.go index 677293da8..58b1f5749 100644 --- a/controllers/core/flagsourceconfiguration/controller_test.go +++ b/controllers/core/flagsourceconfiguration/controller_test.go @@ -27,32 +27,32 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { ) tests := []struct { - name string - fsConfig *v1alpha1.FlagSourceConfiguration - deployment *appsv1.Deployment - restarted1 string - restarted2 string + name string + fsConfig *v1alpha1.FlagSourceConfiguration + deployment *appsv1.Deployment + restartedAtValueBeforeReconcile string + restartedAtValueAfterReconcile string }{ { - name: "deployment gets restarted with rollout", - fsConfig: createTestFSConfig(fsConfigName, testNamespace, deploymentName, true), - deployment: createTestDeployment(fsConfigName, testNamespace, deploymentName), - restarted1: "", - restarted2: time.Now().Format(time.RFC3339), + name: "deployment gets restarted with rollout", + fsConfig: createTestFSConfig(fsConfigName, testNamespace, deploymentName, true), + deployment: createTestDeployment(fsConfigName, testNamespace, deploymentName), + restartedAtValueBeforeReconcile: "", + restartedAtValueAfterReconcile: time.Now().Format(time.RFC3339), }, { - name: "deployment without rollout", - fsConfig: createTestFSConfig(fsConfigName, testNamespace, deploymentName, false), - deployment: createTestDeployment(fsConfigName, testNamespace, deploymentName), - restarted1: "", - restarted2: "", + name: "deployment without rollout", + fsConfig: createTestFSConfig(fsConfigName, testNamespace, deploymentName, false), + deployment: createTestDeployment(fsConfigName, testNamespace, deploymentName), + restartedAtValueBeforeReconcile: "", + restartedAtValueAfterReconcile: "", }, { - name: "no deployment", - fsConfig: createTestFSConfig(fsConfigName, testNamespace, deploymentName, true), - deployment: nil, - restarted1: "", - restarted2: "", + name: "no deployment", + fsConfig: createTestFSConfig(fsConfigName, testNamespace, deploymentName, true), + deployment: nil, + restartedAtValueBeforeReconcile: "", + restartedAtValueAfterReconcile: "", }, } @@ -70,6 +70,7 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(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.FlagSourceConfigurationAnnotation), common.FlagSourceConfigurationIndex).Build() @@ -84,22 +85,25 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { } if tt.deployment != nil { + // checking that the deployment does have 'restartedAt' set to the expected value before reconciliation deployment := &appsv1.Deployment{} err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment) require.Nil(t, err) restartAt := deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"] - require.Equal(t, tt.restarted1, restartAt) + require.Equal(t, tt.restartedAtValueBeforeReconcile, restartAt) } + // running reconcile function _, err = r.Reconcile(ctx, req) require.Nil(t, err) if tt.deployment != nil { + // checking that the deployment does have 'restartedAt' set to the expected value after reconciliation deployment := &appsv1.Deployment{} err = fakeClient.Get(ctx, types.NamespacedName{Name: deploymentName, Namespace: testNamespace}, deployment) require.Nil(t, err) - require.Equal(t, tt.restarted2, deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"]) + require.Equal(t, tt.restartedAtValueAfterReconcile, deployment.Spec.Template.ObjectMeta.Annotations["kubectl.kubernetes.io/restartedAt"]) } })