diff --git a/pkg/controller/framework/framework_controller.go b/pkg/controller/framework/framework_controller.go index b78d0a47d..779d4d795 100644 --- a/pkg/controller/framework/framework_controller.go +++ b/pkg/controller/framework/framework_controller.go @@ -81,9 +81,9 @@ type ReconcileFramework struct { // +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=kudo.k8s.io,resources=frameworks,verbs=get;list;watch;create;update;patch;delete func (r *ReconcileFramework) Reconcile(request reconcile.Request) (reconcile.Result, error) { - // Fetch the Framework instance - instance := &kudov1alpha1.Framework{} - err := r.Get(context.TODO(), request.NamespacedName, instance) + // Fetch the framework + framework := &kudov1alpha1.Framework{} + err := r.Get(context.TODO(), request.NamespacedName, framework) if err != nil { if errors.IsNotFound(err) { // Object not found, return. Created objects are automatically garbage collected. diff --git a/pkg/controller/frameworkversion/frameworkversion_controller.go b/pkg/controller/frameworkversion/frameworkversion_controller.go index 1c8e0f1f9..80e860bb1 100644 --- a/pkg/controller/frameworkversion/frameworkversion_controller.go +++ b/pkg/controller/frameworkversion/frameworkversion_controller.go @@ -74,9 +74,9 @@ type ReconcileFrameworkVersion struct { // +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=kudo.k8s.io,resources=frameworkversions,verbs=get;list;watch;create;update;patch;delete func (r *ReconcileFrameworkVersion) Reconcile(request reconcile.Request) (reconcile.Result, error) { - // Fetch the FrameworkVersion instance - instance := &kudov1alpha1.FrameworkVersion{} - err := r.Get(context.TODO(), request.NamespacedName, instance) + // Fetch the framework version + frameworkVersion := &kudov1alpha1.FrameworkVersion{} + err := r.Get(context.TODO(), request.NamespacedName, frameworkVersion) if err != nil { if errors.IsNotFound(err) { // Object not found, return. Created objects are automatically garbage collected. @@ -87,7 +87,7 @@ func (r *ReconcileFrameworkVersion) Reconcile(request reconcile.Request) (reconc return reconcile.Result{}, err } - log.Printf("FrameworkVersionController: Received Reconcile request for a frameworkversion named: %v", request.Name) + log.Printf("FrameworkVersionController: Received Reconcile request for a frameworkVersion named: %v", request.Name) // TODO: Validate FrameworkVersion is appropriate. return reconcile.Result{}, nil diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 4d406a09c..6e0a850f1 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -120,31 +120,29 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { planName = "upgrade" } } else if !reflect.DeepEqual(old.Spec, new.Spec) { - for k, v := range new.Spec.Parameters { - if old.Spec.Parameters[k] != v { - // Find the right parameter in the FV - for _, param := range fv.Spec.Parameters { - if param.Name == k { - planName = param.Trigger - ok = true - } + for k := range parameterDifference(old.Spec.Parameters, new.Spec.Parameters) { + // Find the right parameter in the FV + for _, param := range fv.Spec.Parameters { + if param.Name == k { + planName = param.Trigger + ok = true } + } + if !ok { + log.Printf("InstanceController: Instance %v updated parameter %v, but parameter not found in FrameworkVersion %v\n", new.Name, k, fv.Name) + } else if planName == "" { + _, ok = fv.Spec.Plans["update"] if !ok { - log.Printf("InstanceController: Instance %v updated parameter %v, but parameter not found in FrameworkVersion %v\n", new.Name, k, fv.Name) - } else if planName == "" { - _, ok = fv.Spec.Plans["update"] + _, ok = fv.Spec.Plans["deploy"] if !ok { - _, ok = fv.Spec.Plans["deploy"] - if !ok { - log.Println("InstanceController: Could not find any plan to use for update") - } else { - planName = "deploy" - } + log.Println("InstanceController: Could not find any plan to use for update") } else { - planName = "update" + planName = "deploy" } - log.Printf("InstanceController: Instance %v updated parameter %v, but no specified trigger. Using default plan %v\n", new.Name, k, planName) + } else { + planName = "update" } + log.Printf("InstanceController: Instance %v updated parameter %v, but no specified trigger. Using default plan %v\n", new.Name, k, planName) } } // Not currently doing anything for Dependency changes. @@ -342,3 +340,23 @@ func (r *ReconcileInstance) Reconcile(request reconcile.Request) (reconcile.Resu // Defer call from above should apply the status changes to the object return reconcile.Result{}, nil } + +func parameterDifference(old, new map[string]string) map[string]string { + diff := make(map[string]string) + + for key, val := range old { + // If a parameter was removed in the new spec + if _, ok := new[key]; !ok { + diff[key] = val + } + } + + for key, val := range new { + // If new spec parameter was added or changed + if v, ok := old[key]; !ok || v != val { + diff[key] = val + } + } + + return diff +} diff --git a/pkg/controller/instance/instance_controller_suite_test.go b/pkg/controller/instance/instance_controller_suite_test.go index 42c5c1db6..a2d2a3155 100644 --- a/pkg/controller/instance/instance_controller_suite_test.go +++ b/pkg/controller/instance/instance_controller_suite_test.go @@ -21,6 +21,8 @@ import ( "path/filepath" "testing" + "github.com/onsi/gomega" + "github.com/kudobuilder/kudo/pkg/apis" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -44,3 +46,28 @@ func TestMain(m *testing.M) { t.Stop() os.Exit(code) } + +func TestSpecParameterDifference(t *testing.T) { + + var testParams = []struct { + name string + new map[string]string + diff map[string]string + }{ + {"update one value", map[string]string{"one": "11", "two": "2"}, map[string]string{"one": "11"}}, + {"update multiple values", map[string]string{"one": "11", "two": "22"}, map[string]string{"one": "11", "two": "22"}}, + {"add new value", map[string]string{"one": "1", "two": "2", "three": "3"}, map[string]string{"three": "3"}}, + {"remove one value", map[string]string{"one": "1"}, map[string]string{"two": "2"}}, + {"no difference", map[string]string{"one": "1", "two": "2"}, map[string]string{}}, + {"empty new map", map[string]string{}, map[string]string{"one": "1", "two": "2"}}, + } + + g := gomega.NewGomegaWithT(t) + + var old = map[string]string{"one": "1", "two": "2"} + + for _, test := range testParams { + diff := parameterDifference(old, test.new) + g.Expect(diff).Should(gomega.Equal(test.diff), test.name) + } +}