diff --git a/pkg/module/ipmi/ipmi.go b/pkg/module/ipmi/ipmi.go index d83b74ea..3939f6e9 100644 --- a/pkg/module/ipmi/ipmi.go +++ b/pkg/module/ipmi/ipmi.go @@ -34,7 +34,9 @@ type IPMI struct { Password string // Password is used for IPMI authentication. WARNING: Unsavely stored as plaintext Timeout string // Timeout is the duration for IPMI commands. Default: 10 seconds - client *ipmi.Client // client is the module's internal entity to forward IPMI commands + timeout time.Duration // timeout is the resolved command timeout; set in Init + client *ipmi.Client // client is the current IPMI session; nil until the first command + connected bool // connected tracks whether client holds a live session } // Ensure implementing the Module interface. @@ -73,19 +75,19 @@ const ( func (i *IPMI) Init(ctx context.Context) error { l := log.FromContext(ctx) - port := i.Port - if port == 0 { - port = defaultPort + if i.Port == 0 { + i.Port = defaultPort l.Debug(fmt.Sprintf("no port configured, using default %d", defaultPort)) } // Parse custom timeout if provided; an unparseable value falls back to the default. - timeout := defaultTimeout + i.timeout = defaultTimeout if i.Timeout != "" { parsedTimeout, err := time.ParseDuration(i.Timeout) if err == nil { - timeout = parsedTimeout + i.timeout = parsedTimeout + l.Debug(fmt.Sprintf("using custom timeout %s", i.timeout)) } else { l.Debug(fmt.Sprintf("invalid timeout %q, using default %s", i.Timeout, defaultTimeout)) } @@ -97,42 +99,105 @@ func (i *IPMI) Init(ctx context.Context) error { return fmt.Errorf("IPMI Host is not set") } - ipmiClient, err := ipmi.NewClient(i.Host, port, i.User, i.Password) + // Deliberately do NOT open the IPMI session here. The BMC may be + // unreachable at agent startup (e.g. the board's outer power is off), and + // the dutagent treats a failed Init() as fatal — it shuts down and + // systemd crash-loops the whole agent, taking down every other device on + // the worker too. The session is opened lazily on the first command and + // re-opened automatically if it goes stale (see connect / withSession). + l.Debug(fmt.Sprintf("init completed for %s:%d (BMC session deferred to first use)", i.Host, i.Port)) + + return nil +} + +// connect ensures a live IPMI session exists. It is a no-op when already +// connected; otherwise it builds a fresh client and opens a new session. A new +// client is used each time so a previously stale session is fully discarded. +// Called lazily on the first command (not in Init) so an unreachable BMC +// surfaces as a normal command error instead of a fatal module-init failure. +func (i *IPMI) connect(ctx context.Context) error { + if i.connected && i.client != nil { + return nil + } + + client, err := ipmi.NewClient(i.Host, i.Port, i.User, i.Password) if err != nil { return fmt.Errorf("failed to create IPMI client: %v", err) } - ipmiClient.WithTimeout(timeout) - ipmiClient.WithRetry(trials) + client.WithTimeout(i.timeout) + client.WithRetry(trials) - // Bounded by ctx: today the init context is a plain background context, but - // passing it (not context.Background()) means a later startup deadline or - // shutdown cancellation on that context will bound this connect. TODO(ctx). - err = ipmiClient.Connect(ctx) + err = client.Connect(ctx) if err != nil { - return fmt.Errorf("failed to connect to IPMI BMC %s:%d: %v", i.Host, port, err) + return fmt.Errorf("failed to connect to IPMI BMC %s: %w", i.Host, err) } - i.client = ipmiClient - - l.Debug(fmt.Sprintf("connected to BMC %s:%d", i.Host, port)) + i.client = client + i.connected = true return nil } -func (i *IPMI) Deinit(ctx context.Context) error { - if i.client == nil { +// disconnect closes the current session (best-effort) and clears it so the next +// connect opens a fresh one. Used to recover from a session that has gone stale. +func (i *IPMI) disconnect(ctx context.Context) { + if i.client != nil { + _ = i.client.Close(ctx) + } + + i.client = nil + i.connected = false +} + +// withSession runs op against a live IPMI session, transparently re-opening the +// session once if op fails. IPMI/RMCP sessions idle-time-out and the BMC drops +// inactive ones, so a cached session goes stale between commands; without this +// the module would keep using the dead session and every command would time out +// until the agent was restarted. On the first failure the session is dropped, +// reconnected, and op is retried once so a stale session self-heals. +func (i *IPMI) withSession(ctx context.Context, op func() error) error { + err := i.connect(ctx) + if err != nil { + return err + } + + err = op() + if err == nil { return nil } - return i.client.Close(ctx) + // First attempt failed — most likely a stale session. Drop it, reconnect, + // and retry once against a fresh session. + log.FromContext(ctx).Debug(fmt.Sprintf("command failed (%v); re-opening session and retrying once", err)) + + i.disconnect(ctx) + + cerr := i.connect(ctx) + if cerr != nil { + return fmt.Errorf("%v; reconnect failed: %w", err, cerr) + } + + return op() } -func (i *IPMI) Run(ctx context.Context, s module.Session, args ...string) error { - if i.client == nil { - return fmt.Errorf("IPMI client not initialized") +func (i *IPMI) Deinit(ctx context.Context) error { + if i.client == nil || !i.connected { + return nil + } + + err := i.client.Close(ctx) + if err != nil { + log.FromContext(ctx).Debug(fmt.Sprintf("Deinit failed to close client: %v", err)) } + i.client = nil + i.connected = false + + return err +} + +func (i *IPMI) Run(ctx context.Context, s module.Session, args ...string) error { if len(args) == 0 { s.Println("No command specified. Try 'help' for usage.") @@ -175,7 +240,11 @@ func (i *IPMI) handlePowerCommand(ctx context.Context, s module.Session, command message = "Power RESET command sent" } - _, err := i.client.ChassisControl(ctx, controlType) + err := i.withSession(ctx, func() error { + _, cerr := i.client.ChassisControl(ctx, controlType) + + return cerr + }) if err != nil { return fmt.Errorf("power %s command failed: %v", command, err) } @@ -187,13 +256,24 @@ func (i *IPMI) handlePowerCommand(ctx context.Context, s module.Session, command } func (i *IPMI) handleStatusCommand(ctx context.Context, s module.Session) error { - status, err := i.client.GetChassisStatus(ctx) + var powerIsOn bool + + err := i.withSession(ctx, func() error { + chassis, cerr := i.client.GetChassisStatus(ctx) + if cerr != nil { + return cerr + } + + powerIsOn = chassis.PowerIsOn + + return nil + }) if err != nil { return fmt.Errorf("failed to get chassis status: %v", err) } powerStatus := "Off" - if status.PowerIsOn { + if powerIsOn { powerStatus = "On" }