-
Notifications
You must be signed in to change notification settings - Fork 19
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 R-builds deploy job #171
base: main
Are you sure you want to change the base?
Conversation
@jspiewak pointed out that we explicitly set Line 11 in c97f0cc
That explains the setting then, and how it was able to work before. The HOME dir is used for the cache, which exists on the host because it's the workspace dir that gets mounted in. Changing it to I'll still propose running the whole deployment in a single container though, to keep things simpler and easier to debug locally. |
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 can't really check if the changes are OK, just saying thanks for fixing the deployment. :)
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.
If setting HOME
to env.WORKSPACE
actually fixes the issue, is it worth separating that fix from the rest and getting it landed?
Otherwise, @stevenolen or @jforest will need to chime in with some historical context on the docker-in-docker situation.
Makefile
Outdated
# The cache directory must either be an absolute path (optionally specified | ||
# via cacheLocation: /path/to/cache), or caching must be disabled for the plugin | ||
# to work. We disable caching completely to avoid all sorts of future issues. | ||
# The cache directory is based on $HOME, which is set to "." when running |
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.
No longer the case
|
||
# Package the service only, for debugging. | ||
# Requires deps and fetch-serverless-custom-file to be run first. | ||
serverless-package: |
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.
Why not declare deps
and fetch-serverless-custom-file
as target dependencies then?
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.
This was to iterate on the packaging (without deploying) step to debug failures with that. I was testing various serverless-python-requirements settings in serverless-custom.yml and didn't want to overwrite it.
@jspiewak Good idea, let's go with that first to get the deploy unblocked asap. I'll follow up with a new PR soon and add some docs. The main problem was just how hard the deploy job was to understand and debug, and I would be fine with the original docker-inside-docker approach as long as it was documented well enough. I'd still prefer reducing it to just one container, but am curious what @stevenolen and @jforest think. |
* Don't Dockerize pip and stop mounting Docker socket in Jenkins. Mounting the socket breaks docker run commands that attempt to mount a directory in the container - the mount uses the host rather than the container. The serverless-python-requirements plugin will do this and fail to locate files. No idea how this was working in Jenkins before, however! * Switch to jammy Docker image with custom-installed dependencies. The language-specific images and lambda images are too tied to distros that make dependencies hard to install or maintain. e.g., the nodesource install script is broken on Debian 11 right now, and AL2 no longer supports newer versions of Node 18+. * Disable caching in the serverless-python-requirements plugin. For some reason, the cache directory in Jenkins changed to a relative .cache directory at some point, based on /home/greg, set to . in Jenkins but an absolute path like /root typically. The plugin assumes that this cache directory is absolute and breaks otherwise with a relative path. To prevent future issues, we just disable caching completely rather than explicitly setting cacheLocation to an absolute path.
…mments * Simplify the deployment as much as possible * HOME can be removed, but needs the jenkins user restored to have a valid HOME dir that's not /, which breaks npm.
3264aa3
to
fbcdbb2
Compare
I updated the docs and tried to simplify the job a little more:
#175 seems to be working fine for now though, but I'll still leave this up for a while. |
Fixes #165. Tested this in staging at https://build.posit.it/blue/organizations/jenkins/r-builds%2Fdeploy-r-builds/detail/deploy-r-builds/195/pipeline, and then confirmed that a staging rebuild works. The CI failure is for an unrelated reason.
If this works well, someone will need to update serverless-custom.yml since I don't think I have permissions for that.
We were using the serverless-python-requirements plugin with
dockerizePip: true
, which installs the Python requirements in a separate docker container. The plugin has to mount some directories (requirements.txt, cache files) into the separate container, and this failed in two ways.The first issue is that the mounted directory was ending up as a relative path starting with a period, which docker doesn't support. With
serverless --verbose
, you get the actual error message:This cache dir comes from the plugin's use of the
appdirectory
npm package, which generates the cache paths based off$HOME
: https://github.com/MrJohz/appdirectory/blob/27f19a6eceb46110cd5d6882a18cae3a4da98331/lib/appdirectory.js#L91-L94When running the plugin locally in Docker, it works fine because
$HOME
resolves to/root
or/home/<user>
. However, in Jenkins, the docker image is run withHOME
set to.
:This may have been a Jenkins behavior change in some upgrade, which could explain why the job suddenly started failing.We were setttingHOME=.
, see #171 (comment)The second issue is that we were mounting the docker socket in the agent container. When you do this, any
docker run -v
mounts from within the container will use paths from the host rather than the container. So even with the cache directory fixed, the plugin was trying to mount directories in the python container that didn't exist, causing a new issue. I'm not sure how the Jenkins job was working before.To fix both of these, I temporarily patched serverless-custom.yml to not dockerize pip
and disable caching:Switched the deploy image to a base OS image rather than a python/node or lambda image, because the language specific images felt hard to maintain. The nodesource install script didn't work on the python/node images' Debian 11, and the lambda images use AL2 which is super old and no longer supports newer versions of Node.