Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Build che-theia plugins from che-theia-dev builder container rather than getting it from released travis built #43

Merged
merged 3 commits into from
Mar 1, 2019
Merged

Build che-theia plugins from che-theia-dev builder container rather than getting it from released travis built #43

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2278419
Build plugins from che-theia-dev builder image rather than getting re…
sunix Feb 27, 2019
dfcf2df
fixup! Build plugins from che-theia-dev builder image rather than get…
sunix Feb 28, 2019
86e93ab
fixup! fixup! Build plugins from che-theia-dev builder image rather t…
sunix Feb 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 42 additions & 4 deletions dockerfiles/theia/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,46 @@ RUN che:theia production
# change permissions
RUN find production -exec sh -c "chgrp 0 {}; chmod g+rwX {}" \; 2>log.txt


Copy link
Contributor

Choose a reason for hiding this comment

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

hello, didn't see it before.

why do we have copy of lines 89--->115 from the previous container ?
couldn't we just add # Clone and build che-theia plugins at the end of previous container ?

here we're duplicating all github token, etc logic in the same file for almost nothing ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe i misunderstood the comments of #43 (comment)
we are duplicating just the github token stuff. I am very fine with cloning and building che-theia plugins at the end of the previous container

###
# Plugin Builder Image
#
FROM ${BUILD_ORGANIZATION}/${BUILD_PREFIX}-theia-dev:${BUILD_TAG} as plugins-builder
WORKDIR ${HOME}

# define in env variable GITHUB_TOKEN only if it is defined
# else check if github rate limit is enough, else will abort requiring to set GITHUB_TOKEN value
ARG GITHUB_TOKEN

# Define che-theia branch to use for plugins
ARG CHE_THEIA_PLUGIN_BRANCH=master

# Check github limit
RUN if [ ! -z "${GITHUB_TOKEN-}" ]; then \
export GITHUB_TOKEN=$GITHUB_TOKEN; \
echo "Setting GITHUB_TOKEN value as provided"; \
else \
export GITHUB_LIMIT=$(curl -s 'https://api.github.com/rate_limit' | jq '.rate .remaining'); \
echo "Current API rate limit https://api.github.com is ${GITHUB_LIMIT}"; \
if [ "${GITHUB_LIMIT}" -lt 10 ]; then \
printf "\033[0;31m\n\n\nRate limit on https://api.github.com is reached so in order to build this image, "; \
printf "the build argument GITHUB_TOKEN needs to be provided so build will not fail.\n\n\n\033[0m"; \
exit 1; \
else \
echo "GITHUB_TOKEN variable is not set but https://api.github.com rate limit has enough slots"; \
fi \
fi

# Clone and build che-theia plugins
Copy link
Contributor

Choose a reason for hiding this comment

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

I would make it part of the theia-generator (the clone will be done in init step)

and then the compilation would occurs as another step of theia-generator like 'build-plugins' or something else

Then it will use same flag that @evidolob is adding for dev mode (master branch) vs tagged version, etc

RUN git clone --branch ${CHE_THEIA_PLUGIN_BRANCH} --single-branch --depth 1 https://github.com/eclipse/che-theia ${HOME}/che-theia-source-code
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not add these steps after the changes of permission

besides I'm in favor of adding it to che-theia-generator, I think that the plug-ins should not be compiled in the same image
It should be part of another stage. So if you modify theia but not plug-ins we may benefit of docker cache ?

Copy link
Contributor

Choose a reason for hiding this comment

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

it looks odd to clone CHE_THEIA while we're inside che-theia there, no ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, on another hand this is what we do with the generator


WORKDIR ${HOME}/che-theia-source-code/plugins/
RUN yarn

RUN mkdir -p ${HOME}/che-theia-plugins/ && \
find ${HOME}/che-theia-source-code/plugins/ -name "*.theia" -exec sh -c "chgrp 0 {}; chmod g+rwX {}; cp {} ${HOME}/che-theia-plugins/" \; 2>log.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

for now it should contain a regexp to only select a subset of plug-ins

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure also why we do chgrp/chmod there

Copy link
Contributor Author

@sunix sunix Feb 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

for now it should contain a regexp to only select a subset of plug-ins

ok which ones do we keep ? @l0rd @evidolob @benoitf

  • containers-plugin
  • factory-plugin
  • ports-plugin
  • ssh-plugin
  • welcome-plugin

I'm not sure also why we do chgrp/chmod there

not sure I thought we had a good reason as we do it for theia production package

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, we do the chgrp/chmod for this folder later in production container so we don't need to do it in the build container

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok i see :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@benoitf added the right filter and removed the permission change commands



###
# Runtime Image
#
Expand All @@ -96,6 +136,8 @@ ENV USE_LOCAL_GIT=true \

EXPOSE 3100 3130

COPY --from=plugins-builder /home/theia-dev/che-theia-plugins /default-theia-plugins
Copy link
Contributor

Choose a reason for hiding this comment

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

--chown=theia:root ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

permissions are updated afterwards for /default-theia-plugins
and group or user doesn't exist yet.


# Install sudo
# Install git
# Install bzip2 to unpack files
Expand All @@ -108,12 +150,8 @@ RUN adduser --disabled-password -S -u 1001 -G root -h ${HOME} -s /bin/sh theia \
&& mkdir /projects \
# Create root node_modules in order to not use node_modules in each project folder
&& mkdir /node_modules \
# Create internal plugins folder
&& mkdir /default-theia-plugins \
# Download yeoman generator plug-in
&& curl -L -o /default-theia-plugins/theia_yeoman_plugin.theia https://github.com/eclipse/theia-yeoman-plugin/releases/download/untagged-04f28ee329e479cc465b/theia_yeoman_plugin.theia \
&& curl -L -o /default-theia-plugins/eclipse_che_theia_factory_plugin.theia https://github.com/eclipse/che-theia/releases/download/0.0.1/eclipse_che_theia_factory_plugin.theia \
&& curl -L -o /default-theia-plugins/eclipse_che_theia_containers_plugin.theia https://github.com/eclipse/che-theia/releases/download/0.0.2/eclipse_che_theia_containers_plugin.theia \
&& for f in "${HOME}" "/etc/passwd" "/etc/group /node_modules /default-theia-plugins /projects"; do\
sudo chgrp -R 0 ${f} && \
sudo chmod -R g+rwX ${f}; \
Expand Down