From 18ec933085d65c1b4ee4cbb083ff146a8282a9fd Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 22 Feb 2021 10:32:21 -0800 Subject: [PATCH] fix: for containers use root-disk detection cleverly (#11593) root-disk implemented currently had issues where root disk partitions getting modified might race and provide incorrect results, to avoid this lets rely again back on DeviceID and match it instead. In-case of containers `/data` is one such extra entity that needs to be verified for root disk, due to how 'overlay' filesystem works and the 'overlay' presents a completely different 'device' id - using `/data` as another entity for fallback helps because our containers describe 'VOLUME' parameter that allows containers to automatically have a virtual `/data` that points to the container root path this can either be at `/` or `/var/lib/` (on different partition) --- .dockerignore | 8 ++++++++ Dockerfile.dev | 9 +++------ Makefile | 3 +-- cmd/xl-storage.go | 32 +++++++++++++++++++++++++++++--- pkg/disk/disk.go | 18 ------------------ pkg/disk/disk_unix.go | 39 +++++++++++++++++++++++++++++++++++++++ pkg/disk/disk_windows.go | 24 ++++++++++++++++++++++++ pkg/disk/root_disk.go | 10 +--------- 8 files changed, 105 insertions(+), 38 deletions(-) create mode 100644 pkg/disk/disk_unix.go create mode 100644 pkg/disk/disk_windows.go diff --git a/.dockerignore b/.dockerignore index f4b11987f..f740e3e3d 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,2 +1,10 @@ .git .github +docs +CREDITS +default.etcd +browser +*.gz +*.tar.gz +*.bzip2 +*.zip diff --git a/Dockerfile.dev b/Dockerfile.dev index cc3496718..1ec4fe57a 100644 --- a/Dockerfile.dev +++ b/Dockerfile.dev @@ -6,8 +6,6 @@ LABEL maintainer="MinIO Inc " COPY dockerscripts/docker-entrypoint.sh /usr/bin/ COPY minio /usr/bin/ -COPY CREDITS /licenses/CREDITS -COPY LICENSE /licenses/LICENSE ENV MINIO_UPDATE=off \ MINIO_ACCESS_KEY_FILE=access_key \ @@ -17,10 +15,9 @@ ENV MINIO_UPDATE=off \ MINIO_KMS_MASTER_KEY_FILE=kms_master_key \ MINIO_SSE_MASTER_KEY_FILE=sse_master_key -RUN \ - microdnf update --nodocs && \ - microdnf install curl ca-certificates shadow-utils util-linux --nodocs && \ - microdnf clean all && \ +RUN microdnf update --nodocs +RUN microdnf install curl ca-certificates shadow-utils util-linux --nodocs +RUN microdnf clean all && \ chmod +x /usr/bin/minio && \ chmod +x /usr/bin/docker-entrypoint.sh diff --git a/Makefile b/Makefile index dbc709322..52ebf4766 100644 --- a/Makefile +++ b/Makefile @@ -81,9 +81,8 @@ docker-hotfix: hotfix checks @echo "Building minio docker image '$(TAG)'" @docker build -t $(TAG) . -f Dockerfile.dev -docker: checks +docker: build checks @echo "Building minio docker image '$(TAG)'" - @GOOS=linux GO111MODULE=on CGO_ENABLED=0 go build -tags kqueue -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null @docker build -t $(TAG) . -f Dockerfile.dev # Builds minio and installs it to $GOPATH/bin. diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go index 04944d38a..41f57cf49 100644 --- a/cmd/xl-storage.go +++ b/cmd/xl-storage.go @@ -217,9 +217,35 @@ func newXLStorage(ep Endpoint) (*xlStorage, error) { if env.Get("MINIO_CI_CD", "") != "" { rootDisk = true } else { - rootDisk, err = disk.IsRootDisk(path, "/") - if err != nil { - return nil, err + if IsDocker() || IsKubernetes() { + // Start with overlay "/" to check if + // possible the path has device id as + // "overlay" that would mean the path + // is emphemeral and we should treat it + // as root disk from the baremetal + // terminology. + rootDisk, err = disk.IsRootDisk(path, "/") + if err != nil { + return nil, err + } + if !rootDisk { + // No root disk was found, its possible that + // path is referenced at "/data" which has + // different device ID that points to the original + // "/" on the host system, fall back to that instead + // to verify of the device id is same. + rootDisk, err = disk.IsRootDisk(path, "/data") + if err != nil { + return nil, err + } + } + + } else { + // On baremetal setups its always "/" is the root disk. + rootDisk, err = disk.IsRootDisk(path, "/") + if err != nil { + return nil, err + } } } diff --git a/pkg/disk/disk.go b/pkg/disk/disk.go index 1ffc15882..5d326d5d7 100644 --- a/pkg/disk/disk.go +++ b/pkg/disk/disk.go @@ -30,21 +30,3 @@ type Info struct { Ffree uint64 FSType string } - -// SameDisk reports whether di1 and di2 describe the same disk. -func SameDisk(di1, di2 Info) bool { - if di1.Total != di2.Total { - // disk total size different - return false - } - - if di1.Files != di2.Files { - // disk total inodes different - return false - } - - // returns true only if Used, Free are same, then its the same disk. - // we are deliberately not using free inodes as that is unreliable - // due the fact that Ffree can vary even for temporary files - return di1.Used == di2.Used && di1.Free == di2.Free -} diff --git a/pkg/disk/disk_unix.go b/pkg/disk/disk_unix.go new file mode 100644 index 000000000..a40fe86cb --- /dev/null +++ b/pkg/disk/disk_unix.go @@ -0,0 +1,39 @@ +// +build !windows + +/* + * MinIO Cloud Storage, (C) 2021 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package disk + +import ( + "syscall" +) + +// SameDisk reports whether di1 and di2 describe the same disk. +func SameDisk(disk1, disk2 string) (bool, error) { + st1 := syscall.Stat_t{} + st2 := syscall.Stat_t{} + + if err := syscall.Stat(disk1, &st1); err != nil { + return false, err + } + + if err := syscall.Stat(disk2, &st2); err != nil { + return false, err + } + + return st1.Dev == st2.Dev, nil +} diff --git a/pkg/disk/disk_windows.go b/pkg/disk/disk_windows.go new file mode 100644 index 000000000..ca22b774c --- /dev/null +++ b/pkg/disk/disk_windows.go @@ -0,0 +1,24 @@ +// +build windows + +/* + * MinIO Cloud Storage, (C) 2021 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package disk + +// SameDisk reports whether di1 and di2 describe the same disk. +func SameDisk(disk1, disk2 string) (bool, error) { + return false, nil +} diff --git a/pkg/disk/root_disk.go b/pkg/disk/root_disk.go index 8e1b91778..9b16f23ee 100644 --- a/pkg/disk/root_disk.go +++ b/pkg/disk/root_disk.go @@ -25,13 +25,5 @@ func IsRootDisk(diskPath string, rootDisk string) (bool, error) { // On windows this function is not implemented. return false, nil } - info, err := GetInfo(diskPath) - if err != nil { - return false, err - } - rootInfo, err := GetInfo(rootDisk) - if err != nil { - return false, err - } - return SameDisk(info, rootInfo), nil + return SameDisk(diskPath, rootDisk) }