From 4b1222c4aa765e7040dc2b9aaf1d549d43e54d37 Mon Sep 17 00:00:00 2001 From: Steve Milner Date: Tue, 26 Mar 2019 12:51:48 -0400 Subject: [PATCH] Add basic kernel tuning functionality Checks /etc/pivot/kernel-args for argument changes. This early version only supports bare arguments that are in the whitelist. See conversation at https://github.com/openshift/machine-config-operator/pull/576 Signed-off-by: Steve Milner --- cmd/root.go | 110 +++++++++++++++++++++++++++++++++++++++++- cmd/root_test.go | 57 ++++++++++++++++++++++ types/tuneargument.go | 8 +++ 3 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 types/tuneargument.go diff --git a/cmd/root.go b/cmd/root.go index 360e848..1a59d14 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,6 +1,7 @@ package cmd import ( + "bufio" "encoding/json" "flag" "fmt" @@ -31,9 +32,17 @@ const ( etcPivotFile = "/etc/pivot/image-pullspec" runPivotRebootFile = "/run/pivot/reboot-needed" // Pull secret. Written by the machine-config-operator - kubeletAuthFile = "/var/lib/kubelet/config.json" + kubeletAuthFile = "/var/lib/kubelet/config.json" + // File containing kernel arg changes for tuning + kernelTuningFile = "/etc/pivot/kernel-args" ) +// TODO: fill out the whitelist +// tuneableArgsWhitelist contains allowed keys for tunable arguments +var tuneableArgsWhitelist = map[string]bool{ + "nosmt": true, +} + // RootCmd houses the cobra config for the main command var RootCmd = &cobra.Command{ Use: "pivot [FLAGS] [IMAGE_PULLSPEC]", @@ -51,6 +60,88 @@ func init() { pflag.CommandLine.AddGoFlagSet(flag.CommandLine) } +// isArgTuneable returns if the argument provided is allowed to be modified +func isArgTunable(arg string) bool { + return tuneableArgsWhitelist[arg] +} + +// parseTuningFile parses the kernel argument tuning file +func parseTuningFile(tuningFilePath string) (addArguments []types.TuneArgument, deleteArguments []types.TuneArgument, err error) { + if tuningFilePath == "" { + tuningFilePath = kernelTuningFile + } + // Return fast if the file does not exist + if _, err := os.Stat(tuningFilePath); os.IsNotExist(err) { + glog.V(2).Infof("no kernel tuning needed as %s does not exist", tuningFilePath) + // This isn't an error. Return out. + return addArguments, deleteArguments, err + } + // Read and parse the file + file, err := os.Open(tuningFilePath) + if err != nil { + // If we have an issue reading return an error + glog.Infof("Unable to open %s for reading: %v", tuningFilePath, err) + return addArguments, deleteArguments, err + } + + // Parse the tuning lines + scanner := bufio.NewScanner(file) + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, "ADD") { + // NOTE: Today only specific bare kernel arguments are allowed so + // there is not a need to split on =. + key := line[4:] + if isArgTunable(key) { + addArguments = append(addArguments, types.TuneArgument{Key: key, Bare: true}) + } else { + glog.Infof("%s not a whitelisted kernel argument", key) + } + } else if strings.HasPrefix(line, "DELETE") { + // NOTE: Today only specific bare kernel arguments are allowed so + // there is not a need to split on =. + key := line[7:] + if isArgTunable(key) { + deleteArguments = append(deleteArguments, types.TuneArgument{Key: key, Bare: true}) + } else { + glog.Infof("%s not a whitelisted kernel argument", key) + } + } else { + glog.V(2).Infof(`skipping malformed line in %s: "%s\n"`, tuningFilePath, line) + } + } + return addArguments, deleteArguments, nil +} + +// updateTuningArgs executes additions and removals of kernel tuning arguments +func updateTuningArgs(tuningFilePath string) (changed bool, err error) { + changed = false + additions, deletions, err := parseTuningFile(tuningFilePath) + if err != nil { + return changed, err + } + + // Execute additions + for _, toAdd := range additions { + if toAdd.Bare { + changed = true + utils.Run("rpm-ostree", "kargs", fmt.Sprintf("--append=%s", toAdd.Key)) + } else { + // TODO: currently not supported + } + } + // Execute deletions + for _, toDelete := range deletions { + if toDelete.Bare { + changed = true + utils.Run("rpm-ostree", "kargs", fmt.Sprintf("--delete=%s", toDelete.Key)) + } else { + // TODO: currently not supported + } + } + return changed, nil +} + // podmanRemove kills and removes a container func podmanRemove(cid string) { utils.RunIgnoreErr("podman", "kill", cid) @@ -241,6 +332,23 @@ func Execute(cmd *cobra.Command, args []string) { utils.RunIgnoreErr("podman", "rmi", imgid) } + // Check to see if we need to tune kernel arguments + tuningChanged, err := updateTuningArgs(kernelTuningFile) + if err != nil { + glog.Infof("unable to parse tuning file %s: %s", kernelTuningFile, err) + } + // If tuning changes but the oscontainer didn't we still denote we changed + // for the reboot + if tuningChanged { + changed = true + // Remove the as it's been processed + err = os.Remove(kernelTuningFile) + if err != nil { + glog.Infof(`Unable to remove kernel tuning file %s: "%s"`, kernelTuningFile, err) + } + + } + if !changed { glog.Info("Already at target pivot; exiting...") if exit_77 { diff --git a/cmd/root_test.go b/cmd/root_test.go index a2209a1..11904b3 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1,6 +1,8 @@ package cmd import ( + "io/ioutil" + "os" "testing" ) @@ -30,3 +32,58 @@ func TestCompareOSImageURL(t *testing.T) { t.Fatalf("Expected err") } } + +// writeTestTuningFile writes out a file to use in the test +func writeTestTuningFile(content []byte) (filePath string, err error) { + tmpfile, err := ioutil.TempFile("", "testParseTuningFile") + if err != nil { + return "", err + } + filePath = tmpfile.Name() + if _, err := tmpfile.Write(content); err != nil { + return filePath, err + } + if err := tmpfile.Close(); err != nil { + return filePath, err + } + return filePath, nil +} + +func TestParseTuningFile(t *testing.T) { + // Test with addition/deletion and verify white list + testFilePath, err := writeTestTuningFile([]byte("ADD nosmt\nADD aaaa\nDELETE nosmt\nDELETE nope")) + defer os.Remove(testFilePath) + if err != nil { + t.Fatalf("unable to write test file %s: %s", testFilePath, err) + } + add, delete, err := parseTuningFile(testFilePath) + if err != nil { + t.Fatalf(`Expected no error, got %s`, err) + } + if len(add) != 1 { + t.Fatalf("Expected 1 addition, got %v", len(add)) + } + + if len(delete) != 1 { + t.Fatalf("Expected 1 deletion, got %v", len(add)) + } + + // Test with no changes + testFilePath, err = writeTestTuningFile([]byte("")) + defer os.Remove(testFilePath) + if err != nil { + t.Fatalf("unable to write test file %s: %s", testFilePath, err) + } + add, delete, err = parseTuningFile(testFilePath) + if err != nil { + t.Fatalf(`Expected no error, got %s`, err) + } + + if len(add) != 0 { + t.Fatalf("Expected 0 addition, got %v", len(add)) + } + + if len(delete) != 0 { + t.Fatalf("Expected 0 deletion, got %v", len(add)) + } +} diff --git a/types/tuneargument.go b/types/tuneargument.go new file mode 100644 index 0000000..3f317e6 --- /dev/null +++ b/types/tuneargument.go @@ -0,0 +1,8 @@ +package types + +// TuneArgument represents a single tuning argument +type TuneArgument struct { + Key string `json:"key"` // The name of the argument (or argument itself if Bare) + Value string `json:"value"` // The value of the argument + Bare bool `json:"bare"` // If the kernel argument is a bare argument (no value expected) +}