Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 153 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"bufio"
"encoding/json"
"flag"
"fmt"
Expand Down Expand Up @@ -31,9 +32,18 @@ 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"
cmdLineFile = "/proc/cmdline"
)

// 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]",
Expand All @@ -51,6 +61,133 @@ 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]
}

// isArgInUse checks to see if the argument is already in use by the system currently
func isArgInUse(arg, cmdLinePath string) (bool, error) {
if cmdLinePath == "" {
cmdLinePath = cmdLineFile
}
content, err := ioutil.ReadFile(cmdLinePath)
if err != nil {
return false, err
}

checkable := string(content)
if strings.Contains(checkable, arg) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think right now this is OK, but in the future we're clearly going to need to actually parse the cmdline properly because this will e.g. succeed if some other user-editable string in the cmdline contains nosmt like rd.lvm.lv=janosmt because their name is "Janos Mountain" or something.

return true, nil
}
return false, nil
}

// parseTuningFile parses the kernel argument tuning file
func parseTuningFile(tuningFilePath, cmdLinePath string) ([]types.TuneArgument, []types.TuneArgument, error) {
addArguments := []types.TuneArgument{}
deleteArguments := []types.TuneArgument{}
if tuningFilePath == "" {
tuningFilePath = kernelTuningFile
}
if cmdLinePath == "" {
cmdLinePath = cmdLineFile
}
// 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)
// Clean up
defer file.Close()
Copy link
Member

@runcom runcom May 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if open fails for any reason (like a not found of the tuningFilePath), this will panic, this statement should go after the error check below

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#52


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 := strings.TrimSpace(line[len("ADD "):])
if isArgTunable(key) {
// Find out if the argument is in use
inUse, err := isArgInUse(key, cmdLinePath)
if err != nil {
return addArguments, deleteArguments, err
}
if !inUse {
addArguments = append(addArguments, types.TuneArgument{Key: key, Bare: true})
} else {
glog.Infof(`skipping "%s" as it is already in use`, key)
}
} 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 := strings.TrimSpace(line[len("DELETE "):])
if isArgTunable(key) {
inUse, err := isArgInUse(key, cmdLinePath)
if err != nil {
return addArguments, deleteArguments, err
}
if inUse {
deleteArguments = append(deleteArguments, types.TuneArgument{Key: key, Bare: true})
} else {
glog.Infof(`skipping "%s" as it is not present in the current argument list`, key)
}
} else {
glog.Infof("%s not a whitelisted kernel argument", key)
}
} else {
glog.V(2).Infof(`skipping malformed line in %s: "%s"`, tuningFilePath, line)
}
}
return addArguments, deleteArguments, nil
}

// updateTuningArgs executes additions and removals of kernel tuning arguments
func updateTuningArgs(tuningFilePath, cmdLinePath string) (bool, error) {
if cmdLinePath == "" {
cmdLinePath = cmdLineFile
}
changed := false
additions, deletions, err := parseTuningFile(tuningFilePath, cmdLinePath)
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)
Expand Down Expand Up @@ -241,6 +378,21 @@ 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, cmdLineFile)
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
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 {
Expand Down
110 changes: 110 additions & 0 deletions cmd/root_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package cmd

import (
"io/ioutil"
"os"
"testing"
)

Expand Down Expand Up @@ -30,3 +32,111 @@ func TestCompareOSImageURL(t *testing.T) {
t.Fatalf("Expected err")
}
}

// writeTestFile writes out a file to use in the test
func writeTestFile(content []byte) (filePath string, err error) {
tmpfile, err := ioutil.TempFile("", "testFile")
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) {
cmdLineFileMock, err := writeTestFile([]byte(
"BOOT_IMAGE=/a/vmlinuz.x86_64 resume=/dev/mapper/swap rhgb quiet root=/a/b/c/root ostree=/ostree/boot.0/a/0"))
defer os.Remove(cmdLineFileMock)

// Test with addition/deletion and verify white list
testFilePath, err := writeTestFile([]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, cmdLineFileMock)
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) != 0 {
t.Fatalf("Expected 0 deletion, got %v", len(delete))
}

deleteCmdLineFileMockWith, err := writeTestFile([]byte(
"BOOT_IMAGE=/a/vmlinuz.x86_64 nosmt resume=/dev/mapper/swap rhgb quiet root=/a/b/c/root ostree=/ostree/boot.0/a/0"))
defer os.Remove(deleteCmdLineFileMockWith)

// Test with addition/deletion and verify white list
testFilePath, err = writeTestFile([]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, deleteCmdLineFileMockWith)
if err != nil {
t.Fatalf(`Expected no error, got %s`, err)
}
if len(add) != 0 {
t.Fatalf("Expected 1 addition, got %v", len(add))
}

if len(delete) != 1 {
t.Fatalf("Expected 1 deletion, got %v", len(delete))
}

// Test with no changes
testFilePath, err = writeTestFile([]byte(""))
defer os.Remove(testFilePath)
if err != nil {
t.Fatalf("unable to write test file %s: %s", testFilePath, err)
}
add, delete, err = parseTuningFile(testFilePath, cmdLineFileMock)
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))
}
}

func TestIsArgInUse(t *testing.T) {
testFilePath, err := writeTestFile([]byte(
"BOOT_IMAGE=/a/vmlinuz.x86_64 resume=/dev/mapper/swap rhgb quiet root=/a/b/c/root ostree=/ostree/boot.0/a/0"))
if err != nil {
t.Fatalf("unable to write test file %s: %s", testFilePath, err)
}
defer os.Remove(testFilePath)

// Should be present
available, err := isArgInUse("quiet", testFilePath)
if err != nil {
t.Fatalf(`Expected no error, got %s`, err)
}
if available != true {
t.Fatalf("Expected true, got false")
}

// Should not be present
available, err = isArgInUse("idonotexist", testFilePath)
if err != nil {
t.Fatalf(`Expected no error, got %s`, err)
}
if available != false {
t.Fatalf("Expected false, got true")
}
}
8 changes: 8 additions & 0 deletions types/tuneargument.go
Original file line number Diff line number Diff line change
@@ -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)
}