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

Adding support for RHEL10 distro release #6011

Conversation

PavamanSubramaniyam
Copy link
Contributor

We need the RHEL10 distro release checking to be included for the avocado misc tests network test bucket to work as it had worked for RHEL9 distro release

@mr-avocado
Copy link

mr-avocado bot commented Aug 27, 2024

Dear contributor,
Avocado is currently at the end of sprint 107, therefore we are in feature freeze state.
Please avoid merging changes that do not fall into these categories:

  • Bug fixes
  • Documentation updates

The feature freeze will be active until the release planned on September 02, 2024.

@PavamanSubramaniyam PavamanSubramaniyam force-pushed the adding_support_for_RHEL10 branch 2 times, most recently from 364050e to 93e9ece Compare August 28, 2024 04:40
Copy link
Collaborator

@PraveenPenguin PraveenPenguin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@abdhaleegit abdhaleegit left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@clebergnu clebergnu left a comment

Choose a reason for hiding this comment

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

Hi @PavamanSubramaniyam ,

Thanks for this update, but I have one concern with the changes introduced here. I was not able to find a place where the "distro_is_rhel9" or the "distro_is_rhel10" conditional is used by themselves. IIUC, there are two possibilities: "RHEL is 9 or later" or "not".

Something like:

diff --git a/avocado/utils/network/interfaces.py b/avocado/utils/network/interfaces.py
index 8a9a815f6..cb915958e 100644
--- a/avocado/utils/network/interfaces.py
+++ b/avocado/utils/network/interfaces.py
@@ -49,14 +49,13 @@ class NetworkInterface:
         self.name = if_name
         self.if_type = if_type
         self.host = host
-        self.distro_is_rhel9 = False
-        self.distro_is_rhel10 = False
+        self.distro_is_rhel9_or_later = False
 
     @property
     def config_filename(self):
         current_distro = distro_detect()
         if current_distro.name in ["rhel", "fedora"]:
-            if self.distro_is_rhel9 or self.distro_is_rhel10:
+            if self.distro_is_rhel9_or_later:
                 path = "/etc/NetworkManager/system-connections"
             else:
                 path = "/etc/sysconfig/network-scripts"
@@ -65,7 +64,7 @@ class NetworkInterface:
         else:
             msg = "Distro not supported by API. Could not get interface filename."
             raise NWException(msg)
-        if self.distro_is_rhel9 or self.distro_is_rhel10:
+        if self.distro_is_rhel9_or_later:
             return f"{path}/{self.name}.nmconnection"
         else:
             return f"{path}/ifcfg-{self.name}"
@@ -74,7 +73,7 @@ class NetworkInterface:
     def config_file_path(self):
         current_distro = distro_detect()
         if current_distro.name in ["rhel", "fedora"]:
-            if self.distro_is_rhel9 or self.distro_is_rhel10:
+            if self.distro_is_rhel9_or_later:
                 return "/etc/NetworkManager/system-connections"
             else:
                 return "/etc/sysconfig/network-scripts"
@@ -88,7 +87,7 @@ class NetworkInterface:
     def slave_config_filename(self):
         try:
             slave_dict = self._get_bondinterface_details()
-            if self.distro_is_rhel9 or self.distro_is_rhel10:
+            if self.distro_is_rhel9_or_later:
                 return [
                     f"{self.config_file_path}/{slave}.nmconnection"
                     for slave in slave_dict["slaves"]
@@ -406,15 +405,13 @@ class NetworkInterface:
             raise NWException(msg)
 
         current_distro = distro_detect()
-        if current_distro.name == "rhel" and current_distro.version == "9":
-            self.distro_is_rhel9 = "rhel9"
-        if current_distro.name == "rhel" and current_distro.version == "10":
-            self.distro_is_rhel10 = "rhel10"
+        if current_distro.name == "rhel" and int(current_distro.version) >= 9:
+            self.distro_is_rhel9_or_later = True
 
         filename = f"ifcfg-{self.name}"
         prefix = self.netmask_to_cidr(netmask)
         if current_distro.name in ["rhel", "fedora"]:
-            if self.distro_is_rhel9 or self.distro_is_rhel10:
+            if self.distro_is_rhel9_or_later:
                 filename = f"{self.name}.nmconnection"
                 path = "/etc/NetworkManager/system-connections"
             else:
@@ -425,7 +422,7 @@ class NetworkInterface:
             msg = "Distro not supported by API. Could not save ipaddr."
             raise NWException(msg)
 
-        if self.distro_is_rhel9 or self.distro_is_rhel10:
+        if self.distro_is_rhel9_or_later:
             ifcfg_dict = ""
             if os.path.exists(f"{path}/{filename}") is False:
                 run_command(
@@ -464,7 +461,7 @@ class NetworkInterface:
 
         if self.if_type == "Bond":
             bond_dict = self._get_bondinterface_details()
-            if self.distro_is_rhel9 or self.distro_is_rhel10:
+            if self.distro_is_rhel9_or_later:
                 if os.path.exists(f"{path}/{filename}") is False:
                     run_command(
                         f"nmcli connection add con-name {self.name} ifname {self.name} type ethernet ipv4.address {ipaddr}/{prefix}",

Please let me know if I'm mistaken or if I'm missing something.

We need the RHEL10 distro release checking to be included
for the avocado misc tests network test bucket to work
as it had worked for RHEL9 distro release

Signed-off-by: Pavaman Subramaniyam <[email protected]>
@PavamanSubramaniyam
Copy link
Contributor Author

Hi @clebergnu,

I have incorporated the suggestions you have made and resent the commit to this PR.

Copy link
Contributor

@clebergnu clebergnu left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@clebergnu clebergnu merged commit 6ad5b87 into avocado-framework:master Sep 11, 2024
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done 108
Development

Successfully merging this pull request may close these issues.

4 participants