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

Upgrade eventlet to the latest version (0.22.1) #4007

Merged
merged 10 commits into from
Feb 22, 2018
Merged

Upgrade eventlet to the latest version (0.22.1) #4007

merged 10 commits into from
Feb 22, 2018

Conversation

Kami
Copy link
Member

@Kami Kami commented Feb 19, 2018

This pull request upgrades eventlet to the latest stable version.

Some tests were failing with this version because of the removal of select.poll() method. To get it to pass, I added a work around which injects original select.poll() for tests.

This issue itself only affected tests.

We don't perform monkey patching in the subprocess which is used to execute Python runner actions - we tried to do monkey patching there in the past, but it negatively affected / didn't work with some 3rd party libraries so we decided to remove it and making it opt-in instead of being enabled by default.

Resolves #3968.

@Kami Kami added this to the 2.7.0 milestone Feb 19, 2018
@Kami Kami changed the title Upgrade eventlet to the latest version (0.22.1) [WIP] Upgrade eventlet to the latest version (0.22.1) Feb 19, 2018
@Kami
Copy link
Member Author

Kami commented Feb 19, 2018

It looks like CI fails with a lot of errors, even though unit tests pass locally.

It looks like some of issues are related to the change in the DNS resolving.

@Kami
Copy link
Member Author

Kami commented Feb 20, 2018

@warrenvw @armab It looks like packages build is failing with:

File "/opt/stackstorm/st2/local/lib/python2.7/site-packages/eventlet/patcher.py", line 94, in inject
    module = __import__(module_name, {}, {}, module_name.split('.')[:-1])
  File "/opt/stackstorm/st2/local/lib/python2.7/site-packages/eventlet/support/dns/rdtypes/IN/WKS.py", line 23, in <module>
    _proto_tcp = socket.getprotobyname('tcp')
socket.error: protocol not found

New version of eventlet uses dnspython (async DNS library) by default which consults /etc/protocols file. I SSHed into the container and it looks like that file doesn't exist.

It looks like we should probably ensure this file exists and install netbase package on Ubuntu (http://senthilthecoder.com/post/ubuntu-getprotobyname/), wdyt?

@warrenvw
Copy link
Contributor

Yes, install netbase.

@arm4b
Copy link
Member

arm4b commented Feb 20, 2018

👍 to verify what @warrenvw said.

An addition to that, if minimal Docker image requires netbase apt package for st2 to function correctly, - we'll need to add it as a dependency in st2-packages for st2 (https://github.com/StackStorm/st2-packages/blob/master/packages/st2/debian/control#L16), as well as find the respective package name for rpm (if any).

@warrenvw
Copy link
Contributor

FWIW, seems we need setup rpm package, not netcfg:

[vagrant@st2el7 ~]$ rpm -q --whatprovides /etc/protocols
setup-2.8.71-7.el7.noarch

@Kami
Copy link
Member Author

Kami commented Feb 21, 2018

Yay, looks like adding netbase as a dependency for st2 package fixed the build - https://circleci.com/gh/StackStorm/st2/6769

@Kami Kami changed the title [WIP] Upgrade eventlet to the latest version (0.22.1) Upgrade eventlet to the latest version (0.22.1) Feb 21, 2018
Copy link
Contributor

@m4dcoder m4dcoder left a comment

Choose a reason for hiding this comment

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

LGTM. But the change is a little bit cryptic. Take InstallPackTestCase for example, i drilled down to https://github.com/StackStorm/st2/blob/master/contrib/packs/actions/pack_mgmt/download.py but I don't see any reference to eventlet and select poll. How is this failing?

@Kami
Copy link
Member Author

Kami commented Feb 21, 2018

@m4dcoder It's the underlying git library we use for git related operations - it shells out and uses .poll() method.

As mentioned in the description - this only affects tests where we use eventlet monkey patching, but not actual deployments, because we don't use monkey patching in the Python action runner subprocess.

@Kami
Copy link
Member Author

Kami commented Feb 22, 2018

And to clarify the test situation - there is actually no need for this and many other tests to use eventlet monkey patching, but the way the test runner works (multiple test files run in a single process) and the way eventlet monkey patching works (import side affect) it's sadly a side affect from some other test.

We had issues with this cross test side affects and pollution in the past, but sadly there is no easy way around it - running each test file in a separate process would be too slow and expensive.

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.

4 participants