✨ KCP adopts existing machines

The KCP controller identifies Machines that belong to the control plane
of an existing cluster and adopts them, including finding PKI materials
that may be owned by the machine's bootstrap config and pivoting their
ownership to the KCP as well.

Prior to adopting machines (which, if unsuccessful, will block the KCP
from taking any management actions), it runs a number of safety checks
including:

- Ensuring the KCP has not been deleted (to prevent re-adoption of
  orphans, though this process races with the garbage collector)
- Checking that the machine's bootstrap provider was KubeadmConfig
- Verifying that the Machine is no further than one minor version off of
  the KCP's spec

Additionally, we set set a "best guess" value for the
kubeadm.controlplane.cluster.x-k8s.io/hash on the adopted machine as if
it were generated by a KCP in the past. The intent is that a KCP will
adopt machines matching its "spec" (to the best of its ability) without
modification, which in practice works well for adopting machines with
the same spec'd version.

We make an educated guess at what parts of the kubeadm config the operator
considers important, and feed that into the hash function.

The goal here is to be able to:
1. Create a KCP over existing machines with ~the same configuration (for
   then new api/equality's package definition of the same) and have the
   KCP make no changes
2. Create a KCP over existing machines with a different configuration
   and have the KCP upgrade those machines to the new config

Finally, replaces PointsTo and introduces IsControlledBy, both of which
search through the owner reference list of their first argument (similar to
metav1.IsControlledBy).

Unlike the metav1 function, they're looking for a match on name plus
api group (not version) and kind. PointsTo returns true if any
OwnerReference matches the pointee, whereas IsControlledBy only returns
true if the sole permitted (by the API) controller reference matches.

fix: handle JoinConfiguration hashing

refactor: util should not check UIDs

This is a behavior change not least for tests, which typically do not
set either Kind or GroupVersion on various objects that end up acting as
referents.

Co-Authored-By: mnguyen <mnguyen@newrelic.com>
Co-Authored-By: Jason DeTiberus <detiberusj@vmware.com>
Co-authored-by: Vince Prignano <vince@vincepri.com>

.dockerignore

@@ -12,8 +12,36 @@ tilt-settings.json
 tilt.d/
 Tiltfile
 **/.tiltbuild
-test/infrastructure/docker/e2e/logs/**
 **/config/**/*.yaml
+**/config/**/*.yaml-e
 _artifacts
 Makefile
 **/Makefile
+
+# ignores changes to test-only code to avoid extra rebuilds
+test/e2e/**
+test/framework/**
+test/infrastructure/docker/e2e/**
+
+.dockerignore
+# We want to ignore any frequently modified files to avoid cache-busting the COPY ./ ./
+# Binaries for programs and plugins
+**/*.exe
+**/*.dll
+**/*.so
+**/*.dylib
+cmd/clusterctl/clusterctl/**
+**/bin/**
+**/out/**
+
+# Test binary, build with `go test -c`
+**/*.test
+
+# Output of the go coverage tool, specifically when used with LiteIDE
+**/*.out
+
+# Common editor / temporary files
+**/*~
+**/*.tmp
+**/.DS_Store
+**/*.swp

api/v1alpha3/cluster_types.go

@@ -18,6 +18,7 @@ package v1alpha3
 
 import (
 	"fmt"
+	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -90,6 +91,13 @@ type NetworkRanges struct {
 	CIDRBlocks []string `json:"cidrBlocks"`
 }
 
+func (n *NetworkRanges) String() string {
+	if n == nil {
+		return ""
+	}
+	return strings.Join(n.CIDRBlocks, "")
+}
+
 // ANCHOR_END: NetworkRanges
 
 // ANCHOR: ClusterStatus

bootstrap/kubeadm/api/equality/doc.go

@@ -0,0 +1,28 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+/*
+Package equality defines equality semantics for KubeadmConfigs, and utility tools for identifying
+equivalent configurations.
+
+There are a number of distinct but not different ways to express the "same" Kubeadm configuration,
+and so this package attempts to sort out what differences are likely meaningful or intentional.
+
+It is inspired by the observation that k/k no longer relies on hashing to identify "current" versions
+ReplicaSets, instead using a semantic equality check that's more amenable to field modifications and
+deletions: https://github.com/kubernetes/kubernetes/blob/0bb125e731/pkg/controller/deployment/util/deployment_util.go#L630-L634
+*/
+package equality

bootstrap/kubeadm/api/equality/semantic.go

@@ -0,0 +1,128 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package equality
+
+import (
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
+	bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha3"
+	kubeadmv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1"
+	"sigs.k8s.io/cluster-api/util/secret"
+)
+
+// SemanticMerge takes two KubeConfigSpecs and produces a third that is semantically equivalent to the other two
+// by way of merging non-behavior-changing fields from incoming into current.
+func SemanticMerge(current, incoming bootstrapv1.KubeadmConfigSpec, cluster *clusterv1.Cluster) bootstrapv1.KubeadmConfigSpec {
+	merged := bootstrapv1.KubeadmConfigSpec{}
+	current.DeepCopyInto(&merged)
+
+	merged = mergeClusterConfiguration(merged, incoming, cluster)
+
+	// Prefer non-nil init configurations: in most cases the init configuration is ignored and so this is purely representative
+	if merged.InitConfiguration == nil && incoming.InitConfiguration != nil {
+		merged.InitConfiguration = incoming.InitConfiguration.DeepCopy()
+	}
+
+	// The type meta for embedded types is currently ignored
+	if merged.ClusterConfiguration != nil && incoming.ClusterConfiguration != nil {
+		merged.ClusterConfiguration.TypeMeta = incoming.ClusterConfiguration.TypeMeta
+	}
+
+	if merged.InitConfiguration != nil && incoming.InitConfiguration != nil {
+		merged.InitConfiguration.TypeMeta = incoming.InitConfiguration.TypeMeta
+	}
+
+	if merged.JoinConfiguration != nil && incoming.JoinConfiguration != nil {
+		merged.JoinConfiguration.TypeMeta = incoming.JoinConfiguration.TypeMeta
+	}
+
+	// The default discovery injected by the kubeadm bootstrap controller is a unique, time-bounded token. However, any
+	// valid token has the same effect of joining the machine to the cluster. We consider the following scenarios:
+	// 1. current has no join configuration (meaning it was never reconciled) -> do nothing
+	// 2. current has a bootstrap token, and incoming has some configured discovery mechanism -> prefer incoming
+	// 3. current has a bootstrap token, and incoming has no discovery set -> delete current's discovery config
+	// 4. in all other cases, prefer current's configuration
+	if merged.JoinConfiguration == nil || merged.JoinConfiguration.Discovery.BootstrapToken == nil {
+		return merged
+	}
+
+	// current has a bootstrap token, check incoming
+	switch {
+	case incoming.JoinConfiguration == nil:
+		merged.JoinConfiguration.Discovery = kubeadmv1.Discovery{}
+	case incoming.JoinConfiguration.Discovery.BootstrapToken != nil:
+		fallthrough
+	case incoming.JoinConfiguration.Discovery.File != nil:
+		incoming.JoinConfiguration.Discovery.DeepCopyInto(&merged.JoinConfiguration.Discovery)
+	default:
+		// Neither token nor file is set on incoming's Discovery config
+		merged.JoinConfiguration.Discovery = kubeadmv1.Discovery{}
+	}
+
+	return merged
+}
+
+func mergeClusterConfiguration(merged, incoming bootstrapv1.KubeadmConfigSpec, cluster *clusterv1.Cluster) bootstrapv1.KubeadmConfigSpec {
+	if merged.ClusterConfiguration == nil && incoming.ClusterConfiguration != nil {
+		merged.ClusterConfiguration = incoming.ClusterConfiguration.DeepCopy()
+	} else if merged.ClusterConfiguration == nil {
+		// incoming's cluster configuration is also nil
+		return merged
+	}
+
+	// Attempt to reconstruct a cluster config in reverse of reconcileTopLevelObjectSettings
+	newCfg := incoming.ClusterConfiguration
+	if newCfg == nil {
+		newCfg = &kubeadmv1.ClusterConfiguration{}
+	}
+
+	if merged.ClusterConfiguration.ControlPlaneEndpoint == cluster.Spec.ControlPlaneEndpoint.String() &&
+		newCfg.ControlPlaneEndpoint == "" {
+		merged.ClusterConfiguration.ControlPlaneEndpoint = ""
+	}
+
+	if newCfg.ClusterName == "" {
+		merged.ClusterConfiguration.ClusterName = ""
+	}
+
+	if cluster.Spec.ClusterNetwork != nil {
+		if merged.ClusterConfiguration.Networking.DNSDomain == cluster.Spec.ClusterNetwork.ServiceDomain && newCfg.Networking.DNSDomain == "" {
+			merged.ClusterConfiguration.Networking.DNSDomain = ""
+		}
+
+		if merged.ClusterConfiguration.Networking.ServiceSubnet == cluster.Spec.ClusterNetwork.Services.String() &&
+			newCfg.Networking.ServiceSubnet == "" {
+			merged.ClusterConfiguration.Networking.ServiceSubnet = ""
+		}
+
+		if merged.ClusterConfiguration.Networking.PodSubnet == cluster.Spec.ClusterNetwork.Pods.String() &&
+			newCfg.Networking.PodSubnet == "" {
+			merged.ClusterConfiguration.Networking.PodSubnet = ""
+		}
+	}
+
+	// We defer to other sources for the version, such as the Machine
+	if newCfg.KubernetesVersion == "" {
+		merged.ClusterConfiguration.KubernetesVersion = ""
+	}
+
+	if merged.ClusterConfiguration.CertificatesDir == secret.DefaultCertificatesDir &&
+		newCfg.CertificatesDir == "" {
+		merged.ClusterConfiguration.CertificatesDir = ""
+	}
+
+	return merged
+}

bootstrap/kubeadm/controllers/kubeadmconfig_controller.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -688,13 +687,13 @@ func (r *KubeadmConfigReconciler) reconcileTopLevelObjectSettings(cluster *clust
 		if config.Spec.ClusterConfiguration.Networking.ServiceSubnet == "" &&
 			cluster.Spec.ClusterNetwork.Services != nil &&
 			len(cluster.Spec.ClusterNetwork.Services.CIDRBlocks) > 0 {
-			config.Spec.ClusterConfiguration.Networking.ServiceSubnet = strings.Join(cluster.Spec.ClusterNetwork.Services.CIDRBlocks, "")
+			config.Spec.ClusterConfiguration.Networking.ServiceSubnet = cluster.Spec.ClusterNetwork.Services.String()
 			log.Info("Altering ClusterConfiguration", "ServiceSubnet", config.Spec.ClusterConfiguration.Networking.ServiceSubnet)
 		}
 		if config.Spec.ClusterConfiguration.Networking.PodSubnet == "" &&
 			cluster.Spec.ClusterNetwork.Pods != nil &&
 			len(cluster.Spec.ClusterNetwork.Pods.CIDRBlocks) > 0 {
-			config.Spec.ClusterConfiguration.Networking.PodSubnet = strings.Join(cluster.Spec.ClusterNetwork.Pods.CIDRBlocks, "")
+			config.Spec.ClusterConfiguration.Networking.PodSubnet = cluster.Spec.ClusterNetwork.Pods.String()
 			log.Info("Altering ClusterConfiguration", "PodSubnet", config.Spec.ClusterConfiguration.Networking.PodSubnet)
 		}
 	}

controllers/cluster_controller.go

@@ -404,7 +404,7 @@ func (c clusterDescendants) filterOwnedDescendants(cluster *clusterv1.Cluster) (
 			return nil
 		}
 
-		if util.PointsTo(acc.GetOwnerReferences(), &cluster.ObjectMeta) {
+		if util.IsOwnedByObject(acc, cluster) {
 			ownedDescendants = append(ownedDescendants, o)
 		}
 

controllers/cluster_controller_test.go

@@ -703,6 +703,10 @@ func TestFilterOwnedDescendants(t *testing.T) {
 	g := NewWithT(t)
 
 	c := clusterv1.Cluster{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: clusterv1.GroupVersion.String(),
+			Kind:       "Cluster",
+		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "c",
 		},

controlplane/kubeadm/config/rbac/role.yaml

@@ -61,6 +61,7 @@ rules:
   - get
   - list
   - patch
+  - update
   - watch
 
 ---