search

scylladb / scylla-jmx

Scylla JMX proxy
GNU Affero General Public License v3.0
29 stars 53 forks source link

select-java: be compatible with el9 and rocky linux #214

Closed tchaikov closed 1 year ago

tchaikov commented 1 year ago

in this change, the compatibility with rocky linux is addressed with following measures:

  1. use the value of "java.specification.version" from the output -XshowSettings as the java version to be matched.
  2. check all JAVA_HOMEs under /usr/lib/jvm, simpler this way
  3. print error message when we are not able to find java executable for running scylla-jmx.

Fixes #212

tchaikov commented 1 year ago

@fruch hi Israel, could you please help review this change?

fruch commented 1 year ago

@tchaikov can you build a unified package out of it ? so we can try it ?

tchaikov commented 1 year ago

@fruch i'd like to. but i am not sure how to accomplish this. anyway, i just created a PR with this change at https://github.com/scylladb/scylladb/pull/13886

fruch commented 1 year ago

@fruch i'd like to. but i am not sure how to accomplish this. anyway, i just created a PR with this change at scylladb/scylladb#13886

there's a pipeline that can help building/uploading what's needed https://jenkins.scylladb.com/view/master/job/scylla-master/job/byo/job/package-byo/

avikivity commented 1 year ago

@tchaikov ninja build/dev/dist/tar/scylla-unified-package-5.3.0~dev-0.20230515.96e912e1cf53.tar.gz

(you'll have to adjust the release string, I don't think we have a virtual target for it)

fruch commented 1 year ago

@tchaikov ninja build/dev/dist/tar/scylla-unified-package-5.3.0~dev-0.20230515.96e912e1cf53.tar.gz

(you'll have to adjust the release string, I don't think we have a virtual target for it)

I think ninja dist-unified-release would do the trick but I would need it up on s3 as well, for run the effected jobs to see if this fixes them.

tchaikov commented 1 year ago

@fruch tested at https://jenkins.scylladb.com/job/scylla-staging/job/tchaikov/job/artifacts-rocky8-test/5/ . could you please take another look?

tchaikov commented 1 year ago

@scylladb/scylla-maint hi scylla maintainers, could you help merge this change?

fruch commented 1 year ago

@fruch tested at https://jenkins.scylladb.com/job/scylla-staging/job/tchaikov/job/artifacts-rocky8-test/5/ . could you please take another look?

this run looks good,

@fruch tested at https://jenkins.scylladb.com/job/scylla-staging/job/tchaikov/job/artifacts-rocky8-test/5/ . could you please take another look?

Looks goods, if it's passing it mean it was working as expected.

I might consider building also RPM/DEB repos and test with that as well, to make sure there no more surprises here.

tchaikov commented 1 year ago

@fruch tested at https://jenkins.scylladb.com/job/scylla-staging/job/tchaikov/job/artifacts-rocky8-test/5/ . could you please take another look?

this run looks good,

@fruch tested at https://jenkins.scylladb.com/job/scylla-staging/job/tchaikov/job/artifacts-rocky8-test/5/ . could you please take another look?

Looks goods, if it's passing it mean it was working as expected.

I might consider building also RPM/DEB repos and test with that as well, to make sure there no more surprises here.

IMHO, the checks are identical as before. and practically, this change only changes the way how we detect the java executable on rocky and the distro whose distro ID does not match with the "known" ones. but if you insist, i will try tomorrow.

tchaikov commented 1 year ago

just tested with the rpm repo created with the byo pipeline, https://jenkins.scylladb.com/job/scylla-staging/job/tchaikov/job/artifacts-rocky8-test/6/ . passed.