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

Introduce runtime python venv for dependencies, add Makefile #332

Closed
wants to merge 1 commit into from

Conversation

tserong
Copy link
Member

@tserong tserong commented Mar 25, 2021

This is work towards being able to have an actual package (rpm, deb, ...) of Aquarium (see #91).

I've added a Makefile with a dist target, so you can now run make dist to get a tarball named something like aquarium-0.1.0-0-gc28eb76.tar.gz, which includes a built glass and all the necessary bits of gravel. It also includes all our python dependencies (per src/requirements.txt) in /usr/share/aquarium/venv, so we don't have to worry about not having packaged versions of these things on whatever distros we deploy on.

This doesn't change the current dev workflow at all. You still build an image by running ./tools/build-image.sh, but that script will in turn call make dist to get a tarball to install in the image. In future (once I've got a spec file and rpm build sorted out), this can be changed so that the kiwi config uses that actual package, instead of the tarball.

That said, if you don't want to build an image, but would rather try a manual Aquarium install on some random Linux, you could run make dist, then take the tarball and extract it on the system you want to deploy on. Might be a bit of fun ;-)

Note: the version string used in the tarball name is based on the latest annotated tag in the form 'vX.Y.Z', which means I've suddenly introduced the notion of actual version numbers and releases to the project. Sorry.

Fixes: #277
Signed-off-by: Tim Serong tserong@suse.com

@tserong tserong added WIP Work in progress - Do not merge feature New feature tooling Related with tools supporting development or deployment images Related to microOS, vagrant, etc, images labels Mar 25, 2021
@tserong tserong requested a review from jecluis March 25, 2021 07:28
@github-actions github-actions bot added the glass Related to the Aquarium Frontend label Mar 25, 2021
@tserong
Copy link
Member Author

tserong commented Mar 25, 2021

I've marked this WIP because I don't know if the CI is going to like this or not, because I've introduced a requirement on having a version tag.

@tserong
Copy link
Member Author

tserong commented Mar 25, 2021

Ah, dammit, I accidentally included src/glass/package-lock.json as well. That's a mistake. Will fix.

@tserong
Copy link
Member Author

tserong commented Mar 25, 2021

Yep, this won't do the right thing in CI without a tag (well, it might work, but the tarball will have a weird versionless name). Hang on...

@jecluis
Copy link
Member

jecluis commented Mar 25, 2021

I'm perfectly fine with adding a version tag to the repo, at the current HEAD, for whatever. This is the first time we've needed a version, so might as well make it a major milestone and give in ;)

(to be clear, it should be a minor version, like 0.0.1 or whatever)

This is work towards being able to have an actual package (rpm, deb, ...)
of Aquarium (see aquarist-labs#91).

I've added a Makefile with a `dist` target, so you can now run `make dist`
to get a tarball named something like aquarium-0.1.0-0-gc28eb76.tar.gz,
which includes a built glass and all the necessary bits of gravel.  It
also includes all our python dependencies (per src/requirements.txt) in
/usr/share/aquarium/venv, so we don't have to worry about not having
packaged versions of these things on whatever distros we deploy on.

This doesn't change the current dev workflow at all.  You still build an
image by running `./tools/build-image.sh`, but that script will in turn
call `make dist` to get a tarball to install in the image.  In future
(once I've got a spec file and rpm build sorted out), this can be changed
so that the kiwi config uses that actual package, instead of the tarball.

That said, if you *don't* want to build an image, but would rather try
a manual Aquarium install on some random Linux, you could run `make dist`,
then take the tarball and extract it on the system you want to deploy
on.  Might be a bit of fun ;-)

Note: the version string used in the tarball name is based on the latest
annotated tag in the form 'vX.Y.Z', which means I've suddenly introduced
the notion of actual version numbers and releases to the project.  Sorry.
Until such time as an actual release is tagged, the version will be set
to v0.0.0.

Fixes: aquarist-labs#277
Signed-off-by: Tim Serong <tserong@suse.com>
@tserong
Copy link
Member Author

tserong commented Mar 25, 2021

(to be clear, it should be a minor version, like 0.0.1 or whatever)

Yeah, totally :-) https://semver.org/ suggests v0.1.0 for that case. But this PR is necessarily a bit chicken-and-eggy, as it (ideally) wants a release tag, except there isn't one yet. So I've made it fall back to a hardcoded v0.0.0 if git describe fails for whatever reason.

@tserong tserong removed the WIP Work in progress - Do not merge label Mar 25, 2021
@tserong
Copy link
Member Author

tserong commented Mar 25, 2021

OK, almost there, just need to figure out why setup-vagrant-tumbleweed succeeded but setup-vagrant-leap failed...

@jecluis
Copy link
Member

jecluis commented Mar 25, 2021

(to be clear, it should be a minor version, like 0.0.1 or whatever)

Yeah, totally :-) https://semver.org/ suggests v0.1.0 for that case. But this PR is necessarily a bit chicken-and-eggy, as it (ideally) wants a release tag, except there isn't one yet. So I've made it fall back to a hardcoded v0.0.0 if git describe fails for whatever reason.

Yeah, 0.1.0 works, whatever ;) Meant as an example.

Copy link
Member

@jecluis jecluis left a comment

Choose a reason for hiding this comment

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

LGTM minus a worry on a for during path replacement.

Will test before giving the a-okay.

$(TAR_USR)/venv/bin/pip install -r src/requirements.txt
# At this point, the shebang lines for everyting in venv/bin will be
# absolute paths on the build host (e.g. /tmp/tmp.QoQm2t2j06/aquarium-0.1.0-0-gc28eb76/usr/share/aquarium/venv/bin/python3)
# but we need them to be appropriate for the host we're *installing* on,
Copy link
Member

Choose a reason for hiding this comment

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

When building the image, this is something that can be achieved by the disk.sh script, which is run inside the chroot'ed environment just before the image is built. So, technically, one could package the requirements.txt file in the tar, drop it somewhere we could reach it within the chroot, create the venv and install the packages, and then remove the requirements.txt file.

Otoh, that would not work for the bare metal host 🤷

fixfiles="$$fixfiles bin/activate*" ; \
for f in $$fixfiles; do \
echo -n "fixing path in $$f: " ; \
grep $(TAR_BASE) "$$f" ; \
Copy link
Member

Choose a reason for hiding this comment

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

meant to continue here? what am I missing?

Copy link
Member Author

Choose a reason for hiding this comment

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

The grep just results in printing out the path it found that it's changing... Maybe it's not necessary? This whole "fixfiles" bit of evil^H^H^H cleverness came from Calamari :-)

@tserong
Copy link
Member Author

tserong commented Mar 25, 2021

From the logs (https://ceph-ci.suse.de/job/aquarium-setup-vagrant-leap/74/consoleFull), I don't think I can tell why it's failing; all I can see is:

+ cd aquarium/tests/vagrant/deployments/foo
+ vagrant ssh -c "sudo systemctl status aquarium"
>>> ��� aquarium.service - Project Aquarium
>>>      Loaded: loaded (/usr/lib/systemd/system/aquarium.service; enabled; vendor preset: disabled)
>>>      Active: failed (Result: exit-code) since Thu 2021-03-25 08:05:31 UTC; 1min 22s ago
>>>    Main PID: 1393 (code=exited, status=1/FAILURE)
>>> 
>>> Mar 25 08:05:31 localhost systemd[1]: aquarium.service: Scheduled restart job, restart counter is at 5.
>>> Mar 25 08:05:31 localhost systemd[1]: Stopped Project Aquarium.
>>> Mar 25 08:05:31 localhost systemd[1]: aquarium.service: Start request repeated too quickly.
>>> Mar 25 08:05:31 localhost systemd[1]: aquarium.service: Failed with result 'exit-code'.
>>> Mar 25 08:05:31 localhost systemd[1]: Failed to start Project Aquarium.

@kshtsk I guess I need to somehow add journalctl -u aquarium or similar if systemctl status aquarium fails at this point, so we can see the full log of what went wrong?

@kshtsk
Copy link
Contributor

kshtsk commented Mar 25, 2021

@tserong you can probably add -n 100 to systemctl status?

<package name="python3-rados"/>
<package name="python3-websockets"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why those dependencies are dropped?

Copy link
Member

Choose a reason for hiding this comment

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

because we're now relying on a virtualenv to run stuff from. These dependencies are being installed in the virtualenv.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a wrong approach, if the venv is prepared on the host. The host system is not necessary the same as the guest one.

Copy link
Member

Choose a reason for hiding this comment

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

Can you be more specific?

Copy link
Contributor

Choose a reason for hiding this comment

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

but I guess it is done in a right way

Copy link
Contributor

Choose a reason for hiding this comment

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

Same true for guys who are trying this out on Ubuntu or RH hosts.

Copy link
Member Author

Choose a reason for hiding this comment

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

They're embedded in the tarball now inside a python venv

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to embed dependencies into tarball? Why not to use pip to install?

Copy link
Member Author

@tserong tserong Mar 26, 2021

Choose a reason for hiding this comment

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

I want to ultimately make a package of aquarium (rpm, deb, etc.), so i'm making a venv inside the tarball and using pip to install in that venv when the tarball is built. The implication of this, which I apparently hadn't realised until yesterday when the leap CI failed, is that the host you build the tarball on, basically has to be the same OS the tarball is deployed on. That will (or should) eventually work for the case of providing an aquarium package for any distro, but right now breaks when using any non-tumbleweed distro to build the MicroOS image.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tserong maybe use kiwi for that?

@kshtsk
Copy link
Contributor

kshtsk commented Mar 25, 2021

Mar 25 12:38:02 localhost uvicorn[1352]:   File "/usr/share/aquarium/venv/bin/uvicorn", line 5, in <module>
Mar 25 12:38:02 localhost uvicorn[1352]:     from uvicorn.main import main
Mar 25 12:38:02 localhost uvicorn[1352]: ModuleNotFoundError: No module named 'uvicorn'

this is what is seen in journalctl

@kshtsk
Copy link
Contributor

kshtsk commented Mar 25, 2021

Traceback (most recent call last):
  File "/usr/share/aquarium/venv/bin/uvicorn", line 5, in <module>
    from uvicorn.main import main
ModuleNotFoundError: No module named 'uvicorn'
vagrant@node01:~> . /usr/share/aquarium/venv/bin/activate
(venv) vagrant@node01:~> pip list
Traceback (most recent call last):
  File "/usr/share/aquarium/venv/bin/pip", line 5, in <module>
    from pip._internal.cli.main import main
ModuleNotFoundError: No module named 'pip'

@kshtsk
Copy link
Contributor

kshtsk commented Mar 25, 2021

Python 3.8.8
vagrant@node01:~> ls /usr/share/aquarium/venv/lib64
python3.6

System and venv python version mismatch.

@kshtsk
Copy link
Contributor

kshtsk commented Mar 25, 2021

@kshtsk I guess I need to somehow add journalctl -u aquarium or similar if systemctl status aquarium fails at this point, so we can see the full log of what went wrong?

that makes sense to me, like:

sudo systemctl status aquarium || {
   RET=$?
   sudo journalctl -u aquarium
   exit $RET
}

@tserong
Copy link
Member Author

tserong commented Mar 26, 2021

System and venv python version mismatch.

Ah, damn, I may need to rethink this approach. The venv in the tarball is created on the build host, using whatever version of python is installed there, so in the Leap case, python 3.6. But then the tarball is installed inside the Tumbleweed-based MicroOS which uses python 3.8...

@kshtsk
Copy link
Contributor

kshtsk commented Mar 26, 2021

System and venv python version mismatch.

Ah, damn, I may need to rethink this approach. The venv in the tarball is created on the build host, using whatever version of python is installed there, so in the Leap case, python 3.6. But then the tarball is installed inside the Tumbleweed-based MicroOS which uses python 3.8...

Maybe use dockerized build? Or run your build script in the vagrant instance? Keep in mind though major version may match it also matching minor version and even system dependencies.

@tserong tserong added the WIP Work in progress - Do not merge label Mar 26, 2021
@tserong
Copy link
Member Author

tserong commented Mar 26, 2021

Marking WIP again, because I obviously have some more work to do here.

@jecluis
Copy link
Member

jecluis commented Apr 2, 2021

@tserong do you thing we could split off the bundling of Aquarium's stuff for make dist without the pip stuff for now, and then deal with pip later? TBH, I find that quite useful as is and the more I think about how that could help us get rid of the build-image.sh script, the more I want it.

@kshtsk
Copy link
Contributor

kshtsk commented Apr 2, 2021

Guys, I still don't get why do you want include all the dependencies in the tar ball, maybe it better to allow more natural way for python to deploy, like using pip so it will install all dependencies from the pypi? Otherwise we need to build rpms for each system? Alternatively, the package can be probably built via docker for the guest system.

@jecluis
Copy link
Member

jecluis commented Apr 2, 2021

Guys, I still don't get why do you want include all the dependencies in the tar ball, maybe it better to allow more natural way for python to deploy, like using pip so it will install all dependencies from the pypi? Otherwise we need to build rpms for each system? Alternatively, the package can be probably built via docker for the guest system.

Tim is still looking into alternatives and the best path forward. What I'm suggesting is having the makefile to do the bundling of aquarium (read, gravel, glass, cephadm, etc), without dependencies, before we figure the rest out.

@tserong
Copy link
Member Author

tserong commented Apr 8, 2021

@tserong do you thing we could split off the bundling of Aquarium's stuff for make dist without the pip stuff for now, and then deal with pip later? TBH, I find that quite useful as is and the more I think about how that could help us get rid of the build-image.sh script, the more I want it.

Alright, I've done that in #382 (I didn't rip the pip stuff out of this one as I'd still rather keep it around for posterity just in case).

Guys, I still don't get why do you want include all the dependencies in the tar ball, maybe it better to allow more natural way for python to deploy, like using pip so it will install all dependencies from the pypi?

My goal was to try to create a native distro package of aquarium (rpm, deb, etc.) that could be installed on openSUSE, Fedora, Debian, Ubuntu. That means that ordinarily we'd need all our python dependencies packaged for all those distros too. Some of them are packaged, and some are not, and I have zero desire to take on package maintenance for a bunch of python packages on multiple Linux distros :-) If we use pip at build time, to embed those dependencies inside the tarball, that dependency problem goes away. Does that help explain why I was experimenting with this approach?

@kshtsk
Copy link
Contributor

kshtsk commented Apr 8, 2021

My goal was to try to create a native distro package of aquarium (rpm, deb, etc.) that could be installed on openSUSE, Fedora, Debian, Ubuntu. That means that ordinarily we'd need all our python dependencies packaged for all those distros too. Some of them are packaged, and some are not, and I have zero desire to take on package maintenance for a bunch of python packages on multiple Linux distros :-)

Yeah, understandable.

If we use pip at build time, to embed those dependencies inside the tarball, that dependency problem goes away. Does that help explain why I was experimenting with this approach?

As you can see it does not go away but makes it even worth. This is why the packaging thing at build time should be run probably in guest container with the destination os instead of the bare host.

@tserong
Copy link
Member Author

tserong commented Apr 9, 2021

As you can see it does not go away but makes it even worth.

Indeed :-/

This is why the packaging thing at build time should be run probably in guest container with the destination os instead of the bare host.

Yeah, that sounds like it'd be more reliable than my first attempt.

@github-actions
Copy link

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@asettle
Copy link
Contributor

asettle commented Jun 30, 2021

Any update on this PR @tserong ?

@tserong
Copy link
Member Author

tserong commented Jul 7, 2021

Any update on this PR @tserong ?

This has been on the back burner for >checks notes< way too long, so I should probably just close it.

@tserong tserong closed this Jul 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New feature glass Related to the Aquarium Frontend images Related to microOS, vagrant, etc, images needs-rebase tooling Related with tools supporting development or deployment WIP Work in progress - Do not merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a Makefile
4 participants