Merge pull request #6495 from fabriziopandini/topology-ssa

✨ Add Server Side Apply helper to the topology controller

Author: k8s-ci-robot

api/v1beta1/common_types.go

@@ -40,6 +40,7 @@ const (
 	// to easily discover which fields have been set by templates + patches/variables at a given reconcile;
 	// instead, it is not necessary to store managed paths for typed objets (e.g. Cluster, MachineDeployments)
 	// given that the topology controller explicitly sets a well-known, immutable list of fields at every reconcile.
+	// Deprecated: Topology controller is now using server side apply and this annotation will be removed in a future release.
 	ClusterTopologyManagedFieldsAnnotation = "topology.cluster.x-k8s.io/managed-field-paths"
 
 	// ClusterTopologyMachineDeploymentLabelName is the label set on the generated  MachineDeployment objects

cmd/clusterctl/client/cluster/client.go

@@ -29,10 +29,6 @@ import (
 	logf "sigs.k8s.io/cluster-api/cmd/clusterctl/log"
 )
 
-const (
-	minimumKubernetesVersion = "v1.20.0"
-)
-
 var (
 	ctx = context.TODO()
 )

cmd/clusterctl/client/cluster/mover.go

@@ -17,6 +17,7 @@ limitations under the License.
 package cluster
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -34,6 +35,7 @@ import (
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	logf "sigs.k8s.io/cluster-api/cmd/clusterctl/log"
+	"sigs.k8s.io/cluster-api/internal/controllers/topology/cluster/structuredmerge"
 	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	"sigs.k8s.io/cluster-api/util/patch"
@@ -851,6 +853,9 @@ func (o *objectMover) createTargetObject(nodeToCreate *node, toProxy Proxy) erro
 	// Rebuild the owne reference chain
 	o.buildOwnerChain(obj, nodeToCreate)
 
+	// Save the old managed fields for topology managed fields migration
+	oldManagedFields := obj.GetManagedFields()
+
 	// FIXME Workaround for https://github.com/kubernetes/kubernetes/issues/32220. Remove when the issue is fixed.
 	// If the resource already exists, the API server ordinarily returns an AlreadyExists error. Due to the above issue, if the resource has a non-empty metadata.generateName field, the API server returns a ServerTimeoutError. To ensure that the API server returns an AlreadyExists error, we set the metadata.generateName field to an empty string.
 	if len(obj.GetName()) > 0 && len(obj.GetGenerateName()) > 0 {
@@ -897,6 +902,10 @@ func (o *objectMover) createTargetObject(nodeToCreate *node, toProxy Proxy) erro
 	// Stores the newUID assigned to the newly created object.
 	nodeToCreate.newUID = obj.GetUID()
 
+	if err := patchTopologyManagedFields(ctx, oldManagedFields, obj, cTo); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -1164,3 +1173,32 @@ func (o *objectMover) checkTargetProviders(toInventory InventoryClient) error {
 
 	return kerrors.NewAggregate(errList)
 }
+
+// patchTopologyManagedFields patches the managed fields of obj if parts of it are owned by the topology controller.
+// This is necessary to ensure the managed fields created by the topology controller are still present and thus to
+// prevent unnecessary machine rollouts. Without patching the managed fields, clusterctl would be the owner of the fields
+// which would lead to co-ownership and also additional machine rollouts.
+func patchTopologyManagedFields(ctx context.Context, oldManagedFields []metav1.ManagedFieldsEntry, obj *unstructured.Unstructured, cTo client.Client) error {
+	var containsTopologyManagedFields bool
+	// Check if the object was owned by the topology controller.
+	for _, m := range oldManagedFields {
+		if m.Operation == metav1.ManagedFieldsOperationApply &&
+			m.Manager == structuredmerge.TopologyManagerName &&
+			m.Subresource == "" {
+			containsTopologyManagedFields = true
+			break
+		}
+	}
+	// Return early if the object was not owned by the topology controller.
+	if !containsTopologyManagedFields {
+		return nil
+	}
+	base := obj.DeepCopy()
+	obj.SetManagedFields(oldManagedFields)
+
+	if err := cTo.Patch(ctx, obj, client.MergeFrom(base)); err != nil {
+		return errors.Wrapf(err, "error patching managed fields %q %s/%s",
+			obj.GroupVersionKind(), obj.GetNamespace(), obj.GetName())
+	}
+	return nil
+}

cmd/clusterctl/client/cluster/proxy.go

@@ -18,6 +18,8 @@ package cluster
 
 import (
 	"fmt"
+	"os"
+	"strconv"
 	"strings"
 	"time"
 
@@ -27,7 +29,6 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
-	utilversion "k8s.io/apimachinery/pkg/util/version"
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -52,7 +53,7 @@ type Proxy interface {
 	// CurrentNamespace returns the namespace from the current context in the kubeconfig file.
 	CurrentNamespace() (string, error)
 
-	// ValidateKubernetesVersion returns an error if management cluster version less than minimumKubernetesVersion.
+	// ValidateKubernetesVersion returns an error if management cluster version less than MinimumKubernetesVersion.
 	ValidateKubernetesVersion() error
 
 	// NewClient returns a new controller runtime Client object for working on the management cluster.
@@ -119,22 +120,12 @@ func (k *proxy) ValidateKubernetesVersion() error {
 		return err
 	}
 
-	client := discovery.NewDiscoveryClientForConfigOrDie(config)
-	serverVersion, err := client.ServerVersion()
-	if err != nil {
-		return errors.Wrap(err, "failed to retrieve server version")
-	}
-
-	compver, err := utilversion.MustParseGeneric(serverVersion.String()).Compare(minimumKubernetesVersion)
-	if err != nil {
-		return errors.Wrap(err, "failed to parse and compare server version")
+	minVer := version.MinimumKubernetesVersion
+	if clusterTopologyFeatureGate, _ := strconv.ParseBool(os.Getenv("CLUSTER_TOPOLOGY")); clusterTopologyFeatureGate {
+		minVer = version.MinimumKubernetesVersionClusterTopology
 	}
 
-	if compver == -1 {
-		return errors.Errorf("unsupported management cluster server version: %s - minimum required version is %s", serverVersion.String(), minimumKubernetesVersion)
-	}
-
-	return nil
+	return version.CheckKubernetesVersion(config, minVer)
 }
 
 // GetConfig returns the config for a kubernetes client.

docs/book/src/clusterctl/commands/alpha-topology-plan.md

@@ -22,6 +22,27 @@ the input should have all the objects needed.
 
 </aside>
 
+<aside class="note">
+
+<h1>Limitations</h1>
+
+The topology controllers uses [Server Side Apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/)
+to support use cases where other controllers are co-authoring the same objects, but this kind of interactions can't be recreated
+in a dry-run scenario.
+
+As a consequence Dry-Run can give some false positives/false negatives when trying to have a preview of
+changes to a set of existing topology owned objects. In other worlds this limitation impacts all the use cases described
+below except for "Designing a new ClusterClass".
+
+More specifically:
+- DryRun doesn't consider OpenAPI schema extension like +ListMap this can lead to false positives when topology
+  dry run is simulating a change to an existing slice (DryRun always reverts external changes, like server side apply when +ListMap=atomic).
+- DryRun doesn't consider existing metadata.managedFields, and this can lead to false negatives when topology dry run
+  is simulating a change where a field is dropped from a template (DryRun always preserve dropped fields, like 
+  server side apply when the field has more than one manager).
+
+</aside>
+
 ## Example use cases
 
 ### Designing a new ClusterClass

docs/book/src/developer/providers/v1.1-to-v1.2.md

@@ -5,7 +5,10 @@ maintainers of providers and consumers of our Go API.
 
 ## Minimum Kubernetes version for the management cluster
 
-* The minimum Kubernetes version that can be used for a management cluster by Cluster API is now 1.20.0
+* The minimum Kubernetes version that can be used for a management cluster is now 1.20.0
+* The minimum Kubernetes version that can be used for a management cluster with ClusterClass is now 1.22.0
+
+NOTE: compliance with minimum Kubernetes version is enforced both by clusterctl and when the CAPI controller starts. 
 
 ## Minimum Go version
 
@@ -28,6 +31,7 @@ in ClusterAPI are kept in sync with the versions used by `sigs.k8s.io/controller
 ### Deprecation
 
 * `util.MachinesByCreationTimestamp` has been deprecated and will be removed in a future release.
+* the `topology.cluster.x-k8s.io/managed-field-paths` annotation has been deprecated and it will be removed in a future release.
 
 ### Removals
 * The `third_party/kubernetes-drain` package has been removed, as we're now using `k8s.io/kubectl/pkg/drain` instead ([PR](https://github.com/kubernetes-sigs/cluster-api/pull/5440)).
@@ -36,11 +40,34 @@ in ClusterAPI are kept in sync with the versions used by `sigs.k8s.io/controller
   `annotations.HasPaused` and `annotations.HasSkipRemediation` respectively instead.
 * `ObjectMeta.ClusterName` has been removed from `k8s.io/apimachinery/pkg/apis/meta/v1`.
 
-### API Changes
+### golang API Changes
 
 - `util.ClusterToInfrastructureMapFuncWithExternallyManagedCheck` was removed and the externally managed check was added to `util.ClusterToInfrastructureMapFunc`, which required changing its signature.
    Users of the former simply need to start using the latter and users of the latter need to add the new arguments to their call.
 
+### Required API Changes for providers
+
+- ClusterClass and managed topologies are now using [Server Side Apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/) 
+  to properly manage other controllers like CAPA/CAPZ coauthoring slices, see [#6320](https://github.com/kubernetes-sigs/cluster-api/issues/6320).
+  In order to take advantage of this feature providers are required to add marker to their API types as described in
+  [merge-strategy](https://kubernetes.io/docs/reference/using-api/server-side-apply/#merge-strategy).
+  NOTE: the change will cause a rollout on existing clusters created with ClusterClass
+
+E.g. in CAPA
+
+```go
+// +optional
+Subnets Subnets `json:"subnets,omitempty"
+```
+Must be modified into:
+
+```go
+// +optional
+// +listType=map
+// +listMapKey=id
+Subnets Subnets `json:"subnets,omitempty"
+```
+
 ### Other
 
 - Logging:

docs/book/src/reference/versions.md

@@ -77,6 +77,7 @@ These diagrams show the relationships between components in a Cluster API releas
 
 \* There is an issue with CRDs in Kubernetes v1.23.{0-2}. ClusterClass with patches is affected by that (for more details please see [this issue](https://github.com/kubernetes-sigs/cluster-api/issues/5990)). Therefore we recommend to use Kubernetes v1.23.3+ with ClusterClass.
    Previous Kubernetes **minor** versions are not affected.
+\** When using CAPI v1.2 with the CLUSTER_TOPOLOGY experimental feature on, the Kubernetes Version for the management cluster must be >= 1.22.0.
 
 The Core Provider also talks to API server of every Workload Cluster. Therefore, the Workload Cluster's Kubernetes version must also be compatible.
 

docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md

@@ -174,21 +174,19 @@ underlying objects like control plane and MachineDeployment act in the same way
 The topology reconciler enforces values defined in the ClusterClass templates into the topology
 owned objects in a Cluster.
 
-A simple way to understand this is to `kubectl get -o json` templates referenced in a ClusterClass;
-then you can consider the topology reconciler to be authoritative on all the values
-under `spec`. Being authoritative means that the user cannot manually change those values in
-the object derived from the template in a specific Cluster (and if they do so the value gets reconciled
-to the value defined in the ClusterClass).
+More specifically, the topology controller uses [Server Side Apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/)
+to write/patch topology owned objects; using SSA allows other controllers to co-author the generated objects, 
+like e.g. adding info for subnets in CAPA.
 
 <aside class="note">
 <h1>What about patches?</h1>
 
 The considerations above apply also when using patches, the only difference being that the
-authoritative fields should be determined by applying patches on top of the `kubectl get -o json` output. 
+set of fields that are enforced should be determined by applying patches on top of the templates. 
 
 </aside>
 
-A corollary of the behaviour described above is that it is technically possible to change non-authoritative
-fields in the object derived from the template in a specific Cluster, but we advise against using the possibility
+A corollary of the behaviour described above is that it is technically possible to change fields in the object 
+which are not derived from the templates and patches, but we advise against using the possibility
 or making ad-hoc changes in generated objects unless otherwise needed for a workaround. It is always
 preferable to improve ClusterClasses by supporting new Cluster variants in a reusable way.

docs/book/src/tasks/experimental-features/cluster-class/index.md

@@ -3,6 +3,14 @@
 The ClusterClass feature introduces a new way to create clusters which reduces boilerplate and enables flexible and powerful customization of clusters.
 ClusterClass is a powerful abstraction implemented on top of existing interfaces and offers a set of tools and operations to streamline cluster lifecycle management while maintaining the same underlying API.
 
+</aside>
+
+<aside class="note warning">
+
+In order to use the ClusterClass (alpha) experimental feature the Kubernetes Version for the management cluster must be >= 1.22.0.
+
+</aside>
+
 **Feature gate name**: `ClusterTopology`
 
 **Variable name to enable/disable the feature gate**: `CLUSTER_TOPOLOGY`

internal/contract/types.go

@@ -29,6 +29,47 @@ var errNotFound = errors.New("not found")
 // Path defines a how to access a field in an Unstructured object.
 type Path []string
 
+// Append a field name to a path.
+func (p Path) Append(k string) Path {
+	return append(p, k)
+}
+
+// IsParentOf check if one path is Parent of the other.
+func (p Path) IsParentOf(other Path) bool {
+	if len(p) >= len(other) {
+		return false
+	}
+	for i := range p {
+		if p[i] != other[i] {
+			return false
+		}
+	}
+	return true
+}
+
+// Equal check if two path are equal (exact match).
+func (p Path) Equal(other Path) bool {
+	if len(p) != len(other) {
+		return false
+	}
+	for i := range p {
+		if p[i] != other[i] {
+			return false
+		}
+	}
+	return true
+}
+
+// Overlaps return true if two paths are Equal or one IsParentOf the other.
+func (p Path) Overlaps(other Path) bool {
+	return other.Equal(p) || other.IsParentOf(p) || p.IsParentOf(other)
+}
+
+// String returns the path as a dotted string.
+func (p Path) String() string {
+	return strings.Join(p, ".")
+}
+
 // Int64 represents an accessor to an int64 path value.
 type Int64 struct {
 	path Path