Merge pull request openshift#102 from azych/feature-gates-lib

OPRUN-3663:  Watch and reconcile feature gates changes

Author: openshift-merge-bot[bot]

cmd/cluster-olm-operator/main.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	goflag "flag"
 	"fmt"
 	"os"
@@ -192,6 +193,16 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error
 
 	cl.StartInformers(ctx)
 
+	log := klog.FromContext(ctx)
+	select {
+	case <-cl.FeatureGatesAccessor.InitialFeatureGatesObserved():
+		featureGates, _ := cl.FeatureGatesAccessor.CurrentFeatureGates()
+		log.Info("FeatureGates initialized", "knownFeatures", featureGates.KnownFeatures())
+	case <-time.After(1 * time.Minute):
+		log.Error(nil, "timed out waiting for FeatureGate detection")
+		return errors.New("timed out waiting for FeatureGate detection")
+	}
+
 	for _, c := range append(staticResourceControllerList, upgradeableConditionController, incompatibleOperatorController, clusterOperatorController, operatorLoggingController, proxyController) {
 		go func(c factory.Controller) {
 			defer runtime.HandleCrash()

internal/featuregates/mapper.go

@@ -0,0 +1,105 @@
+package featuregates
+
+import (
+	"bytes"
+	"errors"
+	"strings"
+
+	configv1 "github.com/openshift/api/config/v1"
+	"github.com/openshift/api/features"
+)
+
+// Add your new upstream feature gate here
+// const (
+// 		MyUpstreamFeature = "MyUpstreamFeature"
+// )
+
+type MapperInterface interface {
+	OperatorControllerUpstreamForDownstream(downstreamGate configv1.FeatureGateName) []string
+	OperatorControllerDownstreamFeatureGates() []configv1.FeatureGateName
+	CatalogdUpstreamForDownstream(downstreamGate configv1.FeatureGateName) []string
+	CatalogdDownstreamFeatureGates() []configv1.FeatureGateName
+}
+
+// Mapper knows the mapping between downstream and upstream feature gates for both OLM components
+type Mapper struct {
+	operatorControllerGates map[configv1.FeatureGateName][]string
+	catalogdGates           map[configv1.FeatureGateName][]string
+}
+
+func NewMapper() *Mapper {
+	// Add your downstream to upstream mapping here
+	operatorControllerGates := map[configv1.FeatureGateName][]string{
+		// features.FeatureGateNewOLMMyDownstreamFeature: {MyUpstreamControllerOperatorFeature}
+	}
+	catalogdGates := map[configv1.FeatureGateName][]string{
+		// features.FeatureGateNewOLMMyDownstreamFeature: {MyUpstreamCatalogdFeature}
+	}
+
+	for _, m := range []map[configv1.FeatureGateName][]string{operatorControllerGates, catalogdGates} {
+		for downstreamGate := range m {
+			// features.FeatureGateNewOLM is a GA-enabled downstream feature gate.
+			// If there is a need to enable upstream alpha/beta features in the downstream GA release
+			// get approval via a merged openshift/enhancement describing the need, then carve out
+			// an exception in this failsafe code
+			if downstreamGate == features.FeatureGateNewOLM {
+				panic(errors.New("FeatureGateNewOLM used in mappings"))
+			}
+			if !strings.HasPrefix(string(downstreamGate), string(features.FeatureGateNewOLM)) {
+				panic(errors.New("all downstream feature gates must use NewOLM prefix by convention"))
+			}
+		}
+	}
+
+	return &Mapper{operatorControllerGates: operatorControllerGates, catalogdGates: catalogdGates}
+}
+
+// OperatorControllerDownstreamFeatureGates returns a list of all downstream feature gates
+// which have an upstream mapping configured for the operator-controller component
+func (m *Mapper) OperatorControllerDownstreamFeatureGates() []configv1.FeatureGateName {
+	return getKeys(m.operatorControllerGates)
+}
+
+// CatalogdDownstreamFeatureGates returns a list of all downstream feature gates
+// which have an upstream mapping configured for the catalogd component
+func (m *Mapper) CatalogdDownstreamFeatureGates() []configv1.FeatureGateName {
+	return getKeys(m.catalogdGates)
+}
+
+// OperatorControllerUpstreamForDownstream returns upstream feature gates which are configured
+// for a given downstream feature gate for the operator-controller component
+func (m *Mapper) OperatorControllerUpstreamForDownstream(downstreamGate configv1.FeatureGateName) []string {
+	return m.operatorControllerGates[downstreamGate]
+}
+
+// CatalogdUpstreamForDownstream returns upstream feature gates which are configured
+// for a given downstream feature gate for the catalogd component
+func (m *Mapper) CatalogdUpstreamForDownstream(downstreamGate configv1.FeatureGateName) []string {
+	return m.catalogdGates[downstreamGate]
+}
+
+// FormatAsEnabledArgs combines list of feature gate names into
+// an all-enabled arg format of <feature_gate_name1>=true,<feature_gate_name1>=true etc.
+func FormatAsEnabledArgs(enabledFeatureGates []string) string {
+	buf := bytes.Buffer{}
+	for _, gateName := range enabledFeatureGates {
+		buf.WriteString(gateName)
+		buf.WriteRune('=')
+		buf.WriteString("true")
+		buf.WriteRune(',')
+	}
+	if buf.Len() > 0 {
+		// get rid of trailing ','
+		buf.Truncate(buf.Len() - 1)
+	}
+
+	return buf.String()
+}
+
+func getKeys(m map[configv1.FeatureGateName][]string) []configv1.FeatureGateName {
+	keys := make([]configv1.FeatureGateName, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
+	}
+	return keys
+}

internal/featuregates/mapper_test.go

@@ -0,0 +1,73 @@
+package featuregates
+
+import (
+	"slices"
+	"testing"
+
+	configv1 "github.com/openshift/api/config/v1"
+	"github.com/openshift/api/features"
+)
+
+func TestMapper_ControllerUpstreamForDownstream(t *testing.T) {
+	t.Run("returns mapped upstream gates for operator-controller", func(t *testing.T) {
+		expectedUpstreamGates := []string{"HelloGate", "WorldGate"}
+
+		mapper := NewMapper()
+		mapper.operatorControllerGates = map[configv1.FeatureGateName][]string{
+			features.FeatureGateNewOLM: expectedUpstreamGates,
+		}
+		upstream := mapper.OperatorControllerUpstreamForDownstream(features.FeatureGateNewOLM)
+		if !slices.Equal(upstream, expectedUpstreamGates) {
+			t.Fatalf("expected and returned upstream gates differ: upstream: %+v, expected: %+v",
+				upstream, expectedUpstreamGates,
+			)
+		}
+	})
+}
+
+func TestMapper_CatalogdUpstreamForDownstream(t *testing.T) {
+	t.Run("returns mapped upstream gates for catalogd", func(t *testing.T) {
+		expectedUpstreamGates := []string{"HelloGate", "WorldGate"}
+
+		mapper := NewMapper()
+		mapper.catalogdGates = map[configv1.FeatureGateName][]string{
+			features.FeatureGateNewOLM: expectedUpstreamGates,
+		}
+		upstream := mapper.CatalogdUpstreamForDownstream(features.FeatureGateNewOLM)
+		if !slices.Equal(upstream, expectedUpstreamGates) {
+			t.Fatalf("expected and returned upstream gates differ: upstream: %+v, expected: %+v",
+				upstream, expectedUpstreamGates,
+			)
+		}
+	})
+}
+
+func TestFormatAsEnabledArgs(t *testing.T) {
+	testCases := []struct {
+		name     string
+		in       []string
+		expected string
+	}{
+		{
+			name: "empty",
+		},
+		{
+			name:     "single feature gate",
+			in:       []string{"testGate1"},
+			expected: "testGate1=true",
+		},
+		{
+			name:     "multiple feature gates",
+			in:       []string{"testGate1", "testGate2", "testGate3"},
+			expected: "testGate1=true,testGate2=true,testGate2=true",
+		},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			result := FormatAsEnabledArgs(testCase.in)
+			if result != testCase.expected {
+				t.Fatalf("result and expected differ, expected: %q, got: %q", testCase.expected, result)
+			}
+		})
+	}
+}

manifests/0000_51_olm_02_operator_clusterrole.yaml

@@ -13,6 +13,8 @@ rules:
     resources:
       - infrastructures
       - proxies
+      - featuregates
+      - clusterversions
     verbs:
       - get
       - list

pkg/clients/clients.go

@@ -21,11 +21,16 @@ import (
 	operatorv1apply "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
 	operatorclient "github.com/openshift/client-go/operator/clientset/versioned"
 	operatorinformers "github.com/openshift/client-go/operator/informers/externalversions"
+	internalfeatures "github.com/openshift/cluster-olm-operator/internal/featuregates"
 	"github.com/openshift/library-go/pkg/apiserver/jsonpatch"
 	"github.com/openshift/library-go/pkg/controller/controllercmd"
+	"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
+	"github.com/openshift/library-go/pkg/operator/events"
 	"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
+	"github.com/openshift/library-go/pkg/operator/status"
 	"github.com/openshift/library-go/pkg/operator/v1helpers"
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
+	corev1 "k8s.io/api/core/v1"
 	apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -62,6 +67,8 @@ type Clients struct {
 	KubeInformerFactory        informers.SharedInformerFactory
 	ConfigInformerFactory      configinformer.SharedInformerFactory
 	KubeInformersForNamespaces v1helpers.KubeInformersForNamespaces
+	FeatureGatesAccessor       featuregates.FeatureGateAccess
+	FeatureGateMapper          internalfeatures.MapperInterface
 }
 
 func New(cc *controllercmd.ControllerContext) (*Clients, error) {
@@ -122,10 +129,13 @@ func New(cc *controllercmd.ControllerContext) (*Clients, error) {
 		ConfigClient:           configClient,
 		KubeInformerFactory:    informers.NewSharedInformerFactory(kubeClient, defaultResyncPeriod),
 		ConfigInformerFactory:  configInformerFactory,
+		FeatureGatesAccessor:   setupFeatureGatesAccessor(kubeClient, configInformerFactory, cc.OperatorNamespace),
+		FeatureGateMapper:      internalfeatures.NewMapper(),
 	}, nil
 }
 
 func (c *Clients) StartInformers(ctx context.Context) {
+	go c.FeatureGatesAccessor.Run(ctx)
 	c.KubeInformerFactory.Start(ctx.Done())
 	c.ConfigInformerFactory.Start(ctx.Done())
 	c.OperatorInformers.Start(ctx.Done())
@@ -148,6 +158,38 @@ func (c *Clients) ClientHolder() *resourceapply.ClientHolder {
 	return cl
 }
 
+func setupFeatureGatesAccessor(
+	kubeClient *kubernetes.Clientset,
+	configInformerFactory configinformer.SharedInformerFactory,
+	operatorNamespace string,
+) featuregates.FeatureGateAccess {
+	eventRecorder := events.NewKubeRecorder(
+		kubeClient.CoreV1().Events(operatorNamespace),
+		"cluster-olm-operator",
+		&corev1.ObjectReference{
+			APIVersion: "apps/v1",
+			Kind:       "Deployment",
+			Namespace:  operatorNamespace,
+			Name:       "cluster-olm-operator",
+		},
+	)
+
+	operatorImageVersion := status.VersionForOperatorFromEnv()
+	missingVersion := "0.0.1-snapshot"
+	featureGateAccessor := featuregates.NewFeatureGateAccess(
+		operatorImageVersion,
+		missingVersion,
+		configInformerFactory.Config().V1().ClusterVersions(), configInformerFactory.Config().V1().FeatureGates(),
+		eventRecorder,
+	)
+	// modify the default behavior of calling exit(0) to noop whenever a FeatureGates set changes in cluster
+	// reconsider this change if there ever comes a feature flag that affects the cluster-olm-operator directly
+	// see: https://github.com/openshift/cluster-olm-operator/pull/102#discussion_r1926861888
+	featureGateAccessor.SetChangeHandler(func(_ featuregates.FeatureChange) {})
+
+	return featureGateAccessor
+}
+
 var _ v1helpers.OperatorClientWithFinalizers = &OperatorClient{}
 
 const (

pkg/controller/builder.go

@@ -8,6 +8,7 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -117,6 +118,7 @@ func (b *Builder) BuildControllers(subDirectories ...string) (map[string]factory
 						replaceImageHook("${KUBE_RBAC_PROXY_IMAGE}", "KUBE_RBAC_PROXY_IMAGE"),
 					},
 					UpdateDeploymentProxyHook(b.Clients.ProxyClient),
+					UpdateDeploymentFeatureGatesHook(b.Clients.FeatureGatesAccessor, b.Clients.FeatureGateMapper),
 				)
 				return nil
 			}
@@ -219,19 +221,19 @@ func setContainerEnv(con *corev1.Container, envs []corev1.EnvVar) error {
 
 func UpdateDeploymentProxyHook(pc clients.ProxyClientInterface) deploymentcontroller.DeploymentHookFunc {
 	return func(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error {
-		klog.FromContext(context.Background()).WithName("builder").V(0).Info("Updating environment", "deployment", deployment.Name)
+		klog.FromContext(context.Background()).WithName("builder").V(0).Info("ProxyHook updating environment", "deployment", deployment.Name)
 		proxyConfig, err := pc.Get("cluster")
 		if err != nil {
 			return fmt.Errorf("error getting proxies.config.openshift.io/cluster: %w", err)
 		}
 
-		var errs []error
 		vars := []corev1.EnvVar{
 			{Name: HTTPSProxy, Value: proxyConfig.Status.HTTPSProxy},
 			{Name: HTTPProxy, Value: proxyConfig.Status.HTTPProxy},
 			{Name: NoProxy, Value: proxyConfig.Status.NoProxy},
 		}
 
+		var errs []error
 		for i := range deployment.Spec.Template.Spec.InitContainers {
 			err = setContainerEnv(&deployment.Spec.Template.Spec.InitContainers[i], vars)
 			if err != nil {
@@ -247,6 +249,22 @@ func UpdateDeploymentProxyHook(pc clients.ProxyClientInterface) deploymentcontro
 		if len(errs) > 0 {
 			return errors.Join(errs...)
 		}
+
+		return nil
+	}
+}
+
+func setContainerArg(con *corev1.Container, arg, value string) error {
+	if slices.ContainsFunc(con.Args, func(oldArg string) bool {
+		return strings.HasPrefix(oldArg, arg)
+	}) {
+		return fmt.Errorf("unexpected argument %q in container %q while building manifests", arg, con.Name)
+	}
+
+	if value == "" {
 		return nil
 	}
+	klog.FromContext(context.Background()).WithName("builder").V(4).Info("Updated args", "container", con.Name, "arg", arg, "value", value)
+	con.Args = append(con.Args, fmt.Sprintf("%s=%s", arg, value))
+	return nil
 }