Skip to content

Commit

Permalink
test: create informers and listers first
Browse files Browse the repository at this point in the history
  • Loading branch information
petr-muller committed Mar 4, 2024
1 parent 6aebb8e commit adc5f6d
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 28 deletions.
40 changes: 22 additions & 18 deletions pkg/featuregates/featurechangestopper.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import (

// ChangeStopper calls stop when the value of the featureset changes
type ChangeStopper struct {
startingRequiredFeatureSet configv1.FeatureSet
startingCvoGates CvoGates
startingRequiredFeatureSet *configv1.FeatureSet
startingCvoGates *CvoGates

featureGateLister configlistersv1.FeatureGateLister
cacheSynced []cache.InformerSynced
Expand All @@ -30,20 +30,12 @@ type ChangeStopper struct {
}

// NewChangeStopper returns a new ChangeStopper.
func NewChangeStopper(
startingRequiredFeatureSet configv1.FeatureSet,
startingCVOGates CvoGates,
featureGateInformer configinformersv1.FeatureGateInformer,
) (*ChangeStopper, error) {
func NewChangeStopper(featureGateInformer configinformersv1.FeatureGateInformer) (*ChangeStopper, error) {
c := &ChangeStopper{
startingRequiredFeatureSet: startingRequiredFeatureSet,
startingCvoGates: startingCVOGates,
featureGateLister: featureGateInformer.Lister(),
cacheSynced: []cache.InformerSynced{featureGateInformer.Informer().HasSynced},
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "feature-gate-stopper"),
featureGateLister: featureGateInformer.Lister(),
cacheSynced: []cache.InformerSynced{featureGateInformer.Informer().HasSynced},
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "feature-gate-stopper"),
}
klog.Infof("NewChangeStopper: startingRequiredFeatureSet=%q", startingRequiredFeatureSet)
klog.Infof("NewChangeStopper: startingCVOGates=%+v", startingCVOGates)
c.queue.Add("cluster") // seed an initial sync, in case startingRequiredFeatureSet is wrong
if _, err := featureGateInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: func(_ interface{}) {
Expand All @@ -62,6 +54,14 @@ func NewChangeStopper(
return c, nil
}

func (c *ChangeStopper) SetStartingFeatures(requiredFeatureSet configv1.FeatureSet, cvoGates CvoGates) {
c.startingRequiredFeatureSet = &requiredFeatureSet
c.startingCvoGates = &cvoGates

klog.Infof("NewChangeStopper: startingRequiredFeatureSet=%s", *c.startingRequiredFeatureSet)
klog.Infof("NewChangeStopper: startingCvoGates=%+v", c.startingCvoGates)
}

// 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).
Expand All @@ -81,8 +81,8 @@ func (c *ChangeStopper) syncHandler(_ context.Context) (done bool, err error) {
klog.Infof("ChangeStopper.syncHandler: not-found error: %v", err)
}

featureSetChanged := current != c.startingRequiredFeatureSet
cvoFeaturesChanged := currentCvoGates != c.startingCvoGates
featureSetChanged := current != *c.startingRequiredFeatureSet
cvoFeaturesChanged := currentCvoGates != *c.startingCvoGates
klog.Infof("ChangeStopper.syncHandler: featureSetChanged=%v", featureSetChanged)
klog.Infof("ChangeStopper.syncHandler: cvoFeaturesChanged=%v", cvoFeaturesChanged)
if featureSetChanged || cvoFeaturesChanged {
Expand All @@ -93,7 +93,7 @@ func (c *ChangeStopper) syncHandler(_ context.Context) (done bool, err error) {
action = "requesting shutdown"
}
if featureSetChanged {
klog.Infof("FeatureSet was %q, but the current feature set is %q; %s.", c.startingRequiredFeatureSet, current, action)
klog.Infof("FeatureSet was %s, but the current feature set is %s; %s.", *c.startingRequiredFeatureSet, current, action)
}
if cvoFeaturesChanged {
klog.Infof("CVO feature flags were %v, but changed to %v; %s.", c.startingCvoGates, currentCvoGates, action)
Expand All @@ -109,6 +109,10 @@ func (c *ChangeStopper) syncHandler(_ context.Context) (done bool, err error) {

// Run launches the controller and blocks until it is canceled or work completes.
func (c *ChangeStopper) Run(ctx context.Context, shutdownFn context.CancelFunc) error {
if c.startingRequiredFeatureSet == nil || c.startingCvoGates == nil {
return errors.New("BUG: startingRequiredFeatureSet and startingCvoGates must be set before calling Run")

}
// don't let panics crash the process
defer utilruntime.HandleCrash()
// make sure the work queue is shutdown which will trigger workers to end
Expand All @@ -121,7 +125,7 @@ func (c *ChangeStopper) Run(ctx context.Context, shutdownFn context.CancelFunc)
}()
c.shutdownFn = shutdownFn

klog.Infof("Starting stop-on-featureset-change controller with %q.", c.startingRequiredFeatureSet)
klog.Infof("Starting stop-on-featureset-change controller with %s.", *c.startingRequiredFeatureSet)

// wait for your secondary caches to fill before starting your work
if !cache.WaitForCacheSync(ctx.Done(), c.cacheSynced...) {
Expand Down
3 changes: 2 additions & 1 deletion pkg/featuregates/featurechangestopper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,11 @@ func TestTechPreviewChangeStopper(t *testing.T) {
client := fakeconfigv1client.NewSimpleClientset(fg)

informerFactory := configv1informer.NewSharedInformerFactory(client, 0)
c, err := NewChangeStopper(tt.startingRequiredFeatureSet, tt.startingCvoFeatureGates, informerFactory.Config().V1().FeatureGates())
c, err := NewChangeStopper(informerFactory.Config().V1().FeatureGates())
if err != nil {
t.Fatal(err)
}
c.SetStartingFeatures(tt.startingRequiredFeatureSet, tt.startingCvoFeatureGates)
informerFactory.Start(ctx.Done())

if err := c.Run(ctx, shutdownFn); err != nil {
Expand Down
21 changes: 12 additions & 9 deletions pkg/start/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"time"

"github.com/google/uuid"
configinformersv1 "github.com/openshift/client-go/config/informers/externalversions/config/v1"
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -436,7 +436,7 @@ type Context struct {
OpenshiftConfigManagedInformerFactory informers.SharedInformerFactory
InformerFactory externalversions.SharedInformerFactory

fgInformer configinformersv1.FeatureGateInformer
fgLister configlistersv1.FeatureGateLister
}

// NewControllerContext initializes the default Context for the current Options. It does
Expand Down Expand Up @@ -479,13 +479,20 @@ func (o *Options) NewControllerContext(cb *ClientBuilder) (*Context, error) {
return nil, err
}

featureChangeStopper, err := featuregates.NewChangeStopper(sharedInformers.Config().V1().FeatureGates())
if err != nil {
return nil, err
}

ctx := &Context{
CVInformerFactory: cvInformer,
OpenshiftConfigInformerFactory: openshiftConfigInformer,
OpenshiftConfigManagedInformerFactory: openshiftConfigManagedInformer,
InformerFactory: sharedInformers,
fgInformer: cvInformer.Config().V1().FeatureGates(),
CVO: cvo,
StopOnFeatureGateChange: featureChangeStopper,

fgLister: sharedInformers.Config().V1().FeatureGates().Lister(),
}

if o.EnableAutoUpdate {
Expand Down Expand Up @@ -522,7 +529,7 @@ func (c *Context) InitializeFromPayload(ctx context.Context, restConfig *rest.Co
// 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 := c.fgInformer.Lister().Get("cluster")
gate, fgErr := c.fgLister.Get("cluster")
switch {
case apierrors.IsNotFound(fgErr):
// if we have no featuregates, then the cluster is using the default featureset, which is "".
Expand Down Expand Up @@ -566,11 +573,7 @@ func (c *Context) InitializeFromPayload(ctx context.Context, restConfig *rest.Co
}
klog.Infof("CVO features for version %s enabled at startup: %+v", payload.Release.Version, cvoGates)

featureChangeStopper, err := featuregates.NewChangeStopper(startingFeatureSet, cvoGates, c.fgInformer)
if err != nil {
return err
}
c.StopOnFeatureGateChange = featureChangeStopper
c.StopOnFeatureGateChange.SetStartingFeatures(startingFeatureSet, cvoGates)

return c.CVO.InitializeFromPayload(payload, startingFeatureSet, cvoGates, restConfig, burstRestConfig)
}

0 comments on commit adc5f6d

Please sign in to comment.