Fix: async (background) mount may cause problems

CHANGELOG.md

@@ -2,6 +2,9 @@
 **Bug Fixes**
 - [#1080](https://github.com/Azure/azure-storage-fuse/issues/1080) HNS rename flow does not encode source path correctly.
 - [#1081](https://github.com/Azure/azure-storage-fuse/issues/1081) Blobfuse will exit with non-zero status code if allow_other option is used but not enabled in fuse config.
+- [#1079](https://github.com/Azure/azure-storage-fuse/issues/1079) Shell returns before child process mounts the container and if user tries to bind the mount it leads to inconsistent state.
+- If mount fails in forked child, blobfuse2 will return back with status error code.
+
 
 ## 2.0.2 (2022-02-23)
 **Bug Fixes**

README.md

@@ -99,8 +99,9 @@ To learn about a specific command, just include the name of the command (For exa
     * `--read-only=true`: Mount container in read-only mode.
     * `--default-working-dir`: The default working directory to store log files and other blobfuse2 related information.
     * `--disable-version-check=true`: Disable the blobfuse2 version check.
-    * `----secure-config=true` : Config file is encrypted suing 'blobfuse2 secure` command.
-    * `----passphrase=<STRING>` : Passphrase used to encrypt/decrypt config file.
+    * `--secure-config=true` : Config file is encrypted suing 'blobfuse2 secure` command.
+    * `--passphrase=<STRING>` : Passphrase used to encrypt/decrypt config file.
+    * `--wait-for-mount=<TIMEOUT IN SECONDS>` : Let parent process wait for given timeout before exit to ensure child has started. 
 - Attribute cache options
     * `--attr-cache-timeout=<TIMEOUT IN SECONDS>`: The timeout for the attribute cache entries.
     * `--no-symlinks=true`: To improve performance disable symlink support.

blobfuse2-nightly.yaml

@@ -650,7 +650,8 @@ stages:
                       sudo yum update -y
                       sudo yum groupinstall "Development Tools" -y
                       if [ $(AgentName) == "blobfuse-rhel7_5" ]; then
-                        sudo yum install git fuse fuse3-libs fuse3-devel fuse3 rh-python36 -y
+                        yum-config-manager --save --setopt=rhui-rhel-7-server-dotnet-rhui-rpms.skip_if_unavailable=true
+                        sudo yum install git fuse fuse3-libs fuse3-devel fuse3 rh-python36 -y --nobest --allowerasing
                       else
                         sudo yum install git fuse fuse3-libs fuse3-devel fuse3 python36 -y --nobest --allowerasing
                       fi

cmd/mount.go

@@ -42,13 +42,15 @@ import (
 	"net/http"
 	_ "net/http/pprof"
 	"os"
+	"os/signal"
 	"path/filepath"
 	"runtime"
 	"runtime/debug"
 	"runtime/pprof"
 	"strconv"
 	"strings"
 	"syscall"
+	"time"
 
 	"github.com/Azure/azure-storage-fuse/v2/common"
 	"github.com/Azure/azure-storage-fuse/v2/common/config"
@@ -85,6 +87,7 @@ type mountOptions struct {
 	ProfilerPort      int            `config:"profiler-port"`
 	ProfilerIP        string         `config:"profiler-ip"`
 	MonitorOpt        monitorOptions `config:"health_monitor"`
+	WaitForMount      time.Duration  `config:"wait-for-mount"`
 
 	// v1 support
 	Streaming      bool     `config:"streaming"`
@@ -390,34 +393,79 @@ var mountCmd = &cobra.Command{
 			return Destroy(fmt.Sprintf("failed to initialize new pipeline [%s]", err.Error()))
 		}
 
+		common.ForegroundMount = options.Foreground
+
 		log.Info("mount: Mounting blobfuse2 on %s", options.MountPath)
 		if !options.Foreground {
 			pidFile := strings.Replace(options.MountPath, "/", "_", -1) + ".pid"
 			pidFileName := filepath.Join(os.ExpandEnv(common.DefaultWorkDir), pidFile)
+
+			pid := os.Getpid()
+			fname := fmt.Sprintf("/tmp/blobfuse2.%v", pid)
+
 			dmnCtx := &daemon.Context{
 				PidFileName: pidFileName,
 				PidFilePerm: 0644,
 				Umask:       022,
+				LogFileName: fname, // this will redirect stderr of child to given file
 			}
 
 			ctx, _ := context.WithCancel(context.Background()) //nolint
-			daemon.SetSigHandler(sigusrHandler(pipeline, ctx), syscall.SIGUSR1, syscall.SIGUSR2)
+
+			// Signal handlers for parent and child to communicate success or failures in mount
+			var sigusr2, sigchild chan os.Signal
+			if !daemon.WasReborn() { // execute in parent only
+				sigusr2 = make(chan os.Signal, 1)
+				signal.Notify(sigusr2, syscall.SIGUSR2)
+
+				sigchild = make(chan os.Signal, 1)
+				signal.Notify(sigchild, syscall.SIGCHLD)
+			} else { // execute in child only
+				daemon.SetSigHandler(sigusrHandler(pipeline, ctx), syscall.SIGUSR1, syscall.SIGUSR2)
+				go func() {
+					_ = daemon.ServeSignals()
+				}()
+			}
+
 			child, err := dmnCtx.Reborn()
 			if err != nil {
 				log.Err("mount : failed to daemonize application [%v]", err)
 				return Destroy(fmt.Sprintf("failed to daemonize application [%s]", err.Error()))
 			}
 
 			log.Debug("mount: foreground disabled, child = %v", daemon.WasReborn())
-			if child == nil {
+			if child == nil { // execute in child only
 				defer dmnCtx.Release() // nolint
 				setGOConfig()
 				go startDynamicProfiler()
 
-				err = runPipeline(pipeline, ctx)
-				if err != nil {
-					return err
+				// In case of failure stderr will have the error emitted by child and parent will read
+				// those logs from the file set in daemon context
+				return runPipeline(pipeline, ctx)
+			} else { // execute in parent only
+				defer os.Remove(fname)
+
+				select {
+				case <-sigusr2:
+					log.Info("mount: Child [%v] mounted successfully at %s", child.Pid, options.MountPath)
+
+				case <-sigchild:
+					// Get error string from the child, stderr or child was redirected to a file
+					log.Info("mount: Child [%v] terminated from %s", child.Pid, options.MountPath)
+
+					buff, err := ioutil.ReadFile(dmnCtx.LogFileName)
+					if err != nil {
+						log.Err("mount: failed to read child [%v] failure logs [%s]", child.Pid, err.Error())
+						return Destroy(fmt.Sprintf("failed to mount, please check logs [%s]", err.Error()))
+					} else {
+						return Destroy(string(buff))
+					}
+
+				case <-time.After(options.WaitForMount):
+					log.Info("mount: Child [%v : %s] status check timeout", child.Pid, options.MountPath)
 				}
+
+				_ = log.Destroy()
 			}
 		} else {
 			if options.CPUProfile != "" {
@@ -441,6 +489,7 @@ var mountCmd = &cobra.Command{
 			if err != nil {
 				return err
 			}
+
 			if options.MemProfile != "" {
 				os.Remove(options.MemProfile)
 				f, err := os.Create(options.MemProfile)
@@ -638,6 +687,8 @@ func init() {
 	config.BindPFlag("libfuse-options", mountCmd.PersistentFlags().ShorthandLookup("o"))
 	mountCmd.PersistentFlags().ShorthandLookup("o").Hidden = true
 
+	mountCmd.PersistentFlags().DurationVar(&options.WaitForMount, "wait-for-mount", 5*time.Second, "Let parent process wait for given timeout before exit")
+
 	config.AttachToFlagSet(mountCmd.PersistentFlags())
 	config.AttachFlagCompletions(mountCmd)
 	config.AddConfigChangeEventListener(config.ConfigChangeEventHandlerFunc(OnConfigChange))

common/util.go

@@ -47,11 +47,13 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"syscall"
 
 	"gopkg.in/ini.v1"
 )
 
 var RootMount bool
+var ForegroundMount bool
 
 //IsDirectoryMounted is a utility function that returns true if the directory is already mounted using fuse
 func IsDirectoryMounted(path string) bool {
@@ -265,3 +267,19 @@ func ExpandPath(path string) string {
 
 	return os.ExpandEnv(path)
 }
+
+// NotifyMountToParent : Send a signal to parent process about successful mount
+func NotifyMountToParent() error {
+	if !ForegroundMount {
+		ppid := syscall.Getppid()
+		if ppid > 1 {
+			if err := syscall.Kill(ppid, syscall.SIGUSR2); err != nil {
+				return err
+			}
+		} else {
+			return fmt.Errorf("failed to get parent pid, received : %v", ppid)
+		}
+	}
+
+	return nil
+}

component/libfuse/libfuse.go

@@ -34,12 +34,8 @@
 package libfuse
 
 import (
-	"bytes"
 	"context"
 	"fmt"
-	"os"
-	"os/exec"
-	"strings"
 
 	"github.com/Azure/azure-storage-fuse/v2/common"
 	"github.com/Azure/azure-storage-fuse/v2/common/config"
@@ -208,26 +204,6 @@ func (lf *Libfuse) Validate(opt *LibfuseOptions) error {
 		lf.negativeTimeout = defaultNegativeEntryExpiration
 	}
 
-	// When root user mounts, allow_other works even if its not set in /etc/fuse.conf file
-	// for other user libfuse init will fail. Adding a pre-condition here for validation
-	if lf.allowOther && 0 != os.Getuid() && 0 != os.Getgid() {
-		cmd := exec.Command("bash", "-c", "cat /etc/fuse.conf | grep -i user_allow_other$")
-
-		var errb bytes.Buffer
-		cmd.Stderr = &errb
-		cliOut, err := cmd.Output()
-		data := string(cliOut)
-		if err == nil {
-			data = strings.TrimSpace(data)
-			if data == "" || data[0] == '#' {
-				log.Err("Libfuse::Validate : config error [user_allow_other is not enabled in /etc/fuse.conf]")
-				return fmt.Errorf("user_allow_other is not enabled in /etc/fuse.conf")
-			}
-		} else {
-			log.Err("Libfuse::Validate : failed to check fuse config file [%s]", err.Error())
-		}
-	}
-
 	var err error
 	lf.ownerUID, lf.ownerGID, err = common.GetCurrentUser()
 	if err != nil {