Merge ~dashmage/charm-sysconfig:bug-2012581/inherit-existing-kernel-params into charm-sysconfig:master

[Ashley James]

Inherit pre-existing kernel cmdline parameters by default.

- Also prevents overwrite of existing console parameters by the charm.

Fix for issue #4 in BSENG-1027 [1] for the LP bugs #2012581 [2], #1854135 [3]. Addresses this comment on the LP bug as well [4].

[1]: https://warthogs.atlassian.net/browse/BSENG-1027
[2]: https://bugs.launchpad.net/charm-sysconfig/+bug/2012581
[3]: https://bugs.launchpad.net/charm-sysconfig/+bug/1854135
[4]: https://bugs.launchpad.net/charm-sysconfig/+bug/1854135/comments/4

[JamesLin (jneo8)]

It's LGTM for implementation part, but I think the unit test for it is required.
Also please provide more detail to explain why this change works.

review: Needs Fixing

[Erhan Sunar (esunar)]

Can you link the bug reports to MR

review: Needs Fixing

[Ashley James (dashmage)]

> It's LGTM for implementation part, but I think the unit test for it is
> required.

There seems to be unit tests already for the `update_grub_file` function [1]. Would I need to add something additionally to test for the change made in the template file?

[1]: https://git.launchpad.net/charm-sysconfig/tree/src/tests/unit/test_lib.py#n250

> Also please provide more detail to explain why this change works.

```
-{% else -%}
-GRUB_CMDLINE_LINUX_DEFAULT=""
```

This is to allow inheriting the pre-existing kernel cmdline parameters the default option. With the charm no longer resetting the value of `GRUB_CMDLINE_LINUX_DEFAULT` to an empty string, it'll only append to the already existing values (ex: coming from MAAS).

```
-GRUB_CMDLINE_LINUX_DEFAULT="$GRUB_CMDLINE_LINUX_DEFAULT console=ttyS0,115200 console=tty0"
+GRUB_CMDLINE_LINUX_DEFAULT="console=ttyS0,115200 console=tty0 $GRUB_CMDLINE_LINUX_DEFAULT"
```

These parameters have been switched around so that the pre-existing console values (ex: coming from MAAS) will not be overwritten by the charm. According to my understanding, the console parameters (if defined) from `$GRUB_CMDLINE_LINUX_DEFAULT` will take precedence since they are declared the last.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:2291db2609ae00ae38158e39b47ee962b3e7aa2c
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/11/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/671/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/227/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/11//rebuild

review: Approve (continuous-integration)

[Ashley James (dashmage)]

Got feedback from @mfo who implemented the initial logic in the grub template file:

```
It looks like that MR is good.

AFAICT it
1) fixes the case I had wrong (do not erase $GRUB... if charm config option is not set), and
2) also addresses the case there are MAAS kernel options.
I haven't done any testing (I know you would / already have, for both issues), but it looks good from what I see.
```

[JamesLin (jneo8)]

> There seems to be unit tests already for the `update_grub_file` function [1]. Would I need to add something additionally to test for the change made in the template file?

The unit test currently doesn't cover the bug case and it should be.

review: Needs Fixing

[Eric Chen (eric-chen)]

> > It's LGTM for implementation part, but I think the unit test for it is
> > required.
>
> There seems to be unit tests already for the `update_grub_file` function [1].
> Would I need to add something additionally to test for the change made in the
> template file?
>
> [1]: https://git.launchpad.net/charm-
> sysconfig/tree/src/tests/unit/test_lib.py#n250
>
> > Also please provide more detail to explain why this change works.
>
> ```
> -{% else -%}
> -GRUB_CMDLINE_LINUX_DEFAULT=""
> ```
>
> This is to allow inheriting the pre-existing kernel cmdline parameters the
> default option. With the charm no longer resetting the value of
> `GRUB_CMDLINE_LINUX_DEFAULT` to an empty string, it'll only append to the
> already existing values (ex: coming from MAAS).
>
> ```
> -GRUB_CMDLINE_LINUX_DEFAULT="$GRUB_CMDLINE_LINUX_DEFAULT console=ttyS0,115200
> console=tty0"
> +GRUB_CMDLINE_LINUX_DEFAULT="console=ttyS0,115200 console=tty0
> $GRUB_CMDLINE_LINUX_DEFAULT"
> ```
>
> These parameters have been switched around so that the pre-existing console
> values (ex: coming from MAAS) will not be overwritten by the charm. According
> to my understanding, the console parameters (if defined) from
> `$GRUB_CMDLINE_LINUX_DEFAULT` will take precedence since they are declared the
> last.

This is a little tricky to me. There already a precedence design under /etc/grub.d/ folder.
If we want to put "console=ttyS0,115200 console=tty0" as a "nice to have" option, we should put it into 00xxxx. But now, we put it into 90sysconfig (almost the biggest number) and use this way to avoid taking precedence.

I don't know if other people have the same feeling. I suggest that we should find the stakeholders to discuss which one should have higher precedence and solve it in a correct way.

[Ashley James (dashmage)]

After discussing with @nobuto, the real issue faced is not that the MAAS console parameters are being overwritten.

```
the original issue was that even if the machine already had ttyS0 and tty, charm added those additionally and it triggered a reboot required flag

as long as the reboot required flag is not triggered, I'm happy with any solutions
```

The charm will certainly trigger a reboot notification at the time of deployment since the template file has minor changes from the original config file on the unit. But this only happens at the time of initial deployment of the charm. Could a possible solution be to have the charm run the `clear-notification` action as part of the install hook handler since the notification is a false positive?

In case the charm does need to support a way to prevent overwriting the MAAS kernel parameters, I've listed down some possible solutions and my proposal in this document here: https://docs.google.com/document/d/19sB_LqBY5xFL0-8JDc1RzgOzQh8quwa3NSsXQIbkfj0/edit?usp=sharing

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:225a3b36c610e0738825658b6c2b139e12757323
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/12/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/786/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/246/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/12//rebuild

review: Needs Fixing (continuous-integration)

[Ashley James (dashmage)]

What this MR does:
- Inherit pre-existing kernel parameters (eg: from MAAS) by default
- Remove hard-coded console parameters
- Introduce new "console" config option

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:954c28f4bc7d01930647930569f7784b00cbb7f8
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/13/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/800/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/251/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/13//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:e31dbba8a8632452c997a62aafc46db55f0f76f0
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/14/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/801/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/252/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/14//rebuild

review: Approve (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:954c28f4bc7d01930647930569f7784b00cbb7f8
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/15/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/802/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/253/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/15//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:e31dbba8a8632452c997a62aafc46db55f0f76f0
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/16/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/803/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/254/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/16//rebuild

review: Approve (continuous-integration)

[Eric Chen (eric-chen)]

1. No unit test for prepare_console_context
2. some inline comment, please fix it.

review: Needs Fixing

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:856dfa2902990f86b739778705825582a05210ac
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/17/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/809/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/262/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/17//rebuild

review: Needs Fixing (continuous-integration)

[Ashley James (dashmage)]

Updated MR:
- Inherit pre-existing kernel parameters (eg: from MAAS) by default
- Remove hard-coded console parameters
- Rework config parser function to support console parameters
- Unit tests for config parser function

[JamesLin (jneo8)]

Please also update the config and doc-string.

review: Needs Fixing

[Eric Chen (eric-chen)]

review: Needs Fixing

[Eric Chen (eric-chen)]

Please see inline comment.

review: Needs Fixing

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:2ca99b5719a8e6e089990cc102717a86ca0b4ebc
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/19/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/811/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/264/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/19//rebuild

review: Needs Fixing (continuous-integration)

[Ashley James (dashmage)]

Resolved comments by @jneo8 and @eric-chen

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:71aa123b8e13fe2cb128c3003116127f5fa86f01
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/20/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/812/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/265/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/20//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:2ca99b5719a8e6e089990cc102717a86ca0b4ebc
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/21/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/813/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/266/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/21//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:71aa123b8e13fe2cb128c3003116127f5fa86f01
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/22/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/814/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/267/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/22//rebuild

review: Needs Fixing (continuous-integration)

[Eric Chen (eric-chen)]

please check inline comment and fix it

review: Needs Fixing

[Eric Chen (eric-chen)]

one more inline comment

review: Needs Fixing

[Ashley James (dashmage)]

> I think a string with "=" assignments is not the key point in case 3. There are also "=" in case 1&2. The major different is double quote. Please mention that the subkey and value can be assigned by double quote.

There are "=" in the prev 2 cases but they do not occur in the values. Here in case 3,
'key1="subkey1=val1 subkey2=val2", key2=val3' -- the double quotes are just to make the assignment between key value pairs more clearer. The key point is that the subkey values themselves contain assignments that include "=". Even if the double quotes aren't present, the parser will correctly parse it into a dictionary.

> This case is weird. Don't we need the double quote?
'key1="subkey1=val1,val2 subkey2=val3"'

Here as well, the double quotes aren't really necessary but just to aid in identifying the key and value clearer. This case is to show how there's "=" assignments and the subkey is assigned multiple values separated by commas unlike the previous case which just contained a single value. The commas between val1 and val2 in "subkey1=val1,val2" is what this case is supposed to capture.

I've also added another unit test where the double quotes aren't present. I've addressed the other inline comments as well.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:41ba0f611fde4f78b2559ffc9bd5f49f26083836
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/23/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/849/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/268/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/23//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:41ba0f611fde4f78b2559ffc9bd5f49f26083836
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/24/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/851/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/269/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/24//rebuild

review: Needs Fixing (continuous-integration)

[Eric Chen (eric-chen)]

Please check my inline comment.

review: Needs Fixing

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:41ba0f611fde4f78b2559ffc9bd5f49f26083836
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/25/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/852/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/270/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/25//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:081d772a25f6fccbd0c3bdc14bb761238e55d4d4
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/26/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/853/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/271/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/26//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:081d772a25f6fccbd0c3bdc14bb761238e55d4d4
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/27/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/854/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/272/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/27//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:081d772a25f6fccbd0c3bdc14bb761238e55d4d4
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/29/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/856/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/274/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/29//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:081d772a25f6fccbd0c3bdc14bb761238e55d4d4
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/30/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/857/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/275/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/30//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:081d772a25f6fccbd0c3bdc14bb761238e55d4d4
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/31/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/858/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/276/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/31//rebuild

review: Approve (continuous-integration)

[Eric Chen (eric-chen)]

Please check inline comment. You miss some of my comments before.

review: Needs Fixing

[Ashley James (dashmage)]

@eric-chen -- addressed all your latest comments

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:c5f37ade0c63fcd4770f9c15ed3133f5486c26b2
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/34/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/861/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/279/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/34//rebuild

review: Approve (continuous-integration)

[Eric Chen (eric-chen)]

review: Approve

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:0e5531c372b4776fcde2f541565fc17c6d8f9886
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/35/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/862/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/280/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-sysconfig-ci/35//rebuild

review: Approve (continuous-integration)

[JamesLin (jneo8)]

LGTM

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision a2e49a8d488bbf5eadcf84dc5934731b5ec44743

diff --git a/src/actions.yaml b/src/actions.yaml
index 1dcaedb..93282ae 100644
--- a/src/actions.yaml
+++ b/src/actions.yaml
@@ -6,7 +6,7 @@ clear-notification:
     By doing so, unit will change the description to 'ready'
 update-grub:
   description: |
-    This action will run 'update-grub' to the generate a GRUB configuration file.
+    This action will run 'update-grub' to generate a GRUB configuration file.
 check-update-grub:
   description: |
     This action will test if there is any GRUB configuration changes available.
diff --git a/src/config.yaml b/src/config.yaml
index ba4a0a6..f7455e4 100644
--- a/src/config.yaml
+++ b/src/config.yaml
@@ -121,6 +121,9 @@ options:
         to avoid them being overwritten, for example
         grub-config-flags='GRUB_CMDLINE_LINUX_DEFAULT="$GRUB_CMDLINE_LINUX_DEFAULT
         nvme_core.multipath=0'
+        Also supports adding values with special characters in them(`$`, `=`, `,` etc.)
+        but they need to be enclosed in double quotes. For example, grub-config-flags='GRUB_CMDLINE_LINUX_DEFAULT="$GRUB_CMDLINE_LINUX_DEFAULT
+        console=ttyS0,115200 console=tty0",GRUB_TIMEOUT=0'
   isolcpus:
     default: ""
     type: string
diff --git a/src/lib/lib_sysconfig.py b/src/lib/lib_sysconfig.py
index 1a9875a..caf4c00 100644
--- a/src/lib/lib_sysconfig.py
+++ b/src/lib/lib_sysconfig.py
@@ -5,6 +5,7 @@ Manage grub, systemd, coufrequtils and kernel version configuration.
 import filecmp
 import hashlib
 import os
+import re
 import subprocess
 from datetime import datetime, timedelta, timezone
 
@@ -35,10 +36,34 @@ IRQBALANCE_CONF = "/etc/default/irqbalance"
 def parse_config_flags(config_flags):
     """Parse config flags into a dict.
 
+    This parsing function supports a few different formats
+    depending on the config flags being provided.
+
+    1. Simple string with key=value pairs. For example,
+    a string in the format of 'key1=val1, key2=val2' will return a dict of:
+    {'key1': 'val1', 'key2': 'val2'}.
+
+    2. A string in the above format, but supporting a comma-delimited
+    list of values for the same key. For example, a string in the format of
+    'key1=val1, key2=val2,val3,val4' will return a dict of:
+    {'key1': 'val1', 'key2': 'val2,val3,val4'}
+
+    3. A string with special characters (`=`, `,`) in the value.
+    Note: This case requires double quotes to properly separate the key and value pairs
+    since there are special characters that need to be protected. Adding the double
+    quotes explicitly ensures that the quotes are present in the dict output as well.
+    For example, a string in the format of 'key1="subkey1=val1,val2 subkey2=val3"'
+    will return a dict of: {'key1': '"subkey1=val1,val2 subkey2=val3"'}
+
     :param config_flags: key pairs list. Format: key1=value1,key2=value2
     :return dict: format {'key1': 'value1', 'key2': 'value2'}
     """
-    key_value_pairs = config_flags.split(",")
+    # This regular expression is used to split the config flags
+    # into a list while preserving the content within the
+    # double quotes even if it contains commas
+    # For example, it splits 'val1, val2, "val3,val4", val5'
+    # as ['val1', 'val2', '"val3, val4"', 'val5]
+    key_value_pairs = re.split(r',(?=(?:[^"]*"[^"]*")*[^"]*$)', config_flags)
     parsed_config_flags = {}
     for index, pair in enumerate(key_value_pairs):
         if "=" in pair:
diff --git a/src/templates/grub.j2 b/src/templates/grub.j2
index a76ca57..5d7cf79 100644
--- a/src/templates/grub.j2
+++ b/src/templates/grub.j2
@@ -5,18 +5,13 @@
 # For full documentation of the options in this file, see:
 #   info -f grub -n 'Simple configuration'
 
-# Evaluate grub-config-flags first, in case it uses $GRUB_CMDLINE_LINUX_DEFAULT.
-# However, if grub-config-flags is not specified, ignore that (current behavior.)
 {% if grub_config_flags is defined and grub_config_flags -%}
 # config-flags
 {% for key, value in grub_config_flags.items() -%}
 {{ key }}={{ value }}
 {% endfor -%}
-{% else -%}
-GRUB_CMDLINE_LINUX_DEFAULT=""
 {% endif -%}
 
-GRUB_CMDLINE_LINUX_DEFAULT="$GRUB_CMDLINE_LINUX_DEFAULT console=ttyS0,115200 console=tty0"
 {% if isolcpus is defined and isolcpus -%}
 GRUB_CMDLINE_LINUX_DEFAULT="$GRUB_CMDLINE_LINUX_DEFAULT isolcpus={{ isolcpus }}"
 {% endif -%}
diff --git a/src/tests/unit/test_lib.py b/src/tests/unit/test_lib.py
index 9721313..b2b5354 100644
--- a/src/tests/unit/test_lib.py
+++ b/src/tests/unit/test_lib.py
@@ -45,6 +45,47 @@ def test_check_update_grub_unavailable(check_output, cmp_file):
     assert "No available grub updates found." in message
 
 
[e-mail](
+    "config_flags, expected",
+    [
+        ("key1=val1,key2=val2", {"key1": "val1", "key2": "val2"}),
+        # whitespace between key value pairs after comma
+        ("key1=val1,  key2=val2", {"key1": "val1", "key2": "val2"}),
+        # multiple values assigned to key
+        ("key1=val1,val2,key2=val3", {"key1": "val1,val2", "key2": "val3"}),
+        ("key1=val1 val2 val3", {"key1": "val1 val2 val3"}),
+        # Subkey assigned to key has a single value (val) assigned with "="
+        ('key="$key subkey=val"', {"key": '"$key subkey=val"'}),
+        (
+            'key1="subkey1=val1 val2",key2="$key2 val3"',
+            {"key1": '"subkey1=val1 val2"', "key2": '"$key2 val3"'},
+        ),
+        # Subkey has multiple values (val1, val2) assigned each separated by comma
+        (
+            'key1="subkey1=val1,val2 subkey2=val3",key2=val4',
+            {"key1": '"subkey1=val1,val2 subkey2=val3"', "key2": "val4"},
+        ),
+        (
+            'key1="$key1 subkey1=val1,val2 subkey2=val3", key2=val4',
+            {"key1": '"$key1 subkey1=val1,val2 subkey2=val3"', "key2": "val4"},
+        ),
+        (
+            (
+                'key1="$key1 subkey1=val1,val2 subkey2=val3", key2="$key2 '
+                'subkey1=val1 subkey2=val2"'
+            ),
+            {
+                "key1": '"$key1 subkey1=val1,val2 subkey2=val3"',
+                "key2": '"$key2 subkey1=val1 subkey2=val2"',
+            },
+        ),
+    ],
+)
+def test_parse_config_flags(config_flags, expected):
+    """Test parsing function for config flags."""
+    assert lib_sysconfig.parse_config_flags(config_flags) == expected
+
+
 class TestBootResourceState:
     """Test BootResourceState class."""