isula-build/patch/0116-bugfix-run-and-data-root-when-cmd-not-set-main-confi.patch
2022-06-15 15:42:51 +08:00

555 lines
18 KiB
Diff

From e1e1ff42c720153b67375d84e3c56262e41d2fa9 Mon Sep 17 00:00:00 2001
From: xingweizheng <xingweizheng@huawei.com>
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