Always check for root disks unless MINIO_CI_CD is set (#14232)

The current code considers a pool with all root disks to be as part
of a testing environment even if there are other pools with mounted
disks. This will result to illegitimate writing in root disks.

Fix this by simplifing the logic: require MINIO_CI_CD in order to skip
root disk check.
This commit is contained in:
Anis Elleuch 2022-02-14 00:42:07 +01:00 committed by GitHub
parent f71b114a84
commit 1f92fc3fc0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 51 additions and 66 deletions

View file

@ -30,6 +30,7 @@ function start_minio_16drive() {
export MC_HOST_minio="http://minio:minio123@127.0.0.1:${start_port}/"
unset MINIO_KMS_AUTO_ENCRYPTION # do not auto-encrypt objects
export _MINIO_SHARD_DISKTIME_DELTA="5s" # do not change this as its needed for tests
export MINIO_CI_CD=1
MC_BUILD_DIR="mc-$RANDOM"
if ! git clone --quiet https://github.com/minio/mc "$MC_BUILD_DIR"; then

View file

@ -22,6 +22,8 @@ export GO111MODULE=on
export GOGC=25
export ENABLE_ADMIN=1
export MINIO_CI_CD=1
MINIO_CONFIG_DIR="$WORK_DIR/.minio"
MINIO=( "$PWD/minio" --config-dir "$MINIO_CONFIG_DIR" )

View file

@ -17,6 +17,7 @@ function start_minio_3_node() {
export MINIO_ROOT_USER=minio
export MINIO_ROOT_PASSWORD=minio123
export MINIO_ERASURE_SET_DRIVE_COUNT=6
export MINIO_CI_CD=1
start_port=$2
args=""

View file

@ -1226,24 +1226,6 @@ func formatsToDrivesInfo(endpoints Endpoints, formats []*formatErasureV3, sErrs
return beforeDrives
}
// If it is a single node Erasure and all disks are root disks, it is most likely a test setup, else it is a production setup.
// On a test setup we allow creation of format.json on root disks to help with dev/testing.
func isTestSetup(infos []DiskInfo, errs []error) bool {
if globalIsCICD {
return true
}
rootDiskCount := 0
for i := range errs {
if errs[i] == nil || errs[i] == errUnformattedDisk {
if infos[i].RootDisk {
rootDiskCount++
}
}
}
// It is a test setup if all disks are root disks in quorum.
return rootDiskCount >= len(infos)/2+1
}
func getHealDiskInfos(storageDisks []StorageAPI, errs []error) ([]DiskInfo, []error) {
infos := make([]DiskInfo, len(storageDisks))
g := errgroup.WithNErrs(len(storageDisks))
@ -1267,20 +1249,18 @@ func getHealDiskInfos(storageDisks []StorageAPI, errs []error) ([]DiskInfo, []er
// Mark root disks as down so as not to heal them.
func markRootDisksAsDown(storageDisks []StorageAPI, errs []error) {
if globalIsCICD {
// Do nothing
return
}
var infos []DiskInfo
infos, errs = getHealDiskInfos(storageDisks, errs)
if !isTestSetup(infos, errs) {
for i := range storageDisks {
if storageDisks[i] != nil && infos[i].RootDisk {
// We should not heal on root disk. i.e in a situation where the minio-administrator has unmounted a
// defective drive we should not heal a path on the root disk.
logger.Info("Disk `%s` the same as the system root disk.\n"+
"Disk will not be used. Please supply a separate disk and restart the server.",
storageDisks[i].String())
storageDisks[i] = nil
}
infos, _ := getHealDiskInfos(storageDisks, errs)
for i := range storageDisks {
if storageDisks[i] != nil && infos[i].RootDisk {
// We should not heal on root disk. i.e in a situation where the minio-administrator has unmounted a
// defective drive we should not heal a path on the root disk.
logger.Info("Disk `%s` the same as the system root disk.\n"+
"Disk will not be used. Please supply a separate disk and restart the server.",
storageDisks[i].String())
storageDisks[i] = nil
}
}
}

View file

@ -111,7 +111,7 @@ func TestMain(m *testing.M) {
resetTestGlobals()
os.Setenv("MINIO_CI_CD", "ci")
globalIsCICD = true
os.Exit(m.Run())
}

View file

@ -91,10 +91,11 @@ func TestReleaseTagToNFromTimeConversion(t *testing.T) {
}
func TestDownloadURL(t *testing.T) {
sci := os.Getenv("MINIO_CI_CD")
os.Setenv("MINIO_CI_CD", "")
defer os.Setenv("MINIO_CI_CD", sci)
sci := globalIsCICD
globalIsCICD = false
defer func() {
globalIsCICD = sci
}()
minioVersion1 := releaseTimeToReleaseTag(UTCNow())
durl := getDownloadURL(minioVersion1)
@ -158,8 +159,8 @@ func TestUserAgent(t *testing.T) {
}
for i, testCase := range testCases {
sci := os.Getenv("MINIO_CI_CD")
os.Setenv("MINIO_CI_CD", "")
sci := globalIsCICD
globalIsCICD = false
os.Setenv(testCase.envName, testCase.envValue)
if testCase.envName == "MESOS_CONTAINER_NAME" {
@ -173,7 +174,7 @@ func TestUserAgent(t *testing.T) {
if str != expectedStr {
t.Errorf("Test %d: expected: %s, got: %s", i+1, expectedStr, str)
}
os.Setenv("MINIO_CI_CD", sci)
globalIsCICD = sci
os.Unsetenv("MARATHON_APP_LABEL_DCOS_PACKAGE_VERSION")
os.Unsetenv(testCase.envName)
}
@ -181,9 +182,11 @@ func TestUserAgent(t *testing.T) {
// Tests if the environment we are running is in DCOS.
func TestIsDCOS(t *testing.T) {
sci := os.Getenv("MINIO_CI_CD")
os.Setenv("MINIO_CI_CD", "")
defer os.Setenv("MINIO_CI_CD", sci)
sci := globalIsCICD
globalIsCICD = false
defer func() {
globalIsCICD = sci
}()
os.Setenv("MESOS_CONTAINER_NAME", "mesos-1111")
dcos := IsDCOS()
@ -200,9 +203,11 @@ func TestIsDCOS(t *testing.T) {
// Tests if the environment we are running is in kubernetes.
func TestIsKubernetes(t *testing.T) {
sci := os.Getenv("MINIO_CI_CD")
os.Setenv("MINIO_CI_CD", "")
defer os.Setenv("MINIO_CI_CD", sci)
sci := globalIsCICD
globalIsCICD = false
defer func() {
globalIsCICD = sci
}()
os.Setenv("KUBERNETES_SERVICE_HOST", "10.11.148.5")
kubernetes := IsKubernetes()

View file

@ -213,29 +213,21 @@ func newXLStorage(ep Endpoint) (s *xlStorage, err error) {
}
var rootDisk bool
if globalIsCICD {
rootDisk = true
} else {
if globalRootDiskThreshold > 0 {
// When you do not want rely on automatic verification
// of rejecting root disks, we need to add this threshold
// to ensure that root disks are ignored properly.
info, err := disk.GetInfo(path)
if err != nil {
return nil, err
}
if globalRootDiskThreshold > 0 {
// Use MINIO_ROOTDISK_THRESHOLD_SIZE to figure out if
// this disk is a root disk.
info, err := disk.GetInfo(path)
if err != nil {
return nil, err
}
// treat those disks with size less than or equal to the
// threshold as rootDisks.
rootDisk = info.Total <= globalRootDiskThreshold
} else {
// When root disk threshold is not set, we rely
// on automatic detection - does not work in
// container environments.
rootDisk, err = disk.IsRootDisk(path, SlashSeparator)
if err != nil {
return nil, err
}
// treat those disks with size less than or equal to the
// threshold as rootDisks.
rootDisk = info.Total <= globalRootDiskThreshold
} else {
rootDisk, err = disk.IsRootDisk(path, SlashSeparator)
if err != nil {
return nil, err
}
}

View file

@ -25,6 +25,7 @@ catch() {
catch
set -e
export MINIO_CI_CD=1
export MINIO_BROWSER=off
export MINIO_ROOT_USER="minio"
export MINIO_ROOT_PASSWORD="minio123"

View file

@ -20,6 +20,7 @@ unset MINIO_KMS_KES_KEY_FILE
unset MINIO_KMS_KES_ENDPOINT
unset MINIO_KMS_KES_KEY_NAME
export MINIO_CI_CD=1
export MINIO_BROWSER=off
export MINIO_ROOT_USER="minio"
export MINIO_ROOT_PASSWORD="minio123"

View file

@ -20,6 +20,7 @@ unset MINIO_KMS_KES_KEY_FILE
unset MINIO_KMS_KES_ENDPOINT
unset MINIO_KMS_KES_KEY_NAME
export MINIO_CI_CD=1
export MINIO_BROWSER=off
export MINIO_ROOT_USER="minio"
export MINIO_ROOT_PASSWORD="minio123"

View file

@ -20,6 +20,7 @@ unset MINIO_KMS_KES_KEY_FILE
unset MINIO_KMS_KES_ENDPOINT
unset MINIO_KMS_KES_KEY_NAME
export MINIO_CI_CD=1
export MINIO_BROWSER=off
export MINIO_ROOT_USER="minio"
export MINIO_ROOT_PASSWORD="minio123"