Merge pull request #3095 from stgraber/main

Small fixes and refactorings

client/incus_instances.go

@@ -1650,8 +1650,8 @@ func (r *ProtocolIncus) DeleteInstanceFile(instanceName string, filePath string)
 	return nil
 }
 
-// rawSFTPConn connects to the apiURL, upgrades to an SFTP raw connection and returns it.
-func (r *ProtocolIncus) rawSFTPConn(apiURL *url.URL) (net.Conn, error) {
+// rawConn connects to the apiURL, upgrades to the requested protocol and returns it.
+func (r *ProtocolIncus) rawConn(apiURL *url.URL, protocol string) (net.Conn, error) {
 	// Get the HTTP transport.
 	httpTransport, err := r.getUnderlyingHTTPTransport()
 	if err != nil {
@@ -1668,7 +1668,7 @@ func (r *ProtocolIncus) rawSFTPConn(apiURL *url.URL) (net.Conn, error) {
 		Host:       apiURL.Host,
 	}
 
-	req.Header["Upgrade"] = []string{"sftp"}
+	req.Header["Upgrade"] = []string{protocol}
 	req.Header["Connection"] = []string{"Upgrade"}
 
 	r.addClientHeaders(req)
@@ -1711,7 +1711,7 @@ func (r *ProtocolIncus) rawSFTPConn(apiURL *url.URL) (net.Conn, error) {
 		}
 	}
 
-	if resp.Header.Get("Upgrade") != "sftp" {
+	if resp.Header.Get("Upgrade") != protocol {
 		return nil, errors.New("Missing or unexpected Upgrade header in response")
 	}
 
@@ -1725,7 +1725,7 @@ func (r *ProtocolIncus) GetInstanceFileSFTPConn(instanceName string) (net.Conn,
 	apiURL.Path("1.0", "instances", instanceName, "sftp")
 	r.setURLQueryAttributes(&apiURL.URL)
 
-	return r.rawSFTPConn(&apiURL.URL)
+	return r.rawConn(&apiURL.URL, "sftp")
 }
 
 // GetInstanceFileSFTP returns an SFTP connection to the instance.

client/incus_storage_volumes.go

@@ -1301,7 +1301,7 @@ func (r *ProtocolIncus) GetStoragePoolVolumeFileSFTPConn(pool string, volType st
 	u.Path("1.0", "storage-pools", pool, "volumes", volType, volName, "sftp")
 	r.setURLQueryAttributes(&u.URL)
 
-	return r.rawSFTPConn(&u.URL)
+	return r.rawConn(&u.URL, "sftp")
 }
 
 // GetStoragePoolVolumeFileSFTP returns an SFTP connection to the volume.

cmd/incusd/daemon_storage.go

@@ -26,7 +26,6 @@ import (
 func daemonStorageVolumesUnmount(s *state.State) error {
 	var storageBackups string
 	var storageImages string
-	var storageLogs string
 
 	err := s.DB.Node.Transaction(context.Background(), func(ctx context.Context, tx *db.NodeTx) error {
 		nodeConfig, err := node.ConfigLoad(ctx, tx)
@@ -36,7 +35,6 @@ func daemonStorageVolumesUnmount(s *state.State) error {
 
 		storageBackups = nodeConfig.StorageBackupsVolume()
 		storageImages = nodeConfig.StorageImagesVolume()
-		storageLogs = nodeConfig.StorageLogsVolume()
 
 		return nil
 	})
@@ -79,14 +77,6 @@ func daemonStorageVolumesUnmount(s *state.State) error {
 		}
 	}
 
-	if storageLogs != "" {
-		err := unmount("logs", storageLogs)
-		if err != nil {
-			// With the logs volume, we may be dealing with log files that are in use by Incus or dnsmasq, those will need a daemon restart.
-			logger.Warn("Unable to unmount logs storage, daemon restart required")
-		}
-	}
-
 	return nil
 }
 

cmd/incusd/instance_sftp.go

@@ -79,5 +79,5 @@ func instanceSFTPHandler(d *Daemon, r *http.Request) response.Response {
 		}
 	}
 
-	return response.SFTPResponse(r, conn)
+	return response.UpgradeResponse(r, conn, "sftp", nil)
 }

cmd/incusd/storage_volumes_sftp.go

@@ -114,5 +114,5 @@ func storagePoolVolumeTypeSFTPHandler(d *Daemon, r *http.Request) response.Respo
 		}
 	}
 
-	return response.SFTPResponse(r, conn)
+	return response.UpgradeResponse(r, conn, "sftp", nil)
 }

internal/server/instance/drivers/driver_lxc.go

@@ -3323,6 +3323,9 @@ func (d *lxc) Shutdown(timeout time.Duration) error {
 		if err != nil {
 			return err
 		}
+
+		// Wait 3s for init to be running enough to get the next signal handle.
+		time.Sleep(3 * time.Second)
 	}
 
 	ctxMap := logger.Ctx{

internal/server/response/response.go

@@ -612,28 +612,34 @@ func Unauthorized(err error) Response {
 	return &errorResponse{http.StatusUnauthorized, message}
 }
 
-// SFTPResponse upgrades the connection for sftp and connects to the backend server.
-func SFTPResponse(r *http.Request, conn net.Conn) Response {
-	return &sftpResponse{req: r, conn: conn}
+// UpgradeResponse upgrades the connection and connects to the backend server.
+func UpgradeResponse(r *http.Request, conn net.Conn, protocol string, cleanup func()) Response {
+	return &upgradeResponse{req: r, conn: conn, protocol: protocol, cleanup: cleanup}
 }
 
-type sftpResponse struct {
-	req  *http.Request
-	conn net.Conn
+type upgradeResponse struct {
+	req      *http.Request
+	conn     net.Conn
+	protocol string
+	cleanup  func()
 }
 
 // String returns the response type name.
-func (r *sftpResponse) String() string {
-	return "sftp handler"
+func (r *upgradeResponse) String() string {
+	return r.protocol + " handler"
 }
 
 // Code returns the HTTP code.
-func (r *sftpResponse) Code() int {
+func (r *upgradeResponse) Code() int {
 	return http.StatusOK
 }
 
 // Render handles the HTTP connection.
-func (r *sftpResponse) Render(w http.ResponseWriter) error {
+func (r *upgradeResponse) Render(w http.ResponseWriter) error {
+	if r.cleanup != nil {
+		defer r.cleanup()
+	}
+
 	defer func() { _ = r.conn.Close() }()
 
 	hijacker, ok := w.(http.Hijacker)
@@ -657,7 +663,7 @@ func (r *sftpResponse) Render(w http.ResponseWriter) error {
 		}
 	}
 
-	err = Upgrade(remoteConn, "sftp")
+	err = Upgrade(remoteConn, r.protocol)
 	if err != nil {
 		return api.StatusErrorf(http.StatusInternalServerError, "%s", err.Error())
 	}
@@ -675,7 +681,7 @@ func (r *sftpResponse) Render(w http.ResponseWriter) error {
 		_, err := io.Copy(remoteConn, r.conn)
 		if err != nil {
 			if ctx.Err() == nil {
-				l.Warn("Failed copying SFTP instance connection to remote connection", logger.Ctx{"err": err})
+				l.Warn("Failed copying data from local to remote connection", logger.Ctx{"err": err})
 			}
 		}
 		cancel()               // Cancel context first so when remoteConn is closed it doesn't cause a warning.
@@ -685,7 +691,7 @@ func (r *sftpResponse) Render(w http.ResponseWriter) error {
 	_, err = io.Copy(r.conn, remoteConn)
 	if err != nil {
 		if ctx.Err() == nil {
-			l.Warn("Failed copying SFTP remote connection to instance connection", logger.Ctx{"err": err})
+			l.Warn("Failed copying data from remote to local connection", logger.Ctx{"err": err})
 		}
 	}
 	cancel() // Cancel context first so when conn is closed it doesn't cause a warning.