Merge pull request #1190 from petr-muller/ota-1531-05-load-gates-early

OTA-1531: [5/x] cvo: read cluster `FeatureGate` early

Author: openshift-merge-bot[bot]

pkg/featuregates/featuregates.go

@@ -5,6 +5,12 @@ import (
 	"github.com/openshift/api/features"
 )
 
+// StubOpenShiftVersion is the default OpenShift version placeholder for the purpose of determining
+// enabled and disabled CVO feature gates. It is assumed to never conflict with a real OpenShift
+// version. Both DefaultCvoGates and CvoGatesFromFeatureGate should return a safe conservative
+// default set of enabled and disabled gates, typically with unknownVersion set to true.
+const StubOpenShiftVersion = "0.0.1-snapshot"
+
 // CvoGateChecker allows CVO code to check which feature gates are enabled
 type CvoGateChecker interface {
 	// UnknownVersion flag is set to true if CVO did not find a matching version in the FeatureGate

pkg/start/start.go

@@ -189,32 +189,11 @@ func (o *Options) Run(ctx context.Context) error {
 		return err
 	}
 
-	payloadRoot := payload.DefaultRootPath
-	if o.PayloadOverride != "" {
-		payloadRoot = payload.RootPath(o.PayloadOverride)
-	}
-
-	cvoOcpVersion := "0.0.1-snapshot"
-	// Peek at the local release metadata to determine the version of OCP this CVO belongs to. This assumes the CVO is
-	// executing in a container from the payload image. Full payload content is only read later once leader lease is
-	// acquired, and here we should only read as little data as possible to determine the version so we can establish
-	// enabled feature gate checker for all following code.
-	//
-	// We cannot refuse to start CVO if for some reason we cannot determine the OCP version on startup from the local
-	// release metadata. The only consequence is we fail to determine enabled/disabled feature gates and will have to use
-	// some defaults.
-	releaseMetadata, err := payloadRoot.LoadReleaseMetadata()
-	switch {
-	case err != nil:
-		klog.Warningf("Failed to read release metadata to determine OCP version for this CVO (will use placeholder version %q): %v", cvoOcpVersion, err)
-	case releaseMetadata.Version == "":
-		klog.Warningf("Version missing from release metadata, cannot determine OCP version for this CVO (will use placeholder version %q)", cvoOcpVersion)
-	default:
-		cvoOcpVersion = releaseMetadata.Version
-		klog.Infof("Determined OCP version for this CVO: %q", cvoOcpVersion)
-	}
-
 	clusterVersionConfigInformerFactory, configInformerFactory := o.prepareConfigInformerFactories(cb)
+	_, _, err = o.processInitialFeatureGate(ctx, configInformerFactory)
+	if err != nil {
+		return fmt.Errorf("error processing feature gates: %w", err)
+	}
 
 	// initialize the controllers and attempt to load the payload information
 	controllerCtx, err := o.NewControllerContext(cb, clusterVersionConfigInformerFactory, configInformerFactory)
@@ -237,6 +216,87 @@ func (o *Options) prepareConfigInformerFactories(cb *ClientBuilder) (configinfor
 	return clusterVersionConfigInformerFactory, configInformerFactory
 }
 
+// getOpenShiftVersion peeks at the local release metadata to determine the version of OpenShift this CVO belongs to.
+// This assumes the CVO is executing in a container from the payload image. This does not and should not fully load
+// whole payload content, that is only loaded later once leader lease is acquired. Here we should only read as little
+// data as possible to determine the version so we can establish enabled feature gate checker for all following code.
+func (o *Options) getOpenShiftVersion() string {
+	payloadRoot := payload.DefaultRootPath
+	if o.PayloadOverride != "" {
+		payloadRoot = payload.RootPath(o.PayloadOverride)
+	}
+
+	// We cannot refuse to start CVO if for some reason we cannot determine the OpenShift version on startup from the local
+	// release metadata. The only consequence is we fail to determine enabled/disabled feature gates and will have to use
+	// some defaults.
+	releaseMetadata, err := payloadRoot.LoadReleaseMetadata()
+	if err != nil {
+		klog.Warningf("Failed to read release metadata to determine OpenShift version for this CVO (will use placeholder version %q): %v", featuregates.StubOpenShiftVersion, err)
+		return featuregates.StubOpenShiftVersion
+	}
+
+	if releaseMetadata.Version == "" {
+		klog.Warningf("Version missing from release metadata, cannot determine OpenShift version for this CVO (will use placeholder version %q)", featuregates.StubOpenShiftVersion)
+		return featuregates.StubOpenShiftVersion
+	}
+
+	klog.Infof("Determined OpenShift version for this CVO: %q", releaseMetadata.Version)
+	return releaseMetadata.Version
+}
+
+func (o *Options) processInitialFeatureGate(ctx context.Context, configInformerFactory configinformers.SharedInformerFactory) (configv1.FeatureSet, *featuregates.CvoGates, error) {
+	featureGates := configInformerFactory.Config().V1().FeatureGates().Lister()
+	configInformerFactory.Start(ctx.Done())
+	configInformerFactory.WaitForCacheSync(ctx.Done())
+
+	cvoOpenShiftVersion := o.getOpenShiftVersion()
+	cvoGates := featuregates.DefaultCvoGates(cvoOpenShiftVersion)
+
+	var startingFeatureSet configv1.FeatureSet
+	var clusterFeatureGate *configv1.FeatureGate
+
+	// client-go automatically retries some network blip errors on GETs for 30s by default, and we want to
+	// retry the remaining ones ourselves. If we fail longer than that, the operator won't be able to do work
+	// anyway. Return the error and crashloop.
+	//
+	// We implement the timeout with a context because the timeout in PollImmediateWithContext does not behave
+	// well when ConditionFunc takes longer time to execute, like here where the GET can be retried by client-go
+	var lastError error
+	if err := wait.PollUntilContextTimeout(context.Background(), 2*time.Second, 25*time.Second, true, func(ctx context.Context) (bool, error) {
+		gate, fgErr := featureGates.Get("cluster")
+		switch {
+		case apierrors.IsNotFound(fgErr):
+			// if we have no featuregates, then the cluster is using the default featureset, which is "".
+			// This excludes everything that could possibly depend on a different feature set.
+			startingFeatureSet = ""
+			klog.Infof("FeatureGate not found in cluster, will assume default feature set %q at startup", startingFeatureSet)
+			return true, nil
+		case fgErr != nil:
+			lastError = fgErr
+			klog.Warningf("Failed to get FeatureGate from cluster: %v", fgErr)
+			return false, nil
+		default:
+			clusterFeatureGate = gate
+			startingFeatureSet = gate.Spec.FeatureSet
+			cvoGates = featuregates.CvoGatesFromFeatureGate(clusterFeatureGate, cvoOpenShiftVersion)
+			klog.Infof("FeatureGate found in cluster, using its feature set %q at startup", startingFeatureSet)
+			return true, nil
+		}
+	}); err != nil {
+		if lastError != nil {
+			return "", nil, lastError
+		}
+		return "", nil, err
+	}
+
+	if cvoGates.UnknownVersion() {
+		klog.Warningf("CVO features for version %s could not be detected from FeatureGate; will use defaults plus special UnknownVersion feature gate", cvoOpenShiftVersion)
+	}
+	klog.Infof("CVO features for version %s enabled at startup: %+v", cvoOpenShiftVersion, cvoGates)
+
+	return startingFeatureSet, &cvoGates, nil
+}
+
 // run launches a number of goroutines to handle manifest application,
 // metrics serving, etc.  It continues operating until ctx.Done(),
 // and then attempts a clean shutdown limited by an internal context