From 4c4cb2c6ade2ee10a25f3cc18e22ffa5a52eb1e0 Mon Sep 17 00:00:00 2001 From: James Rasell Date: Thu, 10 Apr 2025 14:40:21 +0100 Subject: [PATCH] agent: Fix misaligned contextual k/v logging arguments. (#25629) Arguments passed to hclog log lines should always have an even number to provide the expected k/v output. --- client/allocrunner/taskrunner/api_hook.go | 2 +- client/allocrunner/taskrunner/dynamic_users_hook.go | 2 +- client/allocrunner/taskrunner/identity_hook.go | 8 +++++--- client/client.go | 2 +- client/widmgr/widmgr.go | 3 ++- nomad/job_endpoint.go | 2 +- nomad/leader.go | 4 ++-- nomad/locks.go | 4 ++-- scheduler/scheduler_system.go | 2 +- 9 files changed, 16 insertions(+), 13 deletions(-) diff --git a/client/allocrunner/taskrunner/api_hook.go b/client/allocrunner/taskrunner/api_hook.go index 9fa9c9e62..eebcf9bab 100644 --- a/client/allocrunner/taskrunner/api_hook.go +++ b/client/allocrunner/taskrunner/api_hook.go @@ -97,7 +97,7 @@ func (h *apiHook) Stop(ctx context.Context, req *interfaces.TaskStopRequest, res if h.ln != nil { if err := h.ln.Close(); err != nil { if !errors.Is(err, net.ErrClosed) { - h.logger.Debug("error closing task listener: %v", err) + h.logger.Debug("error closing task listener", "error", err) } } h.ln = nil diff --git a/client/allocrunner/taskrunner/dynamic_users_hook.go b/client/allocrunner/taskrunner/dynamic_users_hook.go index 6d7aec006..2c0345832 100644 --- a/client/allocrunner/taskrunner/dynamic_users_hook.go +++ b/client/allocrunner/taskrunner/dynamic_users_hook.go @@ -77,7 +77,7 @@ func (h *dynamicUsersHook) Prestart(_ context.Context, request *interfaces.TaskP // allocate an unused UID/GID from the pool ugid, err := h.pool.Acquire() if err != nil { - h.logger.Error("unable to acquire anonymous UID/GID: %v", err) + h.logger.Error("unable to acquire anonymous UID/GID", "error", err) return err } diff --git a/client/allocrunner/taskrunner/identity_hook.go b/client/allocrunner/taskrunner/identity_hook.go index 32b4d7bd0..d90864df2 100644 --- a/client/allocrunner/taskrunner/identity_hook.go +++ b/client/allocrunner/taskrunner/identity_hook.go @@ -134,12 +134,13 @@ func (h *identityHook) watchIdentity(wid *structs.WorkloadIdentity, runCh chan s if signedWID == nil { // The only way to hit this should be a bug as it indicates the server // did not sign an identity for a task on this alloc. - h.logger.Error("missing workload identity %q", wid.Name) + h.logger.Error("missing workload identity", "identity", wid.Name) return } if err := h.setAltToken(wid, signedWID.JWT); err != nil { - h.logger.Error(err.Error()) + h.logger.Error("failed to set workload identity token", + "identity", wid.Name, "error", err) } // Skip ChangeMode on firstRun and notify caller it can proceed @@ -148,7 +149,8 @@ func (h *identityHook) watchIdentity(wid *structs.WorkloadIdentity, runCh chan s case runCh <- struct{}{}: default: // Not great but not necessarily fatal - h.logger.Warn("task started before identity %q was fetched", wid.Name) + h.logger.Warn("task started before identity was fetched", + "identity", wid.Name) } firstRun = false diff --git a/client/client.go b/client/client.go index 391e84e24..ef08ff5dc 100644 --- a/client/client.go +++ b/client/client.go @@ -1333,7 +1333,7 @@ func (c *Client) restoreState() error { allocState, err := c.stateDB.GetAcknowledgedState(alloc.ID) if err != nil { c.logger.Error("error restoring last acknowledged alloc state, will update again", - err, "alloc_id", alloc.ID) + "error", err, "alloc_id", alloc.ID) } else { ar.AcknowledgeState(allocState) } diff --git a/client/widmgr/widmgr.go b/client/widmgr/widmgr.go index 888befe96..d9327bf40 100644 --- a/client/widmgr/widmgr.go +++ b/client/widmgr/widmgr.go @@ -121,7 +121,8 @@ func (m *WIDMgr) Run() error { hasExpired, err := m.restoreStoredIdentities() if err != nil { - m.logger.Warn("failed to get signed identities from state DB, refreshing from server: %w", err) + m.logger.Warn("failed to get signed identities from state DB, refreshing from server", + "error", err) } if hasExpired { if err := m.getInitialIdentities(); err != nil { diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 827658b28..b786b464c 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -2107,7 +2107,7 @@ func (j *Job) Dispatch(args *structs.JobDispatchRequest, reply *structs.JobDispa // Commit this update via Raft _, jobCreateIndex, err := j.srv.raftApply(structs.JobRegisterRequestType, regReq) if err != nil { - j.logger.Error("dispatched job register failed", "error") + j.logger.Error("dispatched job register failed", "error", err) return err } diff --git a/nomad/leader.go b/nomad/leader.go index 2b8a8378f..fa104c6d7 100644 --- a/nomad/leader.go +++ b/nomad/leader.go @@ -2624,7 +2624,7 @@ func (s *Server) initializeKeyring(stopCh <-chan struct{}) { store := s.fsm.State() key, err := store.GetActiveRootKey(nil) if err != nil { - logger.Error("failed to get active key: %v", err) + logger.Error("failed to get active key", "error", err) return } if key != nil { @@ -2653,7 +2653,7 @@ func (s *Server) initializeKeyring(stopCh <-chan struct{}) { rootKey, err := structs.NewUnwrappedRootKey(structs.EncryptionAlgorithmAES256GCM) rootKey = rootKey.MakeActive() if err != nil { - logger.Error("could not initialize keyring: %v", err) + logger.Error("could not initialize keyring", "error", err) return } diff --git a/nomad/locks.go b/nomad/locks.go index 464e11635..55faa84f3 100644 --- a/nomad/locks.go +++ b/nomad/locks.go @@ -57,7 +57,7 @@ func (s *Server) CreateVariableLockTTLTimer(variable structs.VariableEncrypted) lock := s.lockTTLTimer.Get(lockID) if lock != nil { // If this was to happen, there is a sync issue somewhere else - s.logger.Error("attempting to recreate existing lock: %s", lockID) + s.logger.Error("attempting to recreate existing lock", "lock", lockID) return } @@ -146,7 +146,7 @@ func (s *Server) RemoveVariableLockTTLTimer(variable structs.VariableEncrypted) lock := s.lockTTLTimer.Get(lockID) if lock == nil { // If this was to happen, there is a sync issue somewhere else. - s.logger.Error("attempting to removed missing lock: %s", lockID) + s.logger.Error("attempting to removed missing lock", "lock", lockID) return } diff --git a/scheduler/scheduler_system.go b/scheduler/scheduler_system.go index c80595643..0cdc52882 100644 --- a/scheduler/scheduler_system.go +++ b/scheduler/scheduler_system.go @@ -362,7 +362,7 @@ func (s *SystemScheduler) computePlacements(place []allocTuple) error { node, ok := nodeByID[missing.Alloc.NodeID] if !ok { - s.logger.Debug("could not find node %q", missing.Alloc.NodeID) + s.logger.Debug("could not find node", "node", missing.Alloc.NodeID) continue }