From e1e1ff42c720153b67375d84e3c56262e41d2fa9 Mon Sep 17 00:00:00 2001 From: xingweizheng Date: Wed, 26 Jan 2022 16:21:58 +0800 Subject: [PATCH 15/20] bugfix: run and data root when cmd not set, main configuration.toml first --- cmd/daemon/main.go | 178 +++++++----------- cmd/daemon/main_test.go | 113 ++++++++--- image/context.go | 9 - ..._storage_root_priority_of_configuration.sh | 92 +++++++++ 4 files changed, 244 insertions(+), 148 deletions(-) create mode 100644 tests/src/test_storage_root_priority_of_configuration.sh diff --git a/cmd/daemon/main.go b/cmd/daemon/main.go index 06a53fa..6deb9cb 100644 --- a/cmd/daemon/main.go +++ b/cmd/daemon/main.go @@ -155,22 +155,10 @@ func before(cmd *cobra.Command) error { logrus.SetOutput(os.Stdout) logrus.SetFormatter(&logrus.TextFormatter{FullTimestamp: true}) - runRoot, err := securejoin.SecureJoin(daemonOpts.RunRoot, "storage") - if err != nil { + if err := validateConfigFileAndMerge(cmd); err != nil { return err } - dataRoot, err := securejoin.SecureJoin(daemonOpts.DataRoot, "storage") - if err != nil { - return err - } - store.SetDefaultStoreOptions(store.DaemonStoreOptions{ - RunRoot: runRoot, - DataRoot: dataRoot, - Driver: daemonOpts.StorageDriver, - DriverOption: util.CopyStrings(daemonOpts.StorageOpts), - }) - - if err := checkAndValidateConfig(cmd); err != nil { + if err := setStoreAccordingToDaemonOpts(); err != nil { return err } @@ -187,6 +175,26 @@ func before(cmd *cobra.Command) error { return nil } +func setStoreAccordingToDaemonOpts() error { + runRoot, err := securejoin.SecureJoin(daemonOpts.RunRoot, "storage") + if err != nil { + return err + } + dataRoot, err := securejoin.SecureJoin(daemonOpts.DataRoot, "storage") + if err != nil { + return err + } + + store.SetDefaultStoreOptions(store.DaemonStoreOptions{ + RunRoot: runRoot, + DataRoot: dataRoot, + Driver: daemonOpts.StorageDriver, + DriverOption: util.CopyStrings(daemonOpts.StorageOpts), + }) + + return nil +} + func loadConfig(path string) (config.TomlConfig, error) { var conf config.TomlConfig if err := util.CheckFileInfoAndSize(path, constant.MaxFileSize); err != nil { @@ -202,65 +210,25 @@ func loadConfig(path string) (config.TomlConfig, error) { return conf, err } -func checkRootSetInConfig(path string) (setRunRoot, setGraphRoot bool, err error) { - if err = util.CheckFileInfoAndSize(path, constant.MaxFileSize); err != nil { - return false, false, err - } - - configData, err := ioutil.ReadFile(filepath.Clean(path)) - if err != nil { - return false, false, err - } - conf := struct { - Storage struct { - RunRoot string `toml:"runroot"` - DataRoot string `toml:"graphroot"` - } `toml:"storage"` - }{} - _, err = toml.Decode(string(configData), &conf) - return conf.Storage.RunRoot != "", conf.Storage.DataRoot != "", err -} - func mergeStorageConfig(cmd *cobra.Command) error { store.SetDefaultConfigFilePath(constant.StorageConfigPath) option, err := store.GetDefaultStoreOptions(true) - if err == nil { - if option.GraphDriverName != "" && !cmd.Flag("storage-driver").Changed { - daemonOpts.StorageDriver = option.GraphDriverName - } - if len(option.GraphDriverOptions) > 0 && !cmd.Flag("storage-opt").Changed { - daemonOpts.StorageOpts = option.GraphDriverOptions - } - } - - var storeOpt store.DaemonStoreOptions - storeOpt.RunRoot = option.RunRoot - storeOpt.DataRoot = option.GraphRoot - - setRunRoot, setDataRoot, err := checkRootSetInConfig(constant.StorageConfigPath) if err != nil { return err } - if !setRunRoot { - storeOpt.RunRoot, err = securejoin.SecureJoin(daemonOpts.RunRoot, "storage") - if err != nil { - return err - } + if !cmd.Flag("runroot").Changed && option.RunRoot != "" { + daemonOpts.RunRoot = option.RunRoot } - if !setDataRoot { - storeOpt.DataRoot, err = securejoin.SecureJoin(daemonOpts.DataRoot, "storage") - if err != nil { - return err - } + if !cmd.Flag("dataroot").Changed && option.GraphRoot != "" { + daemonOpts.DataRoot = option.GraphRoot } - if daemonOpts.StorageDriver != "" { - storeOpt.Driver = daemonOpts.StorageDriver + if !cmd.Flag("storage-driver").Changed && option.GraphDriverName != "" { + daemonOpts.StorageDriver = option.GraphDriverName } - if len(daemonOpts.StorageOpts) > 0 { - storeOpt.DriverOption = util.CopyStrings(daemonOpts.StorageOpts) + if !cmd.Flag("storage-opt").Changed && len(option.GraphDriverOptions) > 0 { + daemonOpts.StorageOpts = option.GraphDriverOptions } - store.SetDefaultStoreOptions(storeOpt) return nil } @@ -288,20 +256,6 @@ func mergeConfig(conf config.TomlConfig, cmd *cobra.Command) error { daemonOpts.DataRoot = conf.DataRoot } - runRoot, err := securejoin.SecureJoin(daemonOpts.RunRoot, "storage") - if err != nil { - return err - } - - dataRoot, err := securejoin.SecureJoin(daemonOpts.DataRoot, "storage") - if err != nil { - return err - } - store.SetDefaultStoreOptions(store.DaemonStoreOptions{ - DataRoot: dataRoot, - RunRoot: runRoot, - }) - return nil } @@ -342,48 +296,56 @@ func setupWorkingDirectories() error { return nil } -func checkAndValidateConfig(cmd *cobra.Command) error { - // check if configuration.toml file exists, merge config if exists - if exist, err := util.IsExist(constant.ConfigurationPath); err != nil { - return err - } else if !exist { - logrus.Warnf("Main config file missing, the default configuration is used") - } else { - conf, err := loadConfig(constant.ConfigurationPath) - if err != nil { - logrus.Errorf("Load and parse main config file failed: %v", err) - os.Exit(constant.DefaultFailedCode) +func validateConfigFileAndMerge(cmd *cobra.Command) error { + confFiles := []struct { + path string + needed bool + mergeConfig func(cmd *cobra.Command) error + }{ + {path: constant.StorageConfigPath, needed: false, mergeConfig: mergeStorageConfig}, + {path: constant.RegistryConfigPath, needed: false, mergeConfig: nil}, + // policy.json file must exists + {path: constant.SignaturePolicyPath, needed: true, mergeConfig: nil}, + // main configuration comes last for the final merge operation + {path: constant.ConfigurationPath, needed: false, mergeConfig: loadMainConfiguration}, + } + + for _, file := range confFiles { + if exist, err := util.IsExist(file.path); !exist { + if !file.needed { + logrus.Warnf("Config file %q missing, the default configuration is used", file.path) + continue + } + + if err != nil { + return errors.Wrapf(err, "check config file %q failed", file.path) + } + return errors.Errorf("config file %q is not exist", file.path) } - if err = mergeConfig(conf, cmd); err != nil { + if err := util.CheckFileInfoAndSize(file.path, constant.MaxFileSize); err != nil { + return err + } + if file.mergeConfig == nil { + continue + } + if err := file.mergeConfig(cmd); err != nil { return err } } - // file policy.json must be exist - if exist, err := util.IsExist(constant.SignaturePolicyPath); err != nil { - return err - } else if !exist { - return errors.Errorf("policy config file %v is not exist", constant.SignaturePolicyPath) - } + return nil +} - // check all config files - confFiles := []string{constant.RegistryConfigPath, constant.SignaturePolicyPath, constant.StorageConfigPath} - for _, file := range confFiles { - if exist, err := util.IsExist(file); err != nil { - return err - } else if exist { - if err := util.CheckFileInfoAndSize(file, constant.MaxFileSize); err != nil { - return err - } - } +func loadMainConfiguration(cmd *cobra.Command) error { + conf, err := loadConfig(constant.ConfigurationPath) + if err != nil { + logrus.Errorf("Load and parse main config file failed: %v", err) + os.Exit(constant.DefaultFailedCode) } - // if storage config file exists, merge storage config - if exist, err := util.IsExist(constant.StorageConfigPath); err != nil { + if err = mergeConfig(conf, cmd); err != nil { return err - } else if exist { - return mergeStorageConfig(cmd) } return nil diff --git a/cmd/daemon/main_test.go b/cmd/daemon/main_test.go index 3947f7a..b1b8859 100644 --- a/cmd/daemon/main_test.go +++ b/cmd/daemon/main_test.go @@ -112,6 +112,10 @@ func TestSetupWorkingDirectories(t *testing.T) { func TestRunAndDataRootSet(t *testing.T) { dataRoot := fs.NewDir(t, t.Name()) runRoot := fs.NewDir(t, t.Name()) + result := store.DaemonStoreOptions{ + DataRoot: dataRoot.Join("storage"), + RunRoot: runRoot.Join("storage"), + } conf := config.TomlConfig{ Debug: true, @@ -123,34 +127,41 @@ func TestRunAndDataRootSet(t *testing.T) { } cmd := newDaemonCommand() - result := store.DaemonStoreOptions{ - DataRoot: dataRoot.Join("storage"), - RunRoot: runRoot.Join("storage"), - } - - setStorage := func(content string) func() { - return func() { - if err := mergeConfig(conf, cmd); err != nil { - t.Fatalf("mrege config failed with error: %v", err) - } + setStorage := func(content string) { + // merge main config + if err := mergeConfig(conf, cmd); err != nil { + t.Fatalf("merge config failed with error: %v", err) + } - fileName := "storage.toml" - tmpDir := fs.NewDir(t, t.Name(), fs.WithFile(fileName, content)) - defer tmpDir.Remove() + // simulate storage.toml and merge + fileName := "storage.toml" + tmpDir := fs.NewDir(t, t.Name(), fs.WithFile(fileName, content)) + defer tmpDir.Remove() + filePath := tmpDir.Join(fileName) - filePath := tmpDir.Join(fileName) - store.SetDefaultConfigFilePath(filePath) - option, err := store.GetDefaultStoreOptions(true) - if err != nil { - t.Fatalf("get default store options failed with error: %v", err) - } + store.SetDefaultConfigFilePath(filePath) + option, err := store.GetDefaultStoreOptions(true) + if err != nil { + t.Fatalf("get default store options failed with error: %v", err) + } - var storeOpt store.DaemonStoreOptions - storeOpt.RunRoot = option.RunRoot - storeOpt.DataRoot = option.GraphRoot - store.SetDefaultStoreOptions(storeOpt) + if !cmd.Flag("runroot").Changed && option.RunRoot != "" { + daemonOpts.RunRoot = option.RunRoot + } + if !cmd.Flag("dataroot").Changed && option.GraphRoot != "" { + daemonOpts.DataRoot = option.GraphRoot + } + if !cmd.Flag("storage-driver").Changed && option.GraphDriverName != "" { + daemonOpts.StorageDriver = option.GraphDriverName + } + if !cmd.Flag("storage-opt").Changed && len(option.GraphDriverOptions) > 0 { + daemonOpts.StorageOpts = option.GraphDriverOptions } + // final set + if err := setStoreAccordingToDaemonOpts(); err != nil { + t.Fatalf("set store options failed with error: %v", err) + } } testcases := []struct { @@ -160,28 +171,28 @@ func TestRunAndDataRootSet(t *testing.T) { }{ { // first run so can not be affected by other testcase - name: "TC3 - all not set", - setF: setStorage("[storage]\ndriver = \"overlay\""), + name: "TC1 - all not set", + setF: func() { setStorage("[storage]\ndriver = \"overlay\"") }, expectation: store.DaemonStoreOptions{ DataRoot: "/var/lib/isula-build/storage", RunRoot: "/var/run/isula-build/storage", }, }, { - name: "TC1 - cmd set, configuration and storage not set", + name: "TC2 - cmd set, configuration and storage not set", setF: func() { cmd.PersistentFlags().Set("runroot", runRoot.Path()) cmd.PersistentFlags().Set("dataroot", dataRoot.Path()) - checkAndValidateConfig(cmd) + setStorage("[storage]\ndriver = \"overlay\"") }, expectation: result, }, { - name: "TC2 - cmd and storage not set, configuration set", + name: "TC3 - cmd and storage not set, configuration set", setF: func() { conf.DataRoot = dataRoot.Path() conf.RunRoot = runRoot.Path() - checkAndValidateConfig(cmd) + setStorage("[storage]\ndriver = \"overlay\"") }, expectation: result, }, @@ -190,8 +201,22 @@ func TestRunAndDataRootSet(t *testing.T) { setF: func() { config := fmt.Sprintf("[storage]\ndriver = \"%s\"\nrunroot = \"%s\"\ngraphroot = \"%s\"\n", "overlay", runRoot.Join("storage"), dataRoot.Join("storage")) - sT := setStorage(config) - sT() + setStorage(config) + }, + expectation: result, + }, + { + name: "TC5 - cmd not set, configuration and storage set, configuration first", + setF: func() { + conf.DataRoot = dataRoot.Path() + conf.RunRoot = runRoot.Path() + + dataRootStorage := fs.NewDir(t, t.Name()) + runRootStorage := fs.NewDir(t, t.Name()) + config := fmt.Sprintf("[storage]\ndriver = \"%s\"\nrunroot = \"%s\"\ngraphroot = \"%s\"\n", + "overlay", runRootStorage.Join("storage"), dataRootStorage.Join("storage")) + + setStorage(config) }, expectation: result, }, @@ -210,3 +235,29 @@ func TestRunAndDataRootSet(t *testing.T) { } } + +func TestValidateConfigFileAndMerge(t *testing.T) { + // cmd line args keep default. + cmd := newDaemonCommand() + err := validateConfigFileAndMerge(cmd) + assert.NilError(t, err) + + // cmd line runroot and dataroot args are set. + dataRoot := fs.NewDir(t, t.Name()) + runRoot := fs.NewDir(t, t.Name()) + cmd.PersistentFlags().Set("runroot", runRoot.Path()) + cmd.PersistentFlags().Set("dataroot", dataRoot.Path()) + err = validateConfigFileAndMerge(cmd) + assert.NilError(t, err) + + if err := setStoreAccordingToDaemonOpts(); err != nil { + t.Fatalf("set store options failed with error: %v", err) + } + storeOptions, err := store.GetDefaultStoreOptions(false) + if err != nil { + t.Fatalf("get default store options failed with error: %v", err) + } + + assert.Equal(t, storeOptions.GraphRoot, dataRoot.Join("storage")) + assert.Equal(t, storeOptions.RunRoot, runRoot.Join("storage")) +} diff --git a/image/context.go b/image/context.go index c2d4150..6fd7725 100644 --- a/image/context.go +++ b/image/context.go @@ -19,10 +19,8 @@ import ( cp "github.com/containers/image/v5/copy" "github.com/containers/image/v5/types" - "github.com/sirupsen/logrus" constant "isula.org/isula-build" - "isula.org/isula-build/util" ) var ( @@ -40,13 +38,6 @@ func init() { // SetSystemContext set the values of globalSystemContext func SetSystemContext(dataRoot string) { - configFiles := []string{constant.SignaturePolicyPath, constant.RegistryConfigPath} - for _, cfg := range configFiles { - if err := util.CheckFileInfoAndSize(cfg, constant.MaxFileSize); err != nil { - logrus.Fatalf("check config file %q failed: %v", cfg, err) - } - } - once.Do(func() { globalSystemContext.SignaturePolicyPath = constant.SignaturePolicyPath globalSystemContext.SystemRegistriesConfPath = constant.RegistryConfigPath diff --git a/tests/src/test_storage_root_priority_of_configuration.sh b/tests/src/test_storage_root_priority_of_configuration.sh new file mode 100644 index 0000000..e585d0c --- /dev/null +++ b/tests/src/test_storage_root_priority_of_configuration.sh @@ -0,0 +1,92 @@ +#!/bin/bash + +# Copyright (c) Huawei Technologies Co., Ltd. 2020. All rights reserved. +# isula-build licensed under the Mulan PSL v2. +# You can use this software according to the terms and conditions of the Mulan PSL v2. +# You may obtain a copy of Mulan PSL v2 at: +# http://license.coscl.org.cn/MulanPSL2 +# THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY KIND, EITHER EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO NON-INFRINGEMENT, MERCHANTABILITY OR FIT FOR A PARTICULAR +# PURPOSE. +# See the Mulan PSL v2 for more details. +# Author: Weizheng Xing +# Create: 2022-01-25 +# Description: test priority of data and run root with different configuration locations( binary > configuration.toml > storage.toml) + +top_dir=$(git rev-parse --show-toplevel) +# shellcheck disable=SC1091 +source "$top_dir"/tests/lib/common.sh + +config_file="/etc/isula-build/configuration.toml" +storage_file="/etc/isula-build/storage.toml" +main_run_root="/tmp/run/main-isula-build" +main_data_root="/tmp/lib/main-isula-build" +storage_run_root="/tmp/run/storage-isula-build" +storage_data_root="/tmp/lib/storage-isula-build" + +# change to new data and run root +function pre_test() { + cp $config_file "$config_file".bak + cp $config_file "$config_file".bak + + cp $storage_file "$storage_file".bak + cp $storage_file "$storage_file".bak +} + +function clean() { + rm -f "$config_file" + rm -f "$storage_file" + + mv $config_file.bak "$config_file" + mv $storage_file.bak "$storage_file" +} + +function main_set_storage_not_set() { + sed -i "/run_root/d;/data_root/d" $config_file + sed -i "/runroot/d;/graphroot/d" $storage_file + echo "run_root=\"$main_run_root\"" >>$config_file + echo "data_root=\"$main_data_root\"" >>$config_file + + check_root "$main_run_root" "$main_data_root" +} + +function main_not_set_storage_set() { + sed -i "/run_root/d;/data_root/d" $config_file + sed -i "/runroot/d;/graphroot/d" $storage_file + eval "sed -i '/\[storage\]/a\runroot=\"$storage_run_root\"' $storage_file" + eval "sed -i '/\[storage\]/a\graphroot=\"$storage_data_root\"' $storage_file" + + check_root "$main_run_root" "$main_data_root" +} + +function main_set_storage_set() { + sed -i "/run_root/d;/data_root/d" $config_file + sed -i "/runroot/d;/graphroot/d" $storage_file + echo "run_root = \"$main_run_root}\"" >>$config_file + echo "data_root = \"$main_data_root\"" >>$config_file + eval "sed -i '/\[storage\]/a\runroot=\"$storage_run_root\"' $storage_file" + eval "sed -i '/\[storage\]/a\graphroot=\"$storage_data_root\"' $storage_file" + + check_root "$main_run_root" "$main_data_root" +} + +# run command and check its result +# $1 (run root) +# $1 (data root) +function check_root() { + local -r run_root="$1" + local -r data_root="$2" + + start_time=$(date '+%Y-%m-%d %H:%M:%S') + systemctl restart isula-build + + run_check_result "journalctl -u isula-build --since \"$start_time\" --no-pager | grep $run_root" 0 + run_check_result "journalctl -u isula-build --since \"$start_time\" --no-pager | grep $data_root" 0 +} + +pre_test +main_set_storage_not_set +main_not_set_storage_set +main_set_storage_set +clean +exit "$exit_flag" -- 2.27.0