-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix docker install #5381
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
Fix docker install #5381
Conversation
@@ -44,7 +44,8 @@ RUN useradd -r -u 900 -m -c "ckan account" -d $CKAN_HOME -s /bin/false ckan | |||
RUN mkdir -p $CKAN_VENV $CKAN_CONFIG $CKAN_STORAGE_PATH && \ | |||
virtualenv $CKAN_VENV && \ | |||
ln -s $CKAN_VENV/bin/pip /usr/local/bin/ckan-pip &&\ | |||
ln -s $CKAN_VENV/bin/paster /usr/local/bin/ckan-paster | |||
ln -s $CKAN_VENV/bin/paster /usr/local/bin/ckan-paster &&\ | |||
ln -s $CKAN_VENV/bin/ckan /usr/local/bin/ckan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a symlink to the ckan cli here, similar to what has been done in the past for ckan-paster.
Questions:
- Are we happy with just
ckan
, or maybeckan-cli
? - Do we want to remove the
ckan-paster
symlink?
@@ -62,4 +63,4 @@ ENTRYPOINT ["/ckan-entrypoint.sh"] | |||
USER ckan | |||
EXPOSE 5000 | |||
|
|||
CMD ["ckan-paster","serve","/etc/ckan/production.ini"] | |||
CMD ["ckan","-c","/etc/ckan/production.ini", "run", "--host", "0.0.0.0"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to set the host to 0.0.0.0
to allow host network to access the server in the container. The ckan run
command defaults to localhost
if --host
is not provided.
ckan/cli/__init__.py
Outdated
@@ -5,7 +5,7 @@ | |||
import click | |||
import logging | |||
from logging.config import fileConfig as loggingFileConfig | |||
from configparser import ConfigParser | |||
from six.moves.configparser import ConfigParser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really sure how this is working for anyone else? This looks to be way that this library is imported in other parts of the project.
ckan/cli/generate.py
Outdated
@click.option(u'-o', u'--output-dir', help=u"Location to put the generated " | ||
u"template.", | ||
default=u'.') | ||
def extension(output_dir): | ||
try: | ||
from cookiecutter.main import cookiecutter | ||
except ImportError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this was in the @generate
decorator previously, it was also being applied when running ckan generate config
.
The config command does not need cookiecutter, and should not need to load the dev-requirements in order to generate the config file for a new install.
4d4f142
to
4f4ffc0
Compare
Please note that I'm seeing the issue described in #4715 ( |
@smotornyuk -- hey what was the PR mentioned at the Dev meeting today that (may be) updating ckan/cli/generate.py ? I missed that one - thanks Sergey |
#5150 - this one |
@markstuart - thanks for your work here - did you also change the ckan-entrypoint.sh file in your branch? |
Ah - yes I see the 2 changes in the ckan-entrypoint.sh you made - great - this works for me |
Yep, just updated to use the new ckan CLI commands 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have successfully downloaded/built the 2.9 docker images, run the containers and tested a login - all work fine - Maybe one of the Core Dev's @amercader or @smotornyuk could check the changes in the Python code to see if they are happy
4f4ffc0
to
272b30a
Compare
I see master has moved on a bit since I raised this PR, and there were some conflicts due to @amercader merging #5150. I've rebased my branch on top of master and fixed the conflicts. |
paster is still there, but doesn't load all the commands that are needed Also prevents an error message being thrown about needing to pip install dev-requirements Using 0.0.0.0 instead of localhost in docker so that we can access exposed port from outside the container
272b30a
to
ee83071
Compare
@amercader - Hey can we get this PR merged?...it works fine |
I attempted to install the master (2.9.0a) version of CKAN using docker, but was seeing errors in the logs when running
docker-compose logs -f
. The ckan container would exit almost immediately after starting.The only similar info I can see in issues is this comment by @kowh-ai #5243 (comment)
I haven't raised an accompanying issue, I chose to hopefully provide a fix instead.
Proposed fixes:
Docker installation instructions for 2.9.0a should now work as expected.
Features:
Please [X] all the boxes above that apply