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

Replace Debian Java requirement with an available version [DI-337] #240

Merged
merged 2 commits into from
Nov 28, 2024

Conversation

JackPGreen
Copy link
Collaborator

@JackPGreen JackPGreen commented Nov 7, 2024

In #195, Debian was intended to upgrade it's dependant JRE to 21 - but the JDK specified does not exist (docker run --interactive --rm debian:stable bash -c "apt-get update && apt list | grep jdk")

Instead we should request the latest JDK which currently is 17 (docker run --interactive --rm debian:stable bash -c "apt-get update && apt-get install -y default-jdk-headless && java -version").

Fixes: #237, DI-337

Post-merge action:

In #195, Debian was _intended_ to upgrade it's dependant JRE to `21` - but the JDK specified does not exist (`docker run --interactive --rm debian:stable bash -c "apt-get update && apt list | grep jdk"`)

Instead we should request the _latest_ JDK which currently is `17` (`docker run --interactive --rm debian:stable bash -c "apt-get update && apt-get install -y default-jdk-headless && java -version"`).

Fixes: #237
@JackPGreen JackPGreen self-assigned this Nov 7, 2024
@JackPGreen JackPGreen enabled auto-merge (squash) November 7, 2024 17:05
@@ -5,7 +5,7 @@ Section: imdg
Priority: optional
Architecture: all
Conflicts: ${CONFLICTS}
Depends: java21-sdk-headless
Depends: java21-sdk-headless | default-jdk-headless
Copy link
Contributor

Choose a reason for hiding this comment

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

What if default-jdk-headless ends up being beyond JDK21 when they finally update?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If there is no java21-sdk-headless, there are various outcomes:

  • we don't specify a fallback default-jdk-headless (current behaviour), you can't use Hazelcast (👎)
  • we specify a fallback default-jdk-headless
    • it's older
      • it's too old, Hazelcast doesn't work (👎)
      • it's old, but not too old (e.g. 17) - Hazelcast works (👍)
    • it's 21 - Hazelcast works (👍)
    • it's newer
      • (say) 22 is close enough - Hazelcast works (👍)
      • it's too new (say 99), Hazelcast doesn't work (👎)

At least this way there's a chance it works.

Copy link
Contributor

Choose a reason for hiding this comment

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

At least this way there's a chance it works.

Yes totally agree but if its >21 than this will break our Java policy and if it doesn't work then we are back to current situation

Wondering why this was not flagged with PR builder or something? Not sure I fully grasp #237. Is this compile or runtime issue?

I think if we can't guarantee default-jdk-headless will confirm to our Java policy than may be just use JDK17?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At least this way there's a chance it works.

Yes totally agree but if its >21 than this will break our Java policy and if it doesn't work then we are back to current situation

The other alternatives are that:

  • We don't list a JDK as a prerequisite and let the users install it themselves
  • We leave as-is and Debian is broken until they include JDK21 (which might be a while)

Wondering why this was not flagged with PR builder or something?
Because we test our Debian packages on Ubuntu, where there is a java21-sdk-headless.
I'm investigating this seperately.

Not sure I fully grasp #237. Is this compile or runtime issue?

It's an installation issue (which I guess for this project would be defined as runtime).
When you install Hazelcast, it should install the listed prerequisites as well. But they aren't available, so the installation fails.

I think if we can't guarantee default-jdk-headless will confirm to our Java policy than may be just use JDK17?

But then our JDKs are inconsistent between platforms... (no right answers here).

Copy link
Contributor

Choose a reason for hiding this comment

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

But then our JDKs are inconsistent between platforms... (no right answers here).

Yes very true.

May be Depends: java21-sdk-headless | java17-sdk-headless so its pinned either way?
In Docker we don't set default and I assume we will update all JDK usages when we update our Java policy

Just wondering if there a way to install JDk21 from a different location?

I am just giving my novice 2 cents so lets see what @ldziedziul has to say.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just wondering if there a way to install JDk21 from a different location?

There is, but as it's not via apt I don't think you can tag it as a pre-requisite the same way so not useful for our purposes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Needs backporting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needs backporting?

Yes - already in the description.

@JackPGreen JackPGreen changed the title Replace Debian Java requirement with an available version Replace Debian Java requirement with an available version [DI-337] Nov 8, 2024
@JackPGreen JackPGreen requested a review from nishaatr November 18, 2024 09:06
@JackPGreen
Copy link
Collaborator Author

@ldziedziul are you able to take a look at this, please?

Copy link
Contributor

@ldziedziul ldziedziul left a comment

Choose a reason for hiding this comment

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

The other alternatives are that:

We don't list a JDK as a prerequisite and let the users install it themselves
We leave as-is and Debian is broken until they include JDK21 (which might be a while)

These options break user experience completely.

I think that default-jdk-headless as a fallback makes sense, and usually we end up with version between it's old, but not too old and it's new, but not too new which IMHO is acceptable. But I think the final decision should be agreed with @ElysePrid-Hazel. Also this brings the question if we should mentions this quirk in the docs

EDIT: One other idea would be to simply support ubuntu not debian

@ElysePrid-Hazel
Copy link

I think for this scenario we should go ahead and add the fallback, it adds more value than not and the work is completed.
Regarding Ubuntu that is a separate conversation and strategy which we can discuss.

@JackPGreen JackPGreen merged commit 82d9c00 into master Nov 28, 2024
36 checks passed
@JackPGreen JackPGreen deleted the debian-jdk-fix branch November 28, 2024 09:39
JackPGreen added a commit to hazelcast/management-center-packaging that referenced this pull request Nov 28, 2024
JackPGreen added a commit that referenced this pull request Nov 28, 2024
In #195, Debian was
_intended_ to upgrade it's dependant JRE to `21` - but the JDK specified
does not exist (`docker run --interactive --rm debian:stable bash -c
"apt-get update && apt list | grep jdk"`)

Instead we should request the _latest_ JDK which currently is `17`
(`docker run --interactive --rm debian:stable bash -c "apt-get update &&
apt-get install -y default-jdk-headless && java -version"`).

Fixes: #237
JackPGreen added a commit that referenced this pull request Nov 28, 2024
In #195, Debian was
_intended_ to upgrade it's dependant JRE to `21` - but the JDK specified
does not exist (`docker run --interactive --rm debian:stable bash -c
"apt-get update && apt list | grep jdk"`)

Instead we should request the _latest_ JDK which currently is `17`
(`docker run --interactive --rm debian:stable bash -c "apt-get update &&
apt-get install -y default-jdk-headless && java -version"`).

Fixes: #237
JackPGreen added a commit that referenced this pull request Nov 28, 2024
JackPGreen added a commit that referenced this pull request Nov 28, 2024
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.

Problem with upgrade Debian 12
4 participants