poc(ocpbugs-30080): read OCP version from payload metadada

Author: petr-muller

pkg/cvo/cvo.go

@@ -175,6 +175,7 @@ type Operator struct {
 	exclude string
 
 	enabledFeatureGates featuregates.CvoGateChecker
+	requiredFeatureSet  configv1.FeatureSet
 
 	clusterProfile string
 	uid            types.UID
@@ -210,6 +211,8 @@ func New(
 	injectClusterIdIntoPromQL bool,
 	updateService string,
 	alwaysEnableCapabilities []configv1.ClusterVersionCapability,
+	startingFeatureSet configv1.FeatureSet,
+	cvoGates featuregates.CvoGateChecker,
 ) (*Operator, error) {
 	eventBroadcaster := record.NewBroadcaster()
 	eventBroadcaster.StartLogging(klog.Infof)
@@ -246,7 +249,9 @@ func New(
 		// Because of OCPBUGS-30080, we can only detect the enabled feature gates after Operator loads the initial payload
 		// from disk via LoadInitialPayload. We must not have any gate-checking code until that happens, so we initialize
 		// this field with a checker that panics when used.
-		enabledFeatureGates:      featuregates.PanicOnUsageBeforeInitialization,
+		enabledFeatureGates: cvoGates,
+		requiredFeatureSet:  startingFeatureSet,
+
 		alwaysEnableCapabilities: alwaysEnableCapabilities,
 	}
 
@@ -283,7 +288,7 @@ func New(
 
 // LoadInitialPayload waits until a ClusterVersion object exists. It then retrieves the payload contents, verifies the
 // initial state and returns it. If the payload is invalid, an error is returned.
-func (optr *Operator) LoadInitialPayload(ctx context.Context, startingRequiredFeatureSet configv1.FeatureSet, restConfig *rest.Config) (*payload.Update, error) {
+func (optr *Operator) LoadInitialPayload(ctx context.Context, restConfig, burstRestConfig *rest.Config) error {
 
 	// wait until cluster version object exists
 	if err := wait.PollUntilContextCancel(ctx, 3*time.Second, true, func(ctx context.Context) (bool, error) {
@@ -300,19 +305,19 @@ func (optr *Operator) LoadInitialPayload(ctx context.Context, startingRequiredFe
 		}
 		return true, nil
 	}); err != nil {
-		return nil, fmt.Errorf("Error when attempting to get cluster version object: %w", err)
+		return fmt.Errorf("Error when attempting to get cluster version object: %w", err)
 	}
 
-	update, err := payload.LoadUpdate(optr.defaultPayloadDir(), optr.release.Image, optr.exclude, string(startingRequiredFeatureSet),
+	update, err := payload.LoadUpdate(optr.defaultPayloadDir(), optr.release.Image, optr.exclude, string(optr.requiredFeatureSet),
 		optr.clusterProfile, configv1.KnownClusterVersionCapabilities)
 
 	if err != nil {
-		return nil, fmt.Errorf("the local release contents are invalid - no current version can be determined from disk: %v", err)
+		return fmt.Errorf("the local release contents are invalid - no current version can be determined from disk: %v", err)
 	}
 	httpClientConstructor := sigstore.NewCachedHTTPClientConstructor(optr.HTTPClient, nil)
 	configClient, err := coreclientsetv1.NewForConfig(restConfig)
 	if err != nil {
-		return nil, fmt.Errorf("unable to create a configuration client: %v", err)
+		return fmt.Errorf("unable to create a configuration client: %v", err)
 	}
 
 	customSignatureStore := &customsignaturestore.Store{
@@ -324,7 +329,7 @@ func (optr *Operator) LoadInitialPayload(ctx context.Context, startingRequiredFe
 	// attempt to load a verifier as defined in the payload
 	verifier, signatureStore, err := loadConfigMapVerifierDataFromUpdate(update, httpClientConstructor.HTTPClient, configClient, customSignatureStore)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	if verifier != nil {
 		klog.Infof("Verifying release authenticity: %v", verifier)
@@ -334,13 +339,6 @@ func (optr *Operator) LoadInitialPayload(ctx context.Context, startingRequiredFe
 	}
 	optr.verifier = verifier
 	optr.signatureStore = signatureStore
-	return update, nil
-}
-
-// InitializeFromPayload configures the controller that loads and applies content to the cluster given an initial payload
-// and feature gate data.
-func (optr *Operator) InitializeFromPayload(update *payload.Update, requiredFeatureSet configv1.FeatureSet, cvoFlags featuregates.CvoGateChecker, restConfig *rest.Config, burstRestConfig *rest.Config) {
-	optr.enabledFeatureGates = cvoFlags
 	optr.release = update.Release
 	optr.releaseCreated = update.ImageRef.CreationTimestamp.Time
 
@@ -358,11 +356,13 @@ func (optr *Operator) InitializeFromPayload(update *payload.Update, requiredFeat
 			Cap:      time.Second * 15,
 		},
 		optr.exclude,
-		requiredFeatureSet,
+		optr.requiredFeatureSet,
 		optr.eventRecorder,
 		optr.clusterProfile,
 		optr.alwaysEnableCapabilities,
 	)
+
+	return nil
 }
 
 // ownerReferenceModifier sets the owner reference to the current CV resource if no other reference exists. It also resets

pkg/featuregates/featuregates.go

@@ -35,36 +35,6 @@ type CvoGateChecker interface {
 	CVOConfiguration() bool
 }
 
-type panicOnUsageBeforeInitializationFunc func()
-
-func panicOnUsageBeforeInitialization() {
-	panic("CVO feature flags were used before they were initialized")
-}
-
-// PanicOnUsageBeforeInitialization is a CvoGateChecker that panics if any of its methods are called. This checker should
-// be used before CVO feature gates are actually known and some code tries to check them.
-var PanicOnUsageBeforeInitialization = panicOnUsageBeforeInitializationFunc(panicOnUsageBeforeInitialization)
-
-func (p panicOnUsageBeforeInitializationFunc) ReconciliationIssuesCondition() bool {
-	p()
-	return false
-}
-
-func (p panicOnUsageBeforeInitializationFunc) StatusReleaseArchitecture() bool {
-	p()
-	return false
-}
-
-func (p panicOnUsageBeforeInitializationFunc) UnknownVersion() bool {
-	p()
-	return false
-}
-
-func (p panicOnUsageBeforeInitializationFunc) CVOConfiguration() bool {
-	p()
-	return false
-}
-
 // CvoGates contains flags that control CVO functionality gated by product feature gates. The
 // names do not correspond to product feature gates, the booleans here are "smaller" (product-level
 // gate will enable multiple CVO behaviors).

pkg/payload/payload.go

@@ -141,11 +141,23 @@ type metadata struct {
 func LoadUpdate(dir, releaseImage, excludeIdentifier string, requiredFeatureSet string, profile string,
 	knownCapabilities []configv1.ClusterVersionCapability) (*Update, error) {
 
-	payload, tasks, err := loadUpdatePayloadMetadata(dir, releaseImage, profile)
+	klog.V(2).Infof("Loading updatepayload from %q", dir)
+	if err := ValidateDirectory(dir); err != nil {
+		return nil, err
+	}
+
+	var (
+		releaseDir = filepath.Join(dir, ReleaseManifestDir)
+		cvoDir     = filepath.Join(dir, CVOManifestDir)
+	)
+
+	payload, err := loadMetadata(releaseDir, releaseImage)
 	if err != nil {
 		return nil, err
 	}
 
+	tasks := getPayloadTasks(releaseDir, cvoDir, releaseImage, profile)
+
 	var onlyKnownCaps *configv1.ClusterVersionCapabilitiesStatus
 
 	if knownCapabilities != nil {
@@ -299,38 +311,27 @@ type payloadTasks struct {
 	skipFiles  sets.Set[string]
 }
 
-func loadUpdatePayloadMetadata(dir, releaseImage, clusterProfile string) (*Update, []payloadTasks, error) {
-	klog.V(2).Infof("Loading updatepayload from %q", dir)
-	if err := ValidateDirectory(dir); err != nil {
-		return nil, nil, err
-	}
-	var (
-		cvoDir     = filepath.Join(dir, CVOManifestDir)
-		releaseDir = filepath.Join(dir, ReleaseManifestDir)
-	)
-
-	release, arch, err := loadReleaseFromMetadata(releaseDir)
+func loadMetadata(releaseDir, releaseImage string) (*Update, error) {
+	release, arch, err := LoadReleaseFromMetadata(releaseDir)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 	release.Image = releaseImage
 
 	imageRef, err := loadImageReferences(releaseDir)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	if imageRef.Name != release.Version {
-		return nil, nil, fmt.Errorf("Version from %s (%s) differs from %s (%s)", imageReferencesFile, imageRef.Name, cincinnatiJSONFile, release.Version)
+		return nil, fmt.Errorf("Version from %s (%s) differs from %s (%s)", imageReferencesFile, imageRef.Name, cincinnatiJSONFile, release.Version)
 	}
 
-	tasks := getPayloadTasks(releaseDir, cvoDir, releaseImage, clusterProfile)
-
 	return &Update{
 		Release:      release,
 		ImageRef:     imageRef,
 		Architecture: arch,
-	}, tasks, nil
+	}, nil
 }
 
 func getPayloadTasks(releaseDir, cvoDir, releaseImage, clusterProfile string) []payloadTasks {
@@ -353,7 +354,7 @@ func getPayloadTasks(releaseDir, cvoDir, releaseImage, clusterProfile string) []
 	}}
 }
 
-func loadReleaseFromMetadata(releaseDir string) (configv1.Release, string, error) {
+func LoadReleaseFromMetadata(releaseDir string) (configv1.Release, string, error) {
 	var release configv1.Release
 	path := filepath.Join(releaseDir, cincinnatiJSONFile)
 	data, err := os.ReadFile(path)