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

use centos as the base image #2

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JayjeetAtGithub
Copy link

@JayjeetAtGithub JayjeetAtGithub commented May 28, 2020

Please ignore the changes in .travis.yml and wf.yml. Those are because of the substitutions bug in popper.

.travis.yml Outdated Show resolved Hide resolved
# pip install --upgrade pip
# pip install --upgrade virtualenv
scl enable devtoolset-8 bash
scl enable devtoolset-7 bash
Copy link
Collaborator

Choose a reason for hiding this comment

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

all these packages should be installed already by the install-deps.sh, can you expand on why you needed to install these?

Copy link
Author

@JayjeetAtGithub JayjeetAtGithub May 28, 2020

Choose a reason for hiding this comment

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

It happend that these images were not installed by themselves and errored out due to their unavailabilty. So i installed them explicitly

Copy link
Collaborator

Choose a reason for hiding this comment

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

so then we need to fix install-deps.sh. That script is taken from the upstream Ceph repo, so we need to figure out the root cause, otherwise we won't be able to explain any subsequent failure

Copy link
Author

@JayjeetAtGithub JayjeetAtGithub May 28, 2020

Choose a reason for hiding this comment

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

yeah please take a look at this. Here, removing the devtoolsets, the image build again starts failing here

Copy link
Collaborator

Choose a reason for hiding this comment

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

what I meant is that this script as a whole shouldn't be needed.

Copy link
Author

@JayjeetAtGithub JayjeetAtGithub May 28, 2020

Choose a reason for hiding this comment

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

Oh I see. Yeah thats the ideal. So, what should be done now ? Should we try to fix in the upstream or continue with patches like this ? Patching will be error prone though

Copy link
Collaborator

Choose a reason for hiding this comment

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

we need to remove this script. Building the image should work without it. If there's a problem when building the ceph-builder image after we remove this script, then it'll very likely be something on our end. The upstream script is used extensively by the ceph community, and has some minor tweaks for skyhook (install arrow/parquet). It could still be something on the upstream script, but it's unlikely

apt-get install -y git gnupg2 ccache && \
ADD . /

RUN ./install-preq.sh && \
git clone --branch $GIT_REF --depth 1 $GIT_URL ceph && \
cd ceph && \
./install-deps.sh && \
sh -c 'if [ -n "$EXTRA_PKGS" ]; then apt-get install -y "$EXTRA_PKGS"; fi' && \
Copy link
Collaborator

Choose a reason for hiding this comment

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

for centos, apt-get cannot run. So we can remove this line

Copy link
Author

Choose a reason for hiding this comment

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

oh yeah. missed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants