-
-
Notifications
You must be signed in to change notification settings - Fork 101
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
Add conditionals for Rhel9 in common role #3802
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AswathySK can we make these conditions smarter so we don't have to keep updating them every time a new OS is released? Or do we prefer to leave it like this so we can test the new major OS releases?
@AdamBrousseau instead of changing conditional to do it for everything greater than or equal to rhel 8 instead? |
Sure. We can get other opinions. If we change it to be something like |
@sxa do you have an opinion on the above? |
d93844c
to
6f9a6e3
Compare
Signed-off-by: Aswathy S Kumar <aswathyskumar144@gmail.com>
d831534
to
e9ee426
Compare
@AdamBrousseau , I have made the change as per your suggestion. |
yum: name=https://dl.fedoraproject.org/pub/epel/epel-release-latest-{{ ansible_distribution_major_version }}.noarch.rpm | ||
failed_when: false | ||
when: | ||
- ansible_architecture != "s390x" | ||
- (ansible_distribution_major_version == "8") or (ansible_distribution_major_version == "6") or (ansible_distribution_major_version == "7") | ||
- (ansible_distribution_major_version | int >= 6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is needed for rhel6+ why do we even need a condition for it?
package: "name={{ item }} state=latest" | ||
with_items: "{{ Java_RHEL8 }}" | ||
when: (ansible_distribution_major_version == "8") | ||
when: (ansible_distribution_major_version | int >= 8) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these 3 cases still what we want?
From
infrastructure/ansible/playbooks/AdoptOpenJDK_Unix_Playbook/roles/Common/vars/RedHat.yml
Line 95 in 894d848
Java_RHEL8: |
Java_RHEL8:
- java-1.8.0-openjdk-devel
Java_NOT_RHEL6_PPC64: # Not RHEL8 either
- java-1.7.0-openjdk-devel
- java-1.8.0-openjdk-devel
Java_RHEL6_PPC64:
- java-1.7.0-ibm-devel
- java-1.8.0-ibm-devel
What is java 7 and 8 used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AdamBrousseau @sxa , I am not sure. Should we bump it to 17 or 21?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell it's been there since the beginning. Java is needed for 2 purposes that I know of.
- Connecting agents to jenkins.
- Bootjdk for compiling java.
Since Jenkins now requires jdk17 to run agents, and jdk11 was required 2 years ago, I don't think leaving this at java8 for jenkins connections is necessary.
For bootjdk requirement, the build scripts will pull the needed jdk during the build process so having a permanent install of 8 is not necessary in my opinion.
I would want an opinion from Adopt/@sxa before we remove it completely though.
@@ -52,11 +52,11 @@ | |||
########################## | |||
# Additional build tools # | |||
########################## | |||
- name: Install additional build tools if NOT RHEL 8 | |||
- name: Install additional build tools if NOT RHEL 8 AND RHEL9 | |||
package: "name={{ item }} state=latest" | |||
with_items: "{{ Additional_Build_Tools_NOT_RHEL8 }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we rename this variable to Additional_Build_Tools_NOT_RHEL8Plus
?
@@ -102,11 +102,11 @@ | |||
- (ansible_distribution_major_version == "6") or (ansible_distribution_major_version == "7") | |||
tags: build_tools | |||
|
|||
- name: Install additional build tools for RHEL 8 | |||
- name: Install additional build tools for RHEL >= 8 | |||
package: "name={{ item }} state=latest" | |||
with_items: "{{ Additional_Build_Tools_RHEL8 }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we rename this variable to Additional_Build_Tools_RHEL8Plus
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds good.
The common role for Unix playbook doesnt include conditionals for rhel9 causing playbook failures.
Failures caused due to current condition:
Checklist