xenserver / xe-guest-utilities

XenServer guest utilities for unix-like operating systems
BSD 2-Clause "Simplified" License
59 stars 58 forks source link

CP-36994:Add newly supported guest test case #147

Closed MinghuiHu closed 1 year ago

stormi commented 1 year ago

The commit does more than was the commit message says, as it also removes a lot of test cases.

Why? There's no harm in testing the ability of the xe-linux-distribution script to detect the largest possible sample of distros, even if they are not all officially supported by XenServer. It's better to keep xe-linux-distribution able to detect even distros that are not supported, so that the error message displayed when trying to install the tools is more precise (since it includes the distro name and versions), and these tests help ensure this. By the way XenServer is not the only user of this, and this deletes test cases for distros which are supported elsewhere to some extent.

I suggest to only add the new test cases but not delete the others.

stormi commented 1 year ago

There's also another change which is not described in the commit message: the minor version number is not tested anymore. Why is that so?

xihuan-citrix commented 1 year ago

The commit does more than was the commit message says, as it also removes a lot of test cases.

Why? There's no harm in testing the ability of the xe-linux-distribution script to detect the largest possible sample of distros, even if they are not all officially supported by XenServer. It's better to keep xe-linux-distribution able to detect even distros that are not supported, so that the error message displayed when trying to install the tools is more precise (since it includes the distro name and versions), and these tests help ensure this. By the way XenServer is not the only user of this, and this deletes test cases for distros which are supported elsewhere to some extent.

I suggest to only add the new test cases but not delete the others.

As discussed, Hi @MinghuiHu , please make the commit message the same with what is doing.

Hi @stormi , if you see, we did not remove the support in xe-linux-distribution, all the test cases removed are declared by vendor that will not supported, e.g. almalinux is not used by XenServer, we will still keep it there. But for rhel4 or others, it is hard to believe that user will still use it. We want to keep the xe-guest-utilities up to date.Although we will consider other company's needs, you can pick some distro that seems still will be used to test. Let's delete those that absolutely will not be used.

xihuan-citrix commented 1 year ago

There's also another change which is not described in the commit message: the minor version number is not tested anymore. Why is that so?

Hi @stormi , 1) This is also a point we want some feedback from other users, from our internal script, we actually did not use the minor version currently, as we will use xe-linux-distribution to extract the major version, although we will ensure any minor version could install the guest utilities successfully. But list all the minor version distribution in the test cases is also no need, maybe only keep one minor version is detected is enough. Did you know some cases from other users that will still use the minor version extraction? 2) One rule for UT testing: for xe-linux-distribution provided the ability to extract both major version and minor version, thus we'd better also test the minor extraction.

stormi commented 1 year ago

Did you know some cases from other users that will still use the minor version extraction?

xe-linux-distribution has two uses:

Unless I'm mistaken, the minor version is reported through xenstore, and XAPI clients are able to display it. It's a meaningful information for users, so it's probably good that the tests ensure the script is able to correctly detect it.

stormi commented 1 year ago

We want to keep the xe-guest-utilities up to date.Although we will consider other company's needs, you can pick some distro that seems still will be used to test. Let's delete those that absolutely will not be used.

If you remove, for example, test cases for detecting RHEL 4, then at some point in the future maybe xe-linux-distribution will not be able to detect this distribution anymore. If someone with a very very old system that they can't update tries to install the guest tools, it would be good that the install script is able to clearly explain to them that RHEL 4 is not supported, rather than saying the distro is not recognized. That's why I think that, even if it's not a very important test, there's still some value in testing the ability of xe-linux-distribution to detect old distros.

xihuan-citrix commented 1 year ago

xe-linux-distribution has two uses:

  • detect the distribution and version for the install.sh script
  • report the distribution and version information through Xenstore

Unless I'm mistaken, the minor version is reported through xenstore, and XAPI clients are able to display it. It's a meaningful information for users, so it's probably good that the tests ensure the script is able to correctly detect it.

Hi @stormi , you're right at the second point, it will store the detected minor version into /var/cache/xe-linux-ditribution, when the service xe-linux-distribution is started to execute. Thus in this point, we have to keep the minor version detection.

xihuan-citrix commented 1 year ago

We want to keep the xe-guest-utilities up to date.Although we will consider other company's needs, you can pick some distro that seems still will be used to test. Let's delete those that absolutely will not be used.

If you remove, for example, test cases for detecting RHEL 4, then at some point in the future maybe xe-linux-distribution will not be able to detect this distribution anymore. If someone with a very very old system that they can't update tries to install the guest tools, it would be good that the install script is able to clearly explain to them that RHEL 4 is not supported, rather than saying the distro is not recognized. That's why I think that, even if it's not a very important test, there's still some value in testing the ability of xe-linux-distribution to detect old distros.

I really doubt someone will still using RHEL4, that's actually been out of date between 2017 ~ 2018, for the secure consideration of the OS, they actually should upgrade to use RHEL6 or later distribution, but I will agree with you that some one may still using the recent EOL distribution, we can still keep like ubuntu18.04 been tested. And also when testing the guest tools, we actually already not test the RHEL4 for a long time.

MinghuiHu commented 1 year ago

Hi @stormi, we will only add new supported distro UT test cases in this PR. And I have created another one to handle the out-of-date UT test cases. It won't be merged until we can reach an agreement.