ocket8888 commented on code in PR #7652:
URL: https://github.com/apache/trafficcontrol/pull/7652#discussion_r1269961562
##########
cache-config/t3c-apply/config/config.go:
##########
@@ -188,6 +190,29 @@ func directoryExists(dir string) (bool, os.FileInfo) {
return info.IsDir(), info
}
+const RpmDir = "/var/lib/rpm"
+
+// verifies the rpm database files. if there is any database corruption
+// it will return false
+func VerifyRpmDB() bool {
Review Comment:
GoDoc comments on exported symbols should begin with the name of the symbol
they document (optionally following an article), contain complete sentences,
and end with punctuation.
Alternatively, this doesn't seem to be used outside of the file in which it
is declared, so it doesn't need to be exported at all.
##########
cache-config/t3c-apply/config/config.go:
##########
@@ -188,6 +190,29 @@ func directoryExists(dir string) (bool, os.FileInfo) {
return info.IsDir(), info
}
+const RpmDir = "/var/lib/rpm"
Review Comment:
Exported constants should have GoDoc comments - alternatively, it looks like
this isn't used outside of the file in which it is declared, so it doesn't need
to be exported at all.
##########
cache-config/t3c-apply/torequest/torequest.go:
##########
@@ -587,10 +587,14 @@ func (r *TrafficOpsReq) CheckSystemServices() error {
func (r *TrafficOpsReq) IsPackageInstalled(name string) bool {
for k, v := range r.Pkgs {
if strings.HasPrefix(k, name) {
+ log.Infof("Found in cache for '%v'", k)
Review Comment:
nit: formatting parameter could be more specific; strings may use `%s` for
better compile-time safety
##########
cache-config/t3c-apply/config/config.go:
##########
@@ -580,6 +618,13 @@ If any of the related flags are also set, they override
the mode's default behav
return Cfg{}, errors.New("Initializing loggers: " + err.Error()
+ "\n")
}
+ if len(fatalLogStrs) > 0 {
+ for _, str := range fatalLogStrs {
+ str = strings.TrimSpace(str)
+ log.Warnln(str)
Review Comment:
shouldn't fatal errors be logged to the error stream rather than the warning
stream?
##########
cache-config/t3c-apply/torequest/torequest.go:
##########
@@ -587,10 +587,14 @@ func (r *TrafficOpsReq) CheckSystemServices() error {
func (r *TrafficOpsReq) IsPackageInstalled(name string) bool {
for k, v := range r.Pkgs {
if strings.HasPrefix(k, name) {
+ log.Infof("Found in cache for '%v'", k)
return v
}
}
-
+ if !r.Cfg.RpmDBOk {
+ log.Warnf("RPM DB is corrupted cannot run IsPackageInstalled
for '%v' and package metadata is unavailable", name)
Review Comment:
nit: formatting parameter could be more specific; strings may use `%s` for
better compile-time safety
##########
cache-config/t3c-apply/torequest/torequest.go:
##########
@@ -1030,6 +1034,45 @@ func (r *TrafficOpsReq) ProcessPackages() error {
return nil
}
+func pkgMetaDataToMap(pmd []string) map[string]bool {
+ pkgMap := map[string]bool{}
+ for _, pkg := range pmd {
+ pkgMap[pkg] = true
+ }
+ return pkgMap
+}
+
+func pkgMatch(pkgMetaData []string, pk string) bool {
+ for _, pkg := range pkgMetaData {
+ if ok := strings.Contains(pk, pkg); ok {
+ return true
+ }
+ }
+ return false
+
+}
+
+func (r *TrafficOpsReq) ProcessPackagesWithMetaData(packageMetaData []string)
error {
+ pkgs, err := getPackages(r.Cfg)
+ pkgMdataMap := pkgMetaDataToMap(packageMetaData)
+ if err != nil {
+ return errors.New("getting packages: " + err.Error())
+ }
+ for _, pkg := range pkgs {
+ fullPackage := pkg.Name + "-" + pkg.Version
+ if pkgMdataMap[fullPackage] {
+ log.Infof("package %v is assumed to be installed
according to metadata file", fullPackage)
+ r.Pkgs[fullPackage] = true
+ } else if pkgMatch(packageMetaData, pkg.Name) {
+ log.Infof("package %v is assumed to be installed
according to metadata, but doesn't match traffic ops pkg", fullPackage)
Review Comment:
nit: formatting parameter could be more specific; strings may use `%s` for
better compile-time safety
##########
cache-config/t3c-apply/torequest/torequest.go:
##########
@@ -1030,6 +1034,45 @@ func (r *TrafficOpsReq) ProcessPackages() error {
return nil
}
+func pkgMetaDataToMap(pmd []string) map[string]bool {
+ pkgMap := map[string]bool{}
+ for _, pkg := range pmd {
+ pkgMap[pkg] = true
+ }
+ return pkgMap
+}
+
+func pkgMatch(pkgMetaData []string, pk string) bool {
+ for _, pkg := range pkgMetaData {
+ if ok := strings.Contains(pk, pkg); ok {
+ return true
+ }
+ }
+ return false
+
+}
+
+func (r *TrafficOpsReq) ProcessPackagesWithMetaData(packageMetaData []string)
error {
+ pkgs, err := getPackages(r.Cfg)
+ pkgMdataMap := pkgMetaDataToMap(packageMetaData)
+ if err != nil {
+ return errors.New("getting packages: " + err.Error())
+ }
+ for _, pkg := range pkgs {
+ fullPackage := pkg.Name + "-" + pkg.Version
+ if pkgMdataMap[fullPackage] {
+ log.Infof("package %v is assumed to be installed
according to metadata file", fullPackage)
+ r.Pkgs[fullPackage] = true
+ } else if pkgMatch(packageMetaData, pkg.Name) {
+ log.Infof("package %v is assumed to be installed
according to metadata, but doesn't match traffic ops pkg", fullPackage)
+ r.Pkgs[fullPackage] = true
+ } else {
+ log.Infof("package %v does not appear to be
installed.", pkg.Name+"-"+pkg.Version)
Review Comment:
nit: formatting parameter could be more specific; strings may use `%s` for
better compile-time safety
##########
cache-config/t3c-apply/config/config.go:
##########
@@ -471,6 +497,17 @@ If any of the related flags are also set, they override
the mode's default behav
os.Setenv("TO_PASS", toPass)
}
+ rpmDBisOk := VerifyRpmDB()
+
+ if *installPackagesPtr && !rpmDBisOk {
+ if t3cutil.StrToMode(*runModePtr) == t3cutil.ModeBadAss {
+ fatalLogStrs = append(fatalLogStrs, "RPM database check
failed unable to install packages cannot continue in badass mode")
+ } else {
+ fatalLogStrs = append(fatalLogStrs, "RPM database check
failed unable to install packages cannot continue")
+ }
+ }
+
+ toInfoLog = append(toInfoLog, fmt.Sprintf("rpm database is ok: %v",
rpmDBisOk))
Review Comment:
nit: formatting parameter could be more specific; booleans can use `%b` for
better compile-time safety.
##########
cache-config/t3c-apply/torequest/torequest.go:
##########
@@ -1030,6 +1034,45 @@ func (r *TrafficOpsReq) ProcessPackages() error {
return nil
}
+func pkgMetaDataToMap(pmd []string) map[string]bool {
+ pkgMap := map[string]bool{}
+ for _, pkg := range pmd {
+ pkgMap[pkg] = true
+ }
+ return pkgMap
+}
+
+func pkgMatch(pkgMetaData []string, pk string) bool {
+ for _, pkg := range pkgMetaData {
+ if ok := strings.Contains(pk, pkg); ok {
Review Comment:
you don't need to change this, but note that the assignment and checking of
the variable `ok` is useless; the line is absolutely equivalent to just `if
strings.Contains(pk, pkg) {`
##########
cache-config/t3c-apply/torequest/torequest.go:
##########
@@ -1030,6 +1034,45 @@ func (r *TrafficOpsReq) ProcessPackages() error {
return nil
}
+func pkgMetaDataToMap(pmd []string) map[string]bool {
+ pkgMap := map[string]bool{}
+ for _, pkg := range pmd {
+ pkgMap[pkg] = true
+ }
+ return pkgMap
+}
+
+func pkgMatch(pkgMetaData []string, pk string) bool {
+ for _, pkg := range pkgMetaData {
+ if ok := strings.Contains(pk, pkg); ok {
+ return true
+ }
+ }
+ return false
+
+}
+
+func (r *TrafficOpsReq) ProcessPackagesWithMetaData(packageMetaData []string)
error {
+ pkgs, err := getPackages(r.Cfg)
+ pkgMdataMap := pkgMetaDataToMap(packageMetaData)
+ if err != nil {
+ return errors.New("getting packages: " + err.Error())
Review Comment:
constructing errors from errors should wrap them e.g. using `fmt.Errorf`'s
`%w` formatting directive so that the identity of the underlying error is not
destroyed.
##########
cache-config/t3c-apply/t3c-apply.go:
##########
@@ -276,6 +275,9 @@ func Main() int {
log.Errorf("Error verifying system services: %s\n",
err.Error())
return GitCommitAndExit(ExitCodeServicesError,
FailureExitMsg, cfg, metaData, oldMetaData)
}
+ } else {
+ log.Infoln("======== RPM DB checks failed, package processing
not possible, using installed packages from metadata if available========")
Review Comment:
Should this be a warning?
##########
cache-config/t3c-apply/torequest/torequest.go:
##########
@@ -1030,6 +1034,45 @@ func (r *TrafficOpsReq) ProcessPackages() error {
return nil
}
+func pkgMetaDataToMap(pmd []string) map[string]bool {
+ pkgMap := map[string]bool{}
+ for _, pkg := range pmd {
+ pkgMap[pkg] = true
+ }
+ return pkgMap
+}
+
+func pkgMatch(pkgMetaData []string, pk string) bool {
+ for _, pkg := range pkgMetaData {
+ if ok := strings.Contains(pk, pkg); ok {
+ return true
+ }
+ }
+ return false
+
+}
+
+func (r *TrafficOpsReq) ProcessPackagesWithMetaData(packageMetaData []string)
error {
+ pkgs, err := getPackages(r.Cfg)
+ pkgMdataMap := pkgMetaDataToMap(packageMetaData)
+ if err != nil {
+ return errors.New("getting packages: " + err.Error())
+ }
+ for _, pkg := range pkgs {
+ fullPackage := pkg.Name + "-" + pkg.Version
+ if pkgMdataMap[fullPackage] {
+ log.Infof("package %v is assumed to be installed
according to metadata file", fullPackage)
Review Comment:
nit: formatting parameter could be more specific; strings may use `%s` for
better compile-time safety
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
