Skip to content

Commit

Permalink
plugin: Remove vmStore from AutoscaleEnforcer
Browse files Browse the repository at this point in the history
After the changes from #865 that switched to fetching VmInfo from Pod
objects, the VM store is no longer needed long-term, just during initial
cluster state reading (which can *also* be swapped out) and for event
generation.

This commit changes the VM store from being stored in the
AutoscaleEnforcer struct to being passed in to readClusterState as an
argument.
  • Loading branch information
sharnoff committed May 21, 2024
1 parent e3b2cec commit 69773ef
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 13 deletions.
14 changes: 3 additions & 11 deletions pkg/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,8 @@ type AutoscaleEnforcer struct {
state pluginState
metrics PromMetrics

// vmStore provides access the current-ish state of VMs in the cluster. If something's missing,
// it can be updated with Resync().
//
// Keeping this store allows us to make sure that our event handling is never out-of-sync
// because all the information is coming from the same source.
vmStore IndexedVMStore
// nodeStore is doing roughly the same thing as with vmStore, but for Nodes.
// nodeStore provides access to the current-ish state of Nodes in the cluster. If something's
// missing, it can be updated with Relist().
nodeStore IndexedNodeStore
}

Expand Down Expand Up @@ -99,7 +94,6 @@ func makeAutoscaleEnforcerPlugin(
conf: config,
},
metrics: PromMetrics{}, //nolint:exhaustruct // set by makePrometheusRegistry
vmStore: IndexedVMStore{}, //nolint:exhaustruct // set below
nodeStore: IndexedNodeStore{}, //nolint:exhaustruct // set below
}

Expand Down Expand Up @@ -195,13 +189,11 @@ func makeAutoscaleEnforcerPlugin(
return nil, fmt.Errorf("Error starting VM Migration watcher: %w", err)
}

p.vmStore = watch.NewIndexedStore(vmStore, watch.NewNameIndex[vmapi.VirtualMachine]())

watchMetrics.MustRegister(promReg)

// ... but before handling the events, read the current cluster state:
logger.Info("Reading initial cluster state")
if err = p.readClusterState(ctx, logger); err != nil {
if err = p.readClusterState(ctx, logger, vmStore); err != nil {
return nil, fmt.Errorf("Error reading cluster state: %w", err)
}

Expand Down
8 changes: 6 additions & 2 deletions pkg/plugin/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,11 @@ func (e *AutoscaleEnforcer) startMigration(ctx context.Context, logger *zap.Logg
//
// This method expects that all pluginState fields except pluginState.conf are set to their zero
// value, and will panic otherwise. Accordingly, this is only called during plugin initialization.
func (p *AutoscaleEnforcer) readClusterState(ctx context.Context, logger *zap.Logger) error {
func (p *AutoscaleEnforcer) readClusterState(
ctx context.Context,
logger *zap.Logger,
vmStore *watch.Store[vmapi.VirtualMachine],
) error {
logger = logger.With(zap.String("action", "read cluster state"))

p.state.lock.Lock()
Expand Down Expand Up @@ -1260,7 +1264,7 @@ func (p *AutoscaleEnforcer) readClusterState(ctx context.Context, logger *zap.Lo
// has already been made.

logger.Info("Fetching VMs from existing store")
vms := p.vmStore.Items()
vms := vmStore.Items()

logger.Info("Listing Pods")
pods, err := p.handle.ClientSet().CoreV1().Pods(corev1.NamespaceAll).List(ctx, metav1.ListOptions{})
Expand Down

0 comments on commit 69773ef

Please sign in to comment.