You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by zr...@apache.org on 2021/12/14 21:42:07 UTC
[trafficcontrol] branch master updated: Remove t3c unused plugin symbols (#6414)
This is an automated email from the ASF dual-hosted git repository.
zrhoffman 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 ea85a4b Remove t3c unused plugin symbols (#6414)
ea85a4b is described below
commit ea85a4b39461ddd11a2d09bf17dda9429a238889
Author: Robert O Butts <ro...@users.noreply.github.com>
AuthorDate: Tue Dec 14 14:41:59 2021 -0700
Remove t3c unused plugin symbols (#6414)
t3c wasn't sending the list of installed plugins to t3c-check-reload
like it's designed to take, but that was actually unnecessary,
because installed plugins always require a restart, but syncds
never installs plugins, and badass always restarts. So the entire
check served no purpose.
---
cache-config/t3c-apply/torequest/cmd.go | 9 +-
cache-config/t3c-apply/torequest/torequest.go | 40 +-------
cache-config/t3c-check-reload/t3c-check-reload.go | 23 +----
.../testing/ort-tests/t3c-check-reload_test.go | 112 +++++++--------------
4 files changed, 46 insertions(+), 138 deletions(-)
diff --git a/cache-config/t3c-apply/torequest/cmd.go b/cache-config/t3c-apply/torequest/cmd.go
index c9e4804..420dd2f 100644
--- a/cache-config/t3c-apply/torequest/cmd.go
+++ b/cache-config/t3c-apply/torequest/cmd.go
@@ -334,11 +334,10 @@ func checkRefs(cfg config.Cfg, cfgFile []byte, filesAdding []string) error {
}
// checkReload is a helper for the sub-command t3c-check-reload.
-func checkReload(pluginPackagesInstalled []string, changedConfigFiles []string) (t3cutil.ServiceNeeds, error) {
- log.Infof("t3c-check-reload calling with pluginPackagesInstalled '%v' changedConfigFiles '%v'\n", pluginPackagesInstalled, changedConfigFiles)
+func checkReload(changedConfigFiles []string) (t3cutil.ServiceNeeds, error) {
+ log.Infof("t3c-check-reload calling with changedConfigFiles '%v'\n", changedConfigFiles)
changedFiles := []byte(strings.Join(changedConfigFiles, ","))
- installedPlugins := []byte(strings.Join(pluginPackagesInstalled, ","))
cmd := exec.Command(`t3c-check-reload`)
outBuf := bytes.Buffer{}
@@ -359,10 +358,6 @@ func checkReload(pluginPackagesInstalled []string, changedConfigFiles []string)
return t3cutil.ServiceNeedsInvalid, errors.New("writing opening json to input: " + err.Error())
} else if _, err := stdinPipe.Write(changedFiles); err != nil {
return t3cutil.ServiceNeedsInvalid, errors.New("writing changed files to input: " + err.Error())
- } else if _, err := stdinPipe.Write([]byte(`","installed_plugins":"`)); err != nil {
- return t3cutil.ServiceNeedsInvalid, errors.New("writing installed_plugins key to input: " + err.Error())
- } else if _, err := stdinPipe.Write(installedPlugins); err != nil {
- return t3cutil.ServiceNeedsInvalid, errors.New("writing plugins to input: " + err.Error())
} else if _, err := stdinPipe.Write([]byte(`"}`)); err != nil {
return t3cutil.ServiceNeedsInvalid, errors.New("writing closing json input: " + err.Error())
} else if err := stdinPipe.Close(); err != nil {
diff --git a/cache-config/t3c-apply/torequest/torequest.go b/cache-config/t3c-apply/torequest/torequest.go
index 7a93499..fde5296 100644
--- a/cache-config/t3c-apply/torequest/torequest.go
+++ b/cache-config/t3c-apply/torequest/torequest.go
@@ -58,7 +58,6 @@ type TrafficOpsReq struct {
plugins map[string]bool // map of verified plugins
installedPkgs map[string]struct{} // map of packages which were installed by us.
- pluginPkgs map[string]struct{} // map of packages
changedFiles []string // list of config files which were changed
configFiles map[string]*ConfigFile
@@ -186,7 +185,6 @@ func NewTrafficOpsReq(cfg config.Cfg) *TrafficOpsReq {
plugins: map[string]bool{},
configFiles: map[string]*ConfigFile{},
installedPkgs: map[string]struct{}{},
- pluginPkgs: map[string]struct{}{},
}
}
@@ -246,32 +244,6 @@ func (r *TrafficOpsReq) checkConfigFile(cfg *ConfigFile, filesAdding []string) e
return nil
}
-// checkPlugin verifies ATS plugin requirements are satisfied.
-func (r *TrafficOpsReq) checkPlugin(plugin string) error {
- // already verified
- if r.plugins[plugin] == true {
- return nil
- }
- pluginFile := filepath.Join(config.TSHome, "/libexec/trafficserver/", plugin)
- pkgs, err := util.PackageInfo("pkg-provides", pluginFile)
- if err != nil {
- return errors.New("unable to verify plugin " + pluginFile + ": " + err.Error())
- }
- if len(pkgs) == 0 { // no package is installed that provides the plugin.
- // TODO should this actually be "no package that provides this plugin found in Yum" ?
- return errors.New(plugin + ": Package for plugin: " + plugin + ", is not installed.")
- }
-
- // TODO verify: this only checks packages that have been installed via Paramters, not any package on the system? Does this need to call util.PackageInfo("pkg-query" if it isn't in pkgs??
- // TODO iterate over pkgs, because maybe one is installed that isn't the first
- pkg := pkgs[0]
- if _, ok := r.pkgs[pkg]; !ok {
- return errors.New(plugin + ": Package for plugin: " + plugin + ", is not installed.")
- }
- r.pluginPkgs[pkg] = struct{}{}
- return nil
-}
-
// checkStatusFiles ensures that the cache status files reflect
// the status retrieved from Traffic Ops.
func (r *TrafficOpsReq) checkStatusFiles(svrStatus string) error {
@@ -1073,7 +1045,7 @@ func (r *TrafficOpsReq) StartServices(syncdsUpdate *UpdateStatus) error {
serviceNeeds = t3cutil.ServiceNeedsRestart
} else {
err := error(nil)
- if serviceNeeds, err = checkReload(r.getPluginPackagesInstalled(), r.changedFiles); err != nil {
+ if serviceNeeds, err = checkReload(r.changedFiles); err != nil {
return errors.New("determining if service needs restarted - not reloading or restarting! : " + err.Error())
}
}
@@ -1147,16 +1119,6 @@ func (r *TrafficOpsReq) StartServices(syncdsUpdate *UpdateStatus) error {
return nil
}
-func (r *TrafficOpsReq) getPluginPackagesInstalled() []string {
- installedPluginPkgs := []string{}
- for pluginPkg, _ := range r.pluginPkgs {
- if _, ok := r.installedPkgs[pluginPkg]; ok {
- installedPluginPkgs = append(installedPluginPkgs, pluginPkg)
- }
- }
- return installedPluginPkgs
-}
-
func (r *TrafficOpsReq) UpdateTrafficOps(syncdsUpdate *UpdateStatus) error {
var updateResult bool
diff --git a/cache-config/t3c-check-reload/t3c-check-reload.go b/cache-config/t3c-check-reload/t3c-check-reload.go
index 56ef971..f0607ed 100644
--- a/cache-config/t3c-check-reload/t3c-check-reload.go
+++ b/cache-config/t3c-check-reload/t3c-check-reload.go
@@ -65,19 +65,10 @@ func main() {
changedConfigFiles = StrMap(changedConfigFiles, strings.TrimSpace)
changedConfigFiles = StrRemoveIf(changedConfigFiles, StrIsEmpty)
- // TODO determine if determining which installed packages were plugins should be part of this app's job?
- // Probably not, because whatever told the installer to install them already knew that,
- // we shouldn't re-calculate it.
-
- pluginPackagesInstalled := strings.Split(changedCfg.InstalledPlugins, ",")
- pluginPackagesInstalled = StrMap(pluginPackagesInstalled, strings.TrimSpace)
- pluginPackagesInstalled = StrRemoveIf(pluginPackagesInstalled, StrIsEmpty)
-
// ATS restart is needed if:
// [x] 1. mode was badass
- // [x] 2. any ATS Plugin was installed
- // [x] 3. plugin.config or 50-ats.rules was changed
- // [ ] 4. package 'trafficserver' was installed
+ // [x] 2. plugin.config or 50-ats.rules was changed
+ // [ ] 3. package 'trafficserver' was installed
// ATS reload is needed if:
// [ ] 1. new SSL keys were installed AND ssl_multicert.config was changed
@@ -85,10 +76,6 @@ func main() {
// ssl/*.cer, ssl/*.key, anything else in /trafficserver,
//
- if len(pluginPackagesInstalled) > 0 {
- ExitRestart()
- }
-
for _, fileRequiringRestart := range configFilesRequiringRestart {
for _, changedPath := range changedConfigFiles {
if strings.HasSuffix(changedPath, fileRequiringRestart) {
@@ -118,8 +105,7 @@ func main() {
}
type ChangedCfg struct {
- ChangedFiles string `json:"changed_files"`
- InstalledPlugins string `json:"installed_plugins"`
+ ChangedFiles string `json:"changed_files"`
}
// ExitRestart returns the "needs restart" message and exits.
@@ -168,5 +154,6 @@ func StrIsEmpty(str string) bool { return str == "" }
func usageStr() string {
return `usage: t3c-check-reload [--help]
Accepts json data from stdin in in the following format:
-{"changed_files":"<comma separated list of files>","installed_plugins":"<comma separated list of plugins>"}`
+{"changed_files":"<comma separated list of files>"}
+`
}
diff --git a/cache-config/testing/ort-tests/t3c-check-reload_test.go b/cache-config/testing/ort-tests/t3c-check-reload_test.go
index 717c3d9..0eb2b39 100644
--- a/cache-config/testing/ort-tests/t3c-check-reload_test.go
+++ b/cache-config/testing/ort-tests/t3c-check-reload_test.go
@@ -16,143 +16,107 @@ package orttest
import (
"encoding/json"
- "fmt"
"strings"
"testing"
"github.com/apache/trafficcontrol/cache-config/t3cutil"
)
-type ChangedCfg struct {
- ChangedFiles string `json:"changed_files"`
- InstalledPlugins string `json:"installed_plugins"`
-}
-
-type argsResults struct {
- configs ChangedCfg
- mode string
- expected string
- expectedErr bool
-}
-
-func testCheckReload(t *testing.T, ae argsResults) {
- config, err := json.Marshal(ae.configs)
- if err != nil {
- t.Errorf("failed to encode configs: %v", err)
- }
- out, code := t3cCheckReload(config)
- out = strings.TrimSpace(out)
- if !ae.expectedErr && code != 0 {
- t.Fatalf("expected non-error exit code, actual: %d - output: %s", code, out)
- }
- if ae.expectedErr && code == 0 {
- t.Fatal("expected check-reload to exit with an error, actual: no error")
- }
- if out != ae.expected {
- t.Errorf("expected required action '%s', actual: '%s'", ae.expected, out)
+func TestCheckReload(t *testing.T) {
+ type ChangedCfg struct {
+ ChangedFiles string `json:"changed_files"`
}
-}
-func TestCheckReload(t *testing.T) {
+ type argsResults struct {
+ configs ChangedCfg
+ mode string
+ expected string
+ expectedErr bool
+ }
argsExpected := []argsResults{
{
configs: ChangedCfg{
- ChangedFiles: "/etc/trafficserver/remap.config,/etc/trafficserver/parent.config",
- InstalledPlugins: "",
+ ChangedFiles: "/etc/trafficserver/remap.config,/etc/trafficserver/parent.config",
},
expected: "reload",
},
{
configs: ChangedCfg{
- ChangedFiles: "/etc/trafficserver/anything.foo",
- InstalledPlugins: "",
+ ChangedFiles: "/etc/trafficserver/anything.foo",
},
expected: "reload",
},
{
configs: ChangedCfg{
- ChangedFiles: "/opt/trafficserver/etc/trafficserver/anything.foo",
- InstalledPlugins: "",
+ ChangedFiles: "/opt/trafficserver/etc/trafficserver/anything.foo",
},
expected: "reload",
},
{
configs: ChangedCfg{
- ChangedFiles: "/foo/bar/hdr_rw_foo.config",
- InstalledPlugins: "",
+ ChangedFiles: "/foo/bar/hdr_rw_foo.config",
},
expected: "reload",
},
{
configs: ChangedCfg{
- ChangedFiles: "/foo/bar/uri_signing_dsname.config",
- InstalledPlugins: "",
+ ChangedFiles: "/foo/bar/uri_signing_dsname.config",
},
expected: "reload",
},
{
configs: ChangedCfg{
- ChangedFiles: "/foo/bar/url_sig_dsname.config,foo",
- InstalledPlugins: "",
+ ChangedFiles: "/foo/bar/url_sig_dsname.config,foo",
},
expected: "reload",
},
{
configs: ChangedCfg{
- ChangedFiles: "plugin.config,foo",
- InstalledPlugins: "",
+ ChangedFiles: "plugin.config,foo",
},
expected: "restart",
},
{
configs: ChangedCfg{
- ChangedFiles: "/etc/trafficserver/anything.foo",
- InstalledPlugins: "anything",
- },
- expected: "restart",
- },
- {
- configs: ChangedCfg{
- ChangedFiles: "",
- InstalledPlugins: "anything",
- },
- expected: "restart",
- },
- {
- configs: ChangedCfg{
- ChangedFiles: "",
- InstalledPlugins: "anything,anythingelse",
- },
- expected: "restart",
- },
- {
- configs: ChangedCfg{
- ChangedFiles: "/foo/bar/ssl_multicert.config",
- InstalledPlugins: "",
+ ChangedFiles: "/foo/bar/ssl_multicert.config",
},
expected: "reload",
},
{
configs: ChangedCfg{
- ChangedFiles: "foo",
- InstalledPlugins: "",
+ ChangedFiles: "foo",
},
expected: "",
},
{
configs: ChangedCfg{
- ChangedFiles: "/foo/bar/baz.config",
- InstalledPlugins: "",
+ ChangedFiles: "/foo/bar/baz.config",
},
expected: "",
},
}
for _, ae := range argsExpected {
- testName := fmt.Sprintf("testing check-reload with changed files %+v and installed plugins %+v", ae.configs.ChangedFiles, ae.configs.InstalledPlugins)
- t.Run(testName, func(t *testing.T) { testCheckReload(t, ae) })
-
+ config, err := json.Marshal(ae.configs)
+ if err != nil {
+ t.Errorf("Error: %s", err)
+ }
+ out, code := t3cCheckReload(config)
+ out = strings.TrimSpace(out)
+ if !ae.expectedErr && code != 0 {
+ t.Errorf("expected configs %+v would not error, actual: code %v output '%v'",
+ ae.configs.ChangedFiles, code, out)
+ continue
+ } else if ae.expectedErr && code == 0 {
+ t.Errorf("expected configs %+v would error, actual: no error",
+ ae.configs.ChangedFiles)
+ continue
+ }
+ if out != ae.expected {
+ t.Errorf("expected configs %+v would need '%v', actual: '%v'",
+ ae.configs.ChangedFiles, ae.expected, out)
+ }
}
}