fix #49754 minion zmq connecting to master configured explicitly as IPv6 address or IPv6 only master #49755

aphor · 2018-09-24T05:35:19Z

What does this PR do?

fixes implementation of IPv6 address handling by minion and zeromq transport to enable minions to connect to a master identified only by IPv6 address or address/port

Work:

depends on Bugfix/rework: IPv6 scope errors #49705 [MERGED]
unit tests failing [FIXED]
integration tests failing [FIXED]
parsing config master: "host:port" [FIXED]
missing coverage for host:port parsing [FIXED]

What issues does this PR fix or reference?

#49754, possibly #40515

Tests written?

Yes, or rather test coverage added to existing tests.

Commits signed with GPG?

Yes

salt/minion.py

salt/transport/zeromq.py

salt/utils/win_reg.py

tests/integration/utils/testprogram.py

salt/utils/zeromq.py

salt/minion.py

salt/transport/zeromq.py

salt/utils/zeromq.py

rallytime · 2018-10-04T13:01:20Z

Hrm. The tests aren't liking this change very much. There are quite a few failing tests here.

aphor · 2018-10-04T13:40:17Z

This adds coverage for IPv6 connectivity. I'll check things out after a rebase.

…

On Thu, Oct 4, 2018, 08:01 Nicole Thomas ***@***.***> wrote: Hrm. The tests aren't liking this change very much. There are quite a few failing tests here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#49755 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOKuw9AfvVrqGobmeZsXNPnT7AaRqR4ks5uhgbEgaJpZM4W2FcG> .

aphor · 2018-10-05T14:21:21Z

Hrm. The tests aren't liking this change very much. There are quite a few failing tests here.

I rebased against 2018.3 head last night and only got 2 unit test failures.

This one(tests/unit/transport/test_zeromq.py", line 335) needs some work, so I'll look at that.

 --------  Failed Tests  ----------------------------------------------------------------------------------------------------------------------------------------------------------------
... stuff redacted as irrelevant to this PR ...
   -> unit.transport.test_zeromq.ZMQConfigTest.test_master_uri  .........................................................................................................................
       Traceback (most recent call last):
         File "/Users/aphor/src/salt.git/tests/unit/transport/test_zeromq.py", line 335, in test_master_uri
           source_port=s_port) == 'tcp://[{0}]:{1};[{2}]:{3}'.format(s_ip, s_port, m_ip, m_port)
       AssertionError
   ......................................................................................................................................................................................
 ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
=========================================================================================================================================================================================
FAILED (total=7403, skipped=2100, passed=5298, failures=2, errors=3) 
================================================================================  Overall Tests Report  =================================================================================

isbm · 2018-10-05T14:43:01Z

@aphor BTW, we are running all that with PyTest. 😉 JFYI.

aphor · 2018-10-05T16:52:31Z

@aphor BTW, we are running all that with PyTest. 😉 JFYI.

Trust, but verify @isbm ! :)

I think I will have some time this weekend to fix things up. I appreciate your attention!

rallytime · 2018-10-15T20:46:02Z

@aphor Unfortunately this still isn't quite right. This is failing quite a few integration tests.

aphor · 2018-10-15T23:58:17Z

IDK what is wrong with those tests. The failure logs say that kitchen didn't make a minion, and no targets matched :(

I'm going to fix stuff @isbm recommended, re-push, and then figure out what's wrong with the test kitchen.

cachedout · 2018-10-16T10:15:59Z

@aphor I am re-running the test suite and pulling in the latest upstream branch changes.

aphor · 2018-10-17T04:19:06Z

There's one unit test failure I found that I need to fix with salt.transport.zeromq._get_master_uri().

I tried running the daemon tests --docked, but there's something wrong with the test containers missing the salt test code, and I don't have time to figure out how to make that work so I'm going to just fix the master_uri bug and re-push.

aphor · 2019-01-09T14:06:36Z

@aphor great! Except please do not in a future mention GitHub usernames in the commit messages. Because now every time this commit will be referenced/pushed/touched/transferred/etc, I will get another email. 😆

Sorry about that! I will amend and force push to get rid of that.

Error Message object of type 'NoneType' has no len() Stacktrace Traceback (most recent call last): File "/tmp/kitchen/testing/tests/unit/utils/test_network.py", line 225, in test_parse_host_port host, port = network.parse_host_port(host_port) File "/tmp/kitchen/testing/salt/utils/network.py", line 1957, in parse_host_port raise _e_ TypeError: object of type 'NoneType' has no len()

aphor · 2019-01-10T00:50:51Z

@isbm I'd be grateful if you could review these changes.

I'm not sure why the py2-windows-2016 job is having problems. It doesn't look related to this change set.

This ends up in opts['master_ip'] where it can cause problems. I found this while testng a git+pip based minion installation on Ubuntu Bionic. I'm not sure why it doesn't affect other targeted OS versions in the test framework. On Bionic, if 'salt' resolves to an IPv6 address, zmq fails to connect to the master, and complains about the bracketed address.

TypeError https://docs.python.org/2/library/socket.html RTFD "Note This method has historically accepted a pair of parameters for AF_INET addresses instead of only a tuple. This was never intentional and is no longer available in Python 2.0 and later. "

Bogus random test failures forklifted from develop at 414bfe6

This reverts commit 0af7653. I guess that was way too optimistic.

aphor · 2019-01-11T04:35:49Z

tests.integration.minion.test_blackout is failing off and on for Ubuntu and Windows tests

I tried naïvely bringing the latest changes to that file from develop, but there's too much baggage.

@gtmanfred, @s0undt3ch is there anything you would recommend to make these red lights green?

salt/utils/network.py

s0undt3ch · 2019-01-11T08:31:24Z

We have fixes in develop for that test which cannot be backported into 2018.3, but perhaps the 2019.2 branch changes are small enough to get that test more stable...

(cherry picked from commit 479439a)

(cherry picked from commit 9ab0e98)

confession: tests failed to complete on gitub notification auth failure, so I want to trigger another test.

This reverts commit a07f356. It wasn't necessary, and I want to trigger another round of tests on a Friday night.

aphor · 2019-01-15T05:08:42Z

@rallytime I think this is ready for review.

thatch45 · 2019-01-15T16:27:27Z

@s0undt3ch @DmitryKuzmenko can either of you review and sign off here?

aphor requested a review from a team as a code owner September 24, 2018 05:35

ghost requested review from a team September 24, 2018 05:35

aphor force-pushed the issues/49754 branch 2 times, most recently from da03dc9 to b2f5889 Compare September 24, 2018 05:47

isbm suggested changes Sep 24, 2018

View reviewed changes

KaiSforza reviewed Sep 24, 2018

View reviewed changes

salt/utils/zeromq.py Outdated Show resolved Hide resolved

aphor force-pushed the issues/49754 branch from b2f5889 to fe8c206 Compare September 24, 2018 13:42

KaiSforza reviewed Sep 24, 2018

View reviewed changes

salt/utils/zeromq.py Outdated Show resolved Hide resolved

aphor force-pushed the issues/49754 branch 2 times, most recently from b75b77b to e3e8172 Compare September 24, 2018 19:38

aphor changed the title ~~fix #49754 minion zmq connecting to master configured as IPv6 address~~ fix #49754 minion zmq connecting to master configured as IPv6 address WIP needs #49705 Sep 24, 2018

aphor force-pushed the issues/49754 branch 2 times, most recently from cc0002f to f917636 Compare September 24, 2018 21:24

isbm suggested changes Sep 25, 2018

View reviewed changes

salt/minion.py Outdated Show resolved Hide resolved

salt/transport/zeromq.py Outdated Show resolved Hide resolved

salt/utils/zeromq.py Outdated Show resolved Hide resolved

gtmanfred approved these changes Sep 25, 2018

View reviewed changes

cachedout approved these changes Oct 4, 2018

View reviewed changes

aphor force-pushed the issues/49754 branch from 73290f9 to 3b3e543 Compare October 5, 2018 14:22

aphor force-pushed the issues/49754 branch 2 times, most recently from 06b67be to 372d733 Compare October 9, 2018 22:48

aphor force-pushed the issues/49754 branch from 7dac708 to 8e2c81f Compare October 17, 2018 05:04

aphor changed the title ~~fix #49754 minion zmq connecting to master configured as IPv6 address WIP~~ fix #49754 minion zmq connecting to master configured as IPv6 address Jan 9, 2019

aphor force-pushed the issues/49754 branch from 590603a to 6878de0 Compare January 9, 2019 14:08

isbm approved these changes Jan 10, 2019

View reviewed changes

aphor added 6 commits January 10, 2019 16:20

syntax error bad parentheses

ec1f7f3

socket.connect needs a tuple

ef859b7

TypeError https://docs.python.org/2/library/socket.html RTFD "Note This method has historically accepted a pair of parameters for AF_INET addresses instead of only a tuple. This was never intentional and is no longer available in Python 2.0 and later. "

Merge branch '2018.3' into issues/49754

e9415d6

backport test improvements from develop

0af7653

Bogus random test failures forklifted from develop at 414bfe6

Merge branch 'issues/49754' of github.com:aphor/salt into issues/49754

f02625e

Revert "backport test improvements from develop"

83572c3

This reverts commit 0af7653. I guess that was way too optimistic.

s0undt3ch reviewed Jan 11, 2019

View reviewed changes

salt/utils/network.py Outdated Show resolved Hide resolved

aphor and others added 5 commits January 11, 2019 09:26

fix docstring quote style

9b56485

These should not be considered destructive tests

27aa957

(cherry picked from commit 479439a)

Make sure blackout tests clean up after themselves. Properly.

a2d9e4f

(cherry picked from commit 9ab0e98)

wordsmithing

a07f356

confession: tests failed to complete on gitub notification auth failure, so I want to trigger another test.

Revert "wordsmithing"

dea01b7

This reverts commit a07f356. It wasn't necessary, and I want to trigger another round of tests on a Friday night.

aphor changed the title ~~fix #49754 minion zmq connecting to master configured as IPv6 address~~ fix #49754 minion zmq connecting to master configured explicitly as IPv6 address or IPv6 only master Jan 12, 2019

s0undt3ch approved these changes Jan 15, 2019

View reviewed changes

s0undt3ch merged commit 83c38c6 into saltstack:2018.3 Jan 15, 2019

aphor deleted the issues/49754 branch January 15, 2019 22:14

twangboy mentioned this pull request Jan 22, 2019

Fix minion start issue #51274

Merged

Ch3LL mentioned this pull request Jan 23, 2019

2018.3 - Master address: '127.0.0.1' could not be resolved #51289

Closed

awlx mentioned this pull request Sep 26, 2019

2019.2.1: Breaks Minion-Master Communication #54762

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix #49754 minion zmq connecting to master configured explicitly as IPv6 address or IPv6 only master #49755

fix #49754 minion zmq connecting to master configured explicitly as IPv6 address or IPv6 only master #49755

aphor commented Sep 24, 2018 •

edited

Loading

rallytime commented Oct 4, 2018

aphor commented Oct 4, 2018 via email •

edited

Loading

aphor commented Oct 5, 2018 •

edited

Loading

isbm commented Oct 5, 2018

aphor commented Oct 5, 2018

rallytime commented Oct 15, 2018

aphor commented Oct 15, 2018

cachedout commented Oct 16, 2018

aphor commented Oct 17, 2018

aphor commented Jan 9, 2019

aphor commented Jan 10, 2019

aphor commented Jan 11, 2019

s0undt3ch commented Jan 11, 2019

aphor commented Jan 15, 2019

thatch45 commented Jan 15, 2019

fix #49754 minion zmq connecting to master configured explicitly as IPv6 address or IPv6 only master #49755

fix #49754 minion zmq connecting to master configured explicitly as IPv6 address or IPv6 only master #49755

Conversation

aphor commented Sep 24, 2018 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

Tests written?

Commits signed with GPG?

rallytime commented Oct 4, 2018

aphor commented Oct 4, 2018 via email • edited Loading

aphor commented Oct 5, 2018 • edited Loading

isbm commented Oct 5, 2018

aphor commented Oct 5, 2018

rallytime commented Oct 15, 2018

aphor commented Oct 15, 2018

cachedout commented Oct 16, 2018

aphor commented Oct 17, 2018

aphor commented Jan 9, 2019

aphor commented Jan 10, 2019

aphor commented Jan 11, 2019

s0undt3ch commented Jan 11, 2019

aphor commented Jan 15, 2019

thatch45 commented Jan 15, 2019

aphor commented Sep 24, 2018 •

edited

Loading

aphor commented Oct 4, 2018 via email •

edited

Loading

aphor commented Oct 5, 2018 •

edited

Loading