From b886317fdf2c1f81d682dcef1463dfa6d28cce8a Mon Sep 17 00:00:00 2001
From: Jan Chaloupka <jchaloup@redhat.com>
Date: Fri, 11 Jan 2019 12:21:01 +0100
Subject: [PATCH] Validate machineset before reconciling

Even thought the MachineSet type implements Validate() function,
it's not called by default. The validation function is responsible
for making sure every machine set matchLabels selector is matched with
machine template labels. Given the validation is not performed by default,
it is possible to create an invalid machineset that causes the machineset
controller to start creating machine object one by one without any upper
bound. Causing the machine controller to launch as many instances as
there is machine objects.
---
 .../machinedeployment/machinedeployment_controller_test.go | 2 ++
 pkg/controller/machineset/controller.go                    | 7 +++++++
 pkg/controller/machineset/machineset_controller_test.go    | 7 +++++++
 3 files changed, 16 insertions(+)

diff --git a/pkg/controller/machinedeployment/machinedeployment_controller_test.go b/pkg/controller/machinedeployment/machinedeployment_controller_test.go
index 3029309313de..317d8b6307fe 100644
--- a/pkg/controller/machinedeployment/machinedeployment_controller_test.go
+++ b/pkg/controller/machinedeployment/machinedeployment_controller_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package machinedeployment
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
@@ -174,6 +175,7 @@ func TestReconcile(t *testing.T) {
 		expectInt(t, i, func(ctx context.Context) int {
 			if err = c.Get(ctx, types.NamespacedName{
 				Namespace: newMachineSet.Namespace, Name: newMachineSet.Name}, newMachineSet); err != nil {
+				fmt.Printf("err: %v\n", err)
 				return -1
 			}
 			return int(*newMachineSet.Spec.Replicas)
diff --git a/pkg/controller/machineset/controller.go b/pkg/controller/machineset/controller.go
index a1d3923bec8a..cb537bd4d190 100644
--- a/pkg/controller/machineset/controller.go
+++ b/pkg/controller/machineset/controller.go
@@ -150,6 +150,13 @@ func (r *ReconcileMachineSet) Reconcile(request reconcile.Request) (reconcile.Re
 	}
 
 	klog.V(4).Infof("Reconcile machineset %v", machineSet.Name)
+
+	if errList := machineSet.Validate(); len(errList) > 0 {
+		err := fmt.Errorf("%q machineset validation failed: %v", machineSet.Name, errList.ToAggregate().Error())
+		klog.Error(err)
+		return reconcile.Result{}, err
+	}
+
 	allMachines := &clusterv1alpha1.MachineList{}
 
 	err = r.Client.List(context.Background(), client.InNamespace(machineSet.Namespace), allMachines)
diff --git a/pkg/controller/machineset/machineset_controller_test.go b/pkg/controller/machineset/machineset_controller_test.go
index 153a51e9aa11..87d94ab8503b 100644
--- a/pkg/controller/machineset/machineset_controller_test.go
+++ b/pkg/controller/machineset/machineset_controller_test.go
@@ -37,11 +37,18 @@ const timeout = time.Second * 5
 
 func TestReconcile(t *testing.T) {
 	replicas := int32(2)
+	labels := map[string]string{"foo": "bar"}
 	instance := &clusterv1alpha1.MachineSet{
 		ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
 		Spec: clusterv1alpha1.MachineSetSpec{
 			Replicas: &replicas,
+			Selector: metav1.LabelSelector{
+				MatchLabels: labels,
+			},
 			Template: clusterv1alpha1.MachineTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: labels,
+				},
 				Spec: clusterv1alpha1.MachineSpec{
 					Versions: clusterv1alpha1.MachineVersionInfo{Kubelet: "1.10.3"},
 				},