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

Add required packages #1513

Merged
merged 3 commits into from
Aug 29, 2024
Merged

Add required packages #1513

merged 3 commits into from
Aug 29, 2024

Conversation

Georges-GNM
Copy link
Contributor

@Georges-GNM Georges-GNM commented Jul 15, 2024

What does this change?

Related to this prodmon PR and enables it to work, essentially - otherwise this will lead to an UnsatisfiedLinkError:
image

I haven't dug deeply, but from what I can tell these used to get installed as part of the java 8 role, but for some reason don't anymore after switching to java 11 - which actually can be confirmed from a couple of bakes of the browser-testing-jammy-ARM-WITH-cdk-base-java11: this first one was accidentally still set to use java 8, but when I baked a second one correctly using java 11, the package changes log shows those have not been installed.

From what I can tell they are required to enable selenium/chromedriver to work, so it feels reasonable to include them in the role directly.

How to test

The prodmon PR has temporarily been set to use an AMI baked in code with this role - deploy that to prodmon secondary, and the app should keep working as expected.

Note
If this change adds, updates or removes a role or recipe please note that in your PR and assign appropriate reviewers for the team that will be impacted by those changes.

@Georges-GNM Georges-GNM marked this pull request as ready for review July 17, 2024 08:38
@Georges-GNM Georges-GNM requested a review from a team as a code owner July 17, 2024 08:38
@davidfurey
Copy link
Member

I suspect that this is due to the change from openjdk-8-jdk to java-11-amazon-corretto-jdk
openjdk-8-jdk is a non-headless version of the JDK, and includes a dependency on libx11-6 and libxt-dev which are both graphics related libraries, which probably (unchecked) eventually depend on the missing libraries.

I assume java-11-amazon-corretto-jdk is a headless version of the JDK. The AWS docs don't make this clear, and I haven't downloaded the .deb to check.

It might be more robust to switch to openjdk-11-jdk for this project rather than manually selecting this specific packages.

@Georges-GNM
Copy link
Contributor Author

I suspect that this is due to the change from openjdk-8-jdk to java-11-amazon-corretto-jdk openjdk-8-jdk is a non-headless version of the JDK, and includes a dependency on libx11-6 and libxt-dev which are both graphics related libraries, which probably (unchecked) eventually depend on the missing libraries.

I assume java-11-amazon-corretto-jdk is a headless version of the JDK. The AWS docs don't make this clear, and I haven't downloaded the .deb to check.

That would make sense, although I thought there was a headless variant rather than it being the default... In any case, the missing libraries do seem to indicate that.

It might be more robust to switch to openjdk-11-jdk for this project rather than manually selecting this specific packages.

Will give that a try!

@akash1810
Copy link
Member

Will give that a try!

Marking this PR as draft, to avoid this being included in the daily message to the team via https://github.com/guardian/actions-prnouncer. Please undo this if I'm mistaken!

@akash1810 akash1810 marked this pull request as draft July 29, 2024 20:20
@Georges-GNM
Copy link
Contributor Author

Closing as we've preferred an alternative solution (using a different version of java) than adding these packages.

@Georges-GNM Georges-GNM closed this Aug 7, 2024
@Georges-GNM Georges-GNM reopened this Aug 28, 2024
@Georges-GNM
Copy link
Contributor Author

Reopening as this now seems preferable to using an openjdk version of java.

It might be more robust to switch to openjdk-11-jdk for this project rather than manually selecting this specific packages.

Appreciate this, but trying to use openjdk 17 for the purpose of browser testing felt like it was displaying some unexpected behaviour on the instance (e.g. re cpu utilisation):
image

@Georges-GNM Georges-GNM marked this pull request as ready for review August 29, 2024 09:17
davidfurey
davidfurey previously approved these changes Aug 29, 2024
Copy link
Member

@davidfurey davidfurey left a comment

Choose a reason for hiding this comment

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

Looks good to me

roles/browser-testing/tasks/main.yml Outdated Show resolved Hide resolved
roles/browser-testing/tasks/main.yml Outdated Show resolved Hide resolved
roles/browser-testing/tasks/main.yml Outdated Show resolved Hide resolved
Co-authored-by: David Furey <david.furey@guardian.co.uk>
@Georges-GNM Georges-GNM merged commit 9589d37 into main Aug 29, 2024
4 checks passed
@Georges-GNM Georges-GNM deleted the gl/update-browser-testing branch August 29, 2024 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants