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

Missing, Outdated OS Tests #347

Open
pranavmarla opened this issue Aug 23, 2022 · 9 comments
Open

Missing, Outdated OS Tests #347

pranavmarla opened this issue Aug 23, 2022 · 9 comments

Comments

@pranavmarla
Copy link

pranavmarla commented Aug 23, 2022

Hi, I noticed that there don't appear to be any tests for the following OSs:

  • Ubuntu 18
  • Ubuntu 20
  • Ubuntu 22
  • RHEL 8
  • Amazon Linux (2018)
  • Amazon Linux 2

In addition, although there are tests for the following OSs, they appear to be either outdated or wrong:

  • RHEL 7

I am hoping to fix these myself but, before I do so, I thought I'd open this issue first -- this will be my first time contributing to this repo, so please feel free to let me know if you have any tips, suggestions, etc. 🙂

@pranavmarla pranavmarla changed the title Missing OS Tests Missing + outdated OS Tests Aug 23, 2022
@pranavmarla pranavmarla changed the title Missing + outdated OS Tests Missing, Outdated OS Tests Aug 23, 2022
@HorlogeSkynet
Copy link
Member

Good idea ! Also see #77, #81 and #79. There is also a TODO-list in the sources (to be cleaned).
No particular tips, but if your "real world examples" (rootfs) contain symbolic links, it is a good thing to populate distro test cases with them too (e.g. /etc/os-release -> /usr/lib/os-release), as Git supports them.

Good luck 🙏

@pranavmarla
Copy link
Author

pranavmarla commented Sep 6, 2022

Thanks @HorlogeSkynet -- I had no idea the symlinks mattered, that's good to know!

When you get a chance, I'd appreciate it if you could clarify the following:

  1. Going through the repo, I've seen some offhand comments implying that distro does not consider/care about the /etc/system-release file:
    Eg.

    • PR:

      ... This is the first time the exclusion of system-release hurts.

    • Code comment:
      # Amazon Linux 2014 only contains a system-release file.
      # distro doesn't currently handle it.
      
    • Documentation:

      The distro release file is found by using the first match in the alphabetically sorted list of the files matching the following path name patterns:
      ...
      where the following special path names are excluded:
      /etc/debian_version
      /etc/system-release
      /etc/os-release

    However, I also see that some of the existing OS tests do in fact contain the /etc/system-release file -- eg. RHEL 6.

    So, would you mind clarifying what the current distro policy is towards the /etc/system-release file? Do we consider it or ignore it -- i.e. when I create the new OS test folders, should I upload the /etc/system-release file or not ?

  2. I'm not sure what I should do regarding RHEL 7, and would appreciate your advice.

    In addition, although there are tests for the following OSs, they appear to be either outdated or wrong:

    • RHEL 7

    The issue I was referring to above is that the /etc/os-release file in the current RHEL 7 test folder does not contain the same fields as the RHEL 7 /etc/os-release file from my test VM. Specifically, my os-release file contains two fields (VARIANT, VARIANT_ID) that are not present in distro's /etc/os-release file -- however, I also noticed that my /etc/os-release file is from RHEL 7.9, while distro's is from RHEL 7.0. This suggests to me that one of the following is true:

    • Distro's /etc/os-release file is wrong: In this case, it seems like I should just delete distro's existing /etc/os-release file and replace it with my /etc/os-release file.
    • Distro's /etc/os-release file is correct, but outdated -- in other words, RHEL changed the structure of their /etc/os-release file (added the two new fields) some time between versions 7.0 and 7.9: In this case, it seems like I should not modify distro's existing 7.0 /etc/os-release file but, instead, save my updated file in a new folder -- i.e. create a new test case for RHEL 7.9.
      The problem here is that I'm not sure what I should name this new folder of mine, since it seems like the current test folder naming convention assumes that there will only be 1 folder for each major version: eg. rhel5, rhel6, rhel7. If, instead, I need to create a new folder for the same major version but a different minor version (i.e. 7.9), what naming convention should I use: rhel79, rhel7.9, rhel7_9, rhel7-9, etc.?
      • Also, would I then need to rename the existing rhel7 folder to something like rhel7.0?

@HorlogeSkynet
Copy link
Member

HorlogeSkynet commented Sep 7, 2022

Hey @pranavmarla 🙏

So, would you mind clarifying what the current distro policy is towards the /etc/system-release file? Do we consider it or ignore it -- i.e. when I create the new OS test folders, should I upload the /etc/system-release file or not ?

Actually, whether current distro implementation is broken or not, you should include in the "rootfs" each file that should be ignored, or must be parsed.
It would be simpler for you, closer to the "reality" and eventually will trigger some bugs.

For your point 2, I would opt for a specific new folder for RHEL 7.9, with rhel79 as a naming convention.
distro ignores VARIANT and VARIANT_ID fields so I guess it's OK if RHEL 7.0 folder stays like this.

I hope I've not missed one of your points.
Cheers 👋

@pranavmarla
Copy link
Author

pranavmarla commented Sep 7, 2022

Thank you @HorlogeSkynet -- appreciate the speedy response!

Also, would I then need to rename the existing rhel7 folder to something like rhel7.0?

Would you mind clarifying this as well?

@HorlogeSkynet
Copy link
Member

Yes sorry, why not, renaming the test folder to rhel70 if you opt for rhel79 creation looks OK 👍

@pranavmarla
Copy link
Author

Perfect, thanks!

@HorlogeSkynet
Copy link
Member

Any news on this @pranavmarla ? 🙂

@pranavmarla
Copy link
Author

Any news on this @pranavmarla ? 🙂

@HorlogeSkynet I'm afraid I haven't had a chance to work on this since my last update -- I'm hoping to get back into it next year, but unfortunately can't give an estimate of when that will be

@pranavmarla
Copy link
Author

pranavmarla commented Jul 18, 2023

Test Contribution Guide

For my own reference, summarizing all the info that's useful to know when adding a test case for a particular OS/distro below. Will keep updating this post as I come across more useful info.

What info does distro use to identify an OS?

From highest to lowest precedence:

What does the test structure look like?

Overview

  • First, you need to obtain the relevant system files from the OS and upload them to the distro repo, to the tests/resources/distros/ folder
  • The actual tests are located in test_distro.py
    • There are different classes for the different sources of info distro uses to identify the OS -- under each class, you will add a test case comparing the expected/desired output with the actual output generated by distro processing that particular source of info (i.e. the system file you uploaded)

Which files should we upload?

  • In theory, you could get away with just uploading the files currently used by distro to identify the OS (i.e. the OS release file and the distro release file). To be safe though, as HorlogeSkynet mentioned, you should probably just upload every system file that could possibly be used to identify the OS -- that way, if distro decides to expand its sources of identifying information in the future, your test artifacts will still be sufficient
  • For similar reasons, if the original system file on the OS is merely a symlink to another file, you should maintain that same symlink structure in the respository as well
    Eg. In RHEL 6, /etc/system-release is just a symlink to /etc/redhat-release, and so that relationship is maintained in the repository as well

Test details

Specifically, these are the classes in test_distro.py that are relevant:

  • TestOSRelease, corresponding to the OS release file
  • TestLSBRelease, corresponding to the lsb_release command
    • You need to create a shell script to serve as the lsb_release binary. Note that this is not meant to be a full-fledged replacement for lsb_release -- instead, all it does is produce the output that we expect to get when running the actual lsb_release -a command on the OS
  • TestDistroRelease, corresponding to the distro release file
    • Note: If the OS doesn't have a valid distro release file, then you need to call _test_non_existing_release_file()
      Eg. See here
  • Note: There doesn't appear to be any class corresponding to the uname command
  • Finally, there is a main class called TestOverall
    • Each test within that class tests the "final" output generated by distro after searching through all the possible sources of info in order of precedence
    • In addition, for some reason I didn't quite follow, it looks like each test needs to also check the distro release file output again

Running the tests

Use Linux as local machine

When running the distro test suite locally, note that the tests will only run on a Linux machine. Thus, if you make changes to the distro code on a Windows machine and then run the tests to validate your changes, all the tests will be skipped!

Useful Commands

  • Run full test suite: tox
  • Run only linters: tox -e lint
  • Run only code tests:
    • Eg. Only the Python 3.8, 3.9 and 3.10 tests: tox -e py38,py39,py310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants