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

Fix overlayfs mount location when running Tern in a container #500

Merged
merged 7 commits into from
Dec 3, 2019

Conversation

rnjudge
Copy link
Contributor

@rnjudge rnjudge commented Dec 3, 2019

The overlay mount inside of the docker container was failing as it
was trying to mount on top of another overlay mount point.

This series of commits fixes the working directory when running
in a container to be the bind mount target instead of an absolute path,
$HOME/.tern/temp.

Fixes #498

Signed-off-by: Rose Judge rjudge@vmware.com

Currently the --bind-mount option is only a binary true/false value.
This commit changes the --bind-mount option to hold a value that
defines an absolute path to a bind mount directory. The location
provided will be used to create a working directory inside of the
container and will also be used to overlay mount any potential
diff layers. The --bind-mount option is designed to be used when
running Tern inside of a container.

This commit also adds a call in do_main() to rootfs.set_mount_dir().
set_mount_dir() will set the bind_mount location as specified by the
user if running in a container. If the --bind_mount option is not
invoked, Tern will use /$HOME/.tern/temp as the default working
directory to perform operations. This call to set_mount_dir()
is crucial, as it is used by the get_working_dir() function (defined
in rootfs.py) many times throughout the code.

Works towards tern-tools#498

Signed-off-by: Rose Judge <rjudge@vmware.com>
A previous commit changed the --bind-mount CLI option to store a bind
mount directory value. This commit updates the Dockerfile to reflect
this change in the Dockerfile ENTRYPOINT command. This commit also
changes the name of the bind mount from 'temp' to 'hostmount.'

Lastly, this commit makes changes to the docker_run.sh script to udpate
the name of the bind mount directory from 'temp' to 'hostmount.'

Works towards tern-tools#498

Signed-off-by: Rose Judge <rjudge@vmware.com>
tern/report/report.py currently cleans up the working directory
differently if the --bind-mount CLI option is enabled. Previous commits
have changed the way the working directory is set, however, and this
commit makes changes to the clean_working_dir() function to reflect
that. Instead of checking to see if the bind_mount option is enabled,
clean the working directory provided by rootfs.get_working_dir()

Works towards tern-tools#498

Signed-off-by: Rose Judge <rjudge@vmware.com>
@rnjudge rnjudge requested a review from nishakm December 3, 2019 06:09
This commit makes a few changes:

1) Add a new function in tern/utils/rootfs.py called set_mount_dir()
   that sets the value of a global variable, mount_dir, to the
   value provided when the --bind_mount CLI option is used. If the
   --bind_mount CLI option is not enabled at runtime, assign the
   top_dir value ($HOME/.tern) to the mount_dir variable.

2) Use the mount_dir variable to set the absolute path of the working
   directory in get_working_dir().

3) Make changes to tern/utils/cache.py to change the location of the
   cache.yml file. If the --bind-mount option is used, the cache.yml
   file will be under the mount_dir directory location. If the
   --bind-mount CLI option is not enabled, the cache.yml file will
   be in the top_dir directory ($HOME/.tern).

Fixes tern-tools#498

Signed-off-by: Rose Judge <rjudge@vmware.com>
The clean_working_dir() function does not require any input
arguments. This commit removes the bind_mount argument from
the function definition and any clean_working_dir() invocations.

Signed-off-by: Rose Judge <rjudge@vmware.com>
@rnjudge
Copy link
Contributor Author

rnjudge commented Dec 3, 2019

@nishakm I believe the test running ./docker_run.sh workdir ternd "report -i golang:alpine will fail for this PR since it is using the Dockerfile that installs tern from PyPI to build ternd.

I recommend testing this PR by pulling the changes locally and using the following Dockerfile:

# Copyright (c) 2019 VMware, Inc. All Rights Reserved.
# SPDX-License-Identifier: BSD-2-Clause

FROM photon:3.0

# install system dependencies
# photon:3.0 comes with toybox which conflicts with some dependencies needed for tern to work, so uninstalling
# toybox first
RUN tdnf remove -y toybox && tdnf install -y tar findutils attr util-linux python3 python3-pip python3-setuptools git

# install pip and tern
RUN pip3 install --upgrade pip 

# May need to change this depending on contents of dist/
COPY dist/tern-1.0.1.dev12.tar.gz .
RUN pip install tern-1.0.1.dev12.tar.gz
#  make a mounting directory
RUN mkdir hostmount

ENTRYPOINT ["tern", "-b", "/hostmount"]
CMD ["-h"]

@nishakm
Copy link
Contributor

nishakm commented Dec 3, 2019

@nishakm I believe the test running ./docker_run.sh workdir ternd "report -i golang:alpine will fail for this PR since it is using the Dockerfile that installs tern from PyPI to build ternd.
@rnjudge would you be able to modify the test to use the Dockerfile you have made?

tern/utils/cache.py Outdated Show resolved Hide resolved
tern/utils/rootfs.py Outdated Show resolved Hide resolved
@nishakm
Copy link
Contributor

nishakm commented Dec 3, 2019

@rnjudge apart for the blank lines, the PR looks good. Would appreciate adding your Dockerfile to the test suite :)

This is a straightforward commit to remove two unnecessary blank lines
from tern/utils/cache.py and tern/utils/rootfs.py.

Signed-off-by: Rose Judge <rjudge@vmware.com>
@rnjudge rnjudge changed the title Fix overlayfs in docker Fix overlayfs mount location when running Tern in a container Dec 3, 2019
If changes are made to the Dockerfile in the tern directory, cicleci
will try to build Tern in a container using the Dockerfile. The issue
with this is that the Dockerfile installs Tern from PyPI instead of
testing Tern with the latest changes on master. This commit changes
the test suite in ci/test_files_touched.py to install the latest
version of tern on master in the test container instead of installing
Tern from PyPI.

Signed-off-by: Rose Judge <rjudge@vmware.com>
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.

Overlay mount fails when running Tern in a Docker container
2 participants