From 16c490f956413b9a372bc94ea116005056d126dd Mon Sep 17 00:00:00 2001 From: Thomas Parrott Date: Thu, 27 Jun 2024 12:42:41 +0100 Subject: [PATCH 1/9] lxd/auth: Standardise error field to err Signed-off-by: Thomas Parrott --- lxd/auth/drivers/tls.go | 2 +- lxd/auth/generate/main.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lxd/auth/drivers/tls.go b/lxd/auth/drivers/tls.go index af13ebd3e1ab..2b72cfc83c06 100644 --- a/lxd/auth/drivers/tls.go +++ b/lxd/auth/drivers/tls.go @@ -201,7 +201,7 @@ func (t *tls) GetPermissionChecker(ctx context.Context, r *http.Request, entitle return func(entityURL *api.URL) bool { eType, project, _, pathArgs, err := entity.ParseURL(entityURL.URL) if err != nil { - logger.Warn("Permission checker failed to parse entity URL", logger.Ctx{"entity_url": entityURL, "error": err}) + logger.Warn("Permission checker failed to parse entity URL", logger.Ctx{"entity_url": entityURL, "err": err}) return false } diff --git a/lxd/auth/generate/main.go b/lxd/auth/generate/main.go index 4404379abd06..11a26b46e741 100644 --- a/lxd/auth/generate/main.go +++ b/lxd/auth/generate/main.go @@ -222,7 +222,7 @@ scan: curType = entity.Type(submatch[1]) err := curType.Validate() if err != nil { - logger.Warn("Entity type not defined for OpenFGA model type", logger.Ctx{"model_type": submatch[1], "error": err}) + logger.Warn("Entity type not defined for OpenFGA model type", logger.Ctx{"model_type": submatch[1], "err": err}) continue scan } From ea6108e3f8407c19c04832ac10bc1824e8bab597 Mon Sep 17 00:00:00 2001 From: Thomas Parrott Date: Thu, 27 Jun 2024 12:43:14 +0100 Subject: [PATCH 2/9] lxd/identities: Standardise error field to err Signed-off-by: Thomas Parrott --- lxd/identities.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lxd/identities.go b/lxd/identities.go index fb593f61c506..ac1254b5a0e7 100644 --- a/lxd/identities.go +++ b/lxd/identities.go @@ -832,7 +832,7 @@ func updateIdentityCache(d *Daemon) { if cacheEntry.AuthenticationMethod == api.AuthenticationMethodTLS { cert, err := id.X509() if err != nil { - logger.Warn("Failed to extract x509 certificate from TLS identity metadata", logger.Ctx{"error": err}) + logger.Warn("Failed to extract x509 certificate from TLS identity metadata", logger.Ctx{"err": err}) continue } @@ -840,7 +840,7 @@ func updateIdentityCache(d *Daemon) { } else if cacheEntry.AuthenticationMethod == api.AuthenticationMethodOIDC { subject, err := id.Subject() if err != nil { - logger.Warn("Failed to extract OIDC subject from OIDC identity metadata", logger.Ctx{"error": err}) + logger.Warn("Failed to extract OIDC subject from OIDC identity metadata", logger.Ctx{"err": err}) continue } @@ -853,7 +853,7 @@ func updateIdentityCache(d *Daemon) { if id.Type == api.IdentityTypeCertificateServer { cert, err := id.ToCertificate() if err != nil { - logger.Warn("Failed to convert TLS identity to server certificate", logger.Ctx{"error": err}) + logger.Warn("Failed to convert TLS identity to server certificate", logger.Ctx{"err": err}) } localServerCerts = append(localServerCerts, *cert) @@ -872,7 +872,7 @@ func updateIdentityCache(d *Daemon) { err = d.identityCache.ReplaceAll(identityCacheEntries, idpGroupMapping) if err != nil { - logger.Warn("Failed to update identity cache", logger.Ctx{"error": err}) + logger.Warn("Failed to update identity cache", logger.Ctx{"err": err}) } } @@ -907,7 +907,7 @@ func updateIdentityCacheFromLocal(d *Daemon) error { id, err := dbCert.ToIdentity() if err != nil { - logger.Warn("Failed to convert node certificate into identity entry", logger.Ctx{"error": err}) + logger.Warn("Failed to convert node certificate into identity entry", logger.Ctx{"err": err}) continue } From 48ef3ad627d4b98ce58ec248ac7372a220bba81e Mon Sep 17 00:00:00 2001 From: Thomas Parrott Date: Thu, 27 Jun 2024 12:43:29 +0100 Subject: [PATCH 3/9] lxd: Standardise error field to err Signed-off-by: Thomas Parrott --- lxd/cluster/membership.go | 12 ++++++------ lxd/daemon.go | 2 +- lxd/firewall/drivers/drivers_xtables.go | 2 +- lxd/project/permissions.go | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lxd/cluster/membership.go b/lxd/cluster/membership.go index ef695af94c35..c22fb8ee306a 100644 --- a/lxd/cluster/membership.go +++ b/lxd/cluster/membership.go @@ -405,7 +405,7 @@ func Join(state *state.State, gateway *Gateway, networkCert *shared.CertInfo, se return nil }) if err != nil { - logger.Error("Failed to unlock global database after cluster join error", logger.Ctx{"error": err}) + logger.Error("Failed to unlock global database after cluster join error", logger.Ctx{"err": err}) } }) @@ -437,32 +437,32 @@ func Join(state *state.State, gateway *Gateway, networkCert *shared.CertInfo, se return tx.ReplaceRaftNodes([]db.RaftNode{}) }) if err != nil { - logger.Error("Failed to clear local raft node records after cluster join error", logger.Ctx{"error": err}) + logger.Error("Failed to clear local raft node records after cluster join error", logger.Ctx{"err": err}) return } err = gateway.Shutdown() if err != nil { - logger.Error("Failed to shutdown gateway after cluster join error", logger.Ctx{"error": err}) + logger.Error("Failed to shutdown gateway after cluster join error", logger.Ctx{"err": err}) return } err = os.RemoveAll(state.OS.GlobalDatabaseDir()) if err != nil { - logger.Error("Failed to remove raft data after cluster join error", logger.Ctx{"error": err}) + logger.Error("Failed to remove raft data after cluster join error", logger.Ctx{"err": err}) return } gateway.networkCert = oldCert err = gateway.init(false) if err != nil { - logger.Error("Failed to re-initialize gateway after cluster join error", logger.Ctx{"error": err}) + logger.Error("Failed to re-initialize gateway after cluster join error", logger.Ctx{"err": err}) return } _, err = cluster.EnsureSchema(state.DB.Cluster.DB(), localClusterAddress, state.OS.GlobalDatabaseDir()) if err != nil { - logger.Error("Failed to reload schema after cluster join error", logger.Ctx{"error": err}) + logger.Error("Failed to reload schema after cluster join error", logger.Ctx{"err": err}) return } }) diff --git a/lxd/daemon.go b/lxd/daemon.go index 185c1edc2f14..0424eb2ddff7 100644 --- a/lxd/daemon.go +++ b/lxd/daemon.go @@ -615,7 +615,7 @@ func (d *Daemon) createCmd(restAPI *mux.Router, version string, c APIEndpoint) { var forwardedIdentityProviderGroups []string err = json.Unmarshal([]byte(forwardedIdentityProviderGroupsJSON), &forwardedIdentityProviderGroups) if err != nil { - logger.Error("Failed unmarshalling identity provider groups from forwarded request header", logger.Ctx{"error": err}) + logger.Error("Failed unmarshalling identity provider groups from forwarded request header", logger.Ctx{"err": err}) } else { ctx = context.WithValue(ctx, request.CtxForwardedIdentityProviderGroups, forwardedIdentityProviderGroups) } diff --git a/lxd/firewall/drivers/drivers_xtables.go b/lxd/firewall/drivers/drivers_xtables.go index ca708a70dceb..e83552dfb2c6 100644 --- a/lxd/firewall/drivers/drivers_xtables.go +++ b/lxd/firewall/drivers/drivers_xtables.go @@ -1558,7 +1558,7 @@ func (d Xtables) NetworkApplyForwards(networkName string, rules []AddressForward reverter.Add(func() { err := clearNetworkForwards() if err != nil { - logger.Error("Failed to clear firewall rules after failing to apply network forwards", logger.Ctx{"network_name": networkName, "error": err}) + logger.Error("Failed to clear firewall rules after failing to apply network forwards", logger.Ctx{"network_name": networkName, "err": err}) } }) diff --git a/lxd/project/permissions.go b/lxd/project/permissions.go index 9eea873cf2ac..4202d0621fe9 100644 --- a/lxd/project/permissions.go +++ b/lxd/project/permissions.go @@ -1474,19 +1474,19 @@ func FilterUsedBy(authorizer auth.Authorizer, r *http.Request, entries []string) for _, entry := range entries { u, err := url.Parse(entry) if err != nil { - logger.Warn("Failed to parse project used-by entity URL", logger.Ctx{"url": entry, "error": err}) + logger.Warn("Failed to parse project used-by entity URL", logger.Ctx{"url": entry, "err": err}) continue } entityType, projectName, location, pathArguments, err := entity.ParseURL(*u) if err != nil { - logger.Warn("Failed to parse project used-by entity URL", logger.Ctx{"url": entry, "error": err}) + logger.Warn("Failed to parse project used-by entity URL", logger.Ctx{"url": entry, "err": err}) continue } entityURL, err := entityType.URL(projectName, location, pathArguments...) if err != nil { - logger.Warn("Failed to create canonical entity URL for project used-by filtering", logger.Ctx{"url": entry, "error": err}) + logger.Warn("Failed to create canonical entity URL for project used-by filtering", logger.Ctx{"url": entry, "err": err}) continue } @@ -1522,7 +1522,7 @@ func FilterUsedBy(authorizer auth.Authorizer, r *http.Request, entries []string) // Otherwise get a permission checker for the entity type. canViewEntity, err := authorizer.GetPermissionChecker(r.Context(), r, auth.EntitlementCanView, entityType) if err != nil { - logger.Error("Failed to get permission checker for project used-by filtering", logger.Ctx{"entity_type": entityType, "error": err}) + logger.Error("Failed to get permission checker for project used-by filtering", logger.Ctx{"entity_type": entityType, "err": err}) continue } From 474609ce70d3aa3dc8d285352be1f9392e9c1763 Mon Sep 17 00:00:00 2001 From: Thomas Parrott Date: Thu, 27 Jun 2024 12:44:42 +0100 Subject: [PATCH 4/9] lxd-agent/events: Retry virtiofs hotplug mount Sometimes the hotplug event is received from LXD before the virtiofs device is available in the guest VM. So retry several times before giving up to allow it to be activated. Signed-off-by: Thomas Parrott --- lxd-agent/events.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/lxd-agent/events.go b/lxd-agent/events.go index 475c6cc33cb3..32bb14fac645 100644 --- a/lxd-agent/events.go +++ b/lxd-agent/events.go @@ -6,6 +6,7 @@ import ( "net/http" "os" "strings" + "time" "github.com/canonical/lxd/lxd/events" "github.com/canonical/lxd/lxd/instance/instancetype" @@ -150,12 +151,19 @@ func eventsProcess(event api.Event) { mntSource = e.Mount.Source } + l := logger.AddContext(logger.Ctx{"type": "virtiofs", "source": mntSource, "path": e.Config["path"]}) + _ = os.MkdirAll(e.Config["path"], 0755) - _, err = shared.RunCommand("mount", "-t", "virtiofs", mntSource, e.Config["path"]) - if err != nil { - logger.Infof("Failed to mount hotplug %q (Type: %q) to %q", mntSource, "virtiofs", e.Config["path"]) - return + + for i := 0; i < 5; i++ { + _, err = shared.RunCommand("mount", "-t", "virtiofs", mntSource, e.Config["path"]) + if err == nil { + l.Info("Mounted hotplug") + return + } + + time.Sleep(500 * time.Millisecond) } - logger.Infof("Mounted hotplug %q (Type: %q) to %q", mntSource, "virtiofs", e.Config["path"]) + l.Info("Failed to mount hotplug", logger.Ctx{"err": err}) } From 79a7adcd49805b37d1a8eac7c5793e7e610dde31 Mon Sep 17 00:00:00 2001 From: Thomas Parrott Date: Thu, 27 Jun 2024 12:45:46 +0100 Subject: [PATCH 5/9] lxd-agent: Enable syslog logging Signed-off-by: Thomas Parrott --- lxd-agent/main_agent.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lxd-agent/main_agent.go b/lxd-agent/main_agent.go index 7db2fcc2cd01..b15ec7b6a031 100644 --- a/lxd-agent/main_agent.go +++ b/lxd-agent/main_agent.go @@ -49,7 +49,7 @@ func (c *cmdAgent) Command() *cobra.Command { // Run executes the agent command. func (c *cmdAgent) Run(cmd *cobra.Command, args []string) error { // Setup logger. - err := logger.InitLogger("", "", c.global.flagLogVerbose, c.global.flagLogDebug, nil) + err := logger.InitLogger("", "lxd-agent", c.global.flagLogVerbose, c.global.flagLogDebug, nil) if err != nil { os.Exit(1) } From 31336545baf683077824ff8e10af419ca6844b20 Mon Sep 17 00:00:00 2001 From: Thomas Parrott Date: Thu, 27 Jun 2024 12:46:12 +0100 Subject: [PATCH 6/9] lxd-agent: Match the use of contextual logging for start up mounts Signed-off-by: Thomas Parrott --- lxd-agent/main_agent.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lxd-agent/main_agent.go b/lxd-agent/main_agent.go index b15ec7b6a031..4568ac11e116 100644 --- a/lxd-agent/main_agent.go +++ b/lxd-agent/main_agent.go @@ -281,10 +281,12 @@ func (c *cmdAgent) mountHostShares() { mount.Target = fmt.Sprintf("/%s", mount.Target) } + l := logger.AddContext(logger.Ctx{"source": mount.Source, "path": mount.Target}) + if !shared.PathExists(mount.Target) { err := os.MkdirAll(mount.Target, 0755) if err != nil { - logger.Errorf("Failed to create mount target %q", mount.Target) + l.Error("Failed to create mount target", logger.Ctx{"err": err}) continue // Don't try to mount if mount point can't be created. } } else if filesystem.IsMountPoint(mount.Target) { @@ -307,7 +309,7 @@ func (c *cmdAgent) mountHostShares() { _, err = shared.RunCommand("mount", args...) if err == nil { - logger.Infof("Mounted %q (Type: %q, Options: %v) to %q", mount.Source, "virtiofs", mount.Options, mount.Target) + l.Info("Mounted", logger.Ctx{"type": "virtiofs"}) continue } } @@ -320,10 +322,10 @@ func (c *cmdAgent) mountHostShares() { _, err = shared.RunCommand("mount", args...) if err != nil { - logger.Errorf("Failed mount %q (Type: %q, Options: %v) to %q: %v", mount.Source, mount.FSType, mount.Options, mount.Target, err) + l.Error("Failed to mount", logger.Ctx{"err": err}) continue } - logger.Infof("Mounted %q (Type: %q, Options: %v) to %q", mount.Source, mount.FSType, mount.Options, mount.Target) + l.Info("Mounted", logger.Ctx{"type": mount.FSType}) } } From d34bfbeb05ff4bdd1d441b3c1b1e308699382d3e Mon Sep 17 00:00:00 2001 From: Thomas Parrott Date: Thu, 27 Jun 2024 12:46:42 +0100 Subject: [PATCH 7/9] lxd-agent: Standardise error field to err Signed-off-by: Thomas Parrott --- lxd-agent/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lxd-agent/server.go b/lxd-agent/server.go index 7d03bd82b88c..af983cbb3730 100644 --- a/lxd-agent/server.go +++ b/lxd-agent/server.go @@ -96,7 +96,7 @@ func createCmd(restAPI *mux.Router, version string, c APIEndpoint, cert *x509.Ce if err != nil { writeErr := response.InternalError(err).Render(w) if writeErr != nil { - logger.Error("Failed writing error for HTTP response", logger.Ctx{"url": uri, "error": err, "writeErr": writeErr}) + logger.Error("Failed writing error for HTTP response", logger.Ctx{"url": uri, "err": err, "writeErr": writeErr}) } } }) From 70c0d45d942dc03abfea710e50646dd39aa4719c Mon Sep 17 00:00:00 2001 From: Thomas Parrott Date: Thu, 27 Jun 2024 13:15:06 +0100 Subject: [PATCH 8/9] lxd-agent: Ignore linter complaints about deep exit In this case we need it to ensure non-zero exit code is returned. Signed-off-by: Thomas Parrott --- lxd-agent/main.go | 3 ++- lxd-agent/main_agent.go | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lxd-agent/main.go b/lxd-agent/main.go index 4f023a8605e3..54b24d5b6953 100644 --- a/lxd-agent/main.go +++ b/lxd-agent/main.go @@ -45,6 +45,7 @@ func main() { // Run the main command and handle errors err := app.Execute() if err != nil { - os.Exit(1) + // Ensure we exit with a non-zero exit code. + os.Exit(1) //nolint:revive } } diff --git a/lxd-agent/main_agent.go b/lxd-agent/main_agent.go index 4568ac11e116..db9750663808 100644 --- a/lxd-agent/main_agent.go +++ b/lxd-agent/main_agent.go @@ -51,7 +51,8 @@ func (c *cmdAgent) Run(cmd *cobra.Command, args []string) error { // Setup logger. err := logger.InitLogger("", "lxd-agent", c.global.flagLogVerbose, c.global.flagLogDebug, nil) if err != nil { - os.Exit(1) + // Ensure we exit with a non-zero exit code. + os.Exit(1) //nolint:revive } logger.Info("Starting") @@ -196,7 +197,8 @@ func (c *cmdAgent) Run(cmd *cobra.Command, args []string) error { cancelStatusNotifier() // Ensure STOPPED status is written to QEMU status ringbuffer. cancelFunc() - os.Exit(exitStatus) + // Ensure we exit with a relevant exit code. + os.Exit(exitStatus) //nolint:revive return nil } From 28bd6a9a6d9a9552dc9802f21139987de070ae37 Mon Sep 17 00:00:00 2001 From: Thomas Parrott Date: Thu, 27 Jun 2024 13:58:01 +0100 Subject: [PATCH 9/9] lxd-agent: Log start time mount args on error Signed-off-by: Thomas Parrott --- lxd-agent/main_agent.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lxd-agent/main_agent.go b/lxd-agent/main_agent.go index db9750663808..8dd737730d92 100644 --- a/lxd-agent/main_agent.go +++ b/lxd-agent/main_agent.go @@ -324,7 +324,7 @@ func (c *cmdAgent) mountHostShares() { _, err = shared.RunCommand("mount", args...) if err != nil { - l.Error("Failed to mount", logger.Ctx{"err": err}) + l.Error("Failed to mount", logger.Ctx{"err": err, "args": args}) continue }