allow more than one featureset

go.mod

@@ -10,21 +10,21 @@ require (
 	github.com/google/uuid v1.1.2
 	github.com/imdario/mergo v0.3.8 // indirect
 	github.com/openshift/api v0.0.0-20220811160310-427c7cc57280
-	github.com/openshift/client-go v0.0.0-20220504114320-6aec01bb0754
-	github.com/openshift/library-go v0.0.0-20220523142556-5bcfed822fc6
+	github.com/openshift/client-go v0.0.0-20220525160904-9e1acff93e4a
+	github.com/openshift/library-go v0.0.0-20220823205642-e2ef049de560
 	github.com/pkg/errors v0.9.1
-	github.com/prometheus/client_golang v1.11.1
+	github.com/prometheus/client_golang v1.12.1
 	github.com/prometheus/client_model v0.2.0
-	github.com/prometheus/common v0.28.0
-	github.com/spf13/cobra v1.2.1
+	github.com/prometheus/common v0.32.1
+	github.com/spf13/cobra v1.4.0
 	golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd
 	golang.org/x/time v0.0.0-20220210224613-90d013bbcef8
 	gopkg.in/fsnotify.v1 v1.4.7
 	k8s.io/api v0.24.0
-	k8s.io/apiextensions-apiserver v0.23.0
+	k8s.io/apiextensions-apiserver v0.24.0
 	k8s.io/apimachinery v0.24.0
 	k8s.io/client-go v0.24.0
 	k8s.io/klog/v2 v2.60.1
-	k8s.io/kube-aggregator v0.23.0
+	k8s.io/kube-aggregator v0.24.0
 	k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9
 )

hack/cluster-version-util/task_graph.go

@@ -30,7 +30,7 @@ func newTaskGraphCmd() *cobra.Command {
 
 func runTaskGraphCmd(cmd *cobra.Command, args []string) error {
 	manifestDir := args[0]
-	release, err := payload.LoadUpdate(manifestDir, "", "", false, payload.DefaultClusterProfile, nil)
+	release, err := payload.LoadUpdate(manifestDir, "", "", "", payload.DefaultClusterProfile, nil)
 	if err != nil {
 		return err
 	}

pkg/cvo/cvo.go

@@ -145,9 +145,9 @@ type Operator struct {
 	// via annotation
 	exclude string
 
-	// includeTechPreview is set to true when the CVO should create resources with the `release.openshift.io/feature-gate=TechPreviewNoUpgrade`
-	// label set.  This is set based on whether the featuregates.config.openshift.io|.spec.featureSet is set to "TechPreviewNoUpgrade".
-	includeTechPreview bool
+	// requiredFeatureSet is set the value of featuregates.config.openshift.io|.spec.featureSet.  It's a very slow
+	// moving resource, so it is not re-detected live.
+	requiredFeatureSet string
 
 	clusterProfile string
 	uid            types.UID
@@ -168,7 +168,7 @@ func New(
 	client clientset.Interface,
 	kubeClient kubernetes.Interface,
 	exclude string,
-	includeTechPreview bool,
+	requiredFeatureSet string,
 	clusterProfile string,
 ) *Operator {
 	eventBroadcaster := record.NewBroadcaster()
@@ -197,7 +197,7 @@ func New(
 		upgradeableQueue:      workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "upgradeable"),
 
 		exclude:            exclude,
-		includeTechPreview: includeTechPreview,
+		requiredFeatureSet: requiredFeatureSet,
 		clusterProfile:     clusterProfile,
 	}
 
@@ -245,7 +245,7 @@ func (optr *Operator) InitializeFromPayload(ctx context.Context, restConfig *res
 		return fmt.Errorf("Error when attempting to get cluster version object: %w", err)
 	}
 
-	update, err := payload.LoadUpdate(optr.defaultPayloadDir(), optr.release.Image, optr.exclude, optr.includeTechPreview,
+	update, err := payload.LoadUpdate(optr.defaultPayloadDir(), optr.release.Image, optr.exclude, optr.requiredFeatureSet,
 		optr.clusterProfile, capability.GetKnownCapabilities())
 
 	if err != nil {
@@ -290,7 +290,7 @@ func (optr *Operator) InitializeFromPayload(ctx context.Context, restConfig *res
 			Cap:      time.Second * 15,
 		},
 		optr.exclude,
-		optr.includeTechPreview,
+		optr.requiredFeatureSet,
 		optr.eventRecorder,
 		optr.clusterProfile,
 	)

pkg/cvo/cvo_scenarios_test.go

@@ -125,7 +125,7 @@ func setupCVOTest(payloadDir string) (*Operator, map[string]apiruntime.Object, *
 			Steps: 1,
 		},
 		"exclude-test",
-		false,
+		"",
 		record.NewFakeRecorder(100),
 		o.clusterProfile,
 	)

pkg/cvo/sync_worker.go

@@ -184,15 +184,16 @@ type SyncWorker struct {
 	// of the form exclude.release.openshift.io/<identifier>=true
 	exclude string
 
-	// includeTechPreview is set to true when the CVO should create resources with the `release.openshift.io/feature-gate=TechPreviewNoUpgrade`
-	includeTechPreview bool
+	// requiredFeatureSet is set to the value of Feature.config.openshift.io|spec.featureSet, which contributes to
+	// whether or not some manifests are included for reconciliation.
+	requiredFeatureSet string
 
 	clusterProfile string
 }
 
 // NewSyncWorker initializes a ConfigSyncWorker that will retrieve payloads to disk, apply them via builder
 // to a server, and obey limits about how often to reconcile or retry on errors.
-func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, includeTechPreview bool, eventRecorder record.EventRecorder, clusterProfile string) *SyncWorker {
+func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, requiredFeatureSet string, eventRecorder record.EventRecorder, clusterProfile string) *SyncWorker {
 	return &SyncWorker{
 		retriever:     retriever,
 		builder:       builder,
@@ -208,7 +209,7 @@ func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder,
 		report: make(chan SyncWorkerStatus, 500),
 
 		exclude:            exclude,
-		includeTechPreview: includeTechPreview,
+		requiredFeatureSet: requiredFeatureSet,
 
 		clusterProfile: clusterProfile,
 	}
@@ -217,8 +218,8 @@ func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder,
 // NewSyncWorkerWithPreconditions initializes a ConfigSyncWorker that will retrieve payloads to disk, apply them via builder
 // to a server, and obey limits about how often to reconcile or retry on errors.
 // It allows providing preconditions for loading payload.
-func NewSyncWorkerWithPreconditions(retriever PayloadRetriever, builder payload.ResourceBuilder, preconditions precondition.List, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, includeTechPreview bool, eventRecorder record.EventRecorder, clusterProfile string) *SyncWorker {
-	worker := NewSyncWorker(retriever, builder, reconcileInterval, backoff, exclude, includeTechPreview, eventRecorder, clusterProfile)
+func NewSyncWorkerWithPreconditions(retriever PayloadRetriever, builder payload.ResourceBuilder, preconditions precondition.List, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, requiredFeatureSet string, eventRecorder record.EventRecorder, clusterProfile string) *SyncWorker {
+	worker := NewSyncWorker(retriever, builder, reconcileInterval, backoff, exclude, requiredFeatureSet, eventRecorder, clusterProfile)
 	worker.preconditions = preconditions
 	return worker
 }
@@ -314,7 +315,7 @@ func (w *SyncWorker) syncPayload(ctx context.Context, work *SyncWork,
 		}
 
 		w.eventRecorder.Eventf(cvoObjectRef, corev1.EventTypeNormal, "LoadPayload", "Loading payload version=%q image=%q", desired.Version, desired.Image)
-		payloadUpdate, err := payload.LoadUpdate(info.Directory, desired.Image, w.exclude, w.includeTechPreview, w.clusterProfile,
+		payloadUpdate, err := payload.LoadUpdate(info.Directory, desired.Image, w.exclude, w.requiredFeatureSet, w.clusterProfile,
 			capability.GetKnownCapabilities())
 		if err != nil {
 			msg := fmt.Sprintf("Loading payload failed version=%q image=%q failure=%v", desired.Version, desired.Image, err)

pkg/featurechangestopper/techpreviewchangestopper.go → pkg/featurechangestopper/featurechangestopper.go

@@ -17,10 +17,9 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// TechPreviewChangeStopper calls stop when the value of the featuregate changes from TechPreviewNoUpgrade to anything else
-// or from anything to TechPreviewNoUpgrade.
-type TechPreviewChangeStopper struct {
-	startingTechPreviewState bool
+// FeatureChangeStopper calls stop when the value of the featureset changes
+type FeatureChangeStopper struct {
+	startingRequiredFeatureSet string
 
 	featureGateLister configlistersv1.FeatureGateLister
 	cacheSynced       []cache.InformerSynced
@@ -29,19 +28,19 @@ type TechPreviewChangeStopper struct {
 	shutdownFn context.CancelFunc
 }
 
-// New returns a new TechPreviewChangeStopper.
+// New returns a new FeatureChangeStopper.
 func New(
-	startingTechPreviewState bool,
+	startingRequiredFeatureSet string,
 	featureGateInformer configinformersv1.FeatureGateInformer,
-) *TechPreviewChangeStopper {
-	c := &TechPreviewChangeStopper{
-		startingTechPreviewState: startingTechPreviewState,
-		featureGateLister:        featureGateInformer.Lister(),
-		cacheSynced:              []cache.InformerSynced{featureGateInformer.Informer().HasSynced},
-		queue:                    workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "feature-gate-stopper"),
+) *FeatureChangeStopper {
+	c := &FeatureChangeStopper{
+		startingRequiredFeatureSet: startingRequiredFeatureSet,
+		featureGateLister:          featureGateInformer.Lister(),
+		cacheSynced:                []cache.InformerSynced{featureGateInformer.Informer().HasSynced},
+		queue:                      workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "feature-gate-stopper"),
 	}
 
-	c.queue.Add("cluster") // seed an initial sync, in case startingTechPreviewState is wrong
+	c.queue.Add("cluster") // seed an initial sync, in case startingRequiredFeatureSet is wrong
 	featureGateInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc: func(obj interface{}) {
 			c.queue.Add("cluster")
@@ -60,22 +59,22 @@ func New(
 // syncHandler processes a single work entry, with the
 // processNextWorkItem caller handling the queue management.  It returns
 // done when there will be no more work (because the feature gate changed).
-func (c *TechPreviewChangeStopper) syncHandler(ctx context.Context) (done bool, err error) {
+func (c *FeatureChangeStopper) syncHandler(ctx context.Context) (done bool, err error) {
 	var current configv1.FeatureSet
 	if featureGates, err := c.featureGateLister.Get("cluster"); err == nil {
 		current = featureGates.Spec.FeatureSet
 	} else if !apierrors.IsNotFound(err) {
 		return false, err
 	}
-	techPreviewNowSet := current == configv1.TechPreviewNoUpgrade
-	if techPreviewNowSet != c.startingTechPreviewState {
+
+	if string(current) != c.startingRequiredFeatureSet {
 		var action string
 		if c.shutdownFn == nil {
 			action = "no shutdown function configured"
 		} else {
 			action = "requesting shutdown"
 		}
-		klog.Infof("TechPreviewNoUpgrade was %t, but the current feature set is %q; %s.", c.startingTechPreviewState, current, action)
+		klog.Infof("FeatureSet was %q, but the current feature set is %q; %s.", c.startingRequiredFeatureSet, current, action)
 		if c.shutdownFn != nil {
 			c.shutdownFn()
 		}
@@ -85,7 +84,7 @@ func (c *TechPreviewChangeStopper) syncHandler(ctx context.Context) (done bool,
 }
 
 // Run launches the controller and blocks until it is canceled or work completes.
-func (c *TechPreviewChangeStopper) Run(ctx context.Context, shutdownFn context.CancelFunc) error {
+func (c *FeatureChangeStopper) Run(ctx context.Context, shutdownFn context.CancelFunc) error {
 	// don't let panics crash the process
 	defer utilruntime.HandleCrash()
 	// make sure the work queue is shutdown which will trigger workers to end
@@ -98,22 +97,22 @@ func (c *TechPreviewChangeStopper) Run(ctx context.Context, shutdownFn context.C
 	}()
 	c.shutdownFn = shutdownFn
 
-	klog.Infof("Starting stop-on-techpreview-change controller with %s %t.", configv1.TechPreviewNoUpgrade, c.startingTechPreviewState)
+	klog.Infof("Starting stop-on-featureset-change controller with %q.", c.startingRequiredFeatureSet)
 
 	// wait for your secondary caches to fill before starting your work
 	if !cache.WaitForCacheSync(ctx.Done(), c.cacheSynced...) {
 		return errors.New("feature gate cache failed to sync")
 	}
 
 	err := wait.PollImmediateUntilWithContext(ctx, 30*time.Second, c.runWorker)
-	klog.Info("Shutting down stop-on-techpreview-change controller")
+	klog.Info("Shutting down stop-on-featureset-change controller")
 	return err
 }
 
 // runWorker handles a single worker poll round, processing as many
 // work items as possible, and returning done when there will be no
 // more work.
-func (c *TechPreviewChangeStopper) runWorker(ctx context.Context) (done bool, err error) {
+func (c *FeatureChangeStopper) runWorker(ctx context.Context) (done bool, err error) {
 	// hot loop until we're told to stop.  processNextWorkItem will
 	// automatically wait until there's work available, so we don't worry
 	// about secondary waits
@@ -126,7 +125,7 @@ func (c *TechPreviewChangeStopper) runWorker(ctx context.Context) (done bool, er
 
 // processNextWorkItem deals with one key off the queue.  It returns
 // done when there will be no more work.
-func (c *TechPreviewChangeStopper) processNextWorkItem(ctx context.Context) (done bool, err error) {
+func (c *FeatureChangeStopper) processNextWorkItem(ctx context.Context) (done bool, err error) {
 	// pull the next work item from queue.  It should be a key we use to lookup
 	// something in a cache
 	key, quit := c.queue.Get()

pkg/featurechangestopper/techpreviewchangestopper_test.go → pkg/featurechangestopper/featurechangestopper_test.go

@@ -13,40 +13,40 @@ import (
 
 func TestTechPreviewChangeStopper(t *testing.T) {
 	tests := []struct {
-		name                     string
-		startingTechPreviewState bool
-		featureGate              string
-		expectedShutdownCalled   bool
+		name                       string
+		startingRequiredFeatureSet string
+		featureGate                string
+		expectedShutdownCalled     bool
 	}{
 		{
-			name:                     "default-no-change",
-			startingTechPreviewState: false,
-			featureGate:              "",
-			expectedShutdownCalled:   false,
+			name:                       "default-no-change",
+			startingRequiredFeatureSet: "",
+			featureGate:                "",
+			expectedShutdownCalled:     false,
 		},
 		{
-			name:                     "default-with-change-to-tech-preview",
-			startingTechPreviewState: false,
-			featureGate:              "TechPreviewNoUpgrade",
-			expectedShutdownCalled:   true,
+			name:                       "default-with-change-to-tech-preview",
+			startingRequiredFeatureSet: "",
+			featureGate:                "TechPreviewNoUpgrade",
+			expectedShutdownCalled:     true,
 		},
 		{
-			name:                     "default-with-change-to-other",
-			startingTechPreviewState: false,
-			featureGate:              "AnythingElse",
-			expectedShutdownCalled:   false,
+			name:                       "default-with-change-to-other",
+			startingRequiredFeatureSet: "",
+			featureGate:                "AnythingElse",
+			expectedShutdownCalled:     true,
 		},
 		{
-			name:                     "techpreview-to-techpreview",
-			startingTechPreviewState: true,
-			featureGate:              "TechPreviewNoUpgrade",
-			expectedShutdownCalled:   false,
+			name:                       "techpreview-to-techpreview",
+			startingRequiredFeatureSet: "TechPreviewNoUpgrade",
+			featureGate:                "TechPreviewNoUpgrade",
+			expectedShutdownCalled:     false,
 		},
 		{
-			name:                     "techpreview-to-not-tech-preview", // this isn't allowed today
-			startingTechPreviewState: true,
-			featureGate:              "",
-			expectedShutdownCalled:   true,
+			name:                       "techpreview-to-not-tech-preview", // this isn't allowed today
+			startingRequiredFeatureSet: "TechPreviewNoUpgrade",
+			featureGate:                "",
+			expectedShutdownCalled:     true,
 		},
 	}
 	for _, tt := range tests {
@@ -72,7 +72,7 @@ func TestTechPreviewChangeStopper(t *testing.T) {
 
 			informerFactory := configv1informer.NewSharedInformerFactory(client, 0)
 			featureGates := informerFactory.Config().V1().FeatureGates()
-			c := New(tt.startingTechPreviewState, featureGates)
+			c := New(tt.startingRequiredFeatureSet, featureGates)
 			informerFactory.Start(ctx.Done())
 
 			if err := c.Run(ctx, shutdownFn); err != nil {

pkg/payload/payload.go

@@ -139,7 +139,7 @@ type metadata struct {
 	Metadata map[string]interface{}
 }
 
-func LoadUpdate(dir, releaseImage, excludeIdentifier string, includeTechPreview bool, profile string,
+func LoadUpdate(dir, releaseImage, excludeIdentifier string, requiredFeatureSet string, profile string,
 	knownCapabilities []configv1.ClusterVersionCapability) (*Update, error) {
 
 	payload, tasks, err := loadUpdatePayloadMetadata(dir, releaseImage, profile)
@@ -206,7 +206,7 @@ func LoadUpdate(dir, releaseImage, excludeIdentifier string, includeTechPreview
 			// Filter out manifests that should be excluded based on annotation
 			filteredMs := []manifest.Manifest{}
 			for _, manifest := range ms {
-				if err := manifest.Include(&excludeIdentifier, &includeTechPreview, &profile, onlyKnownCaps); err != nil {
+				if err := manifest.Include(&excludeIdentifier, &requiredFeatureSet, &profile, onlyKnownCaps); err != nil {
 					klog.V(2).Infof("excluding %s group=%s kind=%s namespace=%s name=%s: %v\n", manifest.OriginalFilename, manifest.GVK.Group, manifest.GVK.Kind, manifest.Obj.GetNamespace(), manifest.Obj.GetName(), err)
 					continue
 				}

pkg/payload/payload_test.go

@@ -129,7 +129,7 @@ func TestLoadUpdate(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := LoadUpdate(tt.args.dir, tt.args.releaseImage, "exclude-test", false, DefaultClusterProfile, nil)
+			got, err := LoadUpdate(tt.args.dir, tt.args.releaseImage, "exclude-test", "", DefaultClusterProfile, nil)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("loadUpdatePayload() error = %v, wantErr %v", err, tt.wantErr)
 				return

pkg/payload/task_graph_test.go

@@ -487,7 +487,7 @@ func Test_TaskGraph_real(t *testing.T) {
 	if len(path) == 0 {
 		t.Skip("TEST_GRAPH_PATH unset")
 	}
-	p, err := LoadUpdate(path, "arbitrary/image:1", "", false, DefaultClusterProfile, nil)
+	p, err := LoadUpdate(path, "arbitrary/image:1", "", "", DefaultClusterProfile, nil)
 	if err != nil {
 		t.Fatal(err)
 	}