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

Add support for custom rclone options and fix defuct processes #85

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 5 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,17 @@ VERSION ?= dev
IMAGE_TAG=$(REGISTRY_NAME)/$(IMAGE_NAME):$(VERSION)
FULL_IMAGE_TAG=$(IMAGE_TAG)-full
TEST_IMAGE_TAG=$(REGISTRY_NAME)/$(IMAGE_NAME):test
TAGS ?= "all"

build:
CGO_ENABLED=0 GOOS=linux go build -a -ldflags '-extldflags "-static"' -o _output/s3driver ./cmd/s3driver
CGO_ENABLED=0 GOOS=linux go build --tags $(TAGS) -a -ldflags '-extldflags "-static"' -o _output/s3driver ./cmd/s3driver
test:
docker build -t $(FULL_IMAGE_TAG) -f cmd/s3driver/Dockerfile.full .
docker build -t $(TEST_IMAGE_TAG) -f test/Dockerfile .
docker build -t $(TEST_IMAGE_TAG) --build-arg tags=$(TAGS) -f test/Dockerfile .
docker run --rm --privileged -v $(PWD):$(PROJECT_DIR) --device /dev/fuse $(TEST_IMAGE_TAG)
container:
docker build -t $(IMAGE_TAG) -f cmd/s3driver/Dockerfile .
docker build -t $(FULL_IMAGE_TAG) -f cmd/s3driver/Dockerfile.full .
docker build -t $(IMAGE_TAG) --build-arg TAGS=$(TAGS) -f cmd/s3driver/Dockerfile .
docker build -t $(FULL_IMAGE_TAG) --build-arg TAGS=$(TAGS) -f cmd/s3driver/Dockerfile.full .
push: container
docker push $(IMAGE_TAG)
docker push $(FULL_IMAGE_TAG)
Expand Down
7 changes: 4 additions & 3 deletions cmd/s3driver/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
FROM golang:1.16-alpine as gobuild
FROM golang:1.18-alpine as gobuild

ARG TAGS=all
WORKDIR /build
ADD . /build

RUN go get -d -v ./...
RUN CGO_ENABLED=0 GOOS=linux go build -a -ldflags '-extldflags "-static"' -o ./s3driver ./cmd/s3driver
RUN CGO_ENABLED=0 GOOS=linux go build --tags ${TAGS} -a -ldflags '-extldflags "-static"' -o ./s3driver ./cmd/s3driver

FROM debian:buster-slim
LABEL maintainers="Cyrill Troxler <[email protected]>"
Expand All @@ -17,7 +18,7 @@ RUN apt-get update && \
rm -rf /var/lib/apt/lists/*

# install rclone
ARG RCLONE_VERSION=v1.54.1
ARG RCLONE_VERSION=v1.60.1
RUN cd /tmp \
&& curl -O https://downloads.rclone.org/${RCLONE_VERSION}/rclone-${RCLONE_VERSION}-linux-amd64.zip \
&& unzip /tmp/rclone-${RCLONE_VERSION}-linux-amd64.zip \
Expand Down
8 changes: 5 additions & 3 deletions cmd/s3driver/Dockerfile.full
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
FROM golang:1.16-alpine as gobuild
FROM golang:1.18-alpine as gobuild

WORKDIR /build
ADD . /build

ARG TAGS=all

RUN go get -d -v ./...
RUN CGO_ENABLED=0 GOOS=linux go build -a -ldflags '-extldflags "-static"' -o ./s3driver ./cmd/s3driver
RUN CGO_ENABLED=0 GOOS=linux go build --tags ${TAGS} -a -ldflags '-extldflags "-static"' -o ./s3driver ./cmd/s3driver

FROM debian:buster-slim as s3backer
ARG S3BACKER_VERSION=1.5.0
Expand Down Expand Up @@ -45,7 +47,7 @@ RUN apt-get update && \
rm -rf /var/lib/apt/lists/*

# install rclone
ARG RCLONE_VERSION=v1.54.1
ARG RCLONE_VERSION=v1.60.1
RUN cd /tmp \
&& curl -O https://downloads.rclone.org/${RCLONE_VERSION}/rclone-${RCLONE_VERSION}-linux-amd64.zip \
&& unzip /tmp/rclone-${RCLONE_VERSION}-linux-amd64.zip \
Expand Down
34 changes: 34 additions & 0 deletions cmd/s3driver/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"flag"
"log"
"os"
"os/signal"
"syscall"

"github.com/ctrox/csi-s3/pkg/driver"
)
Expand All @@ -36,6 +38,38 @@ var (
func main() {
flag.Parse()

// We're running in the container as PID-1 which gets some special
// treatment by the kernel. In particular, if a process in the container
// terminates and there are still active child processes, the kernel will move
// those orphaned processes to be child processes of PID-1 and signal it
// by sending a SIGCHLD. Init-systems are expected to handle this case by
// reaping those "orphan" processes once they exit.
//
// Since all available mounters are instructed to daemonize, we need to reap
// the daemonized processes since their parent (the mounter) exists once the daemon
// is running.
go func() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this resolution. You assume you're running as PID-1 which you never check. And you globally reap all children which would mess up any code waiting for a process to exit. IMO the proper resolution would be to reap the child after you signal it to exit.

ch := make(chan os.Signal, 1)

signal.Notify(ch, syscall.SIGCHLD)

for range ch {
var status syscall.WaitStatus
pid, err := syscall.Wait4(-1, &status, 0, nil)
if err != nil {
// we might receive ECHILD when the mounter exits after daemonizing.
// We'll be late calling Wait4 here as that process is already reaped
// since we're using exec.Command().Run() which already calls Waitpid
if val, ok := err.(syscall.Errno); !ok || val != syscall.ECHILD {
log.Printf("failed to call wait4: %s\n", err)
}

} else {
log.Printf("repeated child %d: status=%d\n", pid, status.ExitStatus())
}
}
}()

driver, err := driver.New(*nodeID, *endpoint)
if err != nil {
log.Fatal(err)
Expand Down
8 changes: 8 additions & 0 deletions pkg/driver/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,14 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
FSPath: defaultFsPath,
}

meta.MounterOptions = make(map[string]string)
optionPrefix := meta.Mounter + "-"
for key, value := range params {
if strings.HasPrefix(key, optionPrefix) {
meta.MounterOptions[strings.TrimPrefix(key, optionPrefix)] = value
}
}

client, err := s3.NewClientFromSecret(req.GetSecrets())
if err != nil {
return nil, fmt.Errorf("failed to initialize S3 client: %s", err)
Expand Down
2 changes: 2 additions & 0 deletions pkg/driver/driver_suite_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// go:build all

package driver_test

import (
Expand Down
11 changes: 11 additions & 0 deletions pkg/mounter/goofys.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//go:build all || goofys

package mounter

import (
Expand Down Expand Up @@ -26,12 +28,21 @@ type goofysMounter struct {
secretAccessKey string
}

func init() {
registerMounter(goofysMounterType, newGoofysMounter)
}

func newGoofysMounter(meta *s3.FSMeta, cfg *s3.Config) (Mounter, error) {
region := cfg.Region
// if endpoint is set we need a default region
if region == "" && cfg.Endpoint != "" {
region = defaultRegion
}

if len(meta.MounterOptions) > 0 {
return nil, fmt.Errorf("custom mount options are not supported for goofys")
}

return &goofysMounter{
meta: meta,
endpoint: cfg.Endpoint,
Expand Down
40 changes: 23 additions & 17 deletions pkg/mounter/mounter.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ type Mounter interface {
Mount(source string, target string) error
}

type FactoryFunc func(*s3.FSMeta, *s3.Config) (Mounter, error)

var mounters map[string]FactoryFunc

const (
s3fsMounterType = "s3fs"
goofysMounterType = "goofys"
Expand All @@ -35,40 +39,42 @@ const (
UsePrefix = "usePrefix"
)

func registerMounter(name string, fn FactoryFunc) {
if mounters == nil {
mounters = make(map[string]FactoryFunc)
}
mounters[name] = fn
}

// New returns a new mounter depending on the mounterType parameter
func New(meta *s3.FSMeta, cfg *s3.Config) (Mounter, error) {
mounter := meta.Mounter
// Fall back to mounterType in cfg
if len(meta.Mounter) == 0 {
mounter = cfg.Mounter
}
switch mounter {
case s3fsMounterType:
return newS3fsMounter(meta, cfg)

case goofysMounterType:
return newGoofysMounter(meta, cfg)

case s3backerMounterType:
return newS3backerMounter(meta, cfg)

case rcloneMounterType:
return newRcloneMounter(meta, cfg)

default:
// default to s3backer
return newS3backerMounter(meta, cfg)
fn, ok := mounters[mounter]
if !ok {
return nil, fmt.Errorf("no mounter with name %s available", mounter)
}

return fn(meta, cfg)
}

func fuseMount(path string, command string, args []string) error {
cmd := exec.Command(command, args...)
glog.V(3).Infof("Mounting fuse with command: %s and args: %s", command, args)
glog.V(3).Infof("Mounting fuse with command: %s and args: %#v", command, args)

cmd.SysProcAttr = &syscall.SysProcAttr{
Setsid: true,
}

cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr

if err := cmd.Run(); err != nil {
return fmt.Errorf("Error fuseMount command: %s\nargs: %s\noutput", command, args)
return fmt.Errorf("Error fuseMount command: %s\nargs: %#v\nerror: %s", command, args, err.Error())
}

return waitForMount(path, 10*time.Second)
Expand Down
22 changes: 20 additions & 2 deletions pkg/mounter/rclone.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//go:build all || rclone

package mounter

import (
Expand All @@ -15,19 +17,31 @@ type rcloneMounter struct {
region string
accessKeyID string
secretAccessKey string
customOptions []string
}

const (
rcloneCmd = "rclone"
)

func init() {
registerMounter(rcloneMounterType, newRcloneMounter)
}

func newRcloneMounter(meta *s3.FSMeta, cfg *s3.Config) (Mounter, error) {
customOptions := make([]string, 0, len(meta.MounterOptions))

for key, value := range meta.MounterOptions {
customOptions = append(customOptions, fmt.Sprintf("--%s=%s", key, value))
}

return &rcloneMounter{
meta: meta,
url: cfg.Endpoint,
region: cfg.Region,
accessKeyID: cfg.AccessKeyID,
secretAccessKey: cfg.SecretAccessKey,
customOptions: customOptions,
}, nil
}

Expand All @@ -43,16 +57,20 @@ func (rclone *rcloneMounter) Mount(source string, target string) error {
args := []string{
"mount",
fmt.Sprintf(":s3:%s", path.Join(rclone.meta.BucketName, rclone.meta.Prefix, rclone.meta.FSPath)),
fmt.Sprintf("%s", target),
target,
"--daemon",
"--s3-provider=AWS",
"--s3-env-auth=true",
fmt.Sprintf("--s3-region=%s", rclone.region),
fmt.Sprintf("--s3-endpoint=%s", rclone.url),
"--allow-other",
// TODO: make this configurable
"--vfs-cache-mode=writes",
}

// append any custom rclone options. Later parameters take precedence so
// the user can overwrite the defaults from above (i.e. --allow-other=false)
args = append(args, rclone.customOptions...)

os.Setenv("AWS_ACCESS_KEY_ID", rclone.accessKeyID)
os.Setenv("AWS_SECRET_ACCESS_KEY", rclone.secretAccessKey)
return fuseMount(target, rcloneCmd, args)
Expand Down
11 changes: 11 additions & 0 deletions pkg/mounter/s3backer.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//go:build all || s3backer

package mounter

import (
Expand All @@ -24,6 +26,10 @@ type s3backerMounter struct {
ssl bool
}

func init() {
registerMounter(s3backerMounterType, newS3backerMounter)
}

const (
s3backerCmd = "s3backer"
s3backerFsType = "xfs"
Expand All @@ -45,6 +51,11 @@ func newS3backerMounter(meta *s3.FSMeta, cfg *s3.Config) (Mounter, error) {
if meta.CapacityBytes == 0 {
meta.CapacityBytes = s3backerDefaultSize
}

if len(meta.MounterOptions) > 0 {
return nil, fmt.Errorf("custom mount options are not supported for s3backer")
}

s3backer := &s3backerMounter{
meta: meta,
url: cfg.Endpoint,
Expand Down
10 changes: 10 additions & 0 deletions pkg/mounter/s3fs.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//go:build all || s3fs

package mounter

import (
Expand All @@ -20,7 +22,15 @@ const (
s3fsCmd = "s3fs"
)

func init() {
registerMounter(s3fsMounterType, newS3fsMounter)
}

func newS3fsMounter(meta *s3.FSMeta, cfg *s3.Config) (Mounter, error) {
if len(meta.MounterOptions) > 0 {
return nil, fmt.Errorf("custom mount options are not supported for s3fs")
}

return &s3fsMounter{
meta: meta,
url: cfg.Endpoint,
Expand Down
20 changes: 11 additions & 9 deletions pkg/s3/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/golang/glog"
"github.com/minio/minio-go/v7"
"github.com/minio/minio-go/v7/pkg/credentials"
"io"
"net/url"
"path"

"github.com/golang/glog"
"github.com/minio/minio-go/v7"
"github.com/minio/minio-go/v7/pkg/credentials"
)

const (
Expand All @@ -33,12 +34,13 @@ type Config struct {
}

type FSMeta struct {
BucketName string `json:"Name"`
Prefix string `json:"Prefix"`
UsePrefix bool `json:"UsePrefix"`
Mounter string `json:"Mounter"`
FSPath string `json:"FSPath"`
CapacityBytes int64 `json:"CapacityBytes"`
BucketName string `json:"Name"`
Prefix string `json:"Prefix"`
UsePrefix bool `json:"UsePrefix"`
Mounter string `json:"Mounter"`
FSPath string `json:"FSPath"`
CapacityBytes int64 `json:"CapacityBytes"`
MounterOptions map[string]string `json:"MounterOptions"`
}

func NewClient(cfg *Config) (*s3Client, error) {
Expand Down