This is an automated email from the ASF dual-hosted git repository.
shamrick pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new 93765bb7b1 t3c: Refactor MkDir to return true upon successful
directory creation (#7359)
93765bb7b1 is described below
commit 93765bb7b1cfaed2d50533c0fb49697cc22dabba
Author: Taylor Clayton Frey <[email protected]>
AuthorDate: Tue Mar 21 09:53:42 2023 -0600
t3c: Refactor MkDir to return true upon successful directory creation
(#7359)
* Refactor MkDir to return true if successful on first run
* Avoid passing entire config for single value. Fix error message
* Add tests for MkDir
* Changelog entry
---------
Co-authored-by: Taylor Frey <[email protected]>
---
CHANGELOG.md | 1 +
cache-config/t3c-apply/t3c-apply.go | 2 +-
cache-config/t3c-apply/torequest/torequest.go | 4 +-
cache-config/t3c-apply/util/util.go | 67 +++++++++++--------------
cache-config/testing/ort-tests/t3c-dirs_test.go | 27 ++++++++++
5 files changed, 60 insertions(+), 41 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 46f5bb195e..127a168e97 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -93,6 +93,7 @@ The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).
- [#7346](https://github.com/apache/trafficcontrol/pull/7346) *Traffic Control
Cache Config (t3c)* Fixed issue with stale lock file when using git to track
changes.
- [#7352](https://github.com/apache/trafficcontrol/pull/7352) *Traffic Control
Cache Config (t3c)* Fixed issue with application locking which would allow
multiple instances of `t3c apply` to run concurrently.
- [#6775](https://github.com/apache/trafficcontrol/issues/6775) *Traffic Ops*
Invalid "orgServerFqdn" in Delivery Service creation/update causes Internal
Server Error
+- [#6695](https://github.com/apache/trafficcontrol/issues/6695) *Traffic
Control Cache Config (t3c)* Directory creation was erroneously reporting an
error when actually succeeding.
## [7.0.0] - 2022-07-19
### Added
diff --git a/cache-config/t3c-apply/t3c-apply.go
b/cache-config/t3c-apply/t3c-apply.go
index a84810d79e..f4e7db2dba 100644
--- a/cache-config/t3c-apply/t3c-apply.go
+++ b/cache-config/t3c-apply/t3c-apply.go
@@ -182,7 +182,7 @@ func Main() int {
}
// create and clean the config.TmpBase (/tmp/ort)
- if !util.MkDir(config.TmpBase, cfg) {
+ if !util.MkDir(config.TmpBase, cfg.ReportOnly) {
log.Errorln("mkdir TmpBase '" + config.TmpBase + "' failed,
cannot continue")
log.Infoln(FailureExitMsg)
return ExitCodeGeneralFailure
diff --git a/cache-config/t3c-apply/torequest/torequest.go
b/cache-config/t3c-apply/torequest/torequest.go
index 52fcbfcbb7..da779a3ab7 100644
--- a/cache-config/t3c-apply/torequest/torequest.go
+++ b/cache-config/t3c-apply/torequest/torequest.go
@@ -215,7 +215,7 @@ func (r *TrafficOpsReq) checkConfigFile(cfg *ConfigFile,
filesAdding []string) e
return nil
}
- if !util.MkDirWithOwner(cfg.Dir, r.Cfg, &cfg.Uid, &cfg.Gid) {
+ if !util.MkDirWithOwner(cfg.Dir, r.Cfg.ReportOnly, &cfg.Uid, &cfg.Gid) {
return errors.New("Unable to create the directory '" + cfg.Dir
+ " for " + "'" + cfg.Name + "'")
}
@@ -301,7 +301,7 @@ func (r *TrafficOpsReq) checkStatusFiles(svrStatus string)
error {
}
if !r.Cfg.ReportOnly {
- if !util.MkDir(config.StatusDir, r.Cfg) {
+ if !util.MkDir(config.StatusDir, r.Cfg.ReportOnly) {
return fmt.Errorf("unable to create '%s'\n",
config.StatusDir)
}
fileExists, _ := util.FileExists(statusFile)
diff --git a/cache-config/t3c-apply/util/util.go
b/cache-config/t3c-apply/util/util.go
index cd410fc048..49d77d0748 100644
--- a/cache-config/t3c-apply/util/util.go
+++ b/cache-config/t3c-apply/util/util.go
@@ -387,56 +387,47 @@ func CleanTmpDir(cfg config.Cfg) bool {
return true
}
-func MkDir(name string, cfg config.Cfg) bool {
- return doMkDirWithOwner(name, cfg, nil, nil, false)
+func MkDir(name string, reportOnly bool) bool {
+ return doMkDirWithOwner(name, reportOnly, nil, nil)
}
-func MkDirAll(name string, cfg config.Cfg) bool {
- return doMkDirWithOwner(name, cfg, nil, nil, true)
+func MkDirWithOwner(name string, reportOnly bool, uid *int, gid *int) bool {
+ return doMkDirWithOwner(name, reportOnly, uid, gid)
}
-func MkDirWithOwner(name string, cfg config.Cfg, uid *int, gid *int) bool {
- return doMkDirWithOwner(name, cfg, uid, gid, false)
-}
-
-func doMkDirWithOwner(name string, cfg config.Cfg, uid *int, gid *int, all
bool) bool {
+func doMkDirWithOwner(name string, reportOnly bool, uid *int, gid *int) bool {
+ // Check if already exists
fileInfo, err := os.Stat(name)
- if err == nil && fileInfo.Mode().IsDir() {
- log.Debugf("the directory '%s' already exists", name)
- return true
- }
- if err != nil {
- if cfg.ReportOnly {
- log.Infof("Reporting only: the directory %s does not
exist and was not created", name)
+ if err == nil {
+ if fileInfo.IsDir() {
+ log.Debugf("the directory '%s' already exists", name)
return true
+ } else {
+ log.Errorf("there is a file named, '%s' that is not a
directory", name)
+ return false
}
+ }
- if err != nil { // the path does not exist.
- if all {
- err = os.MkdirAll(name, 0755)
- } else {
- err = os.Mkdir(name, 0755)
- }
- if err != nil {
- log.Errorf("unable to create the directory
'%s': %v", name, err)
- return false
- }
+ if reportOnly {
+ log.Infof("Reporting only: the directory %s does not exist and
was not created", name)
+ return true
+ }
- if uid != nil && gid != nil {
- err = os.Chown(name, *uid, *gid)
- if err != nil {
- log.Errorf("unable to chown directory
uid/gid, '%s': %v", name, err)
- return false
- }
- }
- } else if fileInfo.Mode().IsDir() {
- log.Debugf("the directory: %s, already exists\n", name)
- } else {
- log.Errorf("there is a file named, '%s' that is not a
directory: %v", name, err)
+ err = os.Mkdir(name, 0755)
+ if err != nil {
+ log.Errorf("unable to create the directory '%s': %v", name, err)
+ return false
+ }
+
+ if uid != nil && gid != nil {
+ err = os.Chown(name, *uid, *gid)
+ if err != nil {
+ log.Errorf("unable to chown directory uid/gid, '%s':
%v", name, err)
return false
}
}
- return false
+
+ return true
}
func Touch(fn string) error {
diff --git a/cache-config/testing/ort-tests/t3c-dirs_test.go
b/cache-config/testing/ort-tests/t3c-dirs_test.go
index 09a010e2b2..481e425500 100644
--- a/cache-config/testing/ort-tests/t3c-dirs_test.go
+++ b/cache-config/testing/ort-tests/t3c-dirs_test.go
@@ -17,6 +17,8 @@ package orttest
import (
"os"
"testing"
+
+ "github.com/apache/trafficcontrol/cache-config/t3c-apply/util"
)
// TestDirs tests that the t3c rpm creates directories that t3c-apply requires
to run.
@@ -39,3 +41,28 @@ func TestDirs(t *testing.T) {
}
}
}
+
+// Test_MkDir ensures that the directory creation in t3c-apply succeeds and
fails as expected
+func Test_MkDir(t *testing.T) {
+
+ os.WriteFile("isAFile", []byte("nothing to see here"), 0755)
+
+ mkDirTests := []struct {
+ description string
+ path string
+ reportOnly bool
+ expectedResult bool
+ }{
+ {"Success - Create a test directory", "testDir", false, true},
+ {"Fail - Create a test directory, but file exists", "isAFile",
false, false},
+ {"Success - Report Only even if dir doesn't exist",
"ShouldntExist", true, true},
+ }
+
+ for _, test := range mkDirTests {
+ result := util.MkDir(test.path, test.reportOnly)
+ if result != test.expectedResult {
+ t.Errorf("Directory creation test failed: %s",
test.description)
+ }
+ }
+
+}
