Skip to content

Commit

Permalink
client: fix panic on alloc restore (#18356)
Browse files Browse the repository at this point in the history
When restoring an allocation `WIDMgr` was not being set in the alloc
runner config, resulting in a nil panic when the task runner attempted
to start.

Since we will often require the same configuration values when creating
or restoring a new allocation, this commit moves the logic to a shared
function to ensure that `addAlloc` and `restoreState` configure alloc
runners with the same values.
  • Loading branch information
lgfa29 authored Sep 1, 2023
1 parent 776a26b commit b614ef3
Showing 1 changed file with 40 additions and 49 deletions.
89 changes: 40 additions & 49 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1235,30 +1235,12 @@ func (c *Client) restoreState() error {
prevAllocWatcher := allocwatcher.NoopPrevAlloc{}
prevAllocMigrator := allocwatcher.NoopPrevAlloc{}

arConf := &config.AllocRunnerConfig{
Alloc: alloc,
Logger: c.logger,
ClientConfig: conf,
StateDB: c.stateDB,
StateUpdater: c,
DeviceStatsReporter: c,
Consul: c.consulService,
ConsulSI: c.tokensClient,
ConsulProxies: c.consulProxies,
Vault: c.vaultClient,
PrevAllocWatcher: prevAllocWatcher,
PrevAllocMigrator: prevAllocMigrator,
DynamicRegistry: c.dynamicRegistry,
CSIManager: c.csimanager,
DeviceManager: c.devicemanager,
DriverManager: c.drivermanager,
ServersContactedCh: c.serversContactedCh,
ServiceRegWrapper: c.serviceRegWrapper,
CheckStore: c.checkStore,
RPCClient: c,
Getter: c.getter,
Wranglers: c.wranglers,
}
arConf := c.newAllocRunnerConfig(alloc, prevAllocWatcher, prevAllocMigrator)

// ServerContactedCh is used by task runners on restore failures to
// wait for servers to be contacted before proceeding with the
// restoration process.
arConf.ServersContactedCh = c.serversContactedCh

ar, err := c.allocrunnerFactory(arConf)
if err != nil {
Expand Down Expand Up @@ -2726,31 +2708,7 @@ func (c *Client) addAlloc(alloc *structs.Allocation, migrateToken string) error
}
prevAllocWatcher, prevAllocMigrator := allocwatcher.NewAllocWatcher(watcherConfig)

arConf := &config.AllocRunnerConfig{
Alloc: alloc,
Logger: c.logger,
ClientConfig: c.GetConfig(),
StateDB: c.stateDB,
Consul: c.consulService,
ConsulProxies: c.consulProxies,
ConsulSI: c.tokensClient,
Vault: c.vaultClient,
StateUpdater: c,
DeviceStatsReporter: c,
PrevAllocWatcher: prevAllocWatcher,
PrevAllocMigrator: prevAllocMigrator,
DynamicRegistry: c.dynamicRegistry,
CSIManager: c.csimanager,
DeviceManager: c.devicemanager,
DriverManager: c.drivermanager,
ServiceRegWrapper: c.serviceRegWrapper,
CheckStore: c.checkStore,
RPCClient: c,
Getter: c.getter,
Wranglers: c.wranglers,
WIDMgr: c.widmgr,
}

arConf := c.newAllocRunnerConfig(alloc, prevAllocWatcher, prevAllocMigrator)
ar, err := c.allocrunnerFactory(arConf)
if err != nil {
return err
Expand All @@ -2766,6 +2724,39 @@ func (c *Client) addAlloc(alloc *structs.Allocation, migrateToken string) error
return nil
}

// allocRunnerConfig returns a new AllocRunnerConfig that can be used to start
// or restore an AllocRunner.
func (c *Client) newAllocRunnerConfig(
alloc *structs.Allocation,
prevAllocWatcher config.PrevAllocWatcher,
prevAllocMigrator config.PrevAllocMigrator,
) *config.AllocRunnerConfig {
return &config.AllocRunnerConfig{
Alloc: alloc,
CSIManager: c.csimanager,
CheckStore: c.checkStore,
ClientConfig: c.GetConfig(),
Consul: c.consulService,
ConsulProxies: c.consulProxies,
ConsulSI: c.tokensClient,
DeviceManager: c.devicemanager,
DeviceStatsReporter: c,
DriverManager: c.drivermanager,
DynamicRegistry: c.dynamicRegistry,
Getter: c.getter,
Logger: c.logger,
PrevAllocMigrator: prevAllocMigrator,
PrevAllocWatcher: prevAllocWatcher,
RPCClient: c,
ServiceRegWrapper: c.serviceRegWrapper,
StateDB: c.stateDB,
StateUpdater: c,
Vault: c.vaultClient,
WIDMgr: c.widmgr,
Wranglers: c.wranglers,
}
}

// setupConsulTokenClient configures a tokenClient for managing consul service
// identity tokens.
func (c *Client) setupConsulTokenClient() error {
Expand Down

0 comments on commit b614ef3

Please sign in to comment.