diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index fcf8d13138c8..317ec83d446b 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -110,13 +110,9 @@ rules: - cluster.x-k8s.io resources: - machinedeployments - - machinedeployments/finalizers verbs: - - delete - get - list - - patch - - update - watch - apiGroups: - cluster.x-k8s.io @@ -182,6 +178,14 @@ rules: - get - list - watch +- apiGroups: + - cluster.x-k8s.io + resources: + - machinesets + verbs: + - get + - list + - watch - apiGroups: - cluster.x-k8s.io resources: diff --git a/controllers/topology/machinedeployment_controller.go b/controllers/topology/machinedeployment_controller.go index bb6fb5d76fe3..14fb29231bf8 100644 --- a/controllers/topology/machinedeployment_controller.go +++ b/controllers/topology/machinedeployment_controller.go @@ -18,15 +18,11 @@ package topology import ( "context" - "fmt" "github.com/pkg/errors" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" - "sigs.k8s.io/cluster-api/controllers/external" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/annotations" "sigs.k8s.io/cluster-api/util/patch" @@ -35,41 +31,32 @@ import ( "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=infrastructure.cluster.x-k8s.io;bootstrap.cluster.x-k8s.io,resources=*,verbs=delete // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters,verbs=get;list;watch -// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments;machinedeployments/finalizers,verbs=get;list;watch;update;patch;delete -// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinesets;machinesets/finalizers,verbs=get;list;watch;update;patch;delete +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments,verbs=get;list;watch +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinesets,verbs=get;list;watch -// MachineDeploymentReconciler deletes referenced templates during deletion of topology-owned MachineDeployments and MachineSets. +// MachineDeploymentReconciler deletes referenced templates during deletion of topology-owned MachineDeployments. // The templates are only deleted, if they are not used in other MachineDeployments or MachineSets which are not in deleting, -// i.e. the templates would otherwise be orphaned after the MachineDeployment or MachineSet deletion completes. -// Note: To achieve this the reconciler adds a finalizer, to hook into the MachineDeployment and MachineSet deletions. +// i.e. the templates would otherwise be orphaned after the MachineDeployment deletion completes. +// Note: To achieve this the cluster topology controller sets a finalizer to hook into the MachineDeployment deletions. type MachineDeploymentReconciler struct { Client client.Client WatchFilterValue string } func (r *MachineDeploymentReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error { - _, err := ctrl.NewControllerManagedBy(mgr). + err := ctrl.NewControllerManagedBy(mgr). For(&clusterv1.MachineDeployment{}). Named("machinedeployment/topology"). - Watches( - &source.Kind{Type: &clusterv1.MachineSet{}}, - handler.EnqueueRequestsFromMapFunc(r.machineSetToDeployments), - ). WithOptions(options). WithEventFilter(predicates.All(ctrl.LoggerFrom(ctx), predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(ctx), r.WatchFilterValue), - // Note: Event filters are run for incoming events (i.e. MachineDeployment and MachineSet), not the - // results of EnqueueRequestsFromMapFunc. Thus, it's important that topology-owned MachineDeployments *and* - // MachineSets have the topology owned label to be filtered correctly. predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx)), )). - Build(r) + Complete(r) if err != nil { return errors.Wrap(err, "failed setting up with a controller manager") } @@ -77,346 +64,98 @@ func (r *MachineDeploymentReconciler) SetupWithManager(ctx context.Context, mgr return nil } -// Reconcile deletes referenced templates during deletion of topology-owned MachineDeployments and MachineSets. +// Reconcile deletes referenced templates during deletion of topology-owned MachineDeployments. // The templates are only deleted, if they are not used in other MachineDeployments or MachineSets which are not in deleting, -// i.e. the templates would otherwise be orphaned after the MachineDeployment or MachineSet deletion completes. -// -// This is achieved by: -// * adding finalizers to MachineDeployments and their corresponding MachineSets to hook into the deletion -// * calculating which templates are still used by MachineDeployments or MachineSets not in deleting -// * deleting templates of all MachineDeployments and MachineSets in deleting if they are not still used -// -// Some additional context: +// i.e. the templates would otherwise be orphaned after the MachineDeployment deletion completes. +// Additional context: // * MachineDeployment deletion: -// * MachineDeployments are deleted and garbage collected first (without waiting until all MachineSets are also deleted) -// * After that deletion of MachineSets is automatically triggered by Kubernetes based on owner references -// * MachineSet deletion: -// * MachineSets are deleted and garbage collected first (without waiting until all Machines are also deleted) -// * After that deletion of Machines is automatically triggered by Kubernetes based on owner references -// -// Note: We intentionally also reconcile the MachineSets by reconciling their MachineDeployment to avoid: -// * duplicating the logic -// * race conditions when deleting multiple MachineSets of the same MachineDeployment in parallel -// -// Note: We assume templates are not reused by different MachineDeployments, which is (only) true for topology-owned +// * MachineDeployments are deleted and garbage collected first (without waiting until all MachineSets are also deleted). +// * After that, deletion of MachineSets is automatically triggered by Kubernetes based on owner references. +// Note: We assume templates are not reused by different MachineDeployments, which is only true for topology-owned // MachineDeployments. func (r *MachineDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := ctrl.LoggerFrom(ctx) - // Fetch the MachineDeployment instance (if it still exists). - // Note: After a MachineDeployment has already been deleted, we will still "reconcile" - // the MachineDeployment to clean up its corresponding MachineSets. Thus, it's valid in that case - // that the MachineDeployment doesn't exist anymore. + // Fetch the MachineDeployment instance. md := &clusterv1.MachineDeployment{} if err := r.Client.Get(ctx, req.NamespacedName, md); err != nil { - if !apierrors.IsNotFound(err) { - // Error reading the object - requeue the request. - return ctrl.Result{}, errors.Wrapf(err, "failed to get MachineDeployment/%s", req.NamespacedName.Name) + if apierrors.IsNotFound(err) { + // Object not found, return. + return ctrl.Result{}, nil } - - // If the MachineDeployment doesn't exist anymore, set md to nil, so we can handle that case correctly below. - md = nil + // Error reading the object - requeue the request. + return ctrl.Result{}, errors.Wrapf(err, "failed to get MachineDeployment/%s", req.NamespacedName.Name) } - // Get the corresponding MachineSets. - msList, err := r.getMachineSetsForDeployment(ctx, req.NamespacedName) + cluster, err := util.GetClusterByName(ctx, r.Client, md.Namespace, md.Spec.ClusterName) if err != nil { return ctrl.Result{}, err } - // Return early if the Cluster is paused. - clusterPaused, err := r.isClusterPaused(ctx, md, msList) - if err != nil { - return ctrl.Result{}, err - } - if clusterPaused { + // Return early if the object or Cluster is paused. + if annotations.IsPaused(cluster, md) { log.Info("Reconciliation is paused for this object") return ctrl.Result{}, nil } - // Add finalizers to the MachineDeployment and the corresponding MachineSets. - if err := r.addFinalizers(ctx, msList); err != nil { - return ctrl.Result{}, err - } - - // Calculate which templates are still in use by MachineDeployments or MachineSets which are not in deleting. - templatesInUse, err := calculateTemplatesInUse(md, msList) - if err != nil { - return ctrl.Result{}, err - } + // We don't have to set the finalizer, as it's already set during MachineDeployment creation + // in the cluster topology controller. - // Delete unused templates of MachineSets which are in deleting. - if err := r.deleteMachineSetsTemplatesIfNecessary(ctx, templatesInUse, msList); err != nil { - return ctrl.Result{}, err + // Handle deletion reconciliation loop. + if !md.ObjectMeta.DeletionTimestamp.IsZero() { + return r.reconcileDelete(ctx, md) } - // Delete unused templates of the MachineDeployment if it is in deleting. - if err := r.deleteMachineDeploymentTemplatesIfNecessary(ctx, templatesInUse, md); err != nil { - return ctrl.Result{}, err - } + // Nothing to do. return ctrl.Result{}, nil } -// getMachineSetsForDeployment returns a list of MachineSets associated with a MachineDeployment. -func (r *MachineDeploymentReconciler) getMachineSetsForDeployment(ctx context.Context, md types.NamespacedName) ([]*clusterv1.MachineSet, error) { - // List MachineSets based on the MachineDeployment label. - msList := &clusterv1.MachineSetList{} - if err := r.Client.List(ctx, msList, - client.InNamespace(md.Namespace), client.MatchingLabels{clusterv1.MachineDeploymentLabelName: md.Name}); err != nil { - return nil, errors.Wrapf(err, "failed to list MachineSets for MachineDeployment/%s", md.Name) - } - - // Copy the MachineSets to an array of MachineSet pointers, to avoid MachineSet copying later. - res := make([]*clusterv1.MachineSet, 0, len(msList.Items)) - for i := range msList.Items { - res = append(res, &msList.Items[i]) - } - return res, nil -} - -func (r *MachineDeploymentReconciler) isClusterPaused(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet) (bool, error) { - var cluster *clusterv1.Cluster - var err error - - // If the MachineDeployment still exists, get the Cluster from the MachineDeployment. - if md != nil { - // Fetch the corresponding Cluster. - cluster, err = util.GetClusterByName(ctx, r.Client, md.ObjectMeta.Namespace, md.Spec.ClusterName) - if err != nil { - return false, errors.Wrapf(err, "failed to check if Cluster is paused") - } - } - - // Otherwise, get the Cluster from the first MachineSet, if possible. - if len(msList) > 0 { - cluster, err = util.GetClusterByName(ctx, r.Client, msList[0].ObjectMeta.Namespace, msList[0].Spec.ClusterName) - if err != nil { - return false, errors.Wrapf(err, "failed to check if Cluster is paused") - } - } - - if cluster == nil { - // This should never happen as at least one MachineDeployment or MachineSet should - // always exist during reconciliation. - return false, errors.Errorf("couldn't find a Cluster to check if it is paused") +func (r MachineDeploymentReconciler) reconcileDelete(ctx context.Context, md *clusterv1.MachineDeployment) (ctrl.Result, error) { + // Get the corresponding MachineSets. + msList, err := getMachineSetsForDeployment(ctx, r.Client, types.NamespacedName{Namespace: md.Namespace, Name: md.Name}) + if err != nil { + return ctrl.Result{}, err } - return annotations.IsPaused(cluster, cluster), nil -} - -// calculateTemplatesInUse returns all templates referenced in non-deleting MachineDeployment and MachineSets. -func calculateTemplatesInUse(md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet) (map[string]bool, error) { - templatesInUse := map[string]bool{} - - // Templates of the MachineSet are still in use if the MachineSet is not in deleting. - for _, ms := range msList { - if !ms.DeletionTimestamp.IsZero() { - continue - } - - bootstrapRef := ms.Spec.Template.Spec.Bootstrap.ConfigRef - infrastructureRef := &ms.Spec.Template.Spec.InfrastructureRef - if err := addRef(templatesInUse, bootstrapRef, infrastructureRef); err != nil { - return nil, errors.Wrapf(err, "failed to add templates of %s to templatesInUse", KRef{Obj: ms}) - } + // Calculate which templates are still in use by MachineDeployments or MachineSets which are not in deleting. + templatesInUse, err := calculateTemplatesInUse(md, msList) + if err != nil { + return ctrl.Result{}, err } - // MachineDeployment has already been deleted or still exists and is in deleting. In both cases there - // are no templates referenced in the MachineDeployment which are still in use, so let's return here. - if md == nil || !md.DeletionTimestamp.IsZero() { - return templatesInUse, nil + // Delete unused templates of the MachineDeployment. + if err := r.deleteUnusedMachineDeploymentTemplates(ctx, templatesInUse, md); err != nil { + return ctrl.Result{}, err } - // Templates of the MachineDeployment are still in use if the MachineDeployment is not in deleting. - bootstrapRef := md.Spec.Template.Spec.Bootstrap.ConfigRef - infrastructureRef := &md.Spec.Template.Spec.InfrastructureRef - if err := addRef(templatesInUse, bootstrapRef, infrastructureRef); err != nil { - return nil, errors.Wrapf(err, "failed to add templates of %s to templatesInUse", KRef{Obj: md}) + // Remove the finalizer so the MachineDeployment can be garbage collected by Kubernetes. + patchHelper, err := patch.NewHelper(md, r.Client) + if err != nil { + return ctrl.Result{}, errors.Wrapf(err, "failed to create patch helper for %s", KRef{Obj: md}) } - return templatesInUse, nil -} - -// addFinalizers adds finalizers to the MachineDeployment and the MachineSets, if they aren't already set. -func (r MachineDeploymentReconciler) addFinalizers(ctx context.Context, msList []*clusterv1.MachineSet) error { - // Add finalizer to the MachineSets. - for _, ms := range msList { - if !controllerutil.ContainsFinalizer(ms, clusterv1.MachineSetTopologyFinalizer) { - patchHelper, err := patch.NewHelper(ms, r.Client) - if err != nil { - return errors.Wrapf(err, "failed to create patch helper for %s", KRef{Obj: ms}) - } - controllerutil.AddFinalizer(ms, clusterv1.MachineSetTopologyFinalizer) - if err := patchHelper.Patch(ctx, ms); err != nil { - return errors.Wrapf(err, "failed to patch %s", KRef{Obj: ms}) - } - } + controllerutil.RemoveFinalizer(md, clusterv1.MachineDeploymentTopologyFinalizer) + if err := patchHelper.Patch(ctx, md); err != nil { + return ctrl.Result{}, errors.Wrapf(err, "failed to patch %s", KRef{Obj: md}) } - return nil -} - -// deleteMachineSetsTemplatesIfNecessary deletes templates referenced in MachineSets, if: -// * the MachineSet is in deleting -// * the MachineSet is not paused -// * the templates are not used by other MachineDeployments or MachineSets (i.e. they are not in templatesInUse). -// -// Note: We don't care about Machines, because the Machine deletion is triggered based -// on owner references by Kubernetes *after* we remove the finalizer from the -// MachineSet and the MachineSet has been garbage collected by Kubernetes. -func (r *MachineDeploymentReconciler) deleteMachineSetsTemplatesIfNecessary(ctx context.Context, templatesInUse map[string]bool, msList []*clusterv1.MachineSet) error { - for _, ms := range msList { - // MachineSet is not in deleting, do nothing. - if ms.DeletionTimestamp.IsZero() { - continue - } - // MachineSet is paused, do nothing. - if annotations.HasPausedAnnotation(ms) { - continue - } - // Delete the templates referenced in the MachineSet, if they are not used. - ref := ms.Spec.Template.Spec.Bootstrap.ConfigRef - if err := r.deleteTemplateIfNotUsed(ctx, templatesInUse, ref); err != nil { - return errors.Wrapf(err, "failed to delete bootstrap template for %s", KRef{Obj: ms}) - } - ref = &ms.Spec.Template.Spec.InfrastructureRef - if err := r.deleteTemplateIfNotUsed(ctx, templatesInUse, ref); err != nil { - return errors.Wrapf(err, "failed to delete infrastructure template for %s", KRef{Obj: ms}) - } - - // Remove the finalizer so the MachineSet can be deleted. - patchHelper, err := patch.NewHelper(ms, r.Client) - if err != nil { - return errors.Wrapf(err, "failed to create patch helper for %s", KRef{Obj: ms}) - } - controllerutil.RemoveFinalizer(ms, clusterv1.MachineSetTopologyFinalizer) - if err := patchHelper.Patch(ctx, ms); err != nil { - return errors.Wrapf(err, "failed to patch %s", KRef{Obj: ms}) - } - } - return nil + return ctrl.Result{}, nil } -// deleteMachineDeploymentTemplatesIfNecessary deletes templates referenced in MachineDeployments, if: -// * the MachineDeployment exist and is in deleting -// * the MachineDeployment is not paused -// * the templates are not used by other MachineDeployments or MachineSets (i.e. they are not in templatesInUse). -// +// deleteUnusedMachineDeploymentTemplates deletes templates referenced in MachineDeployments, if +// the templates are not used by other MachineDeployments or MachineSets (i.e. they are not in templatesInUse). // Note: We don't care about MachineSets, because the MachineSet deletion is triggered based // on owner references by Kubernetes *after* we remove the finalizer from the // MachineDeployment and the MachineDeployment has been garbage collected by Kubernetes. -func (r *MachineDeploymentReconciler) deleteMachineDeploymentTemplatesIfNecessary(ctx context.Context, templatesInUse map[string]bool, md *clusterv1.MachineDeployment) error { - // MachineDeployment has already been deleted or still exists and is not in deleting, do nothing. - if md == nil || md.DeletionTimestamp.IsZero() { - return nil - } - // MachineDeployment is paused, do nothing. - if annotations.HasPausedAnnotation(md) { - return nil - } - - // Delete the templates referenced in the MachineDeployment, if they are not used. +func (r *MachineDeploymentReconciler) deleteUnusedMachineDeploymentTemplates(ctx context.Context, templatesInUse map[string]bool, md *clusterv1.MachineDeployment) error { ref := md.Spec.Template.Spec.Bootstrap.ConfigRef - if err := r.deleteTemplateIfNotUsed(ctx, templatesInUse, ref); err != nil { + if err := deleteTemplateIfNotUsed(ctx, r.Client, templatesInUse, ref); err != nil { return errors.Wrapf(err, "failed to delete bootstrap template for %s", KRef{Obj: md}) } + ref = &md.Spec.Template.Spec.InfrastructureRef - if err := r.deleteTemplateIfNotUsed(ctx, templatesInUse, ref); err != nil { + if err := deleteTemplateIfNotUsed(ctx, r.Client, templatesInUse, ref); err != nil { return errors.Wrapf(err, "failed to delete infrastructure template for %s", KRef{Obj: md}) } - // Remove the finalizer so the MachineDeployment can be deleted. - patchHelper, err := patch.NewHelper(md, r.Client) - if err != nil { - return errors.Wrapf(err, "failed to create patch helper for %s", KRef{Obj: md}) - } - controllerutil.RemoveFinalizer(md, clusterv1.MachineDeploymentTopologyFinalizer) - if err := patchHelper.Patch(ctx, md); err != nil { - return errors.Wrapf(err, "failed to patch %s", KRef{Obj: md}) - } return nil } - -// deleteTemplateIfNotUsed deletes the template (ref), if it is not in use (i.e. in templatesInUse). -func (r *MachineDeploymentReconciler) deleteTemplateIfNotUsed(ctx context.Context, templatesInUse map[string]bool, ref *corev1.ObjectReference) error { - // If ref is nil, do nothing (this can happen, because bootstrap templates are optional). - if ref == nil { - return nil - } - - log := loggerFrom(ctx).WithRef(ref) - - refID, err := refID(ref) - if err != nil { - return errors.Wrapf(err, "failed to calculate refID") - } - - // If the template is still in use, do nothing. - if templatesInUse[refID] { - return nil - } - - // TODO(sbueringer) use KRef after 5153 has been merged - log.Infof("Deleting %s/%s", ref.Kind, ref.Name) - if err := external.Delete(ctx, r.Client, ref); err != nil && !apierrors.IsNotFound(err) { - // TODO(sbueringer) use KRef after 5153 has been merged - return errors.Wrapf(err, "failed to delete %s/%s", ref.Kind, ref.Name) - } - return nil -} - -// machineSetToDeployments is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation -// for MachineDeployments after changes to their MachineSets. -func (r *MachineDeploymentReconciler) machineSetToDeployments(o client.Object) []ctrl.Request { - ms, ok := o.(*clusterv1.MachineSet) - if !ok { - panic(fmt.Sprintf("Expected a MachineSet but got a %T", o)) - } - - // Note: When deleting MachineDeployments, MachineDeployments are deleted before MachineSets. - // So it can happen that we reconcile a MachineSet when the corresponding MachineDeployment - // doesn't exist anymore. That's the reason why we intentionally directly return the NamespacedName, - // instead of getting the resource first, as we do in similar funcs in other controllers. - for _, ref := range ms.OwnerReferences { - if ref.Kind != "MachineDeployment" { - continue - } - gv, err := schema.ParseGroupVersion(ref.APIVersion) - if err != nil { - // This should never happen. - panic(fmt.Sprintf("Expected a valid groupVersion but got %q: %v", ref.APIVersion, err)) - } - if gv.Group == clusterv1.GroupVersion.Group { - return []ctrl.Request{{NamespacedName: client.ObjectKey{Namespace: ms.Namespace, Name: ref.Name}}} - } - } - return nil -} - -// addRef adds the refs to the refMap with the refID as key. -func addRef(refMap map[string]bool, refs ...*corev1.ObjectReference) error { - for _, ref := range refs { - if ref != nil { - refID, err := refID(ref) - if err != nil { - return errors.Wrapf(err, "failed to calculate refID") - } - refMap[refID] = true - } - } - return nil -} - -// refID returns the refID of a ObjectReference in the format: g/k/name. -// Note: We don't include the version as references with different versions should be treated as equal. -func refID(ref *corev1.ObjectReference) (string, error) { - if ref == nil { - return "", nil - } - - gv, err := schema.ParseGroupVersion(ref.APIVersion) - if err != nil { - return "", errors.Wrapf(err, "failed to parse apiVersion %q", ref.APIVersion) - } - - return fmt.Sprintf("%s/%s/%s", gv.Group, ref.Kind, ref.Name), nil -} diff --git a/controllers/topology/machinedeployment_controller_test.go b/controllers/topology/machinedeployment_controller_test.go index a7f7786765f3..91cc675960e1 100644 --- a/controllers/topology/machinedeployment_controller_test.go +++ b/controllers/topology/machinedeployment_controller_test.go @@ -20,230 +20,12 @@ import ( "testing" . "github.com/onsi/gomega" - "github.com/pkg/errors" - corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" "sigs.k8s.io/cluster-api/internal/testtypes" - "sigs.k8s.io/cluster-api/util/annotations" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -func TestCalculateTemplatesInUse(t *testing.T) { - t.Run("Calculate templates in use with regular MachineDeployment and MachineSet", func(t *testing.T) { - g := NewWithT(t) - - md := testtypes.NewMachineDeploymentBuilder(metav1.NamespaceDefault, "md"). - WithBootstrapTemplate(testtypes.NewBootstrapTemplateBuilder(metav1.NamespaceDefault, "mdBT").Build()). - WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "mdIMT").Build()). - Build() - ms := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "ms"). - WithBootstrapTemplate(testtypes.NewBootstrapTemplateBuilder(metav1.NamespaceDefault, "msBT").Build()). - WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msIMT").Build()). - Build() - - actual, err := calculateTemplatesInUse(md, []*clusterv1.MachineSet{ms}) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(actual).To(HaveLen(4)) - - g.Expect(actual).To(HaveKey(mustRefID(md.Spec.Template.Spec.Bootstrap.ConfigRef))) - g.Expect(actual).To(HaveKey(mustRefID(&md.Spec.Template.Spec.InfrastructureRef))) - - g.Expect(actual).To(HaveKey(mustRefID(ms.Spec.Template.Spec.Bootstrap.ConfigRef))) - g.Expect(actual).To(HaveKey(mustRefID(&ms.Spec.Template.Spec.InfrastructureRef))) - }) - - t.Run("Calculate templates in use with MachineDeployment and MachineSet without BootstrapTemplate", func(t *testing.T) { - g := NewWithT(t) - - mdWithoutBootstrapTemplate := testtypes.NewMachineDeploymentBuilder(metav1.NamespaceDefault, "md"). - WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "mdIMT").Build()). - Build() - msWithoutBootstrapTemplate := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "ms"). - WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msIMT").Build()). - Build() - - actual, err := calculateTemplatesInUse(mdWithoutBootstrapTemplate, []*clusterv1.MachineSet{msWithoutBootstrapTemplate}) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(actual).To(HaveLen(2)) - - g.Expect(actual).To(HaveKey(mustRefID(&mdWithoutBootstrapTemplate.Spec.Template.Spec.InfrastructureRef))) - - g.Expect(actual).To(HaveKey(mustRefID(&msWithoutBootstrapTemplate.Spec.Template.Spec.InfrastructureRef))) - }) - - t.Run("Calculate templates in use with MachineDeployment and MachineSet ignore templates when resources in deleting", func(t *testing.T) { - g := NewWithT(t) - - deletionTimeStamp := metav1.Now() - - mdInDeleting := testtypes.NewMachineDeploymentBuilder(metav1.NamespaceDefault, "md"). - WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "mdIMT").Build()). - Build() - mdInDeleting.SetDeletionTimestamp(&deletionTimeStamp) - - msInDeleting := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "ms"). - WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msIMT").Build()). - Build() - msInDeleting.SetDeletionTimestamp(&deletionTimeStamp) - - actual, err := calculateTemplatesInUse(mdInDeleting, []*clusterv1.MachineSet{msInDeleting}) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(actual).To(HaveLen(0)) - - g.Expect(actual).ToNot(HaveKey(mustRefID(&mdInDeleting.Spec.Template.Spec.InfrastructureRef))) - - g.Expect(actual).ToNot(HaveKey(mustRefID(&msInDeleting.Spec.Template.Spec.InfrastructureRef))) - }) - - t.Run("Calculate templates in use without MachineDeployment and with MachineSet", func(t *testing.T) { - g := NewWithT(t) - - ms := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "ms"). - WithBootstrapTemplate(testtypes.NewBootstrapTemplateBuilder(metav1.NamespaceDefault, "msBT").Build()). - WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msIMT").Build()). - Build() - - actual, err := calculateTemplatesInUse(nil, []*clusterv1.MachineSet{ms}) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(actual).To(HaveLen(2)) - - g.Expect(actual).To(HaveKey(mustRefID(ms.Spec.Template.Spec.Bootstrap.ConfigRef))) - g.Expect(actual).To(HaveKey(mustRefID(&ms.Spec.Template.Spec.InfrastructureRef))) - }) -} - -func TestDeleteMachineSetsTemplatesIfNecessary(t *testing.T) { - deletionTimeStamp := metav1.Now() - - msBT := testtypes.NewBootstrapTemplateBuilder(metav1.NamespaceDefault, "msBT").Build() - msIMT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msIMT").Build() - ms := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "ms"). - WithBootstrapTemplate(msBT). - WithInfrastructureTemplate(msIMT). - Build() - - msPausedIMT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msInDeletingIMT").Build() - msPaused := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "msInDeleting"). - WithInfrastructureTemplate(msPausedIMT). - Build() - annotations.AddAnnotations(msPaused, map[string]string{clusterv1.PausedAnnotation: ""}) - msPaused.SetDeletionTimestamp(&deletionTimeStamp) - - msInDeletingBT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msInDeletingBT").Build() - msInDeletingIMT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msInDeletingIMT").Build() - msInDeleting := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "msInDeleting"). - WithBootstrapTemplate(msInDeletingBT). - WithInfrastructureTemplate(msInDeletingIMT). - Build() - msInDeleting.SetDeletionTimestamp(&deletionTimeStamp) - - msWithoutBootstrapTemplateInDeletingIMT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msWithoutBootstrapTemplateInDeletingIMT").Build() - msWithoutBootstrapTemplateInDeleting := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "msWithoutBootstrapTemplateInDeleting"). - WithInfrastructureTemplate(msWithoutBootstrapTemplateInDeletingIMT). - Build() - msWithoutBootstrapTemplateInDeleting.SetDeletionTimestamp(&deletionTimeStamp) - - t.Run("Should not delete templates of a MachineSet not in deleting", func(t *testing.T) { - g := NewWithT(t) - - fakeClient := fake.NewClientBuilder(). - WithScheme(fakeScheme). - WithObjects(ms, msBT, msIMT). - Build() - - r := &MachineDeploymentReconciler{ - Client: fakeClient, - } - err := r.deleteMachineSetsTemplatesIfNecessary(ctx, map[string]bool{}, []*clusterv1.MachineSet{ms}) - g.Expect(err).ToNot(HaveOccurred()) - - g.Expect(templateExists(fakeClient, msBT)).To(BeTrue()) - g.Expect(templateExists(fakeClient, msIMT)).To(BeTrue()) - }) - - t.Run("Should not delete templates of a paused MachineSet", func(t *testing.T) { - g := NewWithT(t) - - fakeClient := fake.NewClientBuilder(). - WithScheme(fakeScheme). - WithObjects(msPaused, msPausedIMT). - Build() - - r := &MachineDeploymentReconciler{ - Client: fakeClient, - } - err := r.deleteMachineSetsTemplatesIfNecessary(ctx, map[string]bool{}, []*clusterv1.MachineSet{msPaused}) - g.Expect(err).ToNot(HaveOccurred()) - - g.Expect(templateExists(fakeClient, msPausedIMT)).To(BeTrue()) - }) - - t.Run("Should delete templates of a MachineSet in deleting", func(t *testing.T) { - g := NewWithT(t) - - fakeClient := fake.NewClientBuilder(). - WithScheme(fakeScheme). - WithObjects(msInDeleting, msInDeletingBT, msInDeletingIMT). - Build() - - r := &MachineDeploymentReconciler{ - Client: fakeClient, - } - err := r.deleteMachineSetsTemplatesIfNecessary(ctx, map[string]bool{}, []*clusterv1.MachineSet{msInDeleting}) - g.Expect(err).ToNot(HaveOccurred()) - - g.Expect(templateExists(fakeClient, msInDeletingBT)).To(BeFalse()) - g.Expect(templateExists(fakeClient, msInDeletingIMT)).To(BeFalse()) - }) - - t.Run("Should not delete templates of a MachineSet in deleting when they are still in use", func(t *testing.T) { - g := NewWithT(t) - - fakeClient := fake.NewClientBuilder(). - WithScheme(fakeScheme). - WithObjects(msInDeleting, msInDeletingBT, msInDeletingIMT). - Build() - - templatesInUse := map[string]bool{ - mustRefID(msInDeleting.Spec.Template.Spec.Bootstrap.ConfigRef): true, - mustRefID(&msInDeleting.Spec.Template.Spec.InfrastructureRef): true, - } - - r := &MachineDeploymentReconciler{ - Client: fakeClient, - } - err := r.deleteMachineSetsTemplatesIfNecessary(ctx, templatesInUse, []*clusterv1.MachineSet{msInDeleting}) - g.Expect(err).ToNot(HaveOccurred()) - - g.Expect(templateExists(fakeClient, msInDeletingBT)).To(BeTrue()) - g.Expect(templateExists(fakeClient, msInDeletingIMT)).To(BeTrue()) - }) - - t.Run("Should delete infra template of a MachineSet without a bootstrap template in deleting", func(t *testing.T) { - g := NewWithT(t) - - fakeClient := fake.NewClientBuilder(). - WithScheme(fakeScheme). - WithObjects(msWithoutBootstrapTemplateInDeleting, msWithoutBootstrapTemplateInDeletingIMT). - Build() - - r := &MachineDeploymentReconciler{ - Client: fakeClient, - } - err := r.deleteMachineSetsTemplatesIfNecessary(ctx, map[string]bool{}, []*clusterv1.MachineSet{msWithoutBootstrapTemplateInDeleting}) - g.Expect(err).ToNot(HaveOccurred()) - - g.Expect(templateExists(fakeClient, msWithoutBootstrapTemplateInDeletingIMT)).To(BeFalse()) - }) -} - -func TestDeleteMachineDeploymentTemplatesIfNecessary(t *testing.T) { - deletionTimeStamp := metav1.Now() - +func TestDeleteUnusedMachineDeploymentTemplates(t *testing.T) { mdBT := testtypes.NewBootstrapTemplateBuilder(metav1.NamespaceDefault, "mdBT").Build() mdIMT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "mdIMT").Build() md := testtypes.NewMachineDeploymentBuilder(metav1.NamespaceDefault, "md"). @@ -251,43 +33,12 @@ func TestDeleteMachineDeploymentTemplatesIfNecessary(t *testing.T) { WithInfrastructureTemplate(mdIMT). Build() - mdPausedIMT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "mdInDeletingIMT").Build() - mdPaused := testtypes.NewMachineDeploymentBuilder(metav1.NamespaceDefault, "mdInDeleting"). - WithInfrastructureTemplate(mdPausedIMT). - Build() - annotations.AddAnnotations(mdPaused, map[string]string{clusterv1.PausedAnnotation: ""}) - mdPaused.SetDeletionTimestamp(&deletionTimeStamp) - - mdInDeletingBT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "mdInDeletingBT").Build() - mdInDeletingIMT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "mdInDeletingIMT").Build() - mdInDeleting := testtypes.NewMachineDeploymentBuilder(metav1.NamespaceDefault, "mdInDeleting"). - WithBootstrapTemplate(mdInDeletingBT). - WithInfrastructureTemplate(mdInDeletingIMT). - Build() - mdInDeleting.SetDeletionTimestamp(&deletionTimeStamp) - - mdWithoutBootstrapTemplateInDeletingIMT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "mdWithoutBootstrapTemplateInDeletingIMT").Build() - mdWithoutBootstrapTemplateInDeleting := testtypes.NewMachineDeploymentBuilder(metav1.NamespaceDefault, "mdWithoutBootstrapTemplateInDeleting"). - WithInfrastructureTemplate(mdWithoutBootstrapTemplateInDeletingIMT). + mdWithoutBootstrapTemplateIMT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "mdWithoutBootstrapTemplateIMT").Build() + mdWithoutBootstrapTemplate := testtypes.NewMachineDeploymentBuilder(metav1.NamespaceDefault, "mdWithoutBootstrapTemplate"). + WithInfrastructureTemplate(mdWithoutBootstrapTemplateIMT). Build() - mdWithoutBootstrapTemplateInDeleting.SetDeletionTimestamp(&deletionTimeStamp) - - t.Run("Should not fail if MachineDeployment does not exist", func(t *testing.T) { - g := NewWithT(t) - - fakeClient := fake.NewClientBuilder(). - WithScheme(fakeScheme). - WithObjects(). - Build() - r := &MachineDeploymentReconciler{ - Client: fakeClient, - } - err := r.deleteMachineDeploymentTemplatesIfNecessary(ctx, map[string]bool{}, nil) - g.Expect(err).ToNot(HaveOccurred()) - }) - - t.Run("Should not delete templates of a MachineDeployment not in deleting", func(t *testing.T) { + t.Run("Should delete templates of a MachineDeployment", func(t *testing.T) { g := NewWithT(t) fakeClient := fake.NewClientBuilder(). @@ -298,106 +49,50 @@ func TestDeleteMachineDeploymentTemplatesIfNecessary(t *testing.T) { r := &MachineDeploymentReconciler{ Client: fakeClient, } - err := r.deleteMachineDeploymentTemplatesIfNecessary(ctx, map[string]bool{}, md) - g.Expect(err).ToNot(HaveOccurred()) - - g.Expect(templateExists(fakeClient, mdBT)).To(BeTrue()) - g.Expect(templateExists(fakeClient, mdIMT)).To(BeTrue()) - }) - - t.Run("Should not delete templates of a paused MachineDeployment", func(t *testing.T) { - g := NewWithT(t) - - fakeClient := fake.NewClientBuilder(). - WithScheme(fakeScheme). - WithObjects(mdPaused, mdPausedIMT). - Build() - - r := &MachineDeploymentReconciler{ - Client: fakeClient, - } - err := r.deleteMachineDeploymentTemplatesIfNecessary(ctx, map[string]bool{}, mdPaused) + err := r.deleteUnusedMachineDeploymentTemplates(ctx, map[string]bool{}, md) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(templateExists(fakeClient, mdPausedIMT)).To(BeTrue()) + g.Expect(templateExists(fakeClient, mdBT)).To(BeFalse()) + g.Expect(templateExists(fakeClient, mdIMT)).To(BeFalse()) }) - t.Run("Should delete templates of a MachineDeployment in deleting", func(t *testing.T) { + t.Run("Should not delete templates of a MachineDeployment when they are still in use", func(t *testing.T) { g := NewWithT(t) fakeClient := fake.NewClientBuilder(). WithScheme(fakeScheme). - WithObjects(mdInDeleting, mdInDeletingBT, mdInDeletingIMT). - Build() - - r := &MachineDeploymentReconciler{ - Client: fakeClient, - } - err := r.deleteMachineDeploymentTemplatesIfNecessary(ctx, map[string]bool{}, mdInDeleting) - g.Expect(err).ToNot(HaveOccurred()) - - g.Expect(templateExists(fakeClient, mdInDeletingBT)).To(BeFalse()) - g.Expect(templateExists(fakeClient, mdInDeletingIMT)).To(BeFalse()) - }) - - t.Run("Should not delete templates of a MachineDeployment in deleting when they are still in use", func(t *testing.T) { - g := NewWithT(t) - - fakeClient := fake.NewClientBuilder(). - WithScheme(fakeScheme). - WithObjects(mdInDeleting, mdInDeletingBT, mdInDeletingIMT). + WithObjects(md, mdBT, mdIMT). Build() templatesInUse := map[string]bool{ - mustRefID(mdInDeleting.Spec.Template.Spec.Bootstrap.ConfigRef): true, - mustRefID(&mdInDeleting.Spec.Template.Spec.InfrastructureRef): true, + mustTemplateRefID(md.Spec.Template.Spec.Bootstrap.ConfigRef): true, + mustTemplateRefID(&md.Spec.Template.Spec.InfrastructureRef): true, } r := &MachineDeploymentReconciler{ Client: fakeClient, } - err := r.deleteMachineDeploymentTemplatesIfNecessary(ctx, templatesInUse, mdInDeleting) + err := r.deleteUnusedMachineDeploymentTemplates(ctx, templatesInUse, md) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(templateExists(fakeClient, mdInDeletingBT)).To(BeTrue()) - g.Expect(templateExists(fakeClient, mdInDeletingIMT)).To(BeTrue()) + g.Expect(templateExists(fakeClient, mdBT)).To(BeTrue()) + g.Expect(templateExists(fakeClient, mdIMT)).To(BeTrue()) }) - t.Run("Should delete infra template of a MachineDeployment without a bootstrap template in deleting", func(t *testing.T) { + t.Run("Should delete infra template of a MachineDeployment without a bootstrap template", func(t *testing.T) { g := NewWithT(t) fakeClient := fake.NewClientBuilder(). WithScheme(fakeScheme). - WithObjects(mdWithoutBootstrapTemplateInDeleting, mdWithoutBootstrapTemplateInDeletingIMT). + WithObjects(mdWithoutBootstrapTemplate, mdWithoutBootstrapTemplateIMT). Build() r := &MachineDeploymentReconciler{ Client: fakeClient, } - err := r.deleteMachineDeploymentTemplatesIfNecessary(ctx, map[string]bool{}, mdWithoutBootstrapTemplateInDeleting) + err := r.deleteUnusedMachineDeploymentTemplates(ctx, map[string]bool{}, mdWithoutBootstrapTemplate) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(templateExists(fakeClient, mdWithoutBootstrapTemplateInDeletingIMT)).To(BeFalse()) + g.Expect(templateExists(fakeClient, mdWithoutBootstrapTemplateIMT)).To(BeFalse()) }) } - -func templateExists(fakeClient client.Reader, template *unstructured.Unstructured) bool { - obj := &unstructured.Unstructured{} - obj.SetKind(template.GetKind()) - obj.SetAPIVersion(template.GetAPIVersion()) - - err := fakeClient.Get(ctx, client.ObjectKeyFromObject(template), obj) - if err != nil && !apierrors.IsNotFound(err) { - // This should never happen. - panic(errors.Wrapf(err, "failed to get %s/%s", template.GetKind(), template.GetName())) - } - return err == nil -} - -func mustRefID(ref *corev1.ObjectReference) string { - refID, err := refID(ref) - if err != nil { - panic(errors.Wrap(err, "failed to calculate refID")) - } - return refID -} diff --git a/controllers/topology/machineset_controller.go b/controllers/topology/machineset_controller.go new file mode 100644 index 000000000000..e6a38ac00d89 --- /dev/null +++ b/controllers/topology/machineset_controller.go @@ -0,0 +1,210 @@ +/* +Copyright 2021 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 topology + +import ( + "context" + + "github.com/pkg/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" + "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/cluster-api/util/annotations" + "sigs.k8s.io/cluster-api/util/patch" + "sigs.k8s.io/cluster-api/util/predicates" + 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" +) + +// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io;bootstrap.cluster.x-k8s.io,resources=*,verbs=delete +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters,verbs=get;list;watch +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments,verbs=get;list;watch +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinesets;machinesets/finalizers,verbs=get;list;watch;update;patch;delete + +// MachineSetReconciler deletes referenced templates during deletion of topology-owned MachineSets. +// The templates are only deleted, if they are not used in other MachineDeployments or MachineSets which are not in deleting, +// i.e. the templates would otherwise be orphaned after the MachineSet deletion completes. +// Note: To achieve this the reconciler sets a finalizer to hook into the MachineSet deletions. +type MachineSetReconciler struct { + Client client.Client + WatchFilterValue string +} + +func (r *MachineSetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error { + err := ctrl.NewControllerManagedBy(mgr). + For(&clusterv1.MachineSet{}). + Named("machineset/topology"). + WithOptions(options). + WithEventFilter(predicates.All(ctrl.LoggerFrom(ctx), + predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(ctx), r.WatchFilterValue), + predicates.ResourceIsTopologyOwned(ctrl.LoggerFrom(ctx)), + )). + Complete(r) + if err != nil { + return errors.Wrap(err, "failed setting up with a controller manager") + } + + return nil +} + +// Reconcile deletes referenced templates during deletion of topology-owned MachineSets. +// The templates are only deleted, if they are not used in other MachineDeployments or MachineSets which are not in deleting, +// i.e. the templates would otherwise be orphaned after the MachineSet deletion completes. +// Additional context: +// * MachineSet deletion: +// * MachineSets are deleted and garbage collected first (without waiting until all Machines are also deleted) +// * After that, deletion of Machines is automatically triggered by Kubernetes based on owner references. +// Note: We assume templates are not reused by different MachineDeployments, which is (only) true for topology-owned +// MachineDeployments. +func (r *MachineSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := ctrl.LoggerFrom(ctx) + + // Fetch the MachineSet instance. + ms := &clusterv1.MachineSet{} + if err := r.Client.Get(ctx, req.NamespacedName, ms); err != nil { + if apierrors.IsNotFound(err) { + // Object not found, return. + return ctrl.Result{}, nil + } + // Error reading the object - requeue the request. + return ctrl.Result{}, errors.Wrapf(err, "failed to get MachineSet/%s", req.NamespacedName.Name) + } + + cluster, err := util.GetClusterByName(ctx, r.Client, ms.Namespace, ms.Spec.ClusterName) + if err != nil { + return ctrl.Result{}, err + } + + // Return early if the object or Cluster is paused. + if annotations.IsPaused(cluster, ms) { + log.Info("Reconciliation is paused for this object") + return ctrl.Result{}, nil + } + + // Set finalizer, if it's not already set. + if !controllerutil.ContainsFinalizer(ms, clusterv1.MachineSetTopologyFinalizer) { + patchHelper, err := patch.NewHelper(ms, r.Client) + if err != nil { + return ctrl.Result{}, errors.Wrapf(err, "failed to create patch helper for %s", KRef{Obj: ms}) + } + controllerutil.AddFinalizer(ms, clusterv1.MachineSetTopologyFinalizer) + if err := patchHelper.Patch(ctx, ms); err != nil { + return ctrl.Result{}, errors.Wrapf(err, "failed to patch %s", KRef{Obj: ms}) + } + return ctrl.Result{}, nil + } + + // Handle deletion reconciliation loop. + if !ms.ObjectMeta.DeletionTimestamp.IsZero() { + return r.reconcileDelete(ctx, ms) + } + + // Nothing to do. + return ctrl.Result{}, nil +} + +func (r MachineSetReconciler) reconcileDelete(ctx context.Context, ms *clusterv1.MachineSet) (ctrl.Result, error) { + // Calculate the name of the corresponding MachineDeployment. + mdName, err := calculateMachineDeploymentName(ms) + if err != nil { + return ctrl.Result{}, err + } + + // Get the corresponding MachineSets. + msList, err := getMachineSetsForDeployment(ctx, r.Client, *mdName) + if err != nil { + return ctrl.Result{}, err + } + + // Fetch the MachineDeployment instance, if it still exists. + // Note: This can happen because MachineDeployments are deleted before their corresponding MachineSets. + md := &clusterv1.MachineDeployment{} + if err := r.Client.Get(ctx, *mdName, md); err != nil { + if !apierrors.IsNotFound(err) { + // Error reading the object - requeue the request. + return ctrl.Result{}, errors.Wrapf(err, "failed to get MachineDeployment/%s", mdName.Name) + } + // If the MachineDeployment doesn't exist anymore, set md to nil, so we can handle that case correctly below. + md = nil + } + + // Calculate which templates are still in use by MachineDeployments or MachineSets which are not in deleting. + templatesInUse, err := calculateTemplatesInUse(md, msList) + if err != nil { + return ctrl.Result{}, err + } + + // Delete unused templates of the MachineSet. + if err := r.deleteUnusedMachineSetTemplates(ctx, templatesInUse, ms); err != nil { + return ctrl.Result{}, err + } + + // Remove the finalizer so the MachineSet can be garbage collected by Kubernetes. + patchHelper, err := patch.NewHelper(ms, r.Client) + if err != nil { + return ctrl.Result{}, errors.Wrapf(err, "failed to create patch helper for %s", KRef{Obj: ms}) + } + controllerutil.RemoveFinalizer(ms, clusterv1.MachineSetTopologyFinalizer) + if err := patchHelper.Patch(ctx, ms); err != nil { + return ctrl.Result{}, errors.Wrapf(err, "failed to patch %s", KRef{Obj: ms}) + } + + return ctrl.Result{}, nil +} + +// calculateMachineDeploymentName calculates the MachineDeployment name based on owner references. +func calculateMachineDeploymentName(ms *clusterv1.MachineSet) (*types.NamespacedName, error) { + for _, ref := range ms.OwnerReferences { + if ref.Kind != "MachineDeployment" { + continue + } + gv, err := schema.ParseGroupVersion(ref.APIVersion) + if err != nil { + // This should never happen. + return nil, errors.Errorf("could not calculate MachineDeployment name for %s: invalid apiVersion %q: %v", + KRef{Obj: ms}, ref.APIVersion, err) + } + if gv.Group == clusterv1.GroupVersion.Group { + return &client.ObjectKey{Namespace: ms.Namespace, Name: ref.Name}, nil + } + } + + return nil, errors.Errorf("could not calculate MachineDeployment name for %s", KRef{Obj: ms}) +} + +// deleteUnusedMachineSetTemplates deletes templates referenced in MachineSets, if: +// the templates are not used by other MachineDeployments or MachineSets (i.e. they are not in templatesInUse). +// Note: We don't care about Machines, because the Machine deletion is triggered based +// on owner references by Kubernetes *after* we remove the finalizer from the +// MachineSet and the MachineSet has been garbage collected by Kubernetes. +func (r *MachineSetReconciler) deleteUnusedMachineSetTemplates(ctx context.Context, templatesInUse map[string]bool, ms *clusterv1.MachineSet) error { + ref := ms.Spec.Template.Spec.Bootstrap.ConfigRef + if err := deleteTemplateIfNotUsed(ctx, r.Client, templatesInUse, ref); err != nil { + return errors.Wrapf(err, "failed to delete bootstrap template for %s", KRef{Obj: ms}) + } + + ref = &ms.Spec.Template.Spec.InfrastructureRef + if err := deleteTemplateIfNotUsed(ctx, r.Client, templatesInUse, ref); err != nil { + return errors.Wrapf(err, "failed to delete infrastructure template for %s", KRef{Obj: ms}) + } + + return nil +} diff --git a/controllers/topology/machineset_controller_test.go b/controllers/topology/machineset_controller_test.go new file mode 100644 index 000000000000..4dc38ab71909 --- /dev/null +++ b/controllers/topology/machineset_controller_test.go @@ -0,0 +1,98 @@ +/* +Copyright 2021 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 topology + +import ( + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/cluster-api/internal/testtypes" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestDeleteUnusedMachineSetTemplates(t *testing.T) { + msBT := testtypes.NewBootstrapTemplateBuilder(metav1.NamespaceDefault, "msBT").Build() + msIMT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msIMT").Build() + ms := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "ms"). + WithBootstrapTemplate(msBT). + WithInfrastructureTemplate(msIMT). + Build() + + msWithoutBootstrapTemplateIMT := testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msWithoutBootstrapTemplateIMT").Build() + msWithoutBootstrapTemplate := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "msWithoutBootstrapTemplate"). + WithInfrastructureTemplate(msWithoutBootstrapTemplateIMT). + Build() + + t.Run("Should delete templates of a MachineSet", func(t *testing.T) { + g := NewWithT(t) + + fakeClient := fake.NewClientBuilder(). + WithScheme(fakeScheme). + WithObjects(ms, msBT, msIMT). + Build() + + r := &MachineSetReconciler{ + Client: fakeClient, + } + err := r.deleteUnusedMachineSetTemplates(ctx, map[string]bool{}, ms) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(templateExists(fakeClient, msBT)).To(BeFalse()) + g.Expect(templateExists(fakeClient, msIMT)).To(BeFalse()) + }) + + t.Run("Should not delete templates of a MachineSet when they are still in use", func(t *testing.T) { + g := NewWithT(t) + + fakeClient := fake.NewClientBuilder(). + WithScheme(fakeScheme). + WithObjects(ms, msBT, msIMT). + Build() + + templatesInUse := map[string]bool{ + mustTemplateRefID(ms.Spec.Template.Spec.Bootstrap.ConfigRef): true, + mustTemplateRefID(&ms.Spec.Template.Spec.InfrastructureRef): true, + } + + r := &MachineSetReconciler{ + Client: fakeClient, + } + err := r.deleteUnusedMachineSetTemplates(ctx, templatesInUse, ms) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(templateExists(fakeClient, msBT)).To(BeTrue()) + g.Expect(templateExists(fakeClient, msIMT)).To(BeTrue()) + }) + + t.Run("Should delete infra template of a MachineSet without a bootstrap template", func(t *testing.T) { + g := NewWithT(t) + + fakeClient := fake.NewClientBuilder(). + WithScheme(fakeScheme). + WithObjects(msWithoutBootstrapTemplate, msWithoutBootstrapTemplateIMT). + Build() + + r := &MachineSetReconciler{ + Client: fakeClient, + } + err := r.deleteUnusedMachineSetTemplates(ctx, map[string]bool{}, msWithoutBootstrapTemplate) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(templateExists(fakeClient, msWithoutBootstrapTemplateIMT)).To(BeFalse()) + }) +} diff --git a/controllers/topology/reconcile_state.go b/controllers/topology/reconcile_state.go index d69eb56fef53..3291103c196e 100644 --- a/controllers/topology/reconcile_state.go +++ b/controllers/topology/reconcile_state.go @@ -201,7 +201,7 @@ func (r *ClusterReconciler) updateMachineDeployment(ctx context.Context, cluster log := loggerFrom(ctx).WithMachineDeployment(desiredMD.Object) ctx, _ = log.WithObject(desiredMD.InfrastructureMachineTemplate).Into(ctx) - _, err := r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ + if _, err := r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ ref: &desiredMD.Object.Spec.Template.Spec.InfrastructureRef, current: currentMD.InfrastructureMachineTemplate, desired: desiredMD.InfrastructureMachineTemplate, @@ -209,13 +209,12 @@ func (r *ClusterReconciler) updateMachineDeployment(ctx context.Context, cluster return infrastructureMachineTemplateNamePrefix(clusterName, mdTopologyName) }, compatibilityChecker: check.ReferencedObjectsAreCompatible, - }) - if err != nil { + }); err != nil { return errors.Wrapf(err, "failed to update %s/%s", currentMD.Object.GroupVersionKind(), currentMD.Object.Name) } ctx, _ = log.WithObject(desiredMD.BootstrapTemplate).Into(ctx) - _, err = r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ + if _, err := r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ ref: desiredMD.Object.Spec.Template.Spec.Bootstrap.ConfigRef, current: currentMD.BootstrapTemplate, desired: desiredMD.BootstrapTemplate, @@ -223,8 +222,7 @@ func (r *ClusterReconciler) updateMachineDeployment(ctx context.Context, cluster return bootstrapTemplateNamePrefix(clusterName, mdTopologyName) }, compatibilityChecker: check.ObjectsAreInTheSameNamespace, - }) - if err != nil { + }); err != nil { return errors.Wrapf(err, "failed to update %s/%s", currentMD.Object.GroupVersionKind(), currentMD.Object.Name) } diff --git a/controllers/topology/template_cleanup_helpers.go b/controllers/topology/template_cleanup_helpers.go new file mode 100644 index 000000000000..e807c3d23b19 --- /dev/null +++ b/controllers/topology/template_cleanup_helpers.go @@ -0,0 +1,131 @@ +package topology + +import ( + "context" + "fmt" + + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" + "sigs.k8s.io/cluster-api/controllers/external" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// getMachineSetsForDeployment returns a list of MachineSets associated with a MachineDeployment. +func getMachineSetsForDeployment(ctx context.Context, c client.Client, md types.NamespacedName) ([]*clusterv1.MachineSet, error) { + // List MachineSets based on the MachineDeployment label. + msList := &clusterv1.MachineSetList{} + if err := c.List(ctx, msList, + client.InNamespace(md.Namespace), client.MatchingLabels{clusterv1.MachineDeploymentLabelName: md.Name}); err != nil { + return nil, errors.Wrapf(err, "failed to list MachineSets for MachineDeployment/%s", md.Name) + } + + // Copy the MachineSets to an array of MachineSet pointers, to avoid MachineSet copying later. + res := make([]*clusterv1.MachineSet, 0, len(msList.Items)) + for i := range msList.Items { + res = append(res, &msList.Items[i]) + } + return res, nil +} + +// calculateTemplatesInUse returns all templates referenced in non-deleting MachineDeployment and MachineSets. +func calculateTemplatesInUse(md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet) (map[string]bool, error) { + templatesInUse := map[string]bool{} + + // Templates of the MachineSet are still in use if the MachineSet is not in deleting. + for _, ms := range msList { + if !ms.DeletionTimestamp.IsZero() { + continue + } + + bootstrapRef := ms.Spec.Template.Spec.Bootstrap.ConfigRef + infrastructureRef := &ms.Spec.Template.Spec.InfrastructureRef + if err := addTemplateRef(templatesInUse, bootstrapRef, infrastructureRef); err != nil { + return nil, errors.Wrapf(err, "failed to add templates of %s to templatesInUse", KRef{Obj: ms}) + } + } + + // MachineDeployment has already been deleted or still exists and is in deleting. In both cases there + // are no templates referenced in the MachineDeployment which are still in use, so let's return here. + if md == nil || !md.DeletionTimestamp.IsZero() { + return templatesInUse, nil + } + + // Templates of the MachineDeployment are still in use if the MachineDeployment is not in deleting. + bootstrapRef := md.Spec.Template.Spec.Bootstrap.ConfigRef + infrastructureRef := &md.Spec.Template.Spec.InfrastructureRef + if err := addTemplateRef(templatesInUse, bootstrapRef, infrastructureRef); err != nil { + return nil, errors.Wrapf(err, "failed to add templates of %s to templatesInUse", KRef{Obj: md}) + } + return templatesInUse, nil +} + +// deleteTemplateIfNotUsed deletes the template (ref), if it is not in use (i.e. in templatesInUse). +func deleteTemplateIfNotUsed(ctx context.Context, c client.Client, templatesInUse map[string]bool, ref *corev1.ObjectReference) error { + // If ref is nil, do nothing (this can happen, because bootstrap templates are optional). + if ref == nil { + return nil + } + + log := loggerFrom(ctx).WithRef(ref) + + refID, err := templateRefID(ref) + if err != nil { + return errors.Wrapf(err, "failed to calculate templateRefID") + } + + // If the template is still in use, do nothing. + if templatesInUse[refID] { + return nil + } + + // TODO(sbueringer) use KRef after 5153 has been merged + log.Infof("Deleting %s/%s", ref.Kind, ref.Name) + if err := external.Delete(ctx, c, ref); err != nil && !apierrors.IsNotFound(err) { + // TODO(sbueringer) use KRef after 5153 has been merged + return errors.Wrapf(err, "failed to delete %s/%s", ref.Kind, ref.Name) + } + return nil +} + +// addTemplateRef adds the refs to the refMap with the templateRefID as key. +func addTemplateRef(refMap map[string]bool, refs ...*corev1.ObjectReference) error { + for _, ref := range refs { + if ref != nil { + refID, err := templateRefID(ref) + if err != nil { + return errors.Wrapf(err, "failed to calculate templateRefID") + } + refMap[refID] = true + } + } + return nil +} + +// templateRefID returns the templateRefID of a ObjectReference in the format: g/k/name. +// Note: We don't include the version as references with different versions should be treated as equal. +func templateRefID(ref *corev1.ObjectReference) (string, error) { + if ref == nil { + return "", nil + } + + gv, err := schema.ParseGroupVersion(ref.APIVersion) + if err != nil { + return "", errors.Wrapf(err, "failed to parse apiVersion %q", ref.APIVersion) + } + + return fmt.Sprintf("%s/%s/%s", gv.Group, ref.Kind, ref.Name), nil +} + +// mustTemplateRefID returns the templateRefID as calculated by templateRefID, but panics +// if templateRefID returns an error. +func mustTemplateRefID(ref *corev1.ObjectReference) string { + refID, err := templateRefID(ref) + if err != nil { + panic(errors.Wrap(err, "failed to calculate templateRefID")) + } + return refID +} diff --git a/controllers/topology/template_cleanup_helpers_test.go b/controllers/topology/template_cleanup_helpers_test.go new file mode 100644 index 000000000000..a3852128044f --- /dev/null +++ b/controllers/topology/template_cleanup_helpers_test.go @@ -0,0 +1,127 @@ +/* +Copyright 2021 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 topology + +import ( + "testing" + + . "github.com/onsi/gomega" + "github.com/pkg/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" + "sigs.k8s.io/cluster-api/internal/testtypes" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func TestCalculateTemplatesInUse(t *testing.T) { + t.Run("Calculate templates in use with regular MachineDeployment and MachineSet", func(t *testing.T) { + g := NewWithT(t) + + md := testtypes.NewMachineDeploymentBuilder(metav1.NamespaceDefault, "md"). + WithBootstrapTemplate(testtypes.NewBootstrapTemplateBuilder(metav1.NamespaceDefault, "mdBT").Build()). + WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "mdIMT").Build()). + Build() + ms := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "ms"). + WithBootstrapTemplate(testtypes.NewBootstrapTemplateBuilder(metav1.NamespaceDefault, "msBT").Build()). + WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msIMT").Build()). + Build() + + actual, err := calculateTemplatesInUse(md, []*clusterv1.MachineSet{ms}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(actual).To(HaveLen(4)) + + g.Expect(actual).To(HaveKey(mustTemplateRefID(md.Spec.Template.Spec.Bootstrap.ConfigRef))) + g.Expect(actual).To(HaveKey(mustTemplateRefID(&md.Spec.Template.Spec.InfrastructureRef))) + + g.Expect(actual).To(HaveKey(mustTemplateRefID(ms.Spec.Template.Spec.Bootstrap.ConfigRef))) + g.Expect(actual).To(HaveKey(mustTemplateRefID(&ms.Spec.Template.Spec.InfrastructureRef))) + }) + + t.Run("Calculate templates in use with MachineDeployment and MachineSet without BootstrapTemplate", func(t *testing.T) { + g := NewWithT(t) + + mdWithoutBootstrapTemplate := testtypes.NewMachineDeploymentBuilder(metav1.NamespaceDefault, "md"). + WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "mdIMT").Build()). + Build() + msWithoutBootstrapTemplate := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "ms"). + WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msIMT").Build()). + Build() + + actual, err := calculateTemplatesInUse(mdWithoutBootstrapTemplate, []*clusterv1.MachineSet{msWithoutBootstrapTemplate}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(actual).To(HaveLen(2)) + + g.Expect(actual).To(HaveKey(mustTemplateRefID(&mdWithoutBootstrapTemplate.Spec.Template.Spec.InfrastructureRef))) + + g.Expect(actual).To(HaveKey(mustTemplateRefID(&msWithoutBootstrapTemplate.Spec.Template.Spec.InfrastructureRef))) + }) + + t.Run("Calculate templates in use with MachineDeployment and MachineSet ignore templates when resources in deleting", func(t *testing.T) { + g := NewWithT(t) + + deletionTimeStamp := metav1.Now() + + mdInDeleting := testtypes.NewMachineDeploymentBuilder(metav1.NamespaceDefault, "md"). + WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "mdIMT").Build()). + Build() + mdInDeleting.SetDeletionTimestamp(&deletionTimeStamp) + + msInDeleting := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "ms"). + WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msIMT").Build()). + Build() + msInDeleting.SetDeletionTimestamp(&deletionTimeStamp) + + actual, err := calculateTemplatesInUse(mdInDeleting, []*clusterv1.MachineSet{msInDeleting}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(actual).To(HaveLen(0)) + + g.Expect(actual).ToNot(HaveKey(mustTemplateRefID(&mdInDeleting.Spec.Template.Spec.InfrastructureRef))) + + g.Expect(actual).ToNot(HaveKey(mustTemplateRefID(&msInDeleting.Spec.Template.Spec.InfrastructureRef))) + }) + + t.Run("Calculate templates in use without MachineDeployment and with MachineSet", func(t *testing.T) { + g := NewWithT(t) + + ms := testtypes.NewMachineSetBuilder(metav1.NamespaceDefault, "ms"). + WithBootstrapTemplate(testtypes.NewBootstrapTemplateBuilder(metav1.NamespaceDefault, "msBT").Build()). + WithInfrastructureTemplate(testtypes.NewInfrastructureMachineTemplateBuilder(metav1.NamespaceDefault, "msIMT").Build()). + Build() + + actual, err := calculateTemplatesInUse(nil, []*clusterv1.MachineSet{ms}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(actual).To(HaveLen(2)) + + g.Expect(actual).To(HaveKey(mustTemplateRefID(ms.Spec.Template.Spec.Bootstrap.ConfigRef))) + g.Expect(actual).To(HaveKey(mustTemplateRefID(&ms.Spec.Template.Spec.InfrastructureRef))) + }) +} + +func templateExists(fakeClient client.Reader, template *unstructured.Unstructured) bool { + obj := &unstructured.Unstructured{} + obj.SetKind(template.GetKind()) + obj.SetAPIVersion(template.GetAPIVersion()) + + err := fakeClient.Get(ctx, client.ObjectKeyFromObject(template), obj) + if err != nil && !apierrors.IsNotFound(err) { + // This should never happen. + panic(errors.Wrapf(err, "failed to get %s/%s", template.GetKind(), template.GetName())) + } + return err == nil +} diff --git a/main.go b/main.go index 049da2c31d04..f2bef8324b91 100644 --- a/main.go +++ b/main.go @@ -58,27 +58,26 @@ var ( setupLog = ctrl.Log.WithName("setup") // flags. - metricsBindAddr string - enableLeaderElection bool - leaderElectionLeaseDuration time.Duration - leaderElectionRenewDeadline time.Duration - leaderElectionRetryPeriod time.Duration - watchNamespace string - watchFilterValue string - profilerAddress string - clusterTopologyConcurrency int - machineDeploymentTopologyConcurrency int - clusterConcurrency int - machineConcurrency int - machineSetConcurrency int - machineDeploymentConcurrency int - machinePoolConcurrency int - clusterResourceSetConcurrency int - machineHealthCheckConcurrency int - syncPeriod time.Duration - webhookPort int - webhookCertDir string - healthAddr string + metricsBindAddr string + enableLeaderElection bool + leaderElectionLeaseDuration time.Duration + leaderElectionRenewDeadline time.Duration + leaderElectionRetryPeriod time.Duration + watchNamespace string + watchFilterValue string + profilerAddress string + clusterTopologyConcurrency int + clusterConcurrency int + machineConcurrency int + machineSetConcurrency int + machineDeploymentConcurrency int + machinePoolConcurrency int + clusterResourceSetConcurrency int + machineHealthCheckConcurrency int + syncPeriod time.Duration + webhookPort int + webhookCertDir string + healthAddr string ) func init() { @@ -124,9 +123,6 @@ func InitFlags(fs *pflag.FlagSet) { fs.IntVar(&clusterTopologyConcurrency, "clustertopology-concurrency", 10, "Number of clusters to process simultaneously") - fs.IntVar(&machineDeploymentTopologyConcurrency, "machinedeploymenttopology-concurrency", 10, - "Number of machine deployments to process simultaneously") - fs.IntVar(&clusterConcurrency, "cluster-concurrency", 10, "Number of clusters to process simultaneously") @@ -292,10 +288,18 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager) { if err := (&topology.MachineDeploymentReconciler{ Client: mgr.GetClient(), WatchFilterValue: watchFilterValue, - }).SetupWithManager(ctx, mgr, concurrency(machineDeploymentTopologyConcurrency)); err != nil { + }).SetupWithManager(ctx, mgr, controller.Options{}); err != nil { setupLog.Error(err, "unable to create controller", "controller", "MachineDeploymentTopology") os.Exit(1) } + + if err := (&topology.MachineSetReconciler{ + Client: mgr.GetClient(), + WatchFilterValue: watchFilterValue, + }).SetupWithManager(ctx, mgr, controller.Options{}); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "MachineSetTopology") + os.Exit(1) + } } if err := (&controllers.ClusterReconciler{ Client: mgr.GetClient(),