Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pykickstart: Use RHEL10 kickstart commands #5712

Closed
wants to merge 1 commit into from

Conversation

bcl
Copy link
Contributor

@bcl bcl commented Jun 18, 2024

This switches the following commands to use the RHEL10 version:
network
nvdimm
timezone

It also bumps the pykickstart requirement to version 3.52.4 which contains the new commands. Changes in this version are:

timezone: Remove the --isUtc, --nontp, and --ntpserver arguments
packages: Remove the old camel case arguments on RHEL10
nvdimm: Remove support for the nvdimm command on RHEL10
network: Deprecate network team options on RHEL 10

Related: RHEL-36831

This switches the following commands to use the RHEL10 version:
  network
  nvdimm
  timezone

It also bumps the pykickstart requirement to version 3.52.4 which
contains the new commands. Changes in this version are:

timezone: Remove the --isUtc, --nontp, and --ntpserver arguments
packages: Remove the old camel case arguments on RHEL10
nvdimm: Remove support for the nvdimm command on RHEL10
network: Deprecate network team options on RHEL 10

Related: RHEL-36831
@bcl
Copy link
Contributor Author

bcl commented Jun 18, 2024

This requires pykickstart 3.52.4 to be built first. See pykickstart/pykickstart#494

@jkonecny12 jkonecny12 added the blocked Don't merge this pull request! label Jun 19, 2024
@jkonecny12
Copy link
Member

@bcl please inform us when pykickstart is build and ready in RHEL-10

@bcl
Copy link
Contributor Author

bcl commented Jun 26, 2024

The new pykickstart is now in the nightly AppStream repo so this can be merged.

@jstodola
Copy link
Contributor

jstodola commented Jul 1, 2024

/kickstart-test --testtype smoke

@jstodola jstodola removed the blocked Don't merge this pull request! label Jul 1, 2024
@jstodola
Copy link
Contributor

jstodola commented Jul 1, 2024

Removed the blocked label, since pykickstart is ready

Copy link
Contributor

@M4rtinK M4rtinK left a comment

Choose a reason for hiding this comment

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

Looks good to me as well, thanks! :)

@jkonecny12
Copy link
Member

Hi all. This PR needs more work to fix the tests (or the code?).

First, currently container builds are broken which is resolved by #5735 but that PR is failing because of new pykickstart version. If you build container in #5735 and test these changes you will see unit tests issues which needs to be resolved.

=================================== FAILURES ===================================
__________ PartitioningFactoryTestCase.test_get_method_for_kickstart ___________

self = <tests.unit_tests.pyanaconda_tests.modules.storage.partitioning.test_module_part_factory.PartitioningFactoryTestCase testMethod=test_get_method_for_kickstart>
supported = <PropertyMock name='supported' id='140474766940288'>
formattable = <PropertyMock name='formattable' id='140474770125888'>

    @patch.object(BTRFS, "formattable", new_callable=PropertyMock)
    @patch.object(BTRFS, "supported", new_callable=PropertyMock)
    def test_get_method_for_kickstart(self, supported, formattable):
        """Test get_method_for_kickstart."""
        supported.return_value = True
        formattable.return_value = True
    
        self._check_method(
            PartitioningMethod.AUTOMATIC,
            "autopart"
        )
        self._check_method(
            PartitioningMethod.MANUAL,
            "mount /dev/sda1 /"
        )
        self._check_method(
            PartitioningMethod.CUSTOM,
            "reqpart"
        )
        self._check_method(
            PartitioningMethod.CUSTOM,
            "part /"
        )
        self._check_method(
            PartitioningMethod.CUSTOM,
            "logvol / --name=root --vgname=fedora --size=4000"
        )
        self._check_method(
            PartitioningMethod.CUSTOM,
            "volgroup fedora pv.1 pv.2"
        )
        self._check_method(
            PartitioningMethod.CUSTOM,
            "raid / --level=1 --device=0 raid.01 raid.02"
        )
>       self._check_method(
            PartitioningMethod.CUSTOM,
            "btrfs / --subvol --name=root fedora-btrfs"
        )

unit_tests/pyanaconda_tests/modules/storage/partitioning/test_module_part_factory.py:86: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
unit_tests/pyanaconda_tests/modules/storage/partitioning/test_module_part_factory.py:100: in _check_method
    parser.readKickstartFromString(kickstart)
/usr/lib/python3.12/site-packages/pykickstart/parser.py:797: in readKickstartFromString
    self._stateMachine(i)
/usr/lib/python3.12/site-packages/pykickstart/parser.py:780: in _stateMachine
    self._tryFunc(lambda: self.handleCommand(lineno, args))
/usr/lib/python3.12/site-packages/pykickstart/parser.py:691: in _tryFunc
    fn()
/usr/lib/python3.12/site-packages/pykickstart/parser.py:780: in <lambda>
    self._tryFunc(lambda: self.handleCommand(lineno, args))
/usr/lib/python3.12/site-packages/pykickstart/parser.py:582: in handleCommand
    retval = self.handler.dispatcher(args, lineno)
/usr/lib/python3.12/site-packages/pykickstart/base.py:427: in dispatcher
    obj = self.commands[cmd].parse(args[1:])
../pyanaconda/modules/storage/kickstart.py:107: in parse
    retval = super().parse(args)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <pyanaconda.modules.storage.kickstart.BTRFS object at 0x7fc2d499e1e0>
args = ['/', '--subvol', '--name=root', 'fedora-btrfs']

    def parse(self, args):
        """Raise an error if the command is found in the input file"""
>       raise KickstartParseError(_("%s has been removed.") % self.currentCmd, lineno=self.lineno)
E       pykickstart.errors.KickstartParseError: The following problem occurred on line 1 of the kickstart file:
E       
E       btrfs has been removed.

/usr/lib/python3.12/site-packages/pykickstart/base.py:254: KickstartParseError
________________ StorageInterfaceTestCase.test_btrfs_kickstart _________________

self = <tests.unit_tests.pyanaconda_tests.modules.storage.test_module_storage.StorageInterfaceTestCase testMethod=test_btrfs_kickstart>
supported = <PropertyMock name='formattable' id='140474800179632'>
formattable = <PropertyMock name='supported' id='140474800182992'>
publisher = <MagicMock name='publish_object' id='140474795811200'>

    @patch_dbus_publish_object
    @patch.object(BTRFS, "supported", new_callable=PropertyMock)
    @patch.object(BTRFS, "formattable", new_callable=PropertyMock)
    def test_btrfs_kickstart(self, supported, formattable, publisher):
        """Test the btrfs command."""
        ks_in = """
        btrfs / --subvol --name=root fedora-btrfs
        """
        ks_out = ""
    
        supported.return_value = True
        formattable.return_value = True
        self._apply_partitioning_when_created()
>       self._test_kickstart(ks_in, ks_out)

unit_tests/pyanaconda_tests/modules/storage/test_module_storage.py:1380: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
unit_tests/pyanaconda_tests/modules/storage/test_module_storage.py:388: in _test_kickstart
    check_kickstart_interface(self.storage_interface, ks_in, ks_out, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

interface = <pyanaconda.modules.storage.storage_interface.StorageInterface object at 0x7fc2d677ccb0>
ks_in = 'btrfs / --subvol --name=root fedora-btrfs', ks_out = ''
ks_valid = True

    def check_kickstart_interface(interface, ks_in=None, ks_out=None, ks_valid=True):
        """Test the parsing and generating of a kickstart module.
    
        :param interface: instance of KickstartModuleInterface
        :param ks_in: string with the input kickstart or None
        :param ks_out: string with the output kickstart or None
        :param ks_valid: True if the input kickstart is valid, otherwise False
        """
        callback = PropertiesChangedCallback()
        interface.PropertiesChanged.connect(callback)
    
        result = None
    
        # Read a kickstart,
        if ks_in is not None:
            ks_in = dedent(ks_in).strip()
            result = KickstartReport.from_structure(
                interface.ReadKickstart(ks_in)
            )
>           assert ks_valid == result.is_valid()
E           AssertionError: assert True == False
E            +  where False = <bound method KickstartReport.is_valid of KickstartReport(error_messages=[KickstartMessage(file_name='', line_number=1, message='btrfs has been removed.', module_name='')], warning_messages=[])>()
E            +    where <bound method KickstartReport.is_valid of KickstartReport(error_messages=[KickstartMessage(file_name='', line_number=1, message='btrfs has been removed.', module_name='')], warning_messages=[])> = KickstartReport(error_messages=[KickstartMessage(file_name='', line_number=1, message='btrfs has been removed.', module_name='')], warning_messages=[]).is_valid

unit_tests/pyanaconda_tests/__init__.py:110: AssertionError
------------------------------ Captured log call -------------------------------
DEBUG    anaconda.modules.storage.bootloader.bootloader:bootloader.py:249 The default type is set to 'BootloaderType.DEFAULT'.
DEBUG    anaconda.modules.storage.fcoe.fcoe:fcoe.py:51 Start up the FCoE module.
ERROR    blivet:fcoe.py:45 failed to load libfc: Module 'libfc' doesn't exist
DEBUG    anaconda.modules.storage.iscsi.iscsi:iscsi.py:56 Start up the iSCSI module.
DEBUG    anaconda.modules.storage.nvme.nvme:nvme.py:48 Start up the NVMe module.
DEBUG    anaconda.modules.storage.zfcp.zfcp:zfcp.py:52 Start up the zFCP module.
DEBUG    blivet:__init__.py:109 get_format('ext4') returning Ext4FS instance with object id 2174
DEBUG    blivet:blivet.py:1195 trying to set new default fstype to 'ext4'
DEBUG    blivet:__init__.py:109 get_format('ext4') returning Ext4FS instance with object id 2175
INFO     blivet:fstab.py:317 Fstab file '' does not exist, setting fstab read path to None
INFO     program:util.py:182 Running... lsblk --bytes -a -o NAME,SIZE,OWNER,GROUP,MODE,FSTYPE,LABEL,UUID,PARTUUID,MOUNTPOINT
DEBUG    program:util.py:220 Return code: 0
DEBUG    blivet:blivet.py:98 lsblk output:

DEBUG    blivet:lvm.py:164 lvm filter: clearing the lvm devices list
DEBUG    blivet:lvm.py:164 lvm filter: clearing the lvm devices list
DEBUG    blivet:blivet.py:1195 trying to set new default fstype to 'ext4'
DEBUG    blivet:__init__.py:109 get_format('ext4') returning Ext4FS instance with object id 2176
DEBUG    anaconda.storage:model.py:109 trying to set new default luks version to 'luks2'
DEBUG    blivet:__init__.py:109 get_format('luks') returning LUKS instance with object id 2177
DEBUG    anaconda.modules.storage.storage:storage.py:206 The storage model has changed.
DEBUG    anaconda.modules.common.base.base:base.py:222 Reading kickstart...
________________ ModuleSpecificationsTestCase.test_all_commands ________________

self = <tests.unit_tests.pyanaconda_tests.test_kickstart_specification.ModuleSpecificationsTestCase testMethod=test_all_commands>

    def test_all_commands(self):
        """Check if we process all kickstart commands."""
        # Collect the specified commands.
        specified = set()
    
        for specification in self.SPECIFICATIONS:
            specified.update(specification.commands.keys())
    
        # Collect the expected commands.
        expected = set()
    
        for name, obj in self.pykickstart_commands.items():
            if issubclass(obj, RemovedCommand):
                continue
    
            expected.add(name)
    
        # Ignore specified names if missing.
        for name in self.IGNORED_MISSING_NAMES:
            if name in expected ^ specified:
                warnings.warn("Skipping the missing name: {}".format(name))
                expected.discard(name)
                specified.discard(name)
    
        # Check the differences.
>       assert specified == expected
E       AssertionError: assert {'hmc', 'sshpw', 'sshkey', 'shutdown', 'module', 'part', 'rootpw', 'iscsi', 'network', 'poweroff', 'firstboot', 'eula', 'zerombr', 'skipx', 'nfs', 'raid', 'reqpart', 'snapshot', 'volgroup', 'updates', 'halt', 'graphical', 'rescue', 'liveimg', 'lang', 'bootloader', 'user', 'partition', 'autopart', 'group', 'clearpart', 'ostreecontainer', 'btrfs', 'firewall', 'keyboard', 'nvdimm', 'logvol', 'realm', 'harddrive', 'text', 'zipl', 'driverdisk', 'vnc', 'ostreesetup', 'xconfig', 'timezone', 'timesource', 'authselect', 'iscsiname', 'url', 'selinux', 'logging', 'repo', 'services', 'zfcp', 'ignoredisk', 'rhsm', 'mediacheck', 'fcoe', 'cmdline', 'cdrom', 'mount', 'reboot', 'syspurpose'} == {'hmc', 'sshpw', 'sshkey', 'part', 'module', 'rootpw', 'shutdown', 'iscsi', 'network', 'poweroff', 'firstboot', 'eula', 'zerombr', 'skipx', 'nfs', 'raid', 'reqpart', 'snapshot', 'volgroup', 'updates', 'halt', 'graphical', 'rescue', 'liveimg', 'lang', 'bootloader', 'user', 'partition', 'autopart', 'group', 'clearpart', 'ostreecontainer', 'firewall', 'keyboard', 'logvol', 'realm', 'harddrive', 'driverdisk', 'text', 'zipl', 'vnc', 'ostreesetup', 'xconfig', 'timezone', 'reboot', 'timesource', 'authselect', 'iscsiname', 'selinux', 'url', 'logging', 'mediacheck', 'repo', 'services', 'zfcp', 'rhsm', 'fcoe', 'cmdline', 'cdrom', 'mount', 'ignoredisk', 'syspurpose'}
E         
E         Extra items in the left set:
E         'nvdimm'
E         'btrfs'
E         
E         Full diff:
E           {
E               'authselect',
E               'autopart',
E               'bootloader',
E         +     'btrfs',
E               'cdrom',
E               'clearpart',
E               'cmdline',
E               'driverdisk',
E               'eula',
E               'fcoe',
E               'firewall',
E               'firstboot',
E               'graphical',
E               'group',
E               'halt',
E               'harddrive',
E               'hmc',
E               'ignoredisk',
E               'iscsi',
E               'iscsiname',
E               'keyboard',
E               'lang',
E               'liveimg',
E               'logging',
E               'logvol',
E               'mediacheck',
E               'module',
E               'mount',
E               'network',
E               'nfs',
E         +     'nvdimm',
E               'ostreecontainer',
E               'ostreesetup',
E               'part',
E               'partition',
E               'poweroff',
E               'raid',
E               'realm',
E               'reboot',
E               'repo',
E               'reqpart',
E               'rescue',
E               'rhsm',
E               'rootpw',
E               'selinux',
E               'services',
E               'shutdown',
E               'skipx',
E               'snapshot',
E               'sshkey',
E               'sshpw',
E               'syspurpose',
E               'text',
E               'timesource',
E               'timezone',
E               'updates',
E               'url',
E               'user',
E               'vnc',
E               'volgroup',
E               'xconfig',
E               'zerombr',
E               'zfcp',
E               'zipl',
E           }

unit_tests/pyanaconda_tests/test_kickstart_specification.py:481: AssertionError
=============================== warnings summary ===============================
../../../usr/lib/python3.12/site-packages/gi/overrides/__init__.py:108
../../../usr/lib/python3.12/site-packages/gi/overrides/__init__.py:108
../../../usr/lib/python3.12/site-packages/gi/overrides/__init__.py:108
../../../usr/lib/python3.12/site-packages/gi/overrides/__init__.py:108
../../../usr/lib/python3.12/site-packages/gi/overrides/__init__.py:108
  /usr/lib/python3.12/site-packages/gi/overrides/__init__.py:108: DeprecationWarning: 'pkgutil.get_loader' is deprecated and slated for removal in Python 3.14; use importlib.util.find_spec() instead
    override_loader = get_loader(override_package_name)

../../../usr/lib64/python3.12/pkgutil.py:291
../../../usr/lib64/python3.12/pkgutil.py:291
../../../usr/lib64/python3.12/pkgutil.py:291
../../../usr/lib64/python3.12/pkgutil.py:291
../../../usr/lib64/python3.12/pkgutil.py:291
  /usr/lib64/python3.12/pkgutil.py:291: DeprecationWarning: 'pkgutil.find_loader' is deprecated and slated for removal in Python 3.14; use importlib.util.find_spec() instead
    return find_loader(fullname)

unit_tests/pyanaconda_tests/core/test_threads.py::ThreadManagerTestCase::test_recreate_thread
  /usr/local/lib/python3.12/site-packages/_pytest/threadexception.py:77: PytestUnhandledThreadExceptionWarning: Exception in thread TESTING_THREAD
  
  Traceback (most recent call last):
    File "/usr/lib64/python3.12/threading.py", line 1073, in _bootstrap_inner
      self.run()
    File "/tmp/anaconda/pyanaconda/core/threads.py", line 288, in run
      thread_manager.remove(self.name)
    File "/tmp/anaconda/pyanaconda/core/threads.py", line 78, in remove
      self._objs.pop(name)
  KeyError: 'TESTING_THREAD'
  
    warnings.warn(pytest.PytestUnhandledThreadExceptionWarning(msg))

unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py::DNFManagerTestCase::test_install_packages
unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py::DNFManagerTestCase::test_install_packages_dnf_ts_item_error
unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py::DNFManagerTestCase::test_install_packages_failed
unit_tests/pyanaconda_tests/modules/payloads/payload/test_module_payload_dnf_manager.py::DNFManagerTestCase::test_install_packages_quit
  /usr/lib64/python3.12/multiprocessing/popen_fork.py:66: DeprecationWarning: This process (pid=10421) is multi-threaded, use of fork() may lead to deadlocks in the child.
    self.pid = os.fork()

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================== short test summary info ============================
FAILED unit_tests/pyanaconda_tests/modules/storage/partitioning/test_module_part_factory.py::PartitioningFactoryTestCase::test_get_method_for_kickstart - pykickstart.errors.KickstartParseError: The following problem occurred on line 1 of the kickstart file:

btrfs has been removed.
FAILED unit_tests/pyanaconda_tests/modules/storage/test_module_storage.py::StorageInterfaceTestCase::test_btrfs_kickstart - AssertionError: assert True == False
 +  where False = <bound method KickstartReport.is_valid of KickstartReport(error_messages=[KickstartMessage(file_name='', line_number=1, message='btrfs has been removed.', module_name='')], warning_messages=[])>()
 +    where <bound method KickstartReport.is_valid of KickstartReport(error_messages=[KickstartMessage(file_name='', line_number=1, message='btrfs has been removed.', module_name='')], warning_messages=[])> = KickstartReport(error_messages=[KickstartMessage(file_name='', line_number=1, message='btrfs has been removed.', module_name='')], warning_messages=[]).is_valid
FAILED unit_tests/pyanaconda_tests/test_kickstart_specification.py::ModuleSpecificationsTestCase::test_all_commands - AssertionError: assert {'hmc', 'sshpw', 'sshkey', 'shutdown', 'module', 'part', 'rootpw', 'iscsi', 'network', 'poweroff', 'firstboot', 'eula', 'zerombr', 'skipx', 'nfs', 'raid', 'reqpart', 'snapshot', 'volgroup', 'updates', 'halt', 'graphical', 'rescue', 'liveimg', 'lang', 'bootloader', 'user', 'partition', 'autopart', 'group', 'clearpart', 'ostreecontainer', 'btrfs', 'firewall', 'keyboard', 'nvdimm', 'logvol', 'realm', 'harddrive', 'text', 'zipl', 'driverdisk', 'vnc', 'ostreesetup', 'xconfig', 'timezone', 'timesource', 'authselect', 'iscsiname', 'url', 'selinux', 'logging', 'repo', 'services', 'zfcp', 'ignoredisk', 'rhsm', 'mediacheck', 'fcoe', 'cmdline', 'cdrom', 'mount', 'reboot', 'syspurpose'} == {'hmc', 'sshpw', 'sshkey', 'part', 'module', 'rootpw', 'shutdown', 'iscsi', 'network', 'poweroff', 'firstboot', 'eula', 'zerombr', 'skipx', 'nfs', 'raid', 'reqpart', 'snapshot', 'volgroup', 'updates', 'halt', 'graphical', 'rescue', 'liveimg', 'lang', 'bootloader', 'user', 'partition', 'autopart', 'group', 'clearpart', 'ostreecontainer', 'firewall', 'keyboard', 'logvol', 'realm', 'harddrive', 'driverdisk', 'text', 'zipl', 'vnc', 'ostreesetup', 'xconfig', 'timezone', 'reboot', 'timesource', 'authselect', 'iscsiname', 'selinux', 'url', 'logging', 'mediacheck', 'repo', 'services', 'zfcp', 'rhsm', 'fcoe', 'cmdline', 'cdrom', 'mount', 'ignoredisk', 'syspurpose'}
  
  Extra items in the left set:
  'nvdimm'
  'btrfs'
  
  Full diff:
    {
        'authselect',
        'autopart',
        'bootloader',
  +     'btrfs',
        'cdrom',
        'clearpart',
        'cmdline',
        'driverdisk',
        'eula',
        'fcoe',
        'firewall',
        'firstboot',
        'graphical',
        'group',
        'halt',
        'harddrive',
        'hmc',
        'ignoredisk',
        'iscsi',
        'iscsiname',
        'keyboard',
        'lang',
        'liveimg',
        'logging',
        'logvol',
        'mediacheck',
        'module',
        'mount',
        'network',
        'nfs',
  +     'nvdimm',
        'ostreecontainer',
        'ostreesetup',
        'part',
        'partition',
        'poweroff',
        'raid',
        'realm',
        'reboot',
        'repo',
        'reqpart',
        'rescue',
        'rhsm',
        'rootpw',
        'selinux',
        'services',
        'shutdown',
        'skipx',
        'snapshot',
        'sshkey',
        'sshpw',
        'syspurpose',
        'text',
        'timesource',
        'timezone',
        'updates',
        'url',
        'user',
        'vnc',
        'volgroup',
        'xconfig',
        'zerombr',
        'zfcp',
        'zipl',
    }
= 3 failed, 2022 passed, 7 skipped, 1 xfailed, 15 warnings in 97.09s (0:01:37) =
FAIL unit_tests/unit_tests.sh (exit status: 1)

Three tests are failing. I was looking on the unit_tests/pyanaconda_tests/modules/storage/partitioning/test_module_part_factory.py::PartitioningFactoryTestCase::test_get_method_for_kickstart and if I read it correctly, the test is basically checking kickstart specifications of the module with what returns pykickstart excluding RemovedCommands parts.

Here we are getting into a troubles with how to approach it. The test seems to be designed that modules shouldn't support / process kickstart commands which are removed from the code base. So how should we approach the BTRFS and NVDimm (which were removed in the newest pykickstart):

  • should we remove it from the modules completely
  • should we remove it just from the module specification
  • or should we fix the test

There are also other tests testing BTRFS functionality.

Another thing to consider is if we want to get this into master branch first in backward compatible manner. Unfortunately, I don't have time (PTO from tomorrow) to focus on this so I'm describing it here for someone else to take it.

Until this is resolved and merged we won't get a new containers in quay.io for rhel-10 tag.

@KKoukiou
Copy link
Contributor

KKoukiou commented Jul 3, 2024

Merged as part of #5740

@KKoukiou KKoukiou closed this Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6 participants