From f792d74fc8fe8fc26546c08f018722a23006e855 Mon Sep 17 00:00:00 2001 From: Seth Pellegrino Date: Thu, 14 May 2020 12:49:48 -0700 Subject: [PATCH] :sparkles: 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 Co-Authored-By: Jason DeTiberus Co-authored-by: Vince Prignano --- .dockerignore | 30 ++- api/v1alpha3/cluster_types.go | 8 + bootstrap/kubeadm/api/equality/doc.go | 28 +++ bootstrap/kubeadm/api/equality/semantic.go | 128 ++++++++++ .../controllers/kubeadmconfig_controller.go | 5 +- controllers/cluster_controller.go | 2 +- controllers/cluster_controller_test.go | 4 + controlplane/kubeadm/config/rbac/role.yaml | 1 + .../kubeadm/controllers/controller.go | 171 ++++++++++++- .../kubeadm/controllers/controller_test.go | 206 ++++++++++++++- .../kubeadm/controllers/fakes_test.go | 14 +- controlplane/kubeadm/controllers/scale.go | 4 +- controlplane/kubeadm/controllers/status.go | 11 +- controlplane/kubeadm/internal/cluster.go | 29 ++- .../kubeadm/internal/cluster_labels.go | 8 - controlplane/kubeadm/internal/cluster_test.go | 14 +- .../machinefilters/machine_filters.go | 55 +++- docs/book/src/tasks/kubeadm-control-plane.md | 31 +++ test/e2e/config/docker-ci.yaml | 3 +- test/e2e/config/docker-dev.yaml | 3 +- test/e2e/custom_assertions.go | 59 +++++ .../cluster-template-kcp-adoption.yaml | 151 +++++++++++ test/e2e/kcp_adoption.go | 237 ++++++++++++++++++ test/e2e/kcp_adoption_test.go | 39 +++ test/framework/cluster_proxy.go | 8 + test/framework/control_plane.go | 49 ++++ test/framework/convenience.go | 7 + test/framework/convenience_test.go | 18 +- test/framework/exec/kubectl.go | 7 +- test/framework/machines.go | 89 +++++++ test/infrastructure/docker/.gitignore | 2 + .../examples/simple-cluster-without-kcp.yaml | 107 ++++++++ .../docker/examples/simple-cluster.yaml | 61 ++--- util/secret/certificates.go | 22 +- util/util.go | 80 +++++- util/util_test.go | 172 +++++++++++-- 36 files changed, 1721 insertions(+), 142 deletions(-) create mode 100644 bootstrap/kubeadm/api/equality/doc.go create mode 100644 bootstrap/kubeadm/api/equality/semantic.go create mode 100644 test/e2e/custom_assertions.go create mode 100644 test/e2e/data/infrastructure-docker/cluster-template-kcp-adoption.yaml create mode 100644 test/e2e/kcp_adoption.go create mode 100644 test/e2e/kcp_adoption_test.go create mode 100644 test/framework/control_plane.go create mode 100644 test/framework/machines.go create mode 100644 test/infrastructure/docker/examples/simple-cluster-without-kcp.yaml diff --git a/.dockerignore b/.dockerignore index 5df15783529d..e12d8e1b4f40 100644 --- a/.dockerignore +++ b/.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 diff --git a/api/v1alpha3/cluster_types.go b/api/v1alpha3/cluster_types.go index a9b8b9ac26a2..779b6b294201 100644 --- a/api/v1alpha3/cluster_types.go +++ b/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 diff --git a/bootstrap/kubeadm/api/equality/doc.go b/bootstrap/kubeadm/api/equality/doc.go new file mode 100644 index 000000000000..6639860cec48 --- /dev/null +++ b/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 diff --git a/bootstrap/kubeadm/api/equality/semantic.go b/bootstrap/kubeadm/api/equality/semantic.go new file mode 100644 index 000000000000..1681bad2f7bf --- /dev/null +++ b/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 +} diff --git a/bootstrap/kubeadm/controllers/kubeadmconfig_controller.go b/bootstrap/kubeadm/controllers/kubeadmconfig_controller.go index 8001b364e6bf..d73f260d430a 100644 --- a/bootstrap/kubeadm/controllers/kubeadmconfig_controller.go +++ b/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) } } diff --git a/controllers/cluster_controller.go b/controllers/cluster_controller.go index cd4541e4e7cb..403af4e38ebd 100644 --- a/controllers/cluster_controller.go +++ b/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) } diff --git a/controllers/cluster_controller_test.go b/controllers/cluster_controller_test.go index ab1b27706eba..6c05f9d1b320 100644 --- a/controllers/cluster_controller_test.go +++ b/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", }, diff --git a/controlplane/kubeadm/config/rbac/role.yaml b/controlplane/kubeadm/config/rbac/role.yaml index 481c8a77e27f..59d4d19fdde2 100644 --- a/controlplane/kubeadm/config/rbac/role.yaml +++ b/controlplane/kubeadm/config/rbac/role.yaml @@ -61,6 +61,7 @@ rules: - get - list - patch + - update - watch --- diff --git a/controlplane/kubeadm/controllers/controller.go b/controlplane/kubeadm/controllers/controller.go index be0bfa210226..be060b708f31 100644 --- a/controlplane/kubeadm/controllers/controller.go +++ b/controlplane/kubeadm/controllers/controller.go @@ -21,6 +21,7 @@ import ( "fmt" "time" + "github.com/blang/semver" "github.com/go-logr/logr" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -29,10 +30,21 @@ import ( "k8s.io/apimachinery/pkg/runtime" kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/tools/record" + "k8s.io/utils/pointer" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/source" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" + "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/equality" + bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha3" kubeadmv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" + "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/hash" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" capierrors "sigs.k8s.io/cluster-api/errors" "sigs.k8s.io/cluster-api/util" @@ -40,16 +52,10 @@ import ( "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/cluster-api/util/predicates" "sigs.k8s.io/cluster-api/util/secret" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/source" ) // +kubebuilder:rbac:groups=core,resources=events,verbs=get;list;watch;create;patch -// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;patch +// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;update;patch // +kubebuilder:rbac:groups=core,resources=configmaps,namespace=kube-system,verbs=get;list;watch;create // +kubebuilder:rbac:groups=rbac,resources=roles,namespace=kube-system,verbs=get;list;watch;create // +kubebuilder:rbac:groups=rbac,resources=rolebindings,namespace=kube-system,verbs=get;list;watch;create @@ -229,13 +235,25 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster * return ctrl.Result{}, err } - // TODO: handle proper adoption of Machines - ownedMachines, err := r.managementCluster.GetMachinesForCluster(ctx, util.ObjectKey(cluster), machinefilters.OwnedControlPlaneMachines(kcp.Name)) + controlPlaneMachines, err := r.managementCluster.GetMachinesForCluster(ctx, util.ObjectKey(cluster), machinefilters.ControlPlaneMachines(cluster.Name)) if err != nil { logger.Error(err, "failed to retrieve control plane machines for cluster") return ctrl.Result{}, err } + adoptableMachines := controlPlaneMachines.Filter(machinefilters.AdoptableControlPlaneMachines(cluster.Name)) + if len(adoptableMachines) > 0 { + // We adopt the Machines and then wait for the update event for the ownership reference to re-queue them so the cache is up-to-date + err = r.adoptMachines(ctx, kcp, adoptableMachines, cluster) + return ctrl.Result{}, err + } + + ownedMachines := controlPlaneMachines.Filter(machinefilters.OwnedMachines(kcp)) + if len(ownedMachines) != len(controlPlaneMachines) { + logger.Info("Not all control plane machines are owned by this KubeadmControlPlane, refusing to operate in mixed management mode") + return ctrl.Result{}, nil + } + controlPlane := internal.NewControlPlane(cluster, kcp, ownedMachines) requireUpgrade := controlPlane.MachinesNeedingUpgrade() // Upgrade takes precedence over other operations @@ -302,7 +320,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, clu if err != nil { return ctrl.Result{}, err } - ownedMachines := allMachines.Filter(machinefilters.OwnedControlPlaneMachines(kcp.Name)) + ownedMachines := allMachines.Filter(machinefilters.OwnedMachines(kcp)) // If no control plane machines remain, remove the finalizer if len(ownedMachines) == 0 { @@ -360,7 +378,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileHealth(ctx context.Context, clu logger := controlPlane.Logger() // Do a health check of the Control Plane components - if err := r.managementCluster.TargetClusterControlPlaneIsHealthy(ctx, util.ObjectKey(cluster), kcp.Name); err != nil { + if err := r.managementCluster.TargetClusterControlPlaneIsHealthy(ctx, util.ObjectKey(cluster)); err != nil { logger.V(2).Info("Waiting for control plane to pass control plane health check to continue reconciliation", "cause", err) r.recorder.Eventf(kcp, corev1.EventTypeWarning, "ControlPlaneUnhealthy", "Waiting for control plane to pass control plane health check to continue reconciliation: %v", err) @@ -368,7 +386,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileHealth(ctx context.Context, clu } // Ensure etcd is healthy - if err := r.managementCluster.TargetClusterEtcdIsHealthy(ctx, util.ObjectKey(cluster), kcp.Name); err != nil { + if err := r.managementCluster.TargetClusterEtcdIsHealthy(ctx, util.ObjectKey(cluster)); err != nil { // If there are any etcd members that do not have corresponding nodes, remove them from etcd and from the kubeadm configmap. // This will solve issues related to manual control-plane machine deletion. workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(cluster)) @@ -391,5 +409,134 @@ func (r *KubeadmControlPlaneReconciler) reconcileHealth(ctx context.Context, clu if controlPlane.HasDeletingMachine() { return &capierrors.RequeueAfterError{RequeueAfter: deleteRequeueAfter} } + + return nil +} + +func (r *KubeadmControlPlaneReconciler) adoptMachines(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, machines internal.FilterableMachineCollection, cluster *clusterv1.Cluster) error { + // We do an uncached full quorum read against the KCP to avoid re-adopting Machines the garbage collector just intentionally orphaned + // See https://github.com/kubernetes/kubernetes/issues/42639 + uncached := controlplanev1.KubeadmControlPlane{} + err := r.managementClusterUncached.Get(ctx, client.ObjectKey{Namespace: kcp.Namespace, Name: kcp.Name}, &uncached) + if err != nil { + return errors.Wrapf(err, "failed to check whether %v/%v was deleted before adoption", kcp.GetNamespace(), kcp.GetName()) + } + if !uncached.DeletionTimestamp.IsZero() { + return errors.Errorf("%v/%v has just been deleted at %v", kcp.GetNamespace(), kcp.GetName(), kcp.GetDeletionTimestamp()) + } + + kcpVersion, err := semver.ParseTolerant(kcp.Spec.Version) + if err != nil { + return errors.Wrapf(err, "failed to parse kubernetes version %q", kcp.Spec.Version) + } + + for _, m := range machines { + ref := m.Spec.Bootstrap.ConfigRef + + // TODO instead of returning error here, we should instead Event and add a watch on potentially adoptable Machines + if ref == nil || ref.Kind != "KubeadmConfig" { + return errors.Errorf("unable to adopt Machine %v/%v: expected a ConfigRef of kind KubeadmConfig but instead found %v", m.Namespace, m.Name, ref) + } + + // TODO instead of returning error here, we should instead Event and add a watch on potentially adoptable Machines + if ref.Namespace != "" && ref.Namespace != kcp.Namespace { + return errors.Errorf("could not adopt resources from KubeadmConfig %v/%v: cannot adopt across namespaces", ref.Namespace, ref.Name) + } + + if m.Spec.Version == nil { + // if the machine's version is not immediately apparent, assume the operator knows what they're doing + continue + } + + machineVersion, err := semver.ParseTolerant(*m.Spec.Version) + if err != nil { + return errors.Wrapf(err, "failed to parse kubernetes version %q", *m.Spec.Version) + } + + if !util.IsSupportedVersionSkew(kcpVersion, machineVersion) { + r.recorder.Eventf(kcp, corev1.EventTypeWarning, "AdoptionFailed", "Could not adopt Machine %s/%s: its version (%q) is outside supported +/- one minor version skew from KCP's (%q)", m.Namespace, m.Name, *m.Spec.Version, kcp.Spec.Version) + // avoid returning an error here so we don't cause the KCP controller to spin until the operator clarifies their intent + return nil + } + } + + for _, m := range machines { + ref := m.Spec.Bootstrap.ConfigRef + cfg := &bootstrapv1.KubeadmConfig{} + + if err := r.Client.Get(ctx, client.ObjectKey{Name: ref.Name, Namespace: kcp.Namespace}, cfg); err != nil { + return err + } + + if err := r.adoptOwnedSecrets(ctx, kcp, cfg, cluster.Name); err != nil { + return err + } + + patchHelper, err := patch.NewHelper(m, r.Client) + if err != nil { + return err + } + + if err := controllerutil.SetControllerReference(kcp, m, r.scheme); err != nil { + return err + } + + // 0. get machine.Spec.Version - the easy answer + machineKubernetesVersion := "" + if m.Spec.Version != nil { + machineKubernetesVersion = *m.Spec.Version + } + + // 1. hash the version (kubernetes version) and kubeadm_controlplane's Spec.infrastructureTemplate + syntheticSpec := controlplanev1.KubeadmControlPlaneSpec{ + Version: machineKubernetesVersion, + InfrastructureTemplate: kcp.Spec.InfrastructureTemplate, + KubeadmConfigSpec: equality.SemanticMerge(cfg.Spec, kcp.Spec.KubeadmConfigSpec, cluster), + } + newConfigurationHash := hash.Compute(&syntheticSpec) + // 2. add kubeadm.controlplane.cluster.x-k8s.io/hash as a label in each machine + m.Labels[controlplanev1.KubeadmControlPlaneHashLabelKey] = newConfigurationHash + + // Note that ValidateOwnerReferences() will reject this patch if another + // OwnerReference exists with controller=true. + if err := patchHelper.Patch(ctx, m); err != nil { + return err + } + } + return nil +} + +func (r *KubeadmControlPlaneReconciler) adoptOwnedSecrets(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, currentOwner *bootstrapv1.KubeadmConfig, clusterName string) error { + secrets := corev1.SecretList{} + if err := r.Client.List(ctx, &secrets, client.InNamespace(kcp.Namespace), client.MatchingLabels{clusterv1.ClusterLabelName: clusterName}); err != nil { + return errors.Wrap(err, "error finding secrets for adoption") + } + + for i := range secrets.Items { + s := secrets.Items[i] + if !util.IsControlledBy(&s, currentOwner) { + continue + } + // avoid taking ownership of the bootstrap data secret + if currentOwner.Status.DataSecretName != nil && s.Name == *currentOwner.Status.DataSecretName { + continue + } + + ss := s.DeepCopy() + + ss.SetOwnerReferences(util.ReplaceOwnerRef(ss.GetOwnerReferences(), currentOwner, metav1.OwnerReference{ + APIVersion: controlplanev1.GroupVersion.String(), + Kind: "KubeadmControlPlane", + Name: kcp.Name, + UID: kcp.UID, + Controller: pointer.BoolPtr(true), + BlockOwnerDeletion: pointer.BoolPtr(true), + })) + + if err := r.Client.Update(ctx, ss); err != nil { + return errors.Wrapf(err, "error changing secret %v ownership from KubeadmConfig/%v to KubeadmControlPlane/%v", s.Name, currentOwner.GetName(), kcp.Name) + } + } + return nil } diff --git a/controlplane/kubeadm/controllers/controller_test.go b/controlplane/kubeadm/controllers/controller_test.go index 0b0deac95f02..866572ece4a5 100644 --- a/controlplane/kubeadm/controllers/controller_test.go +++ b/controlplane/kubeadm/controllers/controller_test.go @@ -35,6 +35,7 @@ import ( "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" "k8s.io/klog/klogr" + "k8s.io/utils/pointer" 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" @@ -398,6 +399,206 @@ func TestReconcileClusterNoEndpoints(t *testing.T) { g.Expect(machineList.Items).To(BeEmpty()) } +func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) { + t.Run("adopts existing Machines", func(t *testing.T) { + g := NewWithT(t) + + cluster, kcp, tmpl := createClusterWithControlPlane() + cluster.Spec.ControlPlaneEndpoint.Host = "bar" + kcp.Spec.Version = "v2.0.0" + + fmc := &fakeManagementCluster{ + Machines: internal.FilterableMachineCollection{}, + ControlPlaneHealthy: true, + EtcdHealthy: true, + } + objs := []runtime.Object{cluster.DeepCopy(), kcp.DeepCopy(), tmpl.DeepCopy()} + for i := 0; i < 3; i++ { + name := fmt.Sprintf("test-%d", i) + m := &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cluster.Namespace, + Name: name, + Labels: internal.ControlPlaneLabelsForCluster(cluster.Name), + }, + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + ConfigRef: &corev1.ObjectReference{ + APIVersion: bootstrapv1.GroupVersion.String(), + Kind: "KubeadmConfig", + Name: name, + }, + }, + Version: pointer.StringPtr("v2.0.0"), + }, + } + cfg := &bootstrapv1.KubeadmConfig{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cluster.Namespace, + Name: name, + }, + } + objs = append(objs, m, cfg) + fmc.Machines.Insert(m) + } + + fakeClient := newFakeClient(g, objs...) + fmc.Reader = fakeClient + + log.SetLogger(klogr.New()) + r := &KubeadmControlPlaneReconciler{ + Client: fakeClient, + Log: log.Log, + scheme: scheme.Scheme, + managementCluster: fmc, + managementClusterUncached: fmc, + } + + g.Expect(r.reconcile(context.Background(), cluster, kcp)).To(Equal(ctrl.Result{})) + + machineList := &clusterv1.MachineList{} + g.Expect(fakeClient.List(context.Background(), machineList, client.InNamespace(cluster.Namespace))).To(Succeed()) + g.Expect(machineList.Items).NotTo(BeEmpty()) + g.Expect(machineList.Items).To(HaveLen(3)) + for _, machine := range machineList.Items { + g.Expect(machine.OwnerReferences).To(HaveLen(1)) + g.Expect(machine.OwnerReferences).To(ContainElement(*metav1.NewControllerRef(kcp, controlplanev1.GroupVersion.WithKind("KubeadmControlPlane")))) + g.Expect(machine.Labels).To(Equal(internal.ControlPlaneLabelsForClusterWithHash(cluster.Name, hash.Compute(&kcp.Spec)))) + } + }) + + t.Run("Deleted KubeadmControlPlanes don't adopt machines", func(t *testing.T) { + // Usually we won't get into the inner reconcile with a deleted control plane, but it's possible when deleting with "oprhanDependents": + // 1. The deletion timestamp is set in the API server, but our cache has not yet updated + // 2. The garbage collector removes our ownership reference from a Machine, triggering a re-reconcile (or we get unlucky with the periodic reconciliation) + // 3. We get into the inner reconcile function and re-adopt the Machine + // 4. The update to our cache for our deletion timestamp arrives + g := NewWithT(t) + + cluster, kcp, tmpl := createClusterWithControlPlane() + cluster.Spec.ControlPlaneEndpoint.Host = "nodomain.example.com" + kcp.Spec.Version = "v2.0.0" + + now := metav1.Now() + kcp.DeletionTimestamp = &now + + fmc := &fakeManagementCluster{ + Machines: internal.FilterableMachineCollection{}, + ControlPlaneHealthy: true, + EtcdHealthy: true, + } + objs := []runtime.Object{cluster.DeepCopy(), kcp.DeepCopy(), tmpl.DeepCopy()} + for i := 0; i < 3; i++ { + name := fmt.Sprintf("test-%d", i) + m := &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cluster.Namespace, + Name: name, + Labels: internal.ControlPlaneLabelsForCluster(cluster.Name), + }, + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + ConfigRef: &corev1.ObjectReference{ + APIVersion: bootstrapv1.GroupVersion.String(), + Kind: "KubeadmConfig", + Name: name, + }, + }, + Version: pointer.StringPtr("v2.0.0"), + }, + } + cfg := &bootstrapv1.KubeadmConfig{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cluster.Namespace, + Name: name, + }, + } + objs = append(objs, m, cfg) + fmc.Machines.Insert(m) + } + fakeClient := newFakeClient(g, objs...) + fmc.Reader = fakeClient + + log.SetLogger(klogr.New()) + r := &KubeadmControlPlaneReconciler{ + Client: fakeClient, + Log: log.Log, + scheme: scheme.Scheme, + managementCluster: fmc, + managementClusterUncached: fmc, + } + + result, err := r.reconcile(context.Background(), cluster, kcp) + g.Expect(result).To(Equal(ctrl.Result{})) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("has just been deleted")) + + machineList := &clusterv1.MachineList{} + g.Expect(fakeClient.List(context.Background(), machineList, client.InNamespace(cluster.Namespace))).To(Succeed()) + g.Expect(machineList.Items).NotTo(BeEmpty()) + g.Expect(machineList.Items).To(HaveLen(3)) + for _, machine := range machineList.Items { + g.Expect(machine.OwnerReferences).To(BeEmpty()) + } + }) + + t.Run("refuses to adopt Machines that are more than one version old", func(t *testing.T) { + g := NewWithT(t) + + cluster, kcp, tmpl := createClusterWithControlPlane() + cluster.Spec.ControlPlaneEndpoint.Host = "nodomain.example.com" + kcp.Spec.Version = "v1.17.0" + + fmc := &fakeManagementCluster{ + Machines: internal.FilterableMachineCollection{ + "test0": &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cluster.Namespace, + Name: fmt.Sprintf("test0"), + Labels: internal.ControlPlaneLabelsForCluster(cluster.Name), + }, + Spec: clusterv1.MachineSpec{ + Bootstrap: clusterv1.Bootstrap{ + ConfigRef: &corev1.ObjectReference{ + APIVersion: bootstrapv1.GroupVersion.String(), + Kind: "KubeadmConfig", + }, + }, + Version: pointer.StringPtr("v1.15.0"), + }, + }, + }, + ControlPlaneHealthy: true, + EtcdHealthy: true, + } + + fakeClient := newFakeClient(g, cluster.DeepCopy(), kcp.DeepCopy(), tmpl.DeepCopy(), fmc.Machines["test0"].DeepCopy()) + fmc.Reader = fakeClient + + log.SetLogger(klogr.New()) + recorder := record.NewFakeRecorder(32) + r := &KubeadmControlPlaneReconciler{ + Client: fakeClient, + Log: log.Log, + recorder: recorder, + managementCluster: fmc, + managementClusterUncached: fmc, + } + + g.Expect(r.reconcile(context.Background(), cluster, kcp)).To(Equal(ctrl.Result{})) + // Message: Warning AdoptionFailed Could not adopt Machine test/test0: its version ("v1.15.0") is outside supported +/- one minor version skew from KCP's ("v1.17.0") + g.Expect(recorder.Events).To(Receive(ContainSubstring("minor version"))) + + machineList := &clusterv1.MachineList{} + g.Expect(fakeClient.List(context.Background(), machineList, client.InNamespace(cluster.Namespace))).To(Succeed()) + g.Expect(machineList.Items).NotTo(BeEmpty()) + g.Expect(machineList.Items).To(HaveLen(1)) + for _, machine := range machineList.Items { + g.Expect(machine.OwnerReferences).To(BeEmpty()) + } + }) +} + func TestReconcileInitializeControlPlane(t *testing.T) { g := NewWithT(t) @@ -1017,6 +1218,10 @@ func createClusterWithControlPlane() (*clusterv1.Cluster, *controlplanev1.Kubead } kcp := &controlplanev1.KubeadmControlPlane{ + TypeMeta: metav1.TypeMeta{ + APIVersion: controlplanev1.GroupVersion.String(), + Kind: "KubeadmControlPlane", + }, ObjectMeta: metav1.ObjectMeta{ Name: "kcp-foo", Namespace: cluster.Namespace, @@ -1087,7 +1292,6 @@ func createMachineNodePair(name string, cluster *clusterv1.Cluster, kcp *control }, } } - return machine, node } diff --git a/controlplane/kubeadm/controllers/fakes_test.go b/controlplane/kubeadm/controllers/fakes_test.go index 4efd2be2036a..46af76804696 100644 --- a/controlplane/kubeadm/controllers/fakes_test.go +++ b/controlplane/kubeadm/controllers/fakes_test.go @@ -21,6 +21,7 @@ import ( "errors" "github.com/blang/semver" + "k8s.io/apimachinery/pkg/runtime" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" @@ -34,6 +35,15 @@ type fakeManagementCluster struct { EtcdHealthy bool Machines internal.FilterableMachineCollection Workload fakeWorkloadCluster + Reader client.Reader +} + +func (f *fakeManagementCluster) Get(ctx context.Context, key client.ObjectKey, obj runtime.Object) error { + return f.Reader.Get(ctx, key, obj) +} + +func (f *fakeManagementCluster) List(ctx context.Context, list runtime.Object, opts ...client.ListOption) error { + return f.Reader.List(ctx, list, opts...) } func (f *fakeManagementCluster) GetWorkloadCluster(_ context.Context, _ client.ObjectKey) (internal.WorkloadCluster, error) { @@ -47,14 +57,14 @@ func (f *fakeManagementCluster) GetMachinesForCluster(c context.Context, n clien return f.Machines, nil } -func (f *fakeManagementCluster) TargetClusterControlPlaneIsHealthy(_ context.Context, _ client.ObjectKey, _ string) error { +func (f *fakeManagementCluster) TargetClusterControlPlaneIsHealthy(_ context.Context, _ client.ObjectKey) error { if !f.ControlPlaneHealthy { return errors.New("control plane is not healthy") } return nil } -func (f *fakeManagementCluster) TargetClusterEtcdIsHealthy(_ context.Context, _ client.ObjectKey, _ string) error { +func (f *fakeManagementCluster) TargetClusterEtcdIsHealthy(_ context.Context, _ client.ObjectKey) error { if !f.EtcdHealthy { return errors.New("etcd is not healthy") } diff --git a/controlplane/kubeadm/controllers/scale.go b/controlplane/kubeadm/controllers/scale.go index 8b2a4c95feac..c0e8f4362d49 100644 --- a/controlplane/kubeadm/controllers/scale.go +++ b/controlplane/kubeadm/controllers/scale.go @@ -36,7 +36,7 @@ func (r *KubeadmControlPlaneReconciler) initializeControlPlane(ctx context.Conte // Perform an uncached read of all the owned machines. This check is in place to make sure // that the controller cache is not misbehaving and we end up initializing the cluster more than once. - ownedMachines, err := r.managementClusterUncached.GetMachinesForCluster(ctx, util.ObjectKey(cluster), machinefilters.OwnedControlPlaneMachines(kcp.Name)) + ownedMachines, err := r.managementClusterUncached.GetMachinesForCluster(ctx, util.ObjectKey(cluster), machinefilters.OwnedMachines(kcp)) if err != nil { logger.Error(err, "failed to perform an uncached read of control plane machines for cluster") return ctrl.Result{}, err @@ -120,7 +120,7 @@ func (r *KubeadmControlPlaneReconciler) scaleDownControlPlane( return ctrl.Result{}, err } - if err := r.managementCluster.TargetClusterControlPlaneIsHealthy(ctx, util.ObjectKey(cluster), kcp.Name); err != nil { + if err := r.managementCluster.TargetClusterControlPlaneIsHealthy(ctx, util.ObjectKey(cluster)); err != nil { logger.V(2).Info("Waiting for control plane to pass control plane health check before removing a control plane machine", "cause", err) r.recorder.Eventf(kcp, corev1.EventTypeWarning, "ControlPlaneUnhealthy", "Waiting for control plane to pass control plane health check before removing a control plane machine: %v", err) diff --git a/controlplane/kubeadm/controllers/status.go b/controlplane/kubeadm/controllers/status.go index 051e77e39ca1..6b14342818c0 100644 --- a/controlplane/kubeadm/controllers/status.go +++ b/controlplane/kubeadm/controllers/status.go @@ -20,10 +20,8 @@ import ( "context" "github.com/pkg/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" - "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/hash" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" "sigs.k8s.io/cluster-api/util" @@ -32,17 +30,12 @@ import ( // updateStatus is called after every reconcilitation loop in a defer statement to always make sure we have the // resource status subresourcs up-to-date. func (r *KubeadmControlPlaneReconciler) updateStatus(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster) error { - labelSelector := internal.ControlPlaneSelectorForCluster(cluster.Name) - selector, err := metav1.LabelSelectorAsSelector(labelSelector) - if err != nil { - // Since we are building up the LabelSelector above, this should not fail - return errors.Wrap(err, "failed to parse label selector") - } + selector := machinefilters.ControlPlaneSelectorForCluster(cluster.Name) // Copy label selector to its status counterpart in string format. // This is necessary for CRDs including scale subresources. kcp.Status.Selector = selector.String() - ownedMachines, err := r.managementCluster.GetMachinesForCluster(ctx, util.ObjectKey(cluster), machinefilters.OwnedControlPlaneMachines(kcp.Name)) + ownedMachines, err := r.managementCluster.GetMachinesForCluster(ctx, util.ObjectKey(cluster), machinefilters.OwnedMachines(kcp)) if err != nil { return errors.Wrap(err, "failed to get list of owned machines") } diff --git a/controlplane/kubeadm/internal/cluster.go b/controlplane/kubeadm/internal/cluster.go index 2c74ae5a272c..d9f67d541eda 100644 --- a/controlplane/kubeadm/internal/cluster.go +++ b/controlplane/kubeadm/internal/cluster.go @@ -25,6 +25,7 @@ import ( "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" @@ -38,9 +39,11 @@ import ( // ManagementCluster defines all behaviors necessary for something to function as a management cluster. type ManagementCluster interface { + ctrlclient.Reader + GetMachinesForCluster(ctx context.Context, cluster client.ObjectKey, filters ...machinefilters.Func) (FilterableMachineCollection, error) - TargetClusterEtcdIsHealthy(ctx context.Context, clusterKey client.ObjectKey, controlPlaneName string) error - TargetClusterControlPlaneIsHealthy(ctx context.Context, clusterKey client.ObjectKey, controlPlaneName string) error + TargetClusterEtcdIsHealthy(ctx context.Context, clusterKey client.ObjectKey) error + TargetClusterControlPlaneIsHealthy(ctx context.Context, clusterKey client.ObjectKey) error GetWorkloadCluster(ctx context.Context, clusterKey client.ObjectKey) (WorkloadCluster, error) } @@ -58,6 +61,16 @@ type RemoteClusterConnectionError struct { func (e *RemoteClusterConnectionError) Error() string { return e.Name + ": " + e.Err.Error() } func (e *RemoteClusterConnectionError) Unwrap() error { return e.Err } +// Get implements ctrlclient.Reader +func (m *Management) Get(ctx context.Context, key ctrlclient.ObjectKey, obj runtime.Object) error { + return m.Client.Get(ctx, key, obj) +} + +// List implements ctrlclient.Reader +func (m *Management) List(ctx context.Context, list runtime.Object, opts ...ctrlclient.ListOption) error { + return m.Client.List(ctx, list, opts...) +} + // GetMachinesForCluster returns a list of machines that can be filtered or not. // If no filter is supplied then all machines associated with the target cluster are returned. func (m *Management) GetMachinesForCluster(ctx context.Context, cluster client.ObjectKey, filters ...machinefilters.Func) (FilterableMachineCollection, error) { @@ -131,7 +144,7 @@ type healthCheck func(context.Context) (HealthCheckResult, error) // HealthCheck will run a generic health check function and report any errors discovered. // In addition to the health check, it also ensures there is a 1;1 match between nodes and machines. -func (m *Management) healthCheck(ctx context.Context, check healthCheck, clusterKey client.ObjectKey, controlPlaneName string) error { +func (m *Management) healthCheck(ctx context.Context, check healthCheck, clusterKey client.ObjectKey) error { var errorList []error nodeChecks, err := check(ctx) if err != nil { @@ -147,7 +160,7 @@ func (m *Management) healthCheck(ctx context.Context, check healthCheck, cluster } // Make sure Cluster API is aware of all the nodes. - machines, err := m.GetMachinesForCluster(ctx, clusterKey, machinefilters.OwnedControlPlaneMachines(controlPlaneName)) + machines, err := m.GetMachinesForCluster(ctx, clusterKey, machinefilters.ControlPlaneMachines(clusterKey.Name)) if err != nil { return err } @@ -169,21 +182,21 @@ func (m *Management) healthCheck(ctx context.Context, check healthCheck, cluster } // TargetClusterControlPlaneIsHealthy checks every node for control plane health. -func (m *Management) TargetClusterControlPlaneIsHealthy(ctx context.Context, clusterKey client.ObjectKey, controlPlaneName string) error { +func (m *Management) TargetClusterControlPlaneIsHealthy(ctx context.Context, clusterKey client.ObjectKey) error { // TODO: add checks for expected taints/labels cluster, err := m.GetWorkloadCluster(ctx, clusterKey) if err != nil { return err } - return m.healthCheck(ctx, cluster.ControlPlaneIsHealthy, clusterKey, controlPlaneName) + return m.healthCheck(ctx, cluster.ControlPlaneIsHealthy, clusterKey) } // TargetClusterEtcdIsHealthy runs a series of checks over a target cluster's etcd cluster. // In addition, it verifies that there are the same number of etcd members as control plane Machines. -func (m *Management) TargetClusterEtcdIsHealthy(ctx context.Context, clusterKey client.ObjectKey, controlPlaneName string) error { +func (m *Management) TargetClusterEtcdIsHealthy(ctx context.Context, clusterKey client.ObjectKey) error { cluster, err := m.GetWorkloadCluster(ctx, clusterKey) if err != nil { return err } - return m.healthCheck(ctx, cluster.EtcdIsHealthy, clusterKey, controlPlaneName) + return m.healthCheck(ctx, cluster.EtcdIsHealthy, clusterKey) } diff --git a/controlplane/kubeadm/internal/cluster_labels.go b/controlplane/kubeadm/internal/cluster_labels.go index 165da0817c09..044a81797d7b 100644 --- a/controlplane/kubeadm/internal/cluster_labels.go +++ b/controlplane/kubeadm/internal/cluster_labels.go @@ -17,7 +17,6 @@ limitations under the License. package internal import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" ) @@ -37,10 +36,3 @@ func ControlPlaneLabelsForCluster(clusterName string) map[string]string { clusterv1.MachineControlPlaneLabelName: "", } } - -// ControlPlaneSelectorForCluster returns the label selector necessary to get control plane machines for a given cluster. -func ControlPlaneSelectorForCluster(clusterName string) *metav1.LabelSelector { - return &metav1.LabelSelector{ - MatchLabels: ControlPlaneLabelsForCluster(clusterName), - } -} diff --git a/controlplane/kubeadm/internal/cluster_test.go b/controlplane/kubeadm/internal/cluster_test.go index eab2bd8143bb..c71ae3d5e754 100644 --- a/controlplane/kubeadm/internal/cluster_test.go +++ b/controlplane/kubeadm/internal/cluster_test.go @@ -164,8 +164,8 @@ func TestGetMachinesForCluster(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) g.Expect(machines).To(HaveLen(3)) - // Test the OwnedControlPlaneMachines works - machines, err = m.GetMachinesForCluster(context.Background(), clusterKey, machinefilters.OwnedControlPlaneMachines("my-control-plane")) + // Test the ControlPlaneMachines works + machines, err = m.GetMachinesForCluster(context.Background(), clusterKey, machinefilters.ControlPlaneMachines("my-cluster")) g.Expect(err).NotTo(HaveOccurred()) g.Expect(machines).To(HaveLen(1)) @@ -173,7 +173,7 @@ func TestGetMachinesForCluster(t *testing.T) { nameFilter := func(cluster *clusterv1.Machine) bool { return cluster.Name == "first-machine" } - machines, err = m.GetMachinesForCluster(context.Background(), clusterKey, machinefilters.OwnedControlPlaneMachines("my-control-plane"), nameFilter) + machines, err = m.GetMachinesForCluster(context.Background(), clusterKey, machinefilters.ControlPlaneMachines("my-cluster"), nameFilter) g.Expect(err).NotTo(HaveOccurred()) g.Expect(machines).To(HaveLen(1)) } @@ -314,8 +314,7 @@ func TestManagementCluster_healthCheck_NoError(t *testing.T) { "three": nil, }, nil }, - clusterKey: client.ObjectKey{Namespace: "default", Name: "cluster-name"}, - controlPlaneName: "control-plane-name", + clusterKey: client.ObjectKey{Namespace: "default", Name: "cluster-name"}, }, } for _, tt := range tests { @@ -326,7 +325,7 @@ func TestManagementCluster_healthCheck_NoError(t *testing.T) { m := &Management{ Client: &fakeClient{list: tt.machineList}, } - g.Expect(m.healthCheck(ctx, tt.check, tt.clusterKey, tt.controlPlaneName)).To(Succeed()) + g.Expect(m.healthCheck(ctx, tt.check, tt.clusterKey)).To(Succeed()) }) } } @@ -432,12 +431,11 @@ func TestManagementCluster_healthCheck_Errors(t *testing.T) { ctx := context.Background() clusterKey := client.ObjectKey{Namespace: "default", Name: "cluster-name"} - controlPlaneName := "control-plane-name" m := &Management{ Client: &fakeClient{list: tt.machineList}, } - err := m.healthCheck(ctx, tt.check, clusterKey, controlPlaneName) + err := m.healthCheck(ctx, tt.check, clusterKey) g.Expect(err).To(HaveOccurred()) for _, expectedError := range tt.expectedErrors { diff --git a/controlplane/kubeadm/internal/machinefilters/machine_filters.go b/controlplane/kubeadm/internal/machinefilters/machine_filters.go index 90dad747a0bd..5a75db4736ef 100644 --- a/controlplane/kubeadm/internal/machinefilters/machine_filters.go +++ b/controlplane/kubeadm/internal/machinefilters/machine_filters.go @@ -18,9 +18,13 @@ package machinefilters import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" + "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) type Func func(machine *clusterv1.Machine) bool @@ -56,6 +60,14 @@ func Not(mf Func) Func { } } +// HasControllerRef is a filter that returns true if the machine has a controller ref +func HasControllerRef(machine *clusterv1.Machine) bool { + if machine == nil { + return false + } + return metav1.GetControllerOf(machine) != nil +} + // InFailureDomains returns a filter to find all machines // in any of the given failure domains func InFailureDomains(failureDomains ...*string) Func { @@ -82,21 +94,38 @@ func InFailureDomains(failureDomains ...*string) Func { } } -// OwnedControlPlaneMachines rerturns a filter to find all owned control plane machines. -// Usage: managementCluster.GetMachinesForCluster(ctx, cluster, machinefilters.OwnedControlPlaneMachines(controlPlane.Name)) -func OwnedControlPlaneMachines(controlPlaneName string) Func { +// OwnedMachines returns a filter to find all owned control plane machines. +// Usage: managementCluster.GetMachinesForCluster(ctx, cluster, machinefilters.OwnedMachines(controlPlane)) +func OwnedMachines(owner controllerutil.Object) func(machine *clusterv1.Machine) bool { return func(machine *clusterv1.Machine) bool { if machine == nil { return false } - controllerRef := metav1.GetControllerOf(machine) - if controllerRef == nil { + return util.IsOwnedByObject(machine, owner) + } +} + +// ControlPlaneMachines returns a filter to find all control plane machines for a cluster, regardless of ownership. +// Usage: managementCluster.GetMachinesForCluster(ctx, cluster, machinefilters.ControlPlaneMachines(cluster.Name)) +func ControlPlaneMachines(clusterName string) func(machine *clusterv1.Machine) bool { + selector := ControlPlaneSelectorForCluster(clusterName) + return func(machine *clusterv1.Machine) bool { + if machine == nil { return false } - return controllerRef.Kind == "KubeadmControlPlane" && controllerRef.Name == controlPlaneName + return selector.Matches(labels.Set(machine.Labels)) } } +// AdoptableControlPlaneMachines returns a filter to find all un-controlled control plane machines. +// Usage: managementCluster.GetMachinesForCluster(ctx, cluster, AdoptableControlPlaneMachines(cluster.Name, controlPlane)) +func AdoptableControlPlaneMachines(clusterName string) func(machine *clusterv1.Machine) bool { + return And( + ControlPlaneMachines(clusterName), + Not(HasControllerRef), + ) +} + // HasDeletionTimestamp returns a filter to find all machines that have a deletion timestamp. func HasDeletionTimestamp(machine *clusterv1.Machine) bool { if machine == nil { @@ -143,3 +172,17 @@ func HasAnnotationKey(key string) Func { return false } } + +// ControlPlaneSelectorForCluster returns the label selector necessary to get control plane machines for a given cluster. +func ControlPlaneSelectorForCluster(clusterName string) labels.Selector { + must := func(r *labels.Requirement, err error) labels.Requirement { + if err != nil { + panic(err) + } + return *r + } + return labels.NewSelector().Add( + must(labels.NewRequirement(clusterv1.ClusterLabelName, selection.Equals, []string{clusterName})), + must(labels.NewRequirement(clusterv1.MachineControlPlaneLabelName, selection.Exists, []string{})), + ) +} diff --git a/docs/book/src/tasks/kubeadm-control-plane.md b/docs/book/src/tasks/kubeadm-control-plane.md index 6558f9da3592..a232a3ee8e35 100644 --- a/docs/book/src/tasks/kubeadm-control-plane.md +++ b/docs/book/src/tasks/kubeadm-control-plane.md @@ -44,3 +44,34 @@ transparently manage `MachineSet`s and `Machine`s to allow for a seamless scalin For a more in-depth look at how `MachineDeployments` manage scaling events, take a look at the [`MachineDeployment` controller documentation](../developer/architecture/controllers/machine-deployment.md) and the [`MachineSet` controller documentation](../developer/architecture/controllers/machine-set.md). + +### Adopting existing machines into KubeadmControlPlane management + +If your cluster has existing machines labeled with `cluster.x-k8s.io/control-plane`, you may opt in to management of those machines by creating a new KubeadmControlPlane object and updating the associated Cluster object's `controlPlaneRef` like so: + +``` +--- +apiVersion: "cluster.x-k8s.io/v1alpha3" +kind: Cluster +... +spec: + controlPlaneRef: + apiVersion: controlplane.cluster.x-k8s.io/v1alpha3 + kind: KubeadmControlPlane + name: controlplane + namespace: default +... +``` + +Caveats: + +* The KCP controller will refuse to adopt any control plane Machines not bootstrapped with the kubeadm bootstrapper. +* The KCP controller may immediately begin upgrading Machines post-adoption if they're out of date. +* The KCP controller attempts to behave intelligently when adopting existing Machines, but because the bootstrapping process sets various fields in the KubeadmConfig of a machine it's not always obvious the original user-supplied `KubeadmConfig` would have been for that machine. The controller attempts to guess this intent to not replace Machines unnecessarily, so if it guesses wrongly, the consequence is that the KCP controller will effect an "upgrade" to its current config. For full details, see SemanticMerge in the kubeadm bootstrapper's api/equality package. +* If the cluster's PKI materials were generated by an initial KubeadmConfig reconcile, they'll be owned by the KubeadmConfig bound to that machine. The adoption process re-parents these resources to the KCP so they're not lost during an upgrade, but deleting the KCP post-adoption will destroy those materials. +* The `ClusterConfiguration` is not currently reconciled with their ConfigMaps the workload cluster, and `kubeadm` considers the ConfigMap authoritative. These fields on the KCP will be effectively ignored, and most notably include: + * `kubeadmConfigSpec.clusterConfiguration.apiServer.extraArgs` + * `kubeadmConfigSpec.clusterConfiguration.controllerManager.extraArgs` + * `kubeadmConfigSpec.clusterConfiguration.scheduler.extraArgs` + * Anything underneath `kubeadmConfigSpec.clusterConfiguration.etcd` + * etc. diff --git a/test/e2e/config/docker-ci.yaml b/test/e2e/config/docker-ci.yaml index 06e6658edf3b..886ce12e83c9 100644 --- a/test/e2e/config/docker-ci.yaml +++ b/test/e2e/config/docker-ci.yaml @@ -56,9 +56,10 @@ providers: files: # Add a metadata for docker provider - sourcePath: "../data/infrastructure-docker/metadata.yaml" - # Add a cluster template + # Add cluster templates - sourcePath: "../data/infrastructure-docker/cluster-template-ci.yaml" targetName: "cluster-template.yaml" + - sourcePath: "../data/infrastructure-docker/cluster-template-kcp-adoption.yaml" variables: KUBERNETES_VERSION: "v1.18.2" diff --git a/test/e2e/config/docker-dev.yaml b/test/e2e/config/docker-dev.yaml index fe1dd6ca599c..4e34949b9766 100644 --- a/test/e2e/config/docker-dev.yaml +++ b/test/e2e/config/docker-dev.yaml @@ -87,8 +87,9 @@ providers: files: # Add a metadata for docker provider - sourcePath: "../data/infrastructure-docker/metadata.yaml" - # Add a cluster template + # Add cluster templates - sourcePath: "../data/infrastructure-docker/cluster-template.yaml" + - sourcePath: "../data/infrastructure-docker/cluster-template-kcp-adoption.yaml" variables: KUBERNETES_VERSION: "v1.18.2" diff --git a/test/e2e/custom_assertions.go b/test/e2e/custom_assertions.go new file mode 100644 index 000000000000..2d293935e9d3 --- /dev/null +++ b/test/e2e/custom_assertions.go @@ -0,0 +1,59 @@ +/* +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 e2e + +import ( + "fmt" + + "github.com/onsi/gomega/types" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type controllerMatch struct { + kind string + owner metav1.Object +} + +func (m *controllerMatch) Match(actual interface{}) (success bool, err error) { + actualMeta, err := meta.Accessor(actual) + if err != nil { + return false, fmt.Errorf("unable to read meta for %T: %w", actual, err) + } + + owner := metav1.GetControllerOf(actualMeta) + if owner == nil { + return false, fmt.Errorf("no controller found (owner ref with controller = true) for object %#v", actual) + } + + match := (owner.Kind == m.kind && + owner.Name == m.owner.GetName() && owner.UID == m.owner.GetUID()) + + return match, nil +} + +func (m *controllerMatch) FailureMessage(actual interface{}) string { + return fmt.Sprintf("Expected\n\t%#v to have a controller reference pointing to %s/%s (%v)", actual, m.kind, m.owner.GetName(), m.owner.GetUID()) +} + +func (m *controllerMatch) NegatedFailureMessage(actual interface{}) string { + return fmt.Sprintf("Expected\n\t%#v to not have a controller reference pointing to %s/%s (%v)", actual, m.kind, m.owner.GetName(), m.owner.GetUID()) +} + +func HaveControllerRef(kind string, owner metav1.Object) types.GomegaMatcher { + return &controllerMatch{kind, owner} +} diff --git a/test/e2e/data/infrastructure-docker/cluster-template-kcp-adoption.yaml b/test/e2e/data/infrastructure-docker/cluster-template-kcp-adoption.yaml new file mode 100644 index 000000000000..4d79da4ce2fb --- /dev/null +++ b/test/e2e/data/infrastructure-docker/cluster-template-kcp-adoption.yaml @@ -0,0 +1,151 @@ +## +# these resources are sequenced by label: +# 1. initial: just the resources to bootstrap an initial controlplane +# 2. kcp: the KCP resources (note the duplicated Cluster to associate the control plane) +## +### +# 1. initial +### +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: DockerCluster +metadata: + name: '${ CLUSTER_NAME }' + labels: + initial: '' +--- +apiVersion: cluster.x-k8s.io/v1alpha3 +kind: Cluster +metadata: + name: '${ CLUSTER_NAME }' + labels: + initial: '' +spec: + clusterNetwork: + services: + cidrBlocks: ['${ DOCKER_SERVICE_CIDRS }'] + pods: + cidrBlocks: ['${ DOCKER_POD_CIDRS }'] + serviceDomain: '${ DOCKER_SERVICE_DOMAIN }' + infrastructureRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 + kind: DockerCluster + name: '${ CLUSTER_NAME }' +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: DockerMachine +metadata: + name: "${CLUSTER_NAME}-control-plane-0" + labels: + initial: '' +spec: + extraMounts: + - containerPath: "/var/run/docker.sock" + hostPath: "/var/run/docker.sock" +--- +apiVersion: bootstrap.cluster.x-k8s.io/v1alpha3 +kind: KubeadmConfig +metadata: + name: "${CLUSTER_NAME}-control-plane-0" + labels: + initial: '' +spec: + clusterConfiguration: + controllerManager: + extraArgs: {enable-hostpath-provisioner: 'true'} + apiServer: + certSANs: [localhost, 127.0.0.1] + initConfiguration: + nodeRegistration: + criSocket: /var/run/containerd/containerd.sock + kubeletExtraArgs: {eviction-hard: 'nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0%'} + joinConfiguration: + nodeRegistration: + criSocket: /var/run/containerd/containerd.sock + kubeletExtraArgs: {eviction-hard: 'nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0%'} +--- +apiVersion: cluster.x-k8s.io/v1alpha3 +kind: Machine +metadata: + name: "${CLUSTER_NAME}-control-plane-0" + labels: + initial: '' + cluster.x-k8s.io/control-plane: '' +spec: + clusterName: "${ CLUSTER_NAME }" + version: "${ KUBERNETES_VERSION }" + bootstrap: + configRef: + name: "${ CLUSTER_NAME }-control-plane-0" + apiVersion: bootstrap.cluster.x-k8s.io/v1alpha3 + kind: KubeadmConfig + infrastructureRef: + name: "${ CLUSTER_NAME }-control-plane-0" + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 + kind: DockerMachine +--- +### +# 2. kcp +### +apiVersion: cluster.x-k8s.io/v1alpha3 +kind: Cluster +metadata: + name: '${ CLUSTER_NAME }' + labels: + kcp: '' +spec: + clusterNetwork: + services: + cidrBlocks: ['${ DOCKER_SERVICE_CIDRS }'] + pods: + cidrBlocks: ['${ DOCKER_POD_CIDRS }'] + serviceDomain: '${ DOCKER_SERVICE_DOMAIN }' + controlPlaneRef: + kind: KubeadmControlPlane + apiVersion: controlplane.cluster.x-k8s.io/v1alpha3 + name: "${CLUSTER_NAME}-control-plane" + infrastructureRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 + kind: DockerCluster + name: '${ CLUSTER_NAME }' +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: DockerMachineTemplate +metadata: + name: "${CLUSTER_NAME}-control-plane" + labels: + kcp: '' +spec: + template: + spec: + extraMounts: + - containerPath: "/var/run/docker.sock" + hostPath: "/var/run/docker.sock" +--- +kind: KubeadmControlPlane +apiVersion: controlplane.cluster.x-k8s.io/v1alpha3 +metadata: + name: "${ CLUSTER_NAME }-control-plane" + labels: + cluster.x-k8s.io/cluster-name: "${ CLUSTER_NAME }" + kcp: '' +spec: + replicas: ${ CONTROL_PLANE_MACHINE_COUNT } + infrastructureTemplate: + kind: DockerMachineTemplate + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 + name: "${CLUSTER_NAME}-control-plane" + kubeadmConfigSpec: + clusterConfiguration: + controllerManager: + extraArgs: {enable-hostpath-provisioner: 'true'} + apiServer: + certSANs: [localhost, 127.0.0.1] + initConfiguration: + nodeRegistration: + criSocket: /var/run/containerd/containerd.sock + kubeletExtraArgs: {eviction-hard: 'nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0%'} + joinConfiguration: + nodeRegistration: + criSocket: /var/run/containerd/containerd.sock + kubeletExtraArgs: {eviction-hard: 'nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0%'} + version: "${KUBERNETES_VERSION}" diff --git a/test/e2e/kcp_adoption.go b/test/e2e/kcp_adoption.go new file mode 100644 index 000000000000..1236637495d8 --- /dev/null +++ b/test/e2e/kcp_adoption.go @@ -0,0 +1,237 @@ +/* +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 e2e + +import ( + "context" + "fmt" + "io/ioutil" + "os" + "path/filepath" + "strings" + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + "k8s.io/utils/pointer" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" + bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha3" + "sigs.k8s.io/cluster-api/test/framework" + "sigs.k8s.io/cluster-api/test/framework/clusterctl" + "sigs.k8s.io/cluster-api/util" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +// KCPUpgradeSpecInput is the input for KCPUpgradeSpec. +type KCPAdoptionSpecInput struct { + E2EConfig *clusterctl.E2EConfig + ClusterctlConfigPath string + BootstrapClusterProxy ClusterProxy + ArtifactFolder string + SkipCleanup bool +} + +type ClusterProxy interface { + framework.ClusterProxy + + ApplyWithArgs(context.Context, []byte, ...string) error +} + +// KCPAdoptionSpec implements a test that verifies KCP to properly adopt existing control plane Machines +func KCPAdoptionSpec(ctx context.Context, inputGetter func() KCPAdoptionSpecInput) { + var ( + specName = "kcp-adoption" + input KCPAdoptionSpecInput + namespace *corev1.Namespace + cancelWatches context.CancelFunc + cluster *clusterv1.Cluster + replicas = pointer.Int64Ptr(1) + ) + + SetDefaultEventuallyTimeout(15 * time.Minute) + SetDefaultEventuallyPollingInterval(10 * time.Second) + + BeforeEach(func() { + Expect(ctx).NotTo(BeNil(), "ctx is required for %s spec", specName) + input = inputGetter() + Expect(input.E2EConfig).ToNot(BeNil(), "Invalid argument. input.E2EConfig can't be nil when calling %s spec", specName) + Expect(input.ClusterctlConfigPath).To(BeAnExistingFile(), "Invalid argument. input.ClusterctlConfigPath must be an existing file when calling %s spec", specName) + Expect(input.BootstrapClusterProxy).ToNot(BeNil(), "Invalid argument. input.BootstrapClusterProxy can't be nil when calling %s spec", specName) + Expect(os.MkdirAll(input.ArtifactFolder, 0755)).To(Succeed(), "Invalid argument. input.ArtifactFolder can't be created for %s spec", specName) + + // Setup a Namespace where to host objects for this spec and create a watcher for the namespace events. + namespace, cancelWatches = setupSpecNamespace(ctx, specName, input.BootstrapClusterProxy, input.ArtifactFolder) + }) + + It("Should adopt up-to-date control plane Machines without modification", func() { + + By("Creating a workload cluster") + Expect(input.E2EConfig.Variables).To(HaveKey(KubernetesVersion)) + Expect(input.E2EConfig.Variables).To(HaveKey(CNIPath)) + + clusterName := fmt.Sprintf("cluster-%s", util.RandomString(6)) + client := input.BootstrapClusterProxy.GetClient() + CNIManifestPath := input.E2EConfig.GetVariable(CNIPath) + WaitForClusterIntervals := input.E2EConfig.GetIntervals(specName, "wait-cluster") + WaitForControlPlaneIntervals := input.E2EConfig.GetIntervals(specName, "wait-control-plane") + + workloadClusterTemplate := clusterctl.ConfigCluster(ctx, clusterctl.ConfigClusterInput{ + // pass reference to the management cluster hosting this test + KubeconfigPath: input.BootstrapClusterProxy.GetKubeconfigPath(), + // pass the clusterctl config file that points to the local provider repository created for this test, + ClusterctlConfigPath: input.ClusterctlConfigPath, + // select template + Flavor: "kcp-adoption", + // define template variables + Namespace: namespace.Name, + ClusterName: clusterName, + KubernetesVersion: input.E2EConfig.GetVariable(KubernetesVersion), + InfrastructureProvider: clusterctl.DefaultInfrastructureProvider, + ControlPlaneMachineCount: replicas, + WorkerMachineCount: pointer.Int64Ptr(0), + // setup clusterctl logs folder + LogFolder: filepath.Join(input.ArtifactFolder, "clusters", input.BootstrapClusterProxy.GetName()), + }) + Expect(workloadClusterTemplate).ToNot(BeNil(), "Failed to get the cluster template") + + By("Applying the cluster template yaml to the cluster with the 'initial' selector") + Expect(input.BootstrapClusterProxy.ApplyWithArgs(ctx, workloadClusterTemplate, "--selector", "initial")).ShouldNot(HaveOccurred()) + + cluster = framework.DiscoveryAndWaitForCluster(ctx, framework.DiscoveryAndWaitForClusterInput{ + Getter: client, + Namespace: namespace.Name, + Name: clusterName, + }, WaitForClusterIntervals...) + + framework.WaitForClusterMachineNodeRefs(ctx, framework.WaitForClusterMachineNodeRefsInput{ + GetLister: client, + Cluster: cluster, + }, WaitForControlPlaneIntervals...) + + By("Installing a CNI plugin to the workload cluster") + workloadCluster := input.BootstrapClusterProxy.GetWorkloadCluster(context.TODO(), cluster.Namespace, cluster.Name) + + cniYaml, err := ioutil.ReadFile(CNIManifestPath) + Expect(err).ShouldNot(HaveOccurred()) + + Expect(workloadCluster.Apply(context.TODO(), cniYaml)).ShouldNot(HaveOccurred()) + + framework.WaitForClusterMachinesReady(ctx, framework.WaitForClusterMachinesReadyInput{ + GetLister: input.BootstrapClusterProxy.GetClient(), + NodeGetter: workloadCluster.GetClient(), + Cluster: cluster, + }, WaitForControlPlaneIntervals...) + + By("Applying the cluster template yaml to the cluster with the 'kcp' selector") + Expect(input.BootstrapClusterProxy.ApplyWithArgs(ctx, workloadClusterTemplate, "--selector", "kcp")).ShouldNot(HaveOccurred()) + + controlPlane := framework.GetKubeadmControlPlaneByCluster(ctx, framework.GetKubeadmControlPlaneByClusterInput{ + Lister: client, + ClusterName: clusterName, + Namespace: namespace.Name, + }) + Expect(controlPlane).ToNot(BeNil()) + + framework.WaitForControlPlaneToBeUpToDate(ctx, framework.WaitForControlPlaneToBeUpToDateInput{ + Getter: client, + ControlPlane: controlPlane, + }) + + By("taking stable ownership of the Machines") + must := func(r *labels.Requirement, err error) labels.Requirement { + if err != nil { + panic(err) + } + return *r + } + machines := clusterv1.MachineList{} + Expect(client.List(ctx, &machines, + ctrlclient.InNamespace(namespace.Name), + ctrlclient.MatchingLabelsSelector{ + Selector: labels.NewSelector(). + Add(must(labels.NewRequirement(clusterv1.MachineControlPlaneLabelName, selection.Exists, []string{}))). + Add(must(labels.NewRequirement(clusterv1.ClusterLabelName, selection.Equals, []string{clusterName}))), + }, + )).To(Succeed()) + + for _, m := range machines.Items { + m := m + Expect(&m).To(HaveControllerRef(framework.ObjectToKind(controlPlane), controlPlane)) + // TODO there is a missing unit test here + Expect(m.CreationTimestamp.Time).To(BeTemporally("<", controlPlane.CreationTimestamp.Time), + "The KCP has replaced the control plane machines after adopting them. "+ + "This may have occurred as a result of changes to the KubeadmConfig bootstrap type or reconciler. "+ + "In that case it's possible new defaulting or reconciliation logic made the KCP unable to recognize "+ + "a KubeadmConfig that it should have. "+ + "See ./bootstrap/kubeadm/api/equality/semantic.go and ensure that any new defaults are un-set so the KCP "+ + "can accurately 'guess' whether its template might have created the object.", + ) + } + Expect(machines.Items).To(HaveLen(int(*replicas))) + + bootstrap := bootstrapv1.KubeadmConfigList{} + Expect(client.List(ctx, &bootstrap, + ctrlclient.InNamespace(namespace.Name), + ctrlclient.MatchingLabels{ + clusterv1.ClusterLabelName: clusterName, + })).To(Succeed()) + + By("taking ownership of the cluster's PKI material") + secrets := corev1.SecretList{} + Expect(client.List(ctx, &secrets, ctrlclient.InNamespace(namespace.Name), ctrlclient.MatchingLabels{ + clusterv1.ClusterLabelName: cluster.Name, + })).To(Succeed()) + + bootstrapSecrets := map[string]bootstrapv1.KubeadmConfig{} + for _, b := range bootstrap.Items { + if b.Status.DataSecretName == nil { + continue + } + bootstrapSecrets[*b.Status.DataSecretName] = b + } + + for _, s := range secrets.Items { + s := s + // We don't check the data, and removing it from the object makes assertions much easier to read + s.Data = nil + + // The bootstrap secret should still be owned by the bootstrap config so its cleaned up properly, + // but the cluster PKI materials should have their ownership transferred. + bootstrap, found := bootstrapSecrets[s.Name] + switch { + case strings.HasSuffix(s.Name, "-kubeconfig"): + // Do nothing + case found: + Expect(&s).To(HaveControllerRef(framework.ObjectToKind(&bootstrap), &bootstrap)) + default: + Expect(&s).To(HaveControllerRef(framework.ObjectToKind(controlPlane), controlPlane)) + } + } + Expect(secrets.Items).To(HaveLen(4 /* pki */ + 1 /* kubeconfig */ + int(*replicas))) + + By("PASSED!") + }) + + AfterEach(func() { + // Dumps all the resources in the spec namespace, then cleanups the cluster object and the spec namespace itself. + dumpSpecResourcesAndCleanup(ctx, specName, input.BootstrapClusterProxy, input.ArtifactFolder, namespace, cancelWatches, cluster, input.E2EConfig.GetIntervals, input.SkipCleanup) + }) +} diff --git a/test/e2e/kcp_adoption_test.go b/test/e2e/kcp_adoption_test.go new file mode 100644 index 000000000000..a9febb8bf56d --- /dev/null +++ b/test/e2e/kcp_adoption_test.go @@ -0,0 +1,39 @@ +// +build e2e + +/* +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 e2e + +import ( + "context" + + . "github.com/onsi/ginkgo" +) + +var _ = Describe("When testing KCP adoption", func() { + + KCPAdoptionSpec(context.TODO(), func() KCPAdoptionSpecInput { + return KCPAdoptionSpecInput{ + E2EConfig: e2eConfig, + ClusterctlConfigPath: clusterctlConfigPath, + BootstrapClusterProxy: bootstrapClusterProxy.(ClusterProxy), + ArtifactFolder: artifactFolder, + SkipCleanup: skipCleanup, + } + }) + +}) diff --git a/test/framework/cluster_proxy.go b/test/framework/cluster_proxy.go index 2072e02ad82d..8be27b7d96bd 100644 --- a/test/framework/cluster_proxy.go +++ b/test/framework/cluster_proxy.go @@ -155,6 +155,14 @@ func (p *clusterProxy) Apply(ctx context.Context, resources []byte) error { return exec.KubectlApply(ctx, p.kubeconfigPath, resources) } +// Apply wraps `kubectl apply ...` and prints the output so we can see what gets applied to the cluster. +func (p *clusterProxy) ApplyWithArgs(ctx context.Context, resources []byte, args ...string) error { + Expect(ctx).NotTo(BeNil(), "ctx is required for Apply") + Expect(resources).NotTo(BeNil(), "resources is required for Apply") + + return exec.KubectlApplyWithArgs(ctx, p.kubeconfigPath, resources, args...) +} + func (p *clusterProxy) getConfig() *rest.Config { config, err := clientcmd.LoadFromFile(p.kubeconfigPath) Expect(err).ToNot(HaveOccurred(), "Failed to load Kubeconfig file from %q", p.kubeconfigPath) diff --git a/test/framework/control_plane.go b/test/framework/control_plane.go new file mode 100644 index 000000000000..3607e123b533 --- /dev/null +++ b/test/framework/control_plane.go @@ -0,0 +1,49 @@ +/* +Copyright 2019 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 framework + +import ( + "context" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha3" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// WaitForControlPlaneToBeReadyInput is the input for WaitForControlPlaneToBeReady. +type WaitForControlPlaneToBeUpToDateInput struct { + Getter Getter + ControlPlane *controlplanev1.KubeadmControlPlane +} + +// WaitForControlPlaneToBeUpToDate will wait for a control plane to be fully up-to-date. +func WaitForControlPlaneToBeUpToDate(ctx context.Context, input WaitForControlPlaneToBeUpToDateInput, intervals ...interface{}) { + By("waiting for the control plane to be ready") + Eventually(func() (int32, error) { + controlplane := &controlplanev1.KubeadmControlPlane{} + key := client.ObjectKey{ + Namespace: input.ControlPlane.GetNamespace(), + Name: input.ControlPlane.GetName(), + } + if err := input.Getter.Get(ctx, key, controlplane); err != nil { + return 0, err + } + return controlplane.Status.UpdatedReplicas, nil + }, intervals...).Should(Equal(*input.ControlPlane.Spec.Replicas)) +} diff --git a/test/framework/convenience.go b/test/framework/convenience.go index d46f986e328b..3819601261e4 100644 --- a/test/framework/convenience.go +++ b/test/framework/convenience.go @@ -67,6 +67,13 @@ func TryAddDefaultSchemes(scheme *runtime.Scheme) { // TypeToKind returns the Kind without the package prefix. Pass in a pointer to a struct // This will panic if used incorrectly. +// Deprecated: use ObjectToKind for runtime.Objects for compile-time checking func TypeToKind(i interface{}) string { return reflect.ValueOf(i).Elem().Type().Name() } + +// ObjectToKind returns the Kind without the package prefix. Pass in a pointer to a struct +// This will panic if used incorrectly. +func ObjectToKind(i runtime.Object) string { + return reflect.ValueOf(i).Elem().Type().Name() +} diff --git a/test/framework/convenience_test.go b/test/framework/convenience_test.go index e80ac7f09bcd..64a08dc36398 100644 --- a/test/framework/convenience_test.go +++ b/test/framework/convenience_test.go @@ -20,14 +20,26 @@ import ( "testing" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/cluster-api/test/framework" ) -func TestTypeToKind(t *testing.T) { +func TestObjectToKind(t *testing.T) { g := NewWithT(t) - type hello struct{} + g.Expect(framework.ObjectToKind(&hello{})).To(Equal("hello")) +} + +var _ runtime.Object = &hello{} + +type hello struct{} + +func (*hello) GetObjectKind() schema.ObjectKind { + return schema.EmptyObjectKind +} - g.Expect(framework.TypeToKind(&hello{})).To(Equal("hello")) +func (h *hello) DeepCopyObject() runtime.Object { + return h } diff --git a/test/framework/exec/kubectl.go b/test/framework/exec/kubectl.go index 6b4252707940..e6e5b6d7bd6d 100644 --- a/test/framework/exec/kubectl.go +++ b/test/framework/exec/kubectl.go @@ -24,10 +24,15 @@ import ( // TODO: Remove this usage of kubectl and replace with a function from apply.go using the controller-runtime client. func KubectlApply(ctx context.Context, kubeconfigPath string, resources []byte) error { + return KubectlApplyWithArgs(ctx, kubeconfigPath, resources) +} + +func KubectlApplyWithArgs(ctx context.Context, kubeconfigPath string, resources []byte, args ...string) error { + aargs := append([]string{"apply", "--kubeconfig", kubeconfigPath, "-f", "-"}, args...) rbytes := bytes.NewReader(resources) applyCmd := NewCommand( WithCommand("kubectl"), - WithArgs("apply", "--kubeconfig", kubeconfigPath, "-f", "-"), + WithArgs(aargs...), WithStdin(rbytes), ) stdout, stderr, err := applyCmd.Run(ctx) diff --git a/test/framework/machines.go b/test/framework/machines.go new file mode 100644 index 000000000000..b19c2230140e --- /dev/null +++ b/test/framework/machines.go @@ -0,0 +1,89 @@ +/* +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 framework + +import ( + "context" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" + "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// WaitForClusterMachineNodesRefsInput is the input for WaitForClusterMachineNodesRefs. +type WaitForClusterMachineNodeRefsInput struct { + GetLister GetLister + Cluster *clusterv1.Cluster +} + +// WaitForClusterMachineNodesRefs waits until all nodes associated with a machine deployment exist. +func WaitForClusterMachineNodeRefs(ctx context.Context, input WaitForClusterMachineNodeRefsInput, intervals ...interface{}) { + By("waiting for the machines' nodes to exist") + machines := &clusterv1.MachineList{} + + Expect(input.GetLister.List(ctx, machines, byClusterOptions(input.Cluster.Name, input.Cluster.Namespace)...)).To(Succeed(), "Failed to get Cluster machines %s/%s", input.Cluster.Namespace, input.Cluster.Name) + Eventually(func() (count int, err error) { + for _, m := range machines.Items { + machine := &clusterv1.Machine{} + err = input.GetLister.Get(ctx, client.ObjectKey{Namespace: m.Namespace, Name: m.Name}, machine) + if err != nil { + return + } + if machine.Status.NodeRef != nil { + count++ + } + } + return + }, intervals...).Should(Equal(len(machines.Items))) +} + +type WaitForClusterMachinesReadyInput struct { + GetLister GetLister + NodeGetter Getter + Cluster *clusterv1.Cluster +} + +func WaitForClusterMachinesReady(ctx context.Context, input WaitForClusterMachinesReadyInput, intervals ...interface{}) { + By("waiting for the machines' nodes to be ready") + machines := &clusterv1.MachineList{} + + Expect(input.GetLister.List(ctx, machines, byClusterOptions(input.Cluster.Name, input.Cluster.Namespace)...)).To(Succeed(), "Failed to get Cluster machines %s/%s", input.Cluster.Namespace, input.Cluster.Name) + Eventually(func() (count int, err error) { + for _, m := range machines.Items { + machine := &clusterv1.Machine{} + err = input.GetLister.Get(ctx, client.ObjectKey{Namespace: m.Namespace, Name: m.Name}, machine) + if err != nil { + return + } + if machine.Status.NodeRef == nil { + continue + } + node := &corev1.Node{} + err = input.NodeGetter.Get(ctx, client.ObjectKey{Name: machine.Status.NodeRef.Name}, node) + if err != nil { + return + } + if util.IsNodeReady(node) { + count++ + } + } + return + }, intervals...).Should(Equal(len(machines.Items))) +} diff --git a/test/infrastructure/docker/.gitignore b/test/infrastructure/docker/.gitignore index 7f30e139db31..e93330e1dcb4 100644 --- a/test/infrastructure/docker/.gitignore +++ b/test/infrastructure/docker/.gitignore @@ -16,3 +16,5 @@ manager # test results *.xml + +e2e/resources/** diff --git a/test/infrastructure/docker/examples/simple-cluster-without-kcp.yaml b/test/infrastructure/docker/examples/simple-cluster-without-kcp.yaml new file mode 100644 index 000000000000..d70f827d1031 --- /dev/null +++ b/test/infrastructure/docker/examples/simple-cluster-without-kcp.yaml @@ -0,0 +1,107 @@ +# Creates a cluster with one control-plane node and one worker node +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: DockerCluster +metadata: + name: my-cluster + namespace: default +--- +apiVersion: cluster.x-k8s.io/v1alpha3 +kind: Cluster +metadata: + name: my-cluster + namespace: default +spec: + clusterNetwork: + services: + cidrBlocks: ["10.96.0.0/12"] + pods: + cidrBlocks: ["192.168.0.0/16"] + serviceDomain: "cluster.local" + infrastructureRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 + kind: DockerCluster + name: my-cluster + namespace: default +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: DockerMachine +metadata: + name: controlplane-0 + namespace: default +--- +apiVersion: cluster.x-k8s.io/v1alpha3 +kind: Machine +metadata: + labels: + cluster.x-k8s.io/cluster-name: my-cluster + cluster.x-k8s.io/control-plane: "true" + name: controlplane-0 + namespace: default +spec: + version: "v1.14.2" + clusterName: my-cluster + bootstrap: + configRef: + apiVersion: bootstrap.cluster.x-k8s.io/v1alpha3 + kind: KubeadmConfig + name: controlplane-0-config + namespace: default + infrastructureRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 + kind: DockerMachine + name: controlplane-0 + namespace: default +--- +apiVersion: bootstrap.cluster.x-k8s.io/v1alpha3 +kind: KubeadmConfig +metadata: + name: controlplane-0-config + namespace: default +spec: + clusterConfiguration: + controllerManager: + extraArgs: + enable-hostpath-provisioner: "true" + initConfiguration: + nodeRegistration: + kubeletExtraArgs: + eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 +kind: DockerMachine +metadata: + name: worker-0 + namespace: default +--- +apiVersion: cluster.x-k8s.io/v1alpha3 +kind: Machine +metadata: + labels: + cluster.x-k8s.io/cluster-name: my-cluster + name: worker-0 + namespace: default +spec: + version: "v1.14.2" + clusterName: my-cluster + bootstrap: + configRef: + apiVersion: bootstrap.cluster.x-k8s.io/v1alpha3 + kind: KubeadmConfig + name: worker-0-config + namespace: default + infrastructureRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 + kind: DockerMachine + name: worker-0 + namespace: default +--- +apiVersion: bootstrap.cluster.x-k8s.io/v1alpha3 +kind: KubeadmConfig +metadata: + name: worker-0-config + namespace: default +spec: + joinConfiguration: + nodeRegistration: + kubeletExtraArgs: + eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% diff --git a/test/infrastructure/docker/examples/simple-cluster.yaml b/test/infrastructure/docker/examples/simple-cluster.yaml index d70f827d1031..0757afba7bb8 100644 --- a/test/infrastructure/docker/examples/simple-cluster.yaml +++ b/test/infrastructure/docker/examples/simple-cluster.yaml @@ -17,6 +17,11 @@ spec: pods: cidrBlocks: ["192.168.0.0/16"] serviceDomain: "cluster.local" + controlPlaneRef: + apiVersion: controlplane.cluster.x-k8s.io/v1alpha3 + kind: KubeadmControlPlane + name: controlplane + namespace: default infrastructureRef: apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 kind: DockerCluster @@ -24,48 +29,36 @@ spec: namespace: default --- apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 -kind: DockerMachine +kind: DockerMachineTemplate metadata: - name: controlplane-0 + name: controlplane namespace: default +spec: + template: + spec: {} --- -apiVersion: cluster.x-k8s.io/v1alpha3 -kind: Machine +apiVersion: "controlplane.cluster.x-k8s.io/v1alpha3" +kind: KubeadmControlPlane metadata: - labels: - cluster.x-k8s.io/cluster-name: my-cluster - cluster.x-k8s.io/control-plane: "true" - name: controlplane-0 + name: controlplane namespace: default spec: - version: "v1.14.2" - clusterName: my-cluster - bootstrap: - configRef: - apiVersion: bootstrap.cluster.x-k8s.io/v1alpha3 - kind: KubeadmConfig - name: controlplane-0-config - namespace: default - infrastructureRef: + replicas: 1 + version: v1.14.2 + infrastructureTemplate: apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 - kind: DockerMachine - name: controlplane-0 + kind: DockerMachineTemplate + name: controlplane namespace: default ---- -apiVersion: bootstrap.cluster.x-k8s.io/v1alpha3 -kind: KubeadmConfig -metadata: - name: controlplane-0-config - namespace: default -spec: - clusterConfiguration: - controllerManager: - extraArgs: - enable-hostpath-provisioner: "true" - initConfiguration: - nodeRegistration: - kubeletExtraArgs: - eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% + kubeadmConfigSpec: + clusterConfiguration: + controllerManager: + extraArgs: + enable-hostpath-provisioner: "true" + initConfiguration: + nodeRegistration: + kubeletExtraArgs: + eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% --- apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3 kind: DockerMachine diff --git a/util/secret/certificates.go b/util/secret/certificates.go index fbf53ba8cbd3..cd8ea53e6135 100644 --- a/util/secret/certificates.go +++ b/util/secret/certificates.go @@ -44,7 +44,7 @@ import ( const ( rootOwnerValue = "root:root" - defaultCertificatesDir = "/etc/kubernetes/pki" + DefaultCertificatesDir = "/etc/kubernetes/pki" ) var ( @@ -64,7 +64,7 @@ type Certificates []*Certificate // NewCertificatesForInitialControlPlane returns a list of certificates configured for a control plane node func NewCertificatesForInitialControlPlane(config *v1beta1.ClusterConfiguration) Certificates { if config.CertificatesDir == "" { - config.CertificatesDir = defaultCertificatesDir + config.CertificatesDir = DefaultCertificatesDir } certificates := Certificates{ @@ -116,23 +116,23 @@ func NewCertificatesForJoiningControlPlane() Certificates { return Certificates{ &Certificate{ Purpose: ClusterCA, - CertFile: filepath.Join(defaultCertificatesDir, "ca.crt"), - KeyFile: filepath.Join(defaultCertificatesDir, "ca.key"), + CertFile: filepath.Join(DefaultCertificatesDir, "ca.crt"), + KeyFile: filepath.Join(DefaultCertificatesDir, "ca.key"), }, &Certificate{ Purpose: ServiceAccount, - CertFile: filepath.Join(defaultCertificatesDir, "sa.pub"), - KeyFile: filepath.Join(defaultCertificatesDir, "sa.key"), + CertFile: filepath.Join(DefaultCertificatesDir, "sa.pub"), + KeyFile: filepath.Join(DefaultCertificatesDir, "sa.key"), }, &Certificate{ Purpose: FrontProxyCA, - CertFile: filepath.Join(defaultCertificatesDir, "front-proxy-ca.crt"), - KeyFile: filepath.Join(defaultCertificatesDir, "front-proxy-ca.key"), + CertFile: filepath.Join(DefaultCertificatesDir, "front-proxy-ca.crt"), + KeyFile: filepath.Join(DefaultCertificatesDir, "front-proxy-ca.key"), }, &Certificate{ Purpose: EtcdCA, - CertFile: filepath.Join(defaultCertificatesDir, "etcd", "ca.crt"), - KeyFile: filepath.Join(defaultCertificatesDir, "etcd", "ca.key"), + CertFile: filepath.Join(DefaultCertificatesDir, "etcd", "ca.crt"), + KeyFile: filepath.Join(DefaultCertificatesDir, "etcd", "ca.key"), }, } } @@ -140,7 +140,7 @@ func NewCertificatesForJoiningControlPlane() Certificates { // NewCertificatesForWorker return an initialized but empty set of CA certificates needed to bootstrap a cluster. func NewCertificatesForWorker(caCertPath string) Certificates { if caCertPath == "" { - caCertPath = filepath.Join(defaultCertificatesDir, "ca.crt") + caCertPath = filepath.Join(DefaultCertificatesDir, "ca.crt") } return Certificates{ diff --git a/util/util.go b/util/util.go index f9bccce341a6..31610554d9d9 100644 --- a/util/util.go +++ b/util/util.go @@ -47,6 +47,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" @@ -192,7 +193,7 @@ func GetControlPlaneMachinesFromList(machineList *clusterv1.MachineList) (res [] return } -// GetMachineIfExists gets a machine from the API server if it exists +// GetMachineIfExists gets a machine from the API server if it exists. func GetMachineIfExists(c client.Client, namespace, name string) (*clusterv1.Machine, error) { if c == nil { // Being called before k8s is setup as part of control plane VM creation @@ -262,7 +263,7 @@ func GetClusterByName(ctx context.Context, c client.Client, namespace, name stri return cluster, nil } -// ObjectKey returns client.ObjectKey for the object +// ObjectKey returns client.ObjectKey for the object. func ObjectKey(object metav1.Object) client.ObjectKey { return client.ObjectKey{ Namespace: object.GetNamespace(), @@ -363,6 +364,25 @@ func EnsureOwnerRef(ownerReferences []metav1.OwnerReference, ref metav1.OwnerRef return ownerReferences } +// ReplaceOwnerRef re-parents an object from one OwnerReference to another +// It compares strictly based on UID to avoid reparenting across an intentional deletion: if an object is deleted +// and re-created with the same name and namespace, the only way to tell there was an in-progress deletion +// is by comparing the UIDs. +func ReplaceOwnerRef(ownerReferences []metav1.OwnerReference, source metav1.Object, target metav1.OwnerReference) []metav1.OwnerReference { + fi := -1 + for index, r := range ownerReferences { + if r.UID == source.GetUID() { + fi = index + ownerReferences[index] = target + break + } + } + if fi < 0 { + ownerReferences = append(ownerReferences, target) + } + return ownerReferences +} + // indexOwnerRef returns the index of the owner reference in the slice if found, or -1. func indexOwnerRef(ownerReferences []metav1.OwnerReference, ref metav1.OwnerReference) int { for index, r := range ownerReferences { @@ -373,6 +393,37 @@ func indexOwnerRef(ownerReferences []metav1.OwnerReference, ref metav1.OwnerRefe return -1 } +// PointsTo returns true if any of the owner references point to the given target +// Deprecated: Use IsOwnedByObject to cover differences in API version or backup/restore that changed UIDs. +func PointsTo(refs []metav1.OwnerReference, target *metav1.ObjectMeta) bool { + for _, ref := range refs { + if ref.UID == target.UID { + return true + } + } + return false +} + +// IsOwnedByObject returns true if any of the owner references point to the given target. +func IsOwnedByObject(obj metav1.Object, target controllerutil.Object) bool { + for _, ref := range obj.GetOwnerReferences() { + ref := ref + if refersTo(&ref, target) { + return true + } + } + return false +} + +// IsControlledBy differs from metav1.IsControlledBy in that it checks the group (but not version), kind, and name vs uid. +func IsControlledBy(obj metav1.Object, owner controllerutil.Object) bool { + controllerRef := metav1.GetControllerOfNoCopy(obj) + if controllerRef == nil { + return false + } + return refersTo(controllerRef, owner) +} + // Returns true if a and b point to the same object. func referSameObject(a, b metav1.OwnerReference) bool { aGV, err := schema.ParseGroupVersion(a.APIVersion) @@ -388,15 +439,15 @@ func referSameObject(a, b metav1.OwnerReference) bool { return aGV.Group == bGV.Group && a.Kind == b.Kind && a.Name == b.Name } -// PointsTo returns true if any of the owner references point to the given target -func PointsTo(refs []metav1.OwnerReference, target *metav1.ObjectMeta) bool { - for _, ref := range refs { - if ref.UID == target.UID { - return true - } +// Returns true if ref refers to obj. +func refersTo(ref *metav1.OwnerReference, obj controllerutil.Object) bool { + refGv, err := schema.ParseGroupVersion(ref.APIVersion) + if err != nil { + return false } - return false + gvk := obj.GetObjectKind().GroupVersionKind() + return refGv.Group == gvk.Group && ref.Kind == gvk.Kind && ref.Name == obj.GetName() } // UnstructuredUnmarshalField is a wrapper around json and unstructured objects to decode and copy a specific field @@ -557,3 +608,14 @@ func ObjectReferenceToUnstructured(in corev1.ObjectReference) *unstructured.Unst out.SetName(in.Name) return out } + +// IsSupportedVersionSkew will return true if a and b are no more than one minor version off from each other. +func IsSupportedVersionSkew(a, b semver.Version) bool { + if a.Major != b.Major { + return false + } + if a.Minor > b.Minor { + return a.Minor-b.Minor == 1 + } + return b.Minor-a.Minor <= 1 +} diff --git a/util/util_test.go b/util/util_test.go index 9a438e9921bf..021c185c3262 100644 --- a/util/util_test.go +++ b/util/util_test.go @@ -27,7 +27,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -310,54 +309,118 @@ func TestHasOwner(t *testing.T) { } } -func TestPointsTo(t *testing.T) { +type fakeMeta struct { + metav1.ObjectMeta + metav1.TypeMeta +} + +var _ runtime.Object = &fakeMeta{} + +func (*fakeMeta) DeepCopyObject() runtime.Object { + panic("not implemented") +} + +func TestIsOwnedByObject(t *testing.T) { g := NewWithT(t) - targetID := "fri3ndsh1p" + targetGroup := "ponies.info" + targetKind := "Rainbow" + targetName := "fri3ndsh1p" - meta := metav1.ObjectMeta{ - UID: types.UID(targetID), + meta := fakeMeta{ + metav1.ObjectMeta{ + Name: targetName, + }, + metav1.TypeMeta{ + APIVersion: "ponies.info/v1", + Kind: targetKind, + }, } tests := []struct { name string - refIDs []string + refs []metav1.OwnerReference expected bool }{ { name: "empty owner list", }, { - name: "single wrong owner ref", - refIDs: []string{"m4g1c"}, + name: "single wrong name owner ref", + refs: []metav1.OwnerReference{{ + APIVersion: targetGroup + "/v1", + Kind: targetKind, + Name: "m4g1c", + }}, + }, + { + name: "single wrong group owner ref", + refs: []metav1.OwnerReference{{ + APIVersion: "dazzlings.info/v1", + Kind: "Twilight", + Name: "m4g1c", + }}, + }, + { + name: "single wrong kind owner ref", + refs: []metav1.OwnerReference{{ + APIVersion: targetGroup + "/v1", + Kind: "Twilight", + Name: "m4g1c", + }}, }, { - name: "single right owner ref", - refIDs: []string{targetID}, + name: "single right owner ref", + refs: []metav1.OwnerReference{{ + APIVersion: targetGroup + "/v1", + Kind: targetKind, + Name: targetName, + }}, expected: true, }, { - name: "multiple wrong refs", - refIDs: []string{"m4g1c", "h4rm0ny"}, + name: "single right owner ref (different version)", + refs: []metav1.OwnerReference{{ + APIVersion: targetGroup + "/v2alpha2", + Kind: targetKind, + Name: targetName, + }}, + expected: true, + }, + { + name: "multiple wrong refs", + refs: []metav1.OwnerReference{{ + APIVersion: targetGroup + "/v1", + Kind: targetKind, + Name: "m4g1c", + }, { + APIVersion: targetGroup + "/v1", + Kind: targetKind, + Name: "h4rm0ny", + }}, }, { - name: "multiple refs one right", - refIDs: []string{"m4g1c", targetID}, + name: "multiple refs one right", + refs: []metav1.OwnerReference{{ + APIVersion: targetGroup + "/v1", + Kind: targetKind, + Name: "m4g1c", + }, { + APIVersion: targetGroup + "/v1", + Kind: targetKind, + Name: targetName, + }}, expected: true, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - pointer := &metav1.ObjectMeta{} - - for _, ref := range test.refIDs { - pointer.OwnerReferences = append(pointer.OwnerReferences, metav1.OwnerReference{ - UID: types.UID(ref), - }) + pointer := &metav1.ObjectMeta{ + OwnerReferences: test.refs, } - g.Expect(PointsTo(pointer.OwnerReferences, &meta)).To(Equal(test.expected)) + g.Expect(IsOwnedByObject(pointer, &meta)).To(Equal(test.expected), "Could not find a ref to %+v in %+v", meta, test.refs) }) } } @@ -660,5 +723,72 @@ func TestOrdinalize(t *testing.T) { g.Expect(Ordinalize(tt.input)).To(Equal(tt.expected)) }) } +} +func TestIsSupportedVersionSkew(t *testing.T) { + type args struct { + a semver.Version + b semver.Version + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "same version", + args: args{ + a: semver.MustParse("1.10.0"), + b: semver.MustParse("1.10.0"), + }, + want: true, + }, + { + name: "different patch version", + args: args{ + a: semver.MustParse("1.10.0"), + b: semver.MustParse("1.10.2"), + }, + want: true, + }, + { + name: "a + 1 minor version", + args: args{ + a: semver.MustParse("1.11.0"), + b: semver.MustParse("1.10.2"), + }, + want: true, + }, + { + name: "b + 1 minor version", + args: args{ + a: semver.MustParse("1.10.0"), + b: semver.MustParse("1.11.2"), + }, + want: true, + }, + { + name: "a + 2 minor versions", + args: args{ + a: semver.MustParse("1.12.0"), + b: semver.MustParse("1.10.0"), + }, + want: false, + }, + { + name: "b + 2 minor versions", + args: args{ + a: semver.MustParse("1.10.0"), + b: semver.MustParse("1.12.0"), + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsSupportedVersionSkew(tt.args.a, tt.args.b); got != tt.want { + t.Errorf("IsSupportedVersionSkew() = %v, want %v", got, tt.want) + } + }) + } }