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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add conditionals for Rhel9 in common role #3802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
307ef6f
Add conditionals for Rhel9 in common role
AswathySK Nov 6, 2024
885e859
Add conditionals for Rhel9 in common role
AswathySK Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions ansible/playbooks/AdoptOpenJDK_Unix_Playbook/roles/Common/tasks/RedHat.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
- ansible_architecture == "x86_64"
tags: patch_update

- name: Enable EPEL release for RHEL8 or RHEL6 or RHEL7
- name: Enable EPEL release for RHEL
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")
tags: patch_update

- name: YUM upgrade all packages
Expand Down Expand Up @@ -52,11 +51,11 @@
##########################
# Additional build tools #
##########################
- name: Install additional build tools if NOT RHEL 8
- name: Install additional build tools if NOT RHEL8 and above

Check failure on line 54 in ansible/playbooks/AdoptOpenJDK_Unix_Playbook/roles/Common/tasks/RedHat.yml

View workflow job for this annotation

GitHub Actions / Ansible Lint

yaml[trailing-spaces] 

Trailing spaces

Check failure on line 54 in ansible/playbooks/AdoptOpenJDK_Unix_Playbook/roles/Common/tasks/RedHat.yml

View workflow job for this annotation

GitHub Actions / Yamllint 

54:62 [trailing-spaces] trailing spaces
Copy link
Member

@sxa sxa Dec 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Remove trailing whitespace to fix linter error

Suggested change
- name: Install additional build tools if NOT RHEL8 and above
- name: Install additional build tools if NOT RHEL8 and above

package: "name={{ item }} state=latest"
with_items: "{{ Additional_Build_Tools_NOT_RHEL8 }}"
with_items: "{{ Additional_Build_Tools_NOT_RHEL8Plus }}"
Copy link
Member

Choose a reason for hiding this comment

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

We probably need to go through and sanitise some of these names. This one should probably be Additional_Build_Tools_RHEL6_RHEL7 now but that doesn't need to be changed in this PR

when:
- (ansible_distribution_major_version != "8")
- not (ansible_distribution_major_version | int >= 8)
tags: build_tools

- name: Install additional build tools for RHEL 7
Expand Down Expand Up @@ -102,11 +101,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 RHEL8 and above
package: "name={{ item }} state=latest"
with_items: "{{ Additional_Build_Tools_RHEL8 }}"
with_items: "{{ Additional_Build_Tools_RHEL8Plus }}"
when:
- (ansible_distribution_major_version == "8")
- (ansible_distribution_major_version | int >= 8)
tags: build_tools

- name: Install jq for SBoM parsing for build reproducibility testing
Expand Down Expand Up @@ -136,17 +135,17 @@
with_items: "{{ Java_NOT_RHEL6_PPC64 }}"
when:
- not (ansible_distribution_major_version == "6" and ansible_architecture == "ppc64")
- not (ansible_distribution_major_version == "8")
- not (ansible_distribution_major_version | int >= 8)

- name: Install Java when RedHat 6 on ppc64
package: "name={{ item }} state=latest"
with_items: "{{ Java_RHEL6_PPC64 }}"
when: (ansible_distribution_major_version == "6" and ansible_architecture == "ppc64")

- name: Install Java when RedHat 8
- name: Install Java when RedHat 8 and RedHat 9
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Install Java when RedHat 8 and RedHat 9
- name: Install Java when RedHat 8 and later

package: "name={{ item }} state=latest"
with_items: "{{ Java_RHEL8 }}"
when: (ansible_distribution_major_version == "8")
when: (ansible_distribution_major_version | int >= 8)
Copy link
Contributor

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?

Copy link
Contributor Author

@AswathySK AswathySK Nov 12, 2024 •
edited
Loading

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?

Copy link
Contributor

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.

Copy link
Member

Choose a reason for hiding this comment

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

As far as the boot JDK is concerned I don't want the builds pulling it down dynamically in most cases - that's mostly just a fallback mechanism. Other than pulling down the source, the builds should be able to run without network access.

I suspect those java-1.8.0* packages are no longer required, at least by Temurin, so it would likely be safe to remove them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AdamBrousseau , @sxa ,
Since stewart is of opinion not to pull down the bootjdks by the jobs.And java8 is not required, Should we remove the installing and setting up default java part in this role or should we maybe bump it to jdk17 or 21?

Copy link
Member

Choose a reason for hiding this comment

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

Yes I think that should be safe. It will hopefully be obvious if some part of the process requires it, in which case it's easy to back it out again :-)

Can the 1.7 version be installed from the RHEL9 repositories?

Copy link
Contributor

Choose a reason for hiding this comment

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

@sxa is there another part of the PB installing java for use by the jenkins agent that Adopt is running?
We have an internal Semeru role that installs 21 for this.

Copy link
Member

Choose a reason for hiding this comment

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

Yep. The adoptopenjdk_install role: .

infrastructure/ansible/playbooks/AdoptOpenJDK_Unix_Playbook/main.yml

Line 131 in 028773c

- role: adoptopenjdk_install # JDK21 Build Bootstrap

It's extracting as a tarball and doesn't set any default on the system to point to it, so the Jenkins agent is pointed directly to it under /usr/lub/jvm when it's started


####################
# Set default Java #
Expand Down
15 changes: 2 additions & 13 deletions ansible/playbooks/AdoptOpenJDK_Unix_Playbook/roles/Common/vars/RedHat.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ Build_Tool_Packages:
- xz
- zip

Additional_Build_Tools_NOT_RHEL8:
Additional_Build_Tools_NOT_RHEL8Plus:
- libdwarf-devel # now in CodeReady Linux Builder (CRB) repo
- libmpc-devel # now in CodeReady Linux Builder (CRB) repo
- ntp # in RHEL8, ntp package is replaced by chrony

Additional_Build_Tools_RHEL8:
Additional_Build_Tools_RHEL8Plus:
- glibc-locale-source
- glibc-langpack-ja # required for creating Japanese locales
- glibc-langpack-ko # required for creating Korean locales
Expand Down Expand Up @@ -92,17 +92,6 @@ Additional_Build_Tools_RHEL7_s390x:
- libstdc++.s390 # a dependency required for executing a 32-bit C binary
- yum-utils # yumdownloader required for devkit creation

Java_RHEL8:
Copy link
Member

@sxa sxa Dec 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Since we still have the reference to Java_RHEL8 above I believe this should not be removed

- 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

Test_Tool_Packages:
- acl
- perl
Expand Down
Loading