Bug 1571403

Description of problem: If the grub.cfg file doesn't exist for some reason, then profile cleanup doesn't occur when switching profiles. Example if a custom profile has a [bootloader] and [script] section, when switching from the custom profile to say the virtual-guest profile. The script in the custom profile is executed for every profile switch after that.


Version-Release number of selected component (if applicable): tuned-2.9.0-1.el7


Steps to Reproduce:
1. # mkdir /etc/tuned/test

2. # cat > /etc/tuned/test/tuned.conf <<EOF
[main]
summary=Test profile

[bootloader]
cmdline="transparent_hugepages=never"

[script]
script=${i:PROFILE_DIR}/test.sh
EOF

3. # cat > /etc/tuned/test/test.sh <<EOF
#!/bin/bash

OPERATION=$1

if [ "$OPERATION" = "start" ]; then
	echo "start"
else
	echo "not start"
fi
EOF

4. # mv /boot/grub2/grub.cfg /boot/grub2/grub.cfg.bak
5. # tuned-adm profile test
6. # tuned-adm profile <virtual-guest/balanced>
7. # egrep "loading profile|applied|calling script" /var/log/tuned/tuned.log

Actual results:
2018-04-24 12:48:36,491 INFO     tuned.profiles.loader: loading profile: test
2018-04-24 12:48:36,512 INFO     tuned.plugins.plugin_script: calling script '/etc/tuned/test/cpu_tweaks.sh' with arguments '['start']'
2018-04-24 12:48:36,596 INFO     tuned.plugins.plugin_script: calling script '/etc/tuned/test/test.sh' with arguments '['start']'
2018-04-24 12:48:36,601 INFO     tuned.daemon.daemon: static tuning from profile 'test' applied
2018-04-24 12:48:51,619 INFO     tuned.plugins.plugin_script: calling script '/etc/tuned/test/test.sh' with arguments '['stop', 'full_rollback']'
2018-04-24 12:48:51,655 INFO     tuned.profiles.loader: loading profile: virtual-guest
2018-04-24 12:48:51,696 INFO     tuned.plugins.plugin_script: calling script '/etc/tuned/test/cpu_tweaks.sh' with arguments '['start']'
2018-04-24 12:48:51,786 INFO     tuned.plugins.plugin_script: calling script '/etc/tuned/test/test.sh' with arguments '['start']'
2018-04-24 12:48:51,854 INFO     tuned.daemon.daemon: static tuning from profile 'virtual-guest' applied


Expected results:
2018-04-24 13:02:17,080 INFO     tuned.profiles.loader: loading profile: test
2018-04-24 13:02:17,095 INFO     tuned.plugins.plugin_script: calling script '/etc/tuned/test/test.sh' with arguments '['start']'
2018-04-24 13:02:17,107 INFO     tuned.daemon.daemon: static tuning from profile 'test' applied
2018-04-24 13:02:29,126 INFO     tuned.plugins.plugin_script: calling script '/etc/tuned/test/test.sh' with arguments '['stop', 'full_rollback']'
2018-04-24 13:02:29,147 INFO     tuned.profiles.loader: loading profile: virtual-guest
2018-04-24 13:02:29,260 INFO     tuned.daemon.daemon: static tuning from profile 'virtual-guest' applied


Additional info:
From my testing I found that if the grub.cfg file is missing, it hits an error that isn't caught. That causes the cleanup scripts to not run, which flushes the array that holds any scripts from the config file. Since the array isn't flushed, the test.sh script will get ran over and over until the service is restarted. Below is the error traceback from the journal, and code snippet causing the issue.

1135]: Exception in thread Thread-7:
1135]: Traceback (most recent call last):
1135]: File "/usr/lib64/python2.7/threading.py", line 812, in __bootstrap_inner
1135]: self.run()
1135]: File "/usr/lib64/python2.7/threading.py", line 765, in run
1135]: self.__target(*self.__args, **self.__kwargs)
1135]: File "/usr/lib/python2.7/site-packages/tuned/daemon/daemon.py", line 181, in _thread_code
1135]: self._unit_manager.stop_tuning(full_rollback)
1135]: File "/usr/lib/python2.7/site-packages/tuned/units/manager.py", line 113, in stop_tuning
1135]: instance.unapply_tuning(full_rollback)
1135]: File "/usr/lib/python2.7/site-packages/tuned/plugins/instance/instance.py", line 82, in unapply_t
1135]: self._plugin.instance_unapply_tuning(self, full_rollback)
1135]: File "/usr/lib/python2.7/site-packages/tuned/plugins/base.py", line 289, in instance_unapply_tuni
1135]: self._instance_unapply_static(instance, full_rollback)
1135]: File "/usr/lib/python2.7/site-packages/tuned/plugins/plugin_bootloader.py", line 104, in _instanc
1135]: self._remove_grub2_tuning()
1135]: File "/usr/lib/python2.7/site-packages/tuned/plugins/plugin_bootloader.py", line 96, in _remove_g
1135]: self._cmd.add_modify_option_in_file(self._grub2_cfg_file_name, {"set\s+" + consts.GRUB2_TUNED_VAR
1135]: File "/usr/lib/python2.7/site-packages/tuned/utils/commands.py", line 187, in add_modify_option_i
1135]: data = self.read_file(f)
1135]: File "/usr/lib/python2.7/site-packages/tuned/utils/commands.py", line 117, in read_file
1135]: f = open(f, "r")
1135]: TypeError: coercing to Unicode: need string or buffer, NoneType found

** Below is from /usr/lib/python2.7/site-packages/tuned/utils/commands.py

        def add_modify_option_in_file(self, f, d, add = True):
                data = self.read_file(f)--,
                ,-------------------------'
        def read_file(self, f, err_ret = "", no_error = False):
                old_value = err_ret
                try:
                        f = open(f, "r") <<<<----It errors out here.
                        old_value = f.read()
                        f.close()
                except (OSError,IOError) as e: <<<<----This except doesn't catch the error because it's a TypeError
                        if not no_error:
                                self._error("Error when reading file '%s': '%s'" % (f, e))
                self._debug("Read data from file: '%s' > '%s'" % (f, old_value))
                return old_value

If you fix the above by adding TypeError to the except, then it would just hit the same issue at the bottom of the add_modify_option_in_file function, as you can see below.

        def add_modify_option_in_file(self, f, d, add = True):
                data = self.read_file(f)
                for opt in d:
                        o = str(opt)
                        v = str(d[opt])
                        if re.search(r"\b" + o + r"\s*=.*$", data, flags = re.MULTILINE) is None:
                                if add:
                                        if len(data) > 0 and data[-1] != "\n":
                                                data += "\n"
                                        data += "%s=\"%s\"\n" % (o, v)
                        else:
                                data = re.sub(r"\b(" + o + r"\s*=).*$", r"\1" + "\"" + v + "\"", data, flags = re.MULTILINE)

                return self.write_to_file(f, data)----,
                  ,-----------------------------------'
        def write_to_file(self, f, data, makedir = False, no_error = False):
                self._debug("Writing to file: '%s' < '%s'" % (f, data))
                if makedir:
                        d = os.path.dirname(f)
                        if os.path.isdir(d):
                                makedir = False
                try:
                        if makedir:
                                os.makedirs(d)
                        fd = open(f, "w") <<<<<-----It would error here due to the TypeError
                        fd.write(str(data))
                        fd.close()
                        rc = True
                except (OSError,IOError) as e: <<<----It wouldn't catch here due to TypeError not being specified.
                        rc = False
                        if not no_error:
                                self._error("Writing to file '%s'" % (f))
                return rc

I fill there should be a check prior to calling the add_modify_option_in_file function to check if self._grub2_cfg_file_name actually contains a file name.