MachineSet feature gating

This change adds feature gate support to the MachineSet controller.
We will need this when adding the paused condition, as it will sit
behind a feature gate.

Author: theobarberbany

cmd/machineset/main.go

@@ -20,34 +20,41 @@ import (
 	"flag"
 	"fmt"
 	"log"
+	"strings"
 	"time"
 
+	osconfigv1 "github.com/openshift/api/config/v1"
+	apifeatures "github.com/openshift/api/features"
+	machinev1 "github.com/openshift/api/machine/v1beta1"
+	mapiwebhooks "github.com/openshift/machine-api-operator/pkg/webhooks"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	k8sflag "k8s.io/component-base/cli/flag"
+	ctrl "sigs.k8s.io/controller-runtime"
+
+	"github.com/openshift/library-go/pkg/config/leaderelection"
+	"github.com/openshift/library-go/pkg/features"
+	"github.com/openshift/machine-api-operator/pkg/controller"
+	"github.com/openshift/machine-api-operator/pkg/controller/machineset"
+	"github.com/openshift/machine-api-operator/pkg/metrics"
+	"github.com/openshift/machine-api-operator/pkg/operator"
+	"github.com/openshift/machine-api-operator/pkg/util"
+	"k8s.io/apiserver/pkg/util/feature"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
+	"k8s.io/component-base/featuregate"
 	"k8s.io/klog/v2"
-	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
 	"sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
-
-	osconfigv1 "github.com/openshift/api/config/v1"
-	machinev1 "github.com/openshift/api/machine/v1beta1"
-	"github.com/openshift/library-go/pkg/config/leaderelection"
-	"github.com/openshift/machine-api-operator/pkg/controller"
-	"github.com/openshift/machine-api-operator/pkg/controller/machineset"
-	"github.com/openshift/machine-api-operator/pkg/metrics"
-	"github.com/openshift/machine-api-operator/pkg/operator"
-	"github.com/openshift/machine-api-operator/pkg/util"
-	mapiwebhooks "github.com/openshift/machine-api-operator/pkg/webhooks"
 )
 
 const (
 	defaultWebhookPort    = operator.MachineSetWebhookPort
 	defaultWebhookCertdir = "/etc/machine-api-operator/tls"
+	timeout               = 10 * time.Minute
 )
 
 func main() {
@@ -102,6 +109,17 @@ func main() {
 		fmt.Sprintf("The duration that non-leader candidates will wait after observing a leadership renewal until attempting to acquire leadership of a led but unrenewed leader slot. This is effectively the maximum duration that a leader can be stopped before it is replaced by another candidate. This is only applicable if leader election is enabled. Default: (%s)", defaultLeaderElectionValues.LeaseDuration.Duration),
 	)
 
+	// Sets up feature gates
+	defaultMutableGate := feature.DefaultMutableFeatureGate
+	_, err := features.NewFeatureGateOptions(defaultMutableGate, apifeatures.SelfManaged, apifeatures.FeatureGateVSphereStaticIPs, apifeatures.FeatureGateMachineAPIMigration)
+	if err != nil {
+		klog.Fatalf("Error setting up feature gates: %v", err)
+	}
+
+	featureGateArgs := map[string]bool{}
+	flag.Var(k8sflag.NewMapStringBool(&featureGateArgs), "feature-gates", "A set of key=value pairs that describe feature gates for alpha/experimental features. "+
+		"Options are:\n"+strings.Join(defaultMutableGate.KnownFeatures(), "\n"))
+
 	flag.Parse()
 	if *watchNamespace != "" {
 		log.Printf("Watching cluster-api objects only in namespace %q for reconciliation.", *watchNamespace)
@@ -120,7 +138,7 @@ func main() {
 	})
 
 	// Create a new Cmd to provide shared dependencies and start components
-	syncPeriod := 10 * time.Minute
+	syncPeriod := timeout
 	opts := manager.Options{
 		Metrics: server.Options{
 			BindAddress: *metricsAddress,
@@ -152,6 +170,14 @@ func main() {
 		log.Fatal(err)
 	}
 
+	// Sets feature gates from flags
+	klog.Infof("Initializing feature gates: %s", strings.Join(defaultMutableGate.KnownFeatures(), ", "))
+	err = defaultMutableGate.SetFromMap(featureGateArgs)
+	if err != nil {
+		klog.Fatalf("Error setting feature gates from flags: %v", err)
+	}
+	klog.Infof("FeatureGateMachineAPIMigration initialised: %t", defaultMutableGate.Enabled(featuregate.Feature(apifeatures.FeatureGateMachineAPIMigration)))
+
 	// Enable defaulting and validating webhooks
 	machineDefaulter, err := mapiwebhooks.NewMachineDefaulter()
 	if err != nil {
@@ -188,7 +214,7 @@ func main() {
 	}
 
 	// Setup all Controllers
-	if err := controller.AddToManager(mgr, opts, machineset.Add); err != nil {
+	if err := controller.AddToManagerWithFeatureGates(mgr, opts, defaultMutableGate, machineset.Add); err != nil {
 		log.Fatal(err)
 	}
 

pkg/controller/controller.go

@@ -1,9 +1,20 @@
 package controller
 
 import (
+	"k8s.io/component-base/featuregate"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
+// AddToManager adds all Controllers to the Manager along with a feature gate accessor
+func AddToManagerWithFeatureGates(m manager.Manager, opts manager.Options, featureGate featuregate.MutableFeatureGate, fnList ...func(manager.Manager, manager.Options, featuregate.MutableFeatureGate) error) error {
+	for _, f := range fnList {
+		if err := f(m, opts, featureGate); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // AddToManager adds all Controllers to the Manager
 func AddToManager(m manager.Manager, opts manager.Options, fnList ...func(manager.Manager, manager.Options) error) error {
 	for _, f := range fnList {

pkg/controller/machineset/controller.go

@@ -35,6 +35,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/component-base/featuregate"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -60,14 +61,18 @@ var (
 
 // Add creates a new MachineSet Controller and adds it to the Manager with default RBAC.
 // The Manager will set fields on the Controller and Start it when the Manager is Started.
-func Add(mgr manager.Manager, opts manager.Options) error {
-	r := newReconciler(mgr)
+func Add(mgr manager.Manager, opts manager.Options, gate featuregate.MutableFeatureGate) error {
+	r := newReconciler(mgr, gate)
 	return add(mgr, r, r.MachineToMachineSets)
 }
 
 // newReconciler returns a new reconcile.Reconciler.
-func newReconciler(mgr manager.Manager) *ReconcileMachineSet {
-	return &ReconcileMachineSet{Client: mgr.GetClient(), scheme: mgr.GetScheme(), recorder: mgr.GetEventRecorderFor(controllerName)}
+func newReconciler(mgr manager.Manager, gate featuregate.MutableFeatureGate) *ReconcileMachineSet {
+	return &ReconcileMachineSet{
+		Client: mgr.GetClient(), scheme: mgr.GetScheme(),
+		recorder: mgr.GetEventRecorderFor(controllerName),
+		gate:     gate,
+	}
 }
 
 // add adds a new Controller to mgr with r as the reconcile.Reconciler.
@@ -108,6 +113,7 @@ type ReconcileMachineSet struct {
 	client.Client
 	scheme   *runtime.Scheme
 	recorder record.EventRecorder
+	gate     featuregate.MutableFeatureGate
 }
 
 func (r *ReconcileMachineSet) MachineToMachineSets(ctx context.Context, o *machinev1.Machine) []reconcile.Request {

pkg/controller/machineset/controller_test.go

@@ -25,6 +25,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	machinev1 "github.com/openshift/api/machine/v1beta1"
+	testutils "github.com/openshift/machine-api-operator/pkg/util/testing"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -273,10 +274,13 @@ var _ = Describe("MachineSet Reconcile", func() {
 
 	BeforeEach(func() {
 		rec = record.NewFakeRecorder(32)
+		gate, err := testutils.NewDefaultMutableFeatureGate()
+		Expect(err).NotTo(HaveOccurred())
 
 		r = &ReconcileMachineSet{
 			scheme:   scheme.Scheme,
 			recorder: rec,
+			gate:     gate,
 		}
 	})
 

pkg/controller/machineset/machineset_controller_test.go

@@ -22,18 +22,25 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+
 	machinev1 "github.com/openshift/api/machine/v1beta1"
+	machinev1resourcebuilder "github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder/machine/v1beta1"
+	testutils "github.com/openshift/machine-api-operator/pkg/util/testing"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
+
 	"sigs.k8s.io/controller-runtime/pkg/metrics/server"
 )
 
 var _ = Describe("MachineSet Reconciler", func() {
-	namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ms-test"}}
 	var mgrCtxCancel context.CancelFunc
 	var mgrStopped chan struct{}
+	var namespace *corev1.Namespace
+	var machineSetBuilder machinev1resourcebuilder.MachineSetBuilder
+	var replicas int32 = int32(2)
 
 	BeforeEach(func() {
 		By("Setting up a new manager")
@@ -46,11 +53,14 @@ var _ = Describe("MachineSet Reconciler", func() {
 
 		k8sClient = mgr.GetClient()
 
+		By("Setting up feature gates")
+		gate, err := testutils.NewDefaultMutableFeatureGate()
+		Expect(err).NotTo(HaveOccurred())
+
 		By("Setting up a new reconciler")
-		reconciler := newReconciler(mgr)
+		reconciler := newReconciler(mgr, gate)
 
-		err = add(mgr, reconciler, reconciler.MachineToMachineSets)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(add(mgr, reconciler, reconciler.MachineToMachineSets)).To(Succeed())
 
 		var mgrCtx context.Context
 		mgrCtx, mgrCtxCancel = context.WithCancel(ctx)
@@ -65,12 +75,21 @@ var _ = Describe("MachineSet Reconciler", func() {
 		}()
 
 		By("Creating the namespace")
+		namespace = &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{GenerateName: "ms-test"}}
 		Expect(k8sClient.Create(ctx, namespace)).To(Succeed())
+
+		By("Setting up the machine set builder")
+		machineSetBuilder = machinev1resourcebuilder.MachineSet().
+			WithNamespace(namespace.ObjectMeta.Name).
+			WithName("foo").
+			WithReplicas(replicas).
+			WithLabels(map[string]string{"foo": "bar"})
+
 	})
 
 	AfterEach(func() {
 		By("Deleting the machinesets")
-		Expect(cleanResources()).To(Succeed())
+		Expect(cleanResources(namespace.Name)).To(Succeed())
 
 		By("Deleting the namespace")
 		Expect(k8sClient.Delete(ctx, namespace)).To(Succeed())
@@ -81,29 +100,12 @@ var _ = Describe("MachineSet Reconciler", func() {
 	})
 
 	It("Should reconcile a MachineSet", func() {
-		replicas := int32(2)
-		labels := map[string]string{"foo": "bar"}
-
-		instance := &machinev1.MachineSet{
-			ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: namespace.Name},
-			Spec: machinev1.MachineSetSpec{
-				Replicas: &replicas,
-				Selector: metav1.LabelSelector{
-					MatchLabels: labels,
-				},
-				Template: machinev1.MachineTemplateSpec{
-					ObjectMeta: machinev1.ObjectMeta{
-						Labels: labels,
-					},
-				},
-			},
-		}
+		instance := machineSetBuilder.Build()
 
 		By("Creating the MachineSet")
 		Expect(k8sClient.Create(ctx, instance)).To(Succeed())
 
 		machines := &machinev1.MachineList{}
-
 		By("Verifying that we have 2 replicas")
 		Eventually(func() (int, error) {
 			if err := k8sClient.List(ctx, machines, client.InNamespace(namespace.Name)); err != nil {
@@ -133,28 +135,16 @@ var _ = Describe("MachineSet Reconciler", func() {
 	})
 })
 
-func cleanResources() error {
-	machineSets := &machinev1.MachineSetList{}
-	if err := k8sClient.List(ctx, machineSets); err != nil {
+func cleanResources(namespace string) error {
+	machineSet := &machinev1.MachineSet{}
+	if err := k8sClient.DeleteAllOf(ctx, machineSet, client.InNamespace(namespace)); err != nil {
 		return err
 	}
-	for _, machineSet := range machineSets.Items {
-		ms := machineSet
-		if err := k8sClient.Delete(ctx, &ms); err != nil {
-			return err
-		}
-	}
 
-	machines := &machinev1.MachineList{}
-	if err := k8sClient.List(ctx, machines); err != nil {
+	machine := &machinev1.Machine{}
+	if err := k8sClient.DeleteAllOf(ctx, machine, client.InNamespace(namespace)); err != nil {
 		return err
 	}
-	for _, machine := range machines.Items {
-		m := machine
-		if err := k8sClient.Delete(ctx, &m); err != nil {
-			return err
-		}
-	}
 
 	return nil
 }

pkg/util/testing/testing.go

@@ -6,10 +6,14 @@ import (
 
 	machinev1 "github.com/openshift/api/machine/v1beta1"
 
+	openshiftfeatures "github.com/openshift/api/features"
+	"github.com/openshift/library-go/pkg/features"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/uuid"
+	"k8s.io/apiserver/pkg/util/feature"
+	"k8s.io/component-base/featuregate"
 	"k8s.io/utils/ptr"
 )
 
@@ -169,3 +173,18 @@ func NewMachineHealthCheck(name string) *machinev1.MachineHealthCheck {
 		Status: machinev1.MachineHealthCheckStatus{},
 	}
 }
+
+func NewDefaultMutableFeatureGate() (featuregate.MutableFeatureGate, error) {
+	defaultMutableGate := feature.DefaultMutableFeatureGate
+	_, err := features.NewFeatureGateOptions(defaultMutableGate, openshiftfeatures.SelfManaged, openshiftfeatures.FeatureGateMachineAPIMigration)
+	if err != nil {
+		return nil, err
+	}
+
+	err = defaultMutableGate.SetFromMap(map[string]bool{"MachineAPIMigration": true})
+	if err != nil {
+		return nil, err
+	}
+
+	return defaultMutableGate, nil
+}