Skip to content

kvm/ceph: Only if a port number has been specified define in the XML#4231

Merged
yadvr merged 1 commit intoapache:masterfrom
CLDIN:rbd-ceph-default-mon-port
Aug 5, 2020
Merged

kvm/ceph: Only if a port number has been specified define in the XML#4231
yadvr merged 1 commit intoapache:masterfrom
CLDIN:rbd-ceph-default-mon-port

Conversation

@wido
Copy link
Contributor

@wido wido commented Jul 30, 2020

Description

Ceph used to use port 6789 (no need to specify it), but with the messenger v2
from Ceph it switched to port 3300 while 6789 still works.

librados/librbd/libvirt will automatically figure out the ports to use if none is
specified.

Therefor there is no need for CloudStack to explicitely define the port in the XML
passed to Libvirt or Qemu.

Leave blank if no port number has been defined by the user.

Types of changes

  • Enhancement (improves an existing feature and functionality)

Ceph used to use port 6789 (no need to specify it), but with the messenger v2
from Ceph it switched to port 3300 while 6789 still works.

librados/librbd/libvirt will automatically figure out the ports to use if none is
specified.

Therefor there is no need for CloudStack to explicitely define the port in the XML
passed to Libvirt or Qemu.

Leave blank if no port number has been defined by the user.
@wido wido added this to the 4.15.0.0 milestone Jul 30, 2020
@wido wido requested a review from GabrielBrascher July 30, 2020 10:35
@wido wido self-assigned this Jul 30, 2020
@yadvr
Copy link
Member

yadvr commented Jul 30, 2020

@wido reminder - can you help do the next https://github.com/ceph/rados-java release? It would be great if we can do it soon/before 4.15 to support the latest ceph release. Thanks.
@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔centos7 ✔debian. JID-1629

Copy link
Member

@GabrielBrascher GabrielBrascher left a comment

Choose a reason for hiding this comment

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

Code LGTM.

@wido
Copy link
Contributor Author

wido commented Jul 30, 2020

@rhtyd I actually have rados-java 0.6.0 ready. Just need to have somebody upload it to maven central.

@yadvr
Copy link
Member

yadvr commented Aug 4, 2020

@blueorangutan test

@blueorangutan
Copy link

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

Trillian test result (tid-2252)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41608 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4231-t2252-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

@DaanHoogland
Copy link
Contributor

seems ready to merge, right @wido @rhtyd ?

@wido
Copy link
Contributor Author

wido commented Aug 5, 2020

For me it is

@yadvr yadvr merged commit c3554ec into apache:master Aug 5, 2020
rbdOpts += ":mon_host=" + monHost;
if (monPort != 6789) {
if (monPort > 0) {
rbdOpts += "\\\\:" + monPort;
Copy link
Contributor

Choose a reason for hiding this comment

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

this double escape seems to byte us now ;P during release of 4.15

Copy link
Contributor

Choose a reason for hiding this comment

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

@wido @weizhouapache can you comment on if this is the only issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants