This is an automated email from the ASF dual-hosted git repository.

rob 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 1ee0a2d  Reduce Mutable State in t3c-apply (#6313)
1ee0a2d is described below

commit 1ee0a2d1844672f518ff4789d4cb1b86c1581581
Author: Joe Pappano <[email protected]>
AuthorDate: Thu Dec 2 18:17:49 2021 -0500

    Reduce Mutable State in t3c-apply (#6313)
    
    * updated the way it uses the TrafficOpsReq struct to reduce mutable state
    
    * Added comment to reloadRestart func.
    
    * Added change log entry.
---
 CHANGELOG.md                                  |  1 +
 cache-config/t3c-apply/torequest/torequest.go | 73 +++++++++++++++++++++------
 2 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index da785de..19d4aeb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -43,6 +43,7 @@ The format is based on [Keep a 
Changelog](http://keepachangelog.com/en/1.0.0/).
 - Traffic Portal no longer uses `ruby compass` to compile sass and now uses 
`dart-sass`.
 - Changed Invalidation Jobs throughout (TO, TP, T3C, etc.) to account for the 
ability to do both REFRESH and REFETCH requests for resources.
 - The `admin` Role is now always guaranteed to exist, and can't be deleted or 
modified.
+- Updated `t3c-apply` to reduce mutable state in `TrafficOpsReq` struct.
 
 ### Deprecated
 - Deprecated the endpoints and docs associated with `api_capability`.
diff --git a/cache-config/t3c-apply/torequest/torequest.go 
b/cache-config/t3c-apply/torequest/torequest.go
index fee93ed..10fefd6 100644
--- a/cache-config/t3c-apply/torequest/torequest.go
+++ b/cache-config/t3c-apply/torequest/torequest.go
@@ -71,6 +71,19 @@ type TrafficOpsReq struct {
        unixTimeStr          string // unix time string at program startup.
 }
 
+type ShouldReloadRestart struct {
+       ReloadRestart []RestartData
+}
+type RestartData struct {
+       Name                 string
+       TrafficCtlReload     bool // a traffic_ctl_reload is required
+       SysCtlReload         bool // a reload of the sysctl.conf is required
+       NtpdRestart          bool // ntpd needs restarting
+       TeakdRestart         bool // a restart of teakd is required
+       TrafficServerRestart bool // a trafficserver restart is required
+       RemapConfigReload    bool // remap.config should be reloaded
+}
+
 type ConfigFile struct {
        Name              string // file name
        Dir               string // install directory
@@ -462,12 +475,12 @@ func (r *TrafficOpsReq) readCfgFile(cfg *ConfigFile, dir 
string) ([]byte, error)
 const configFileTempSuffix = `.tmp`
 
 // replaceCfgFile replaces an ATS configuration file with one from Traffic Ops.
-func (r *TrafficOpsReq) replaceCfgFile(cfg *ConfigFile) error {
+func (r *TrafficOpsReq) replaceCfgFile(cfg *ConfigFile) (*RestartData, error) {
        if r.Cfg.ReportOnly ||
                (r.Cfg.Files != t3cutil.ApplyFilesFlagAll && r.Cfg.Files != 
t3cutil.ApplyFilesFlagReval) {
                log.Infof("You elected not to replace %s with the version from 
Traffic Ops.\n", cfg.Name)
                cfg.ChangeApplied = false
-               return nil
+               return &RestartData{Name: cfg.Name}, nil
        }
 
        tmpFileName := cfg.Path + configFileTempSuffix
@@ -479,18 +492,17 @@ func (r *TrafficOpsReq) replaceCfgFile(cfg *ConfigFile) 
error {
        // we'd end up with malformed files.
 
        if _, err := util.WriteFileWithOwner(tmpFileName, cfg.Body, &cfg.Uid, 
&cfg.Gid, cfg.Perm); err != nil {
-               return errors.New("Failed to write temp config file '" + 
tmpFileName + "': " + err.Error())
+               return &RestartData{Name: cfg.Name}, errors.New("Failed to 
write temp config file '" + tmpFileName + "': " + err.Error())
        }
 
        log.Infof("Copying temp file '%s' to real '%s'\n", tmpFileName, 
cfg.Path)
        if err := os.Rename(tmpFileName, cfg.Path); err != nil {
-               return errors.New("Failed to move temp '" + tmpFileName + "' to 
real '" + cfg.Path + "': " + err.Error())
+               return &RestartData{Name: cfg.Name}, errors.New("Failed to move 
temp '" + tmpFileName + "' to real '" + cfg.Path + "': " + err.Error())
        }
        cfg.ChangeApplied = true
        r.changedFiles = append(r.changedFiles, cfg.Path)
 
-       r.RemapConfigReload = r.RemapConfigReload ||
-               cfg.RemapPluginConfig ||
+       remapConfigReload := cfg.RemapPluginConfig ||
                cfg.Name == "remap.config" ||
                strings.HasPrefix(cfg.Name, "bg_fetch") ||
                strings.HasPrefix(cfg.Name, "hdr_rw_") ||
@@ -500,22 +512,28 @@ func (r *TrafficOpsReq) replaceCfgFile(cfg *ConfigFile) 
error {
                strings.HasPrefix(cfg.Name, "uri_signing") ||
                strings.HasSuffix(cfg.Name, ".lua")
 
-       r.TrafficCtlReload = r.TrafficCtlReload ||
-               strings.HasSuffix(cfg.Dir, "trafficserver") ||
-               r.RemapConfigReload ||
+       trafficCtlReload := strings.HasSuffix(cfg.Dir, "trafficserver") ||
+               remapConfigReload ||
                cfg.Name == "ssl_multicert.config" ||
                cfg.Name == "records.config" ||
                (strings.HasSuffix(cfg.Dir, "ssl") && 
strings.HasSuffix(cfg.Name, ".cer")) ||
                (strings.HasSuffix(cfg.Dir, "ssl") && 
strings.HasSuffix(cfg.Name, ".key"))
 
-       r.TrafficServerRestart = r.TrafficServerRestart || (cfg.Name == 
"plugin.config")
-       r.NtpdRestart = r.NtpdRestart || (cfg.Name == "ntpd.conf")
-       r.SysCtlReload = r.SysCtlReload || (cfg.Name == "sysctl.conf")
+       trafficServerRestart := cfg.Name == "plugin.config"
+       ntpdRestart := cfg.Name == "ntpd.conf"
+       sysCtlReload := cfg.Name == "sysctl.conf"
 
-       log.Debugf("Reload state after %s: remap.config: %t reload: %t restart: 
%t ntpd: %t sysctl: %t", cfg.Name, r.RemapConfigReload, r.TrafficCtlReload, 
r.TrafficServerRestart, r.NtpdRestart, r.SysCtlReload)
+       log.Debugf("Reload state after %s: remap.config: %t reload: %t restart: 
%t ntpd: %t sysctl: %t", cfg.Name, remapConfigReload, trafficCtlReload, 
trafficServerRestart, ntpdRestart, sysCtlReload)
 
        log.Debugf("Setting change applied for '%s'\n", cfg.Name)
-       return nil
+       return &RestartData{
+               Name:                 cfg.Name,
+               TrafficCtlReload:     trafficCtlReload,
+               SysCtlReload:         sysCtlReload,
+               NtpdRestart:          ntpdRestart,
+               TrafficServerRestart: trafficServerRestart,
+               RemapConfigReload:    remapConfigReload,
+       }, nil
 }
 
 // CheckSystemServices is used to verify that packages installed
@@ -774,6 +792,27 @@ func (r *TrafficOpsReq) CheckSyncDSState() (UpdateStatus, 
error) {
        return updateStatus, nil
 }
 
+// CheckReloadRestart determines the final reload/restart state after all 
config files are processed.
+func (r *TrafficOpsReq) CheckReloadRestart(data []RestartData) (bool, bool, 
bool, bool, bool, bool) {
+       trafficCtlReload := false
+       sysCtlReload := false
+       ntpdRestart := false
+       teakdRestart := false
+       trafficServerRestart := false
+       remapConfigReload := false
+
+       for _, changedFile := range data {
+               trafficCtlReload = trafficCtlReload || 
changedFile.TrafficCtlReload
+               sysCtlReload = sysCtlReload || changedFile.SysCtlReload
+               ntpdRestart = ntpdRestart || changedFile.NtpdRestart
+               teakdRestart = teakdRestart || changedFile.TeakdRestart
+               trafficServerRestart = trafficServerRestart || 
changedFile.TrafficServerRestart
+               remapConfigReload = remapConfigReload || 
changedFile.RemapConfigReload
+       }
+
+       return trafficCtlReload, sysCtlReload, ntpdRestart, teakdRestart, 
trafficServerRestart, remapConfigReload
+}
+
 // ProcessConfigFiles processes all config files retrieved from Traffic Ops.
 func (r *TrafficOpsReq) ProcessConfigFiles() (UpdateStatus, error) {
        var updateStatus UpdateStatus = UpdateTropsNotNeeded
@@ -811,6 +850,7 @@ func (r *TrafficOpsReq) ProcessConfigFiles() (UpdateStatus, 
error) {
        }
 
        changesRequired := 0
+       shouldRestartReload := ShouldReloadRestart{[]RestartData{}}
 
        for _, cfg := range r.configFiles {
                if cfg.ChangeNeeded &&
@@ -833,14 +873,17 @@ func (r *TrafficOpsReq) ProcessConfigFiles() 
(UpdateStatus, error) {
                                continue
                        } else {
                                log.Debugf("All Prereqs passed for replacing %s 
on disk with that in Traffic Ops.\n", cfg.Name)
-                               err := r.replaceCfgFile(cfg)
+                               reData, err := r.replaceCfgFile(cfg)
                                if err != nil {
                                        log.Errorf("failed to replace the 
config file, '%s',  on disk with data in Traffic Ops.\n", cfg.Name)
                                }
+                               shouldRestartReload.ReloadRestart = 
append(shouldRestartReload.ReloadRestart, *reData)
                        }
                }
        }
 
+       r.TrafficCtlReload, r.SysCtlReload, r.NtpdRestart, r.TeakdRestart, 
r.TrafficServerRestart, r.RemapConfigReload = 
r.CheckReloadRestart(shouldRestartReload.ReloadRestart)
+
        if 0 < len(r.changedFiles) {
                log.Infof("Final state: remap.config: %t reload: %t restart: %t 
ntpd: %t sysctl: %t", r.RemapConfigReload, r.TrafficCtlReload, 
r.TrafficServerRestart, r.NtpdRestart, r.SysCtlReload)
        }

Reply via email to