Merge Conda and regular Python s2i wrappers

[adriangonz]

Context

To include support for custom Conda environments on #1108, we added a new seldonio/seldon-core-s2i-conda s2i wrapper. However, it could be a good idea to merge it with the existing seldonio/seldon-core-s2i-python. This makes sense if we consider that adding Conda is just extending the feature set of the Python wrapper. It could also help with maintainability of the s2i wrappers.

Before agreeing to this change, one of the caveats that we need to consider is that the new seldonio/seldon-core-s2i-conda wrapper uses continuum/miniconda3 as a Docker base image. This image is based on debian:buster-slim, which is a smaller version of debian:buster and which could break some uses of the existing seldonio/seldon-core-s2i-python.

[RafalSkolasinski]

Just an observation:
Images seldon-core-s2i-python we have in few python versions (we base e.g. on python:3.6 and python:3.7) now.
The continuum/miniconda3 images are tagged by conda version and will have version of Python that ships with that conda.

[RafalSkolasinski]

miniconda3 now comes with Python 3.7

We could always create Python 3.6 env there but this would just add another layer to base image and unnecessarily increase its size.

Do we still need to support Python 3.6 image?

[RafalSkolasinski]

It would be 214 MB difference in image size:

satriani ➜  ~ docker images | grep rafalskolasinski | grep mini 
rafalskolasinski/miniconda                3.6                               478c75cf6da1        About a minute ago   644MB
rafalskolasinski/miniconda                3.7                               406f2b43ea59        3 months ago         430MB

The 3.6 is made using following Dockerfile

satriani ➜  playground cat Dockerfile
FROM continuumio/miniconda3:latest
RUN conda install --yes python=3.6

And seems to give fully functional Python 3.6 base.

[adriangonz]

You raise a good point about being forced to use Conda's distribution if we switch to the continuum/miniconda3 image. The way I see it, we should keep using "vanilla" Python by default and perhaps use Conda's just inside a Conda env. That way Conda remains completely optional.

I assume this would only be possible if we installed Miniconda on top of our Python image? That sounds like a potential rabbit hole...

What are your thoughts on this @RafalSkolasinski?

[axsaucedo]

Interesting - We do still need to support python 3.6 / 3.7 as users would also be on those, same with data science libraries.

Why don't we just explicitly remove the 3.7 environment from Conda on the docker file once 3.6 is installed?

[RafalSkolasinski]

That won't work as the 3.7 will still be present due to how Docker handle layers.

[aghagol]

Related to this change, in the s2i python run script, executive seldon-core-microservice uses base conda env regardless of activated conda env (it is using /opt/conda/bin/python in shebang directive). So installed packages in CONDA_ENV_NAME will not be used once the service is running resulting in import errors.

[RafalSkolasinski]

I am pretty sure this is working as expected. The seldon-core-microservice that is available in the PATH after activating the conda requirement is a wrapper script around the defined entry point created by setup.py or pip installation. If I check locally this have the right shebang:

❯❯❯ which python                      
/home/rskolasinski/miniconda3/envs/seldon-core/bin/python

❯❯❯ cat (which seldon-core-microservice)                                             
#!/home/rskolasinski/miniconda3/envs/seldon-core/bin/python3.8
# EASY-INSTALL-ENTRY-SCRIPT: 'seldon-core','console_scripts','seldon-core-microservice'
import re
import sys

# for compatibility with easy_install; see #2198
__requires__ = 'seldon-core'

try:
    from importlib.metadata import distribution
except ImportError:
    try:
        from importlib_metadata import distribution
    except ImportError:
        from pkg_resources import load_entry_point


def importlib_load_entry_point(spec, group, name):
    dist_name, _, _ = spec.partition('==')
    matches = (
        entry_point
        for entry_point in distribution(dist_name).entry_points
        if entry_point.group == group and entry_point.name == name
    )
    return next(matches).load()


globals().setdefault('load_entry_point', importlib_load_entry_point)


if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
    sys.exit(load_entry_point('seldon-core', 'console_scripts', 'seldon-core-microservice')())

[RafalSkolasinski]

@aghagol Do you have the example that fails and does not work properly?

[aghagol]

Thanks @RafalSkolasinski for getting back quickly. That is interesting to know.

I was looking at the prebuilt seldon-core image seldonio/seldon-core-s2i-python37:1.2.2 and was looking at the executable seldon-core-microservice inside that container. Also, when I build my seldon image using s2i build ... and my customized environment.yml, it looks like that these conda packages are installed in the microservice env (or alternatively in $CONDA_ENV_NAME if specified) while the conda base env is being used inside seldon-core-microservice shebang. So I guess seldon-core-microservice is not picking up the microservice env. Am I doing something wrong?

[aghagol]

Ah sorry I forgot that I need to activate the conda env first. Will try that next.

[aghagol]

When I run the docker image, this is what I get:

~/devops/dai/dai-harmony/seldon-deployment managed_sql ❯ docker run -it --rm -p 5000:5000 pilot-adl-test:tip                                                                                                                                         2m 57s
Activating Conda environment 'microservice'
starting microservice
2020-08-14 13:57:21,082 - seldon_core.microservice:main:207 - INFO:  Starting microservice.py:main
2020-08-14 13:57:21,082 - seldon_core.microservice:main:208 - INFO:  Seldon Core version: 1.2.2
2020-08-14 13:57:21,084 - seldon_core.microservice:main:310 - INFO:  Parse JAEGER_EXTRA_TAGS []
2020-08-14 13:57:21,084 - seldon_core.microservice:main:313 - INFO:  Annotations: {}
2020-08-14 13:57:21,084 - seldon_core.microservice:main:317 - INFO:  Importing XGB
Traceback (most recent call last):
  File "/opt/conda/bin/seldon-core-microservice", line 11, in <module>
    load_entry_point('seldon-core', 'console_scripts', 'seldon-core-microservice')()
  File "/microservice/python/seldon_core/microservice.py", line 318, in main
    interface_file = importlib.import_module(args.interface_name)
  File "/opt/conda/lib/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/microservice/XGB.py", line 5, in <module>
    import xgboost as xgb
ModuleNotFoundError: No module named 'xgboost'

Even though it says it is activating env microservice, it calls /opt/conda/bin/seldon-core-microservice. This is resulting in import errors down the line since my packages are being installed in the microservice env.

I looked inside the container. When I manually activate the microservice env, I can successfully import my python packages (in this case xgboost). However, the path to seldon-core-microservice seems to be on base env still:

~/devops/dai/dai-harmony/seldon-deployment managed_sql ❯ docker run -it --rm --entrypoint bash pilot-adl-test:tip                                                                                                                                        3s
(base) root@b4520c7ac6ba:/microservice# conda env list
# conda environments:
#
base                  *  /opt/conda
microservice             /opt/conda/envs/microservice

(base) root@b4520c7ac6ba:/microservice# conda activate microservice
(microservice) root@b4520c7ac6ba:/microservice# python
Python 3.8.5 (default, Aug  5 2020, 08:36:46) 
[GCC 7.3.0] :: Anaconda, Inc. on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import xgboost
>>> 
(microservice) root@b4520c7ac6ba:/microservice# which seldon-core-microservice
/opt/conda/bin/seldon-core-microservice
(microservice) root@b4520c7ac6ba:/microservice# 

[aghagol]

Figured it out. The problem is that I wasn't including seldon-core as a dependency in my environment.yml and it was not installing it in the microservice env - thus defaulting to the base's seldon-core-microservice. This is also mentioned in https://docs.seldon.io/projects/seldon-core/en/latest/python/python_wrapping_docker.html?highlight=requirements.txt#requirements-txt (for the requirements.txt file - would be nice to mention the same for environment.yml in doc) but I missed that.