Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: async (background) mount may cause problems #1088

Merged
merged 30 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
75157cc
Adding cli flag to wait after daemonization to ensure child process h…
vibhansa-msft Mar 17, 2023
bec96f5
update
vibhansa-msft Mar 17, 2023
dbac12c
spell correction
vibhansa-msft Mar 17, 2023
be68a5a
Update readme for new cli option
vibhansa-msft Mar 17, 2023
7739601
Correct cli help
vibhansa-msft Mar 17, 2023
8fc9110
fix : mount
cvvz Mar 18, 2023
1494afd
fix
cvvz Mar 18, 2023
9517925
remove timeout
cvvz Mar 18, 2023
84d7f70
fix
cvvz Mar 20, 2023
637f3a3
fix
cvvz Mar 20, 2023
2c817e8
format
cvvz Mar 20, 2023
e6f13bc
make no difference
cvvz Mar 20, 2023
5b93098
do not exit when creating tmp file failed
cvvz Mar 20, 2023
f04ea61
fix
cvvz Mar 20, 2023
597f808
fix
cvvz Mar 20, 2023
3b1f155
fix
cvvz Mar 20, 2023
67d9fd0
Adding wait for mount as an alternative
vibhansa-msft Mar 20, 2023
5ee5f7c
Merge branch 'vibhansa/v2/waitformount' of https://github.com/Azure/a…
cvvz Mar 20, 2023
8d277d1
fix signal
cvvz Mar 20, 2023
4007661
fix for-select code
cvvz Mar 20, 2023
1c304b8
fix errcheck
cvvz Mar 20, 2023
4c9ab71
Use PIPE for communication between child and parent
cvvz Mar 21, 2023
f2e1eac
Correcting the code
vibhansa-msft Mar 21, 2023
d3ac13a
Updating changlog
vibhansa-msft Mar 21, 2023
7a59567
Sync with main
vibhansa-msft Mar 21, 2023
baf70b2
Using daemon context logfile to redirect child's stderr to file
vibhansa-msft Mar 21, 2023
1fd148d
Rolling back go-daemon to standard version
vibhansa-msft Mar 21, 2023
a63ea54
Adding step to rhel 7.5 pipeline to run
vibhansa-msft Mar 21, 2023
62fcf3f
Correcting nightly
vibhansa-msft Mar 21, 2023
c5a23e0
Send signal only if mount was done in background
vibhansa-msft Mar 21, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**
Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
56 changes: 51 additions & 5 deletions cmd/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"`
Expand Down Expand Up @@ -394,29 +397,69 @@ var mountCmd = &cobra.Command{
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,
}

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
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
log.Info("mount: Child [%v] terminated from %s", child.Pid, options.MountPath)
if dmnCtx.LogFileName != "" {
vibhansa-msft marked this conversation as resolved.
Show resolved Hide resolved
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)
}
}
} else {
Expand All @@ -441,6 +484,7 @@ var mountCmd = &cobra.Command{
if err != nil {
return err
}

if options.MemProfile != "" {
os.Remove(options.MemProfile)
f, err := os.Create(options.MemProfile)
Expand Down Expand Up @@ -638,6 +682,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))
Expand Down
15 changes: 15 additions & 0 deletions common/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (
"strconv"
"strings"
"sync"
"syscall"

"gopkg.in/ini.v1"
)
Expand Down Expand Up @@ -265,3 +266,17 @@ func ExpandPath(path string) string {

return os.ExpandEnv(path)
}

// NotifyMountToParent : Send a signal to parent process about successful mount
func NotifyMountToParent() error {
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)
cvvz marked this conversation as resolved.
Show resolved Hide resolved
}

return nil
}
24 changes: 0 additions & 24 deletions component/libfuse/libfuse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
Loading