cephadm: fix port_in_use when IPv6 is disabled by p-se · Pull Request #39421 · ceph/ceph

p-se · 2021-02-11T15:25:19Z

Do not return "port is in use" when the protocol family tested is not
supported (due to being deactivated).

Differentiates between

OSError: [Errno 97] Address family not supported by protocol

and

OSError: [Errno 98] Address already in use

Signed-off-by: Patrick Seidensal pseidensal@suse.com

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

github-actions · 2021-02-11T15:25:40Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

sebastian-philipp

lgrm, but unrelated whitespace changes

src/cephadm/cephadm

adk3798

LGTM

p-se · 2021-02-15T13:21:41Z

jenkins retest this please

p-se · 2021-02-15T14:46:47Z

jenkins test make check

p-se · 2021-02-15T15:19:47Z

Building remotely on irvingi03

tests/test_cephadm.py:529: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
cephadm:6554: in port_active
    return port_in_use(self.ctx, self.port)
cephadm:966: in port_in_use
    (socket.AF_INET6, '::')
cephadm:964: in <genexpr>
    return any(_port_in_use(af, address) for af, address in (
cephadm:961: in _port_in_use
    raise e
cephadm:954: in _port_in_use
    attempt_bind(ctx, s, address, port_num)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

ctx = <cephadm.CephadmContext object at 0x7f14bb673dd8>
s = <socket.socket [closed] fd=-1, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0>
address = '0.0.0.0', port = 9443

    def attempt_bind(ctx, s, address, port):
        # type: (CephadmContext, socket.socket, str, int) -> None
        try:
            s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
            s.bind((address, port))
        except (socket.error, OSError) as e:  # py2 and py3
            msg = 'Cannot bind to IP %s port %d: %s' % (address, port, e)
            logger.warning(msg)
            if e.errno == errno.EADDRINUSE:
>               raise OSError(msg)
E               OSError: Cannot bind to IP 0.0.0.0 port 9443: [Errno 98] Address already in use

cephadm:938: OSError

However, running the tests locally seems to work:

user@home ~/src/ceph/src/pybind/mgr ±wip-pse-cephadm-port-in-use » ../../script/run_tox.sh mgr -- cephadm | tail 
flake8 installed: DEPRECATION: --find-links option in pip freeze is deprecated. pip 21.2 will remove support for this functionality. You can find discussion regarding this at https://github.com/pypa/pip/issues/9069.,-f /home/user/src/ceph/src/pybind/mgr/wheelhouse,flake8==3.8.4,mccabe==0.6.1,pycodestyle==2.6.0,pyflakes==2.2.0
flake8 run-test-pre: PYTHONHASHSEED='3595092527'
flake8 run-test: commands[0] | flake8 --config=tox.ini cephadm cephadm
___________________________________ summary ____________________________________
  py3: commands succeeded
  mypy: commands succeeded
  test: commands succeeded
  fix: commands succeeded
  flake8: commands succeeded
  congratulations :)
user@home ~/src/ceph/src/pybind/mgr ±wip-pse-cephadm-port-in-use »

p-se · 2021-02-16T13:09:31Z

jenkins test make check

sebastian-philipp · 2021-02-16T13:20:27Z

Building remotely on irvingi03

I think we need to mock socket.bind. Maybe

@mock.patch("socket.bind")
    def test_port_active(self, _, exporter):
        assert exporter.port_active == True

p-se · 2021-02-16T15:55:32Z

Building remotely on irvingi03

I think we need to mock socket.bind. Maybe
@mock.patch("socket.bind")
    def test_port_active(self, _, exporter):
        assert exporter.port_active == True

What I haven't understood yet is why this test, all of a sudden, starts making problems and hasn't done so before. I wonder if that's due to the machine or the test (maybe being canceled at the wrong moment?). But in any case, adapting the test should get us going.

sebastian-philipp · 2021-02-16T16:58:10Z

Building remotely on irvingi03

I think we need to mock socket.bind. Maybe
@mock.patch("socket.bind")
    def test_port_active(self, _, exporter):
        assert exporter.port_active == True
What I haven't understood yet is why this test, all of a sudden, starts making problems and hasn't done so before. I wonder if that's due to the machine or the test (maybe being canceled at the wrong moment?). But in any case, adapting the test should get us going.

cause we are no longer hiding those exceptions from the caller. which is a good thing, but now some tests did call that function and are now throwing errors.

src/cephadm/cephadm

Do not return "port is in use" when the protocol family tested is not supported (due to being deactivated). Fixes: https://tracker.ceph.com/issues/49273 Signed-off-by: Patrick Seidensal <pseidensal@suse.com>

src/cephadm/cephadm

p-se · 2021-02-18T22:42:33Z

Building remotely on irvingi03

I think we need to mock socket.bind. Maybe
@mock.patch("socket.bind")
    def test_port_active(self, _, exporter):
        assert exporter.port_active == True
What I haven't understood yet is why this test, all of a sudden, starts making problems and hasn't done so before. I wonder if that's due to the machine or the test (maybe being canceled at the wrong moment?). But in any case, adapting the test should get us going.
cause we are no longer hiding those exceptions from the caller. which is a good thing, but now some tests did call that function and are now throwing errors.

Turns out the code was faulty. Fixed it and created some tests to ensure it works as expected.

sebastian-philipp · 2021-02-23T11:07:32Z

p-se requested a review from sebastian-philipp 1 hour ago

I'm waiting for https://pulpito.ceph.com/swagner-2021-02-22_13:54:50-rados:cephadm-wip-swagner4-testing-2021-02-22-1119-distro-basic-smithi/ to be scheduled.

sebastian-philipp · 2021-02-24T14:39:28Z

https://pulpito.ceph.com/swagner-2021-02-22_13:54:50-rados:cephadm-wip-swagner4-testing-2021-02-22-1119-distro-basic-smithi/

smithfarm · 2021-11-19T10:14:44Z

NOTE: this was backported to octopus via #41602

p-se requested a review from sebastian-philipp February 11, 2021 15:25

github-actions bot added cephadm needs-rebase labels Feb 11, 2021

p-se force-pushed the wip-pse-cephadm-port-in-use branch from b451274 to eecef03 Compare February 11, 2021 15:38

github-actions bot removed the needs-rebase label Feb 11, 2021

sebastian-philipp suggested changes Feb 11, 2021

View reviewed changes

src/cephadm/cephadm Outdated Show resolved Hide resolved

sebastian-philipp reviewed Feb 11, 2021

View reviewed changes

src/cephadm/cephadm Outdated Show resolved Hide resolved

sebastian-philipp added the wip-swagner4-testing label Feb 11, 2021

sebastian-philipp requested a review from adk3798 February 11, 2021 23:58

p-se force-pushed the wip-pse-cephadm-port-in-use branch from eecef03 to 6c0e059 Compare February 12, 2021 09:54

p-se marked this pull request as ready for review February 12, 2021 10:00

p-se requested a review from a team as a code owner February 12, 2021 10:00

p-se force-pushed the wip-pse-cephadm-port-in-use branch 3 times, most recently from 8ecdee9 to 512f899 Compare February 12, 2021 13:14

adk3798 approved these changes Feb 14, 2021

View reviewed changes

sebastian-philipp mentioned this pull request Feb 16, 2021

cephadm: also print occupying process of port in use #39326

Closed

3 tasks

p-se force-pushed the wip-pse-cephadm-port-in-use branch from 512f899 to 03d2262 Compare February 18, 2021 10:01

p-se commented Feb 18, 2021

View reviewed changes

src/cephadm/cephadm Outdated Show resolved Hide resolved

cephadm: fix port_in_use when IPv6 is disabled

a0ffcec

Do not return "port is in use" when the protocol family tested is not supported (due to being deactivated). Fixes: https://tracker.ceph.com/issues/49273 Signed-off-by: Patrick Seidensal <pseidensal@suse.com>

p-se force-pushed the wip-pse-cephadm-port-in-use branch from 03d2262 to a0ffcec Compare February 18, 2021 22:39

p-se commented Feb 18, 2021

View reviewed changes

src/cephadm/cephadm Show resolved Hide resolved

p-se requested a review from sebastian-philipp February 23, 2021 10:20

sebastian-philipp approved these changes Feb 24, 2021

View reviewed changes

sebastian-philipp merged commit d383284 into ceph:master Feb 24, 2021

This was referenced Feb 26, 2021

[DNM BECAUSE NEEDS TEST FIX] octopus: cephadm: fix port_in_use when IPv6 is disabled #39721

Closed

pacific: cephadm: Batch backport March (1) #39807

Merged

Conversation

p-se commented Feb 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

github-actions bot commented Feb 11, 2021

Uh oh!

sebastian-philipp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adk3798 left a comment

Choose a reason for hiding this comment

Uh oh!

p-se commented Feb 15, 2021

Uh oh!

p-se commented Feb 15, 2021

Uh oh!

p-se commented Feb 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

p-se commented Feb 16, 2021

Uh oh!

sebastian-philipp commented Feb 16, 2021

Uh oh!

p-se commented Feb 16, 2021

Uh oh!

sebastian-philipp commented Feb 16, 2021

Uh oh!

Uh oh!

Uh oh!

p-se commented Feb 18, 2021

Uh oh!

sebastian-philipp commented Feb 23, 2021

Uh oh!

sebastian-philipp commented Feb 24, 2021

Uh oh!

smithfarm commented Nov 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

p-se commented Feb 11, 2021 •

edited

Loading

p-se commented Feb 15, 2021 •

edited

Loading