Merge pull request #177 from angelabriel/master

fix refresh and logging

Author: angelabriel

app/parameter.go

@@ -31,17 +31,9 @@ func collectChangedParameterInfo(noteID, fileName string, comparisons map[string
 	// to get the changes: diff between note in working area and info stored
 	// in sections file /run/saptune/sections/<noteid>.sections
 	for _, param := range content.AllValues {
-		switch param.Section {
-		case note.INISectionVersion, note.INISectionRpm, note.INISectionGrub, note.INISectionFS, note.INISectionReminder:
-			// These parameters are only checked, but not applied.
-			// So nothing to do during refresh
-			continue
-		case note.INISectionCPU, note.INISectionMEM, note.INISectionService, note.INISectionBlock, note.INISectionLimits, note.INISectionLogin, note.INISectionPagecache:
-			// currently not supported for 'refresh'
-			system.InfoLog("parameters from section '%s' currently not supported and not evaluated for 'refresh' operation", param.Section)
+		if !isSectionSupportedForRefresh(param.Key, param.Section) {
 			continue
 		}
-
 		// initialise parameter entry
 		paramEntry := initChangedParams(param, noteID, "newchange")
 		// check for new parameter
@@ -74,6 +66,22 @@ func collectChangedParameterInfo(noteID, fileName string, comparisons map[string
 	return chgParams
 }
 
+// isSectionSupportedForRefresh skips these sections which are currently not
+// supported for refresh
+func isSectionSupportedForRefresh(param, section string) bool {
+	switch section {
+	case note.INISectionVersion, note.INISectionRpm, note.INISectionGrub, note.INISectionFS, note.INISectionReminder:
+		// These parameters are only checked, but not applied.
+		// So nothing to do during refresh
+		return false
+	case note.INISectionCPU, note.INISectionMEM, note.INISectionService, note.INISectionBlock, note.INISectionLimits, note.INISectionLogin, note.INISectionPagecache:
+		// currently not supported for 'refresh'
+		system.InfoLog("parameters (%s) from section '%s' currently not supported and not evaluated for 'refresh' operation", param, section)
+		return false
+	}
+	return true
+}
+
 // initChangedParams initialises the parameter entry
 func initChangedParams(param txtparser.INIEntry, noteID, mode string) map[string]interface{} {
 	paramEntry := make(map[string]interface{})
@@ -279,8 +287,11 @@ func chkParameterValue(paramEntry map[string]interface{}, changed bool, app *App
 // available in the note definition file
 func chkForDeletedParams(noteID string, sectCont []txtparser.INIEntry, noteCont map[string]map[string]txtparser.INIEntry, ovCont map[string]map[string]txtparser.INIEntry, chgParams map[string]map[string]interface{}) {
 	system.DebugLog("chkForDeletedParams - chgParams is '%+v'", chgParams)
-	deleted := false
 	for _, sectParam := range sectCont {
+		if !isSectionSupportedForRefresh(sectParam.Key, sectParam.Section) {
+			continue
+		}
+		deleted := false
 		paramEntry := initChangedParams(sectParam, noteID, "del")
 		if _, ok := noteCont[sectParam.Section]; !ok {
 			// section no longer available in note definition file

development.md

@@ -11,7 +11,7 @@ the sources should be available at $GOPATH/src/github.com/SUSE/saptune
 	version="3.2.0-test"
 	bdate=$(date +"%Y/%m/%d")
 	bvers=15
-	go build -ldflags "-X 'github.com/SUSE/saptune/actions.RPMVersion=$version' -X 'github.com/SUSE/saptune/actions.RPMDate=$bdate' -X 'github.com/SUSE/saptune/system.RPMBldVers=$bvers'"
+	go build -buildmode=pie -ldflags "-X 'github.com/SUSE/saptune/actions.RPMVersion=$version' -X 'github.com/SUSE/saptune/actions.RPMDate=$bdate' -X 'github.com/SUSE/saptune/system.RPMBldVers=$bvers'"
 
 ## lint and format checks for the sources before committing changes
 
@@ -22,7 +22,7 @@ the sources should be available at $GOPATH/src/github.com/SUSE/saptune
 and run the unit tests (in a docker container)
 
 ## unit tests for saptune:
-after committing the changes to git travis is used for automatic testing
+after committing the changes to git github actions are used for automatic testing
 
 But before committing the sources, run the tests locally by using docker and the same workflow as with github actions
 

main.go

@@ -8,7 +8,6 @@ import (
 	"github.com/SUSE/saptune/sap/solution"
 	"github.com/SUSE/saptune/system"
 	"github.com/SUSE/saptune/txtparser"
-	"io"
 	"os"
 	"os/exec"
 	"strings"
@@ -30,13 +29,23 @@ var logSwitch = map[string]string{"verbose": os.Getenv("SAPTUNE_VERBOSE"), "debu
 var SaptuneVersion = ""
 
 func main() {
+	logSwitchFromConfig(system.SaptuneConfigFile(), logSwitch)
+	// special log switch settings for json
 	system.InitOut(logSwitch)
+
+	// activate logging
+	system.LogInit(logFile, logSwitch)
+	// now system.ErrorExit can write to log and os.Stderr. No longer extra
+	// care is needed.
+	system.InfoLog("saptune (%s) started with '%s'", actions.RPMVersion, strings.Join(os.Args, " "))
+	system.InfoLog("build for '%d'", system.IfdefVers())
+
 	if !system.ChkCliSyntax() {
 		actions.PrintHelpAndExit(os.Stdout, 1)
 	}
 
-	// get saptune version and log switches from saptune sysconfig file
-	SaptuneVersion = checkSaptuneConfigFile(os.Stderr, system.SaptuneConfigFile(), logSwitch)
+	// get saptune version from saptune config file and check file content
+	SaptuneVersion = checkSaptuneConfigFile(system.SaptuneConfigFile())
 
 	arg1 := system.CliArg(1)
 	if arg1 == "version" || system.IsFlagSet("version") {
@@ -54,17 +63,9 @@ func main() {
 
 	// All other actions require super user privilege
 	if os.Geteuid() != 0 {
-		fmt.Fprintf(os.Stderr, "Please run saptune with root privilege.\n")
-		system.ErrorExit("", 1)
+		system.ErrorExit("Please run saptune with root privilege.\n", 1)
 	}
 
-	// activate logging
-	system.LogInit(logFile, logSwitch)
-	// now system.ErrorExit can write to log and os.Stderr. No longer extra
-	// care is needed.
-	system.InfoLog("saptune (%s) started with '%s'", actions.RPMVersion, strings.Join(os.Args, " "))
-	system.InfoLog("build for '%d'", system.IfdefVers())
-
 	if arg1 == "lock" {
 		if arg2 := system.CliArg(2); arg2 == "remove" {
 			system.JnotSupportedYet()
@@ -265,27 +266,17 @@ func checkWorkingArea() {
 // checkSaptuneConfigFile checks the config file /etc/sysconfig/saptune
 // if it exists, if it contains all needed variables and for some variables
 // checks, if the values is valid
-// returns the saptune version and changes some log switches
-func checkSaptuneConfigFile(writer io.Writer, saptuneConf string, lswitch map[string]string) string {
+// returns the saptune version
+func checkSaptuneConfigFile(saptuneConf string) string {
 	if system.CliArg(1) == "configure" && system.CliArg(2) == "reset" {
-		if lswitch["debug"] == "" {
-			lswitch["debug"] = "off"
-		}
-		if lswitch["verbose"] == "" {
-			lswitch["verbose"] = "on"
-		}
-		if lswitch["error"] == "" {
-			lswitch["error"] = "on"
-		}
+		// skip check
 		return "3"
 	}
-
 	missingKey := []string{}
 	keyList := actions.MandKeyList()
 	sconf, err := txtparser.ParseSysconfigFile(saptuneConf, false)
 	if err != nil {
-		fmt.Fprintf(writer, "Error: Checking saptune configuration file - Unable to read file '%s': %v\n", saptuneConf, err)
-		system.ErrorExit("", 128)
+		system.ErrorExit("Checking saptune configuration file - Unable to read file '%s': %v", saptuneConf, err, 128)
 	}
 	// check, if all needed variables are available in the saptune
 	// config file
@@ -295,14 +286,12 @@ func checkSaptuneConfigFile(writer io.Writer, saptuneConf string, lswitch map[st
 		}
 	}
 	if len(missingKey) != 0 {
-		fmt.Fprintf(writer, "Error: File '%s' is broken. Missing variables '%s'\n", saptuneConf, strings.Join(missingKey, ", "))
-		system.ErrorExit("", 128)
+		system.ErrorExit("File '%s' is broken. Missing variables '%s'", saptuneConf, strings.Join(missingKey, ", "), 128)
 	}
 	txtparser.GetSysctlExcludes(sconf.GetString("SKIP_SYSCTL_FILES", ""))
 	stageVal := sconf.GetString("STAGING", "")
 	if stageVal != "true" && stageVal != "false" {
-		fmt.Fprintf(writer, "Error: Variable 'STAGING' from file '%s' contains a wrong value '%s'. Needs to be 'true' or 'false'\n", saptuneConf, stageVal)
-		system.ErrorExit("", 128)
+		system.ErrorExit("Variable 'STAGING' from file '%s' contains a wrong value '%s'. Needs to be 'true' or 'false'", saptuneConf, stageVal, 128)
 	}
 
 	// check saptune-discovery-period of the Trento Agent
@@ -313,10 +302,18 @@ func checkSaptuneConfigFile(writer io.Writer, saptuneConf string, lswitch map[st
 	// set values read from the config file
 	saptuneVers := sconf.GetString("SAPTUNE_VERSION", "")
 	if saptuneVers != "1" && saptuneVers != "2" && saptuneVers != "3" {
-		fmt.Fprintf(writer, "Error: Wrong saptune version in file '%s': %s\n", saptuneConf, saptuneVers)
-		system.ErrorExit("", 128)
+		system.ErrorExit("Wrong saptune version in file '%s': %s", saptuneConf, saptuneVers, 128)
 	}
+	return saptuneVers
+}
 
+// logSwitchFromConfig reads log switch settings from the saptune
+// config file
+func logSwitchFromConfig(saptuneConf string, lswitch map[string]string) {
+	sconf, err := txtparser.ParseSysconfigFile(saptuneConf, false)
+	if err != nil {
+		system.ErrorExit("Checking saptune configuration file - Unable to read file '%s': %v", saptuneConf, err, 128)
+	}
 	// Switch Debug on ("on") or off ("off" - default)
 	// Switch verbose mode on ("on" - default) or off ("off")
 	// Switch error mode on ("on" - default) or off ("off")
@@ -330,5 +327,4 @@ func checkSaptuneConfigFile(writer io.Writer, saptuneConf string, lswitch map[st
 	if lswitch["error"] == "" {
 		lswitch["error"] = sconf.GetString("ERROR", "on")
 	}
-	return saptuneVers
 }

main_test.go

@@ -137,58 +137,60 @@ func TestCheckSaptuneConfigFile(t *testing.T) {
 	logSwitch := map[string]string{"verbose": "", "debug": ""}
 	// check saptune version and debug
 	saptuneConf := fmt.Sprintf("%s/saptune_VersAndDebug", TstFilesInGOPATH)
-	buffer := bytes.Buffer{}
+	//buffer := bytes.Buffer{}
 
 	lSwitch := logSwitch
-	saptuneVers := checkSaptuneConfigFile(&buffer, saptuneConf, lSwitch)
+	saptuneVers := checkSaptuneConfigFile(saptuneConf)
 	if saptuneVers != "5" {
 		t.Errorf("wrong value for 'SAPTUNE_VERSION' - '%+v' instead of ''\n", saptuneVers)
 	}
+	logSwitchFromConfig(saptuneConf, lSwitch)
 	if lSwitch["debug"] != "on" {
 		t.Errorf("wrong value for 'DEBUG' - '%+v' instead of 'on'\n", lSwitch["debug"])
 	}
 	if lSwitch["verbose"] != "on" {
 		t.Errorf("wrong value for 'VERBOSE' - '%+v' instead of 'on'\n", lSwitch["debug"])
 	}
 
-	buffer.Reset()
+	//buffer.Reset()
+
 	errExitbuffer := bytes.Buffer{}
 	tstwriter = &errExitbuffer
 
 	// check missing variable
 	saptuneConf = fmt.Sprintf("%s/saptune_MissingVar", TstFilesInGOPATH)
-	matchTxt := fmt.Sprintf("Error: File '%s' is broken. Missing variables 'COLOR_SCHEME'\n", saptuneConf)
-	lSwitch = logSwitch
-	_ = checkSaptuneConfigFile(&buffer, saptuneConf, lSwitch)
+	matchTxt := fmt.Sprintf("ERROR: File '%s' is broken. Missing variables 'COLOR_SCHEME'\n", saptuneConf)
+	//lSwitch = logSwitch
+	_ = checkSaptuneConfigFile(saptuneConf)
 
-	txt := buffer.String()
-	checkOut(t, txt, matchTxt)
+	//txt := buffer.String()
+	//checkOut(t, txt, matchTxt)
 	if tstRetErrorExit != 128 {
 		t.Errorf("error exit should be '128' and NOT '%v'\n", tstRetErrorExit)
 	}
 	errExOut := errExitbuffer.String()
-	if errExOut != "" {
+	if errExOut != matchTxt {
 		t.Errorf("wrong text returned by ErrorExit: '%v' instead of '%v'\n", errExOut, matchTxt)
 	}
 
 	// initialise next test
-	buffer.Reset()
+	//buffer.Reset()
 	errExitbuffer.Reset()
 
 	// check wrong STAGING value
 	saptuneConf = fmt.Sprintf("%s/saptune_WrongStaging", TstFilesInGOPATH)
 	saptuneVers = ""
-	matchTxt = fmt.Sprintf("Error: Variable 'STAGING' from file '%s' contains a wrong value 'hugo'. Needs to be 'true' or 'false'\n", saptuneConf)
-	lSwitch = logSwitch
-	_ = checkSaptuneConfigFile(&buffer, saptuneConf, lSwitch)
+	matchTxt = fmt.Sprintf("ERROR: Variable 'STAGING' from file '%s' contains a wrong value 'hugo'. Needs to be 'true' or 'false'\n", saptuneConf)
+	//lSwitch = logSwitch
+	_ = checkSaptuneConfigFile(saptuneConf)
 
-	txt = buffer.String()
-	checkOut(t, txt, matchTxt)
+	//txt = buffer.String()
+	//checkOut(t, txt, matchTxt)
 	if tstRetErrorExit != 128 {
 		t.Errorf("error exit should be '128' and NOT '%v'\n", tstRetErrorExit)
 	}
 	errExOut = errExitbuffer.String()
-	if errExOut != "" {
-		t.Errorf("wrong text returned by ErrorExit: '%v' instead of ''\n", errExOut)
+	if errExOut != matchTxt {
+		t.Errorf("wrong text returned by ErrorExit: '%v' instead of '%v'\n", errExOut, matchTxt)
 	}
 }

ospackage/usr/share/saptune/notes/2205917

@@ -36,7 +36,7 @@ THP=never
 #
 KSM=0
 
-[cpu]
+[cpu:arch=x86_64]
 # Energy Performance Bias EPB (applies to Intel-based systems only)
 # energy_perf_bias: performance - 0, normal - 6, powersave - 15
 # cpupower set -b 0, if system supports Intel's performance bias setting
@@ -127,7 +127,7 @@ kernel-default 4.12.14-95.13.1
 [rpm:os=12-SP5:arch=ppc64le]
 kernel-default 4.12.14-122.17
 
-[grub]
+[grub:arch=x86_64]
 # /etc/default/grub GRUB_CMDLINE_LINUX_DEFAULT
 # saptune only checks the values. Changing the grub configuration is not
 # supported by saptune

system/cpu.go

@@ -225,8 +225,6 @@ func GetGovernor() map[string]string {
 			gGov[cpuName] = gov
 		}
 	}
-	fmt.Printf("setAll is '%+v'\n", setAll)
-	fmt.Printf("gGov is '%+v'\n", gGov)
 	if setAll {
 		gGov = make(map[string]string)
 		gGov["all"] = oldgov