controller: fix duplicated plan merge already done outside renderTemp…

…late
tsuru · Feb 19, 2024 · fe8348d · fe8348d
1 parent fe33045
commit fe8348d
Show file tree

Hide file tree

Showing 2 changed files with 2 additions and 135 deletions.
diff --git a/controllers/controller.go b/controllers/controller.go
@@ -941,14 +941,9 @@ func (r *RpaasInstanceReconciler) renderTemplate(ctx context.Context, instance *
 		return "", err
 	}
 
-	instanceWithMergedConfig, err := mergeInstanceWithConfig(instance, &plan.Spec.Config)
-	if err != nil {
-		return "", err
-	}
-
 	config := nginx.ConfigurationData{
-		Instance: instanceWithMergedConfig,
-		Config:   &instanceWithMergedConfig.Spec.PlanTemplate.Config,
+		Instance: instance,
+		Config:   &plan.Spec.Config,
 	}
 
 	return cr.Render(config)
@@ -1325,34 +1320,6 @@ func mergePlans(base v1alpha1.RpaasPlanSpec, override v1alpha1.RpaasPlanSpec) (m
 	return
 }
 
-func mergeConfig(base v1alpha1.NginxConfig, override v1alpha1.NginxConfig) (merged v1alpha1.NginxConfig, err error) {
-	err = genericMerge(&merged, base, override)
-	return
-}
-
-func mergeInstanceWithConfig(instance *v1alpha1.RpaasInstance, config *v1alpha1.NginxConfig) (*v1alpha1.RpaasInstance, error) {
-	instanceConfig := v1alpha1.NginxConfig{}
-	if instance.Spec.PlanTemplate != nil {
-		instanceConfig = instance.Spec.PlanTemplate.Config
-	}
-
-	if config == nil {
-		config = &v1alpha1.NginxConfig{}
-	}
-
-	mergedConfig, err := mergeConfig(*config, instanceConfig)
-	if err != nil {
-		return nil, err
-	}
-	instanceWithMergedConfig := instance.DeepCopy()
-	if instanceWithMergedConfig.Spec.PlanTemplate == nil {
-		instanceWithMergedConfig.Spec.PlanTemplate = &v1alpha1.RpaasPlanSpec{}
-	}
-	instanceWithMergedConfig.Spec.PlanTemplate.Config = mergedConfig
-
-	return instanceWithMergedConfig, nil
-}
-
 func genericMerge(dst interface{}, overrides ...interface{}) error {
 	transformers := []func(*mergo.Config){
 		mergo.WithOverride,

diff --git a/controllers/controller_test.go b/controllers/controller_test.go
@@ -25,7 +25,6 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/tools/record"
-	"k8s.io/utils/pointer"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -400,105 +399,6 @@ func Test_mergePlans(t *testing.T) {
 	}
 }
 
-func Test_mergeInstanceWithConfig(t *testing.T) {
-	t.Parallel()
-	tests := []struct {
-		instance *v1alpha1.RpaasInstance
-		config   *v1alpha1.NginxConfig
-		expected *v1alpha1.RpaasInstance
-	}{
-		{
-			// case 0: nil config
-			instance: &v1alpha1.RpaasInstance{},
-			config:   nil,
-			expected: &v1alpha1.RpaasInstance{
-				Spec: v1alpha1.RpaasInstanceSpec{
-					PlanTemplate: &v1alpha1.RpaasPlanSpec{
-						Config: v1alpha1.NginxConfig{},
-					},
-				},
-			},
-		},
-		{
-			// case 1: empty config
-			instance: &v1alpha1.RpaasInstance{},
-			config:   &v1alpha1.NginxConfig{},
-			expected: &v1alpha1.RpaasInstance{
-				Spec: v1alpha1.RpaasInstanceSpec{
-					PlanTemplate: &v1alpha1.RpaasPlanSpec{
-						Config: v1alpha1.NginxConfig{},
-					},
-				},
-			},
-		},
-
-		{
-			// case 2: merge without conflits
-			instance: &v1alpha1.RpaasInstance{
-				Spec: v1alpha1.RpaasInstanceSpec{
-					PlanTemplate: &v1alpha1.RpaasPlanSpec{
-						Config: v1alpha1.NginxConfig{
-							User: "nobody",
-						},
-					},
-				},
-			},
-			config: &v1alpha1.NginxConfig{
-				CachePath: "/tmp/cache",
-			},
-			expected: &v1alpha1.RpaasInstance{
-				Spec: v1alpha1.RpaasInstanceSpec{
-					PlanTemplate: &v1alpha1.RpaasPlanSpec{
-						Config: v1alpha1.NginxConfig{
-							User:      "nobody",
-							CachePath: "/tmp/cache",
-						},
-					},
-				},
-			},
-		},
-		{
-			// case 3: merge with conflicts, instance have precedence
-			instance: &v1alpha1.RpaasInstance{
-				Spec: v1alpha1.RpaasInstanceSpec{
-					PlanTemplate: &v1alpha1.RpaasPlanSpec{
-						Config: v1alpha1.NginxConfig{
-							User:         "nobody",
-							CacheEnabled: pointer.Bool(false),
-						},
-					},
-				},
-			},
-			config: &v1alpha1.NginxConfig{
-				User:         "root",
-				CachePath:    "/tmp/cache",
-				CacheEnabled: pointer.Bool(true),
-				VTSEnabled:   pointer.Bool(true),
-			},
-			expected: &v1alpha1.RpaasInstance{
-				Spec: v1alpha1.RpaasInstanceSpec{
-					PlanTemplate: &v1alpha1.RpaasPlanSpec{
-						Config: v1alpha1.NginxConfig{
-							User:         "nobody",
-							CachePath:    "/tmp/cache",
-							VTSEnabled:   pointer.Bool(true),
-							CacheEnabled: pointer.Bool(false),
-						},
-					},
-				},
-			},
-		},
-	}
-
-	for i, tt := range tests {
-		t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) {
-			result, err := mergeInstanceWithConfig(tt.instance, tt.config)
-			require.NoError(t, err)
-			assert.Equal(t, tt.expected, result)
-		})
-	}
-}
-
 func TestReconcileRpaasInstance_getRpaasInstance(t *testing.T) {
 	t.Parallel()