Stronger link between Machine* <-> Cluster (#728)

Signed-off-by: Vince Prignano <[email protected]>

docs/book/common_code/machine_controller.md

@@ -10,16 +10,22 @@ the host with a new one matching the updated spec. If a `Machine` object is
 deleted, the corresponding `Node` should have its external resources released by
 the provider-specific controller, and should be deleted as well.
 
+Machines can be associated with a Cluster using a custom label
+`cluster.k8s.io/cluster-name`. When the label is set and non-empty,
+then it must reference the name of a cluster residing in the same namespace.
+The label must be set only once and updates are not permitted,
+an admission controller is going to enforce the change in a future version.
+
 {% method %}
 ## Machine
 
 `Machine` has 4 fields:
 
-`Spec` contains the desired cluster state specified by the object. While much
+`Spec` contains the desired machine state specified by the object. While much
 of the `Spec` is defined by users, unspecified parts may be filled in with
 defaults or by Controllers such as autoscalers.
 
-`Status` contains only observed cluster state and is only written by
+`Status` contains only observed machine state and is only written by
 controllers. `Status` is not the source of truth for any information, but
 instead aggregates and publishes observed state.
 
@@ -93,7 +99,7 @@ methods.
 `Delete()` will only be called when the `Machine` is in the process of being
 deleted.
 
-The definition of `Exist()` is determined by the provider.
+The definition of `Exists()` is determined by the provider.
 
 **TODO**: Provide more guidance on `Exists()`.
 
@@ -108,7 +114,7 @@ We need a diagram tracing the logic from resource creation through updates
 and finally deletion.
 {% endpanel %}
 
-0. Determine the `Cluster` associated with the `Machine`.
+0. Determine the `Cluster` associated with the `Machine` from its `cluster.k8s.io/cluster-name` label.
 - If the `Machine` hasn't been deleted and doesn't have a finalizer, add one.
 - If the `Machine` is being deleted, and there is no finalizer, we're done
   - Check if the `Machine` is allowed to be deleted. [^1]

pkg/controller/machine/BUILD.bazel

@@ -15,6 +15,7 @@ go_library(
         "//pkg/util:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/fields:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
         "//vendor/sigs.k8s.io/controller-runtime/pkg/client:go_default_library",

pkg/controller/machine/controller.go

@@ -21,6 +21,8 @@ import (
 	"errors"
 	"os"
 
+	"k8s.io/apimachinery/pkg/fields"
+
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -36,7 +38,10 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/source"
 )
 
-const NodeNameEnvVar = "NODE_NAME"
+const (
+	NodeNameEnvVar          = "NODE_NAME"
+	MachineClusterLabelName = "cluster.k8s.io/cluster-name"
+)
 
 var DefaultActuator Actuator
 
@@ -116,8 +121,7 @@ func (r *ReconcileMachine) Reconcile(request reconcile.Request) (reconcile.Resul
 	// for machine management.
 	cluster, err := r.getCluster(ctx, m)
 	if err != nil {
-		// Just log the error here.
-		klog.V(4).Infof("Cluster not found, machine actuation might fail: %v", err)
+		return reconcile.Result{}, err
 	}
 	// If object hasn't been deleted and doesn't have a finalizer, add one
 	// Add a finalizer to newly created objects.
@@ -193,9 +197,15 @@ func (r *ReconcileMachine) Reconcile(request reconcile.Request) (reconcile.Resul
 }
 
 func (r *ReconcileMachine) getCluster(ctx context.Context, machine *clusterv1.Machine) (*clusterv1.Cluster, error) {
+	if machine.Labels[MachineClusterLabelName] == "" {
+		klog.Infof("Machine %q in namespace %q doesn't specify %q label, assuming nil cluster", machine.Name, MachineClusterLabelName, machine.Namespace)
+		return nil, nil
+	}
+
 	clusterList := clusterv1.ClusterList{}
 	listOptions := &client.ListOptions{
-		Namespace: machine.Namespace,
+		FieldSelector: fields.OneTermEqualSelector("metadata.name", machine.Labels[MachineClusterLabelName]),
+		Namespace:     machine.Namespace,
 	}
 
 	if err := r.Client.List(ctx, listOptions, &clusterList); err != nil {

pkg/controller/machine/controller_test.go

@@ -37,6 +37,9 @@ func TestReconcileRequest(t *testing.T) {
 			Name:       "create",
 			Namespace:  "default",
 			Finalizers: []string{v1alpha1.MachineFinalizer},
+			Labels: map[string]string{
+				MachineClusterLabelName: "testcluster",
+			},
 		},
 	}
 	machine2 := v1alpha1.Machine{
@@ -47,6 +50,9 @@ func TestReconcileRequest(t *testing.T) {
 			Name:       "update",
 			Namespace:  "default",
 			Finalizers: []string{v1alpha1.MachineFinalizer},
+			Labels: map[string]string{
+				MachineClusterLabelName: "testcluster",
+			},
 		},
 	}
 	time := metav1.Now()
@@ -59,6 +65,9 @@ func TestReconcileRequest(t *testing.T) {
 			Namespace:         "default",
 			Finalizers:        []string{v1alpha1.MachineFinalizer},
 			DeletionTimestamp: &time,
+			Labels: map[string]string{
+				MachineClusterLabelName: "testcluster",
+			},
 		},
 	}
 	clusterList := v1alpha1.ClusterList{
@@ -71,10 +80,19 @@ func TestReconcileRequest(t *testing.T) {
 					Kind: "Cluster",
 				},
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "cluster",
+					Name:      "testcluster",
 					Namespace: "default",
 				},
 			},
+			{
+				TypeMeta: metav1.TypeMeta{
+					Kind: "Cluster",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "rainbow",
+					Namespace: "foo",
+				},
+			},
 		},
 	}
 

pkg/controller/machinedeployment/util/util_test.go

@@ -698,7 +698,6 @@ func TestMaxUnavailable(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		t.Log(test.name)
 		t.Run(test.name, func(t *testing.T) {
 			maxUnavailable := MaxUnavailable(test.deployment)
 			if test.expected != maxUnavailable {