diff --git a/Gopkg.lock b/Gopkg.lock index 08029cbaece2..770b13d52c35 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -985,6 +985,7 @@ "k8s.io/apimachinery/pkg/apis/meta/v1", "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured", "k8s.io/apimachinery/pkg/apis/meta/v1/validation", + "k8s.io/apimachinery/pkg/fields", "k8s.io/apimachinery/pkg/labels", "k8s.io/apimachinery/pkg/runtime", "k8s.io/apimachinery/pkg/runtime/schema", diff --git a/docs/book/common_code/machine_controller.md b/docs/book/common_code/machine_controller.md index 7d6e8e2d8b68..198b3baeae77 100644 --- a/docs/book/common_code/machine_controller.md +++ b/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] diff --git a/pkg/controller/machine/BUILD.bazel b/pkg/controller/machine/BUILD.bazel index 2266261c0d27..8984f0c53a0d 100644 --- a/pkg/controller/machine/BUILD.bazel +++ b/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", diff --git a/pkg/controller/machine/controller.go b/pkg/controller/machine/controller.go index f748359b8590..6662d19ece0f 100644 --- a/pkg/controller/machine/controller.go +++ b/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 { diff --git a/pkg/controller/machine/controller_test.go b/pkg/controller/machine/controller_test.go index 5e3201d78626..44cb42c6ec4d 100644 --- a/pkg/controller/machine/controller_test.go +++ b/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", + }, + }, }, } diff --git a/pkg/controller/machinedeployment/util/util_test.go b/pkg/controller/machinedeployment/util/util_test.go index 4809a2c01b57..66c1b2bace95 100644 --- a/pkg/controller/machinedeployment/util/util_test.go +++ b/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 {