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

Fixes Xenial docker build #5477

Merged
merged 4 commits into from
Sep 3, 2020
Merged

Fixes Xenial docker build #5477

merged 4 commits into from
Sep 3, 2020

Conversation

conorsch
Copy link
Contributor

@conorsch conorsch commented Sep 2, 2020

Status

Ready for review

Description of Changes

Fixes #5476. Refs #5471

Changes proposed in this pull request:

We need a recent version of dh-virtualenv. As for Focal, for Xenial we now pull from the Debian [sic] unstable [sic] repos to get a recent version. In Focal, we could reuse the already-available debian-archive-keyring to verify integrity, but in Xenial, the debian-archive-keyring is too old, Jessie-era (2014-11 vs 2018-09, judging by the timestamps). So for Xenial, we must fetch the required key over HTTPS and provide that to apt manually.

We must also explicitly reference the "unstable" target for dh-virtualenv, since an older version of the package is available in
Xenial, and will be preferrred to the unstable version given the cautious apt preferences we configure.

Testing

  • make dev results in a working container
  • cd molecule/builder-xenial && make builds successfully
  • run an interactive shell inside the latest xenial build container and confirm that apt-cache policy dh-virtualenv shows 1.2.1-1 (not 0.11-1 from Xenial)
  • make debs builds xenial packages without error

Deployment

Affects packaging build logic, will be used for upcoming release. No implications for admin intervention, but we must be confident in the build logic prior to entering QA period.

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make test) pass in the development container

If you made changes to securedrop-admin:

  • Linting and tests (make -C admin test) pass in the admin development container

If you made changes to the system configuration:

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

If you made changes to documentation:

  • Doc linting (make docs-lint) passed locally

If you added or updated a code dependency:

Choose one of the following:

  • I have performed a diff review and pasted the contents to the packaging wiki
  • I would like someone else to do the diff review

We need a recent version of dh-virtualenv. As for Focal, for Xenial we
now pull from the Debian [sic] unstable [sic] repos to get a recent
version. In Focal, we could reuse the already-available
debian-archive-keyring to verify integrity, but in Xenial, the
debian-archive-keyring is too old, Jessie-era (2014-11 vs 2018-09,
judging by the timestamps). So for Xenial, we must fetch the required
key over HTTPS and provide that to apt manually.

We must also explicitly reference the "unstable" target for
dh-virtualenv, since an older version of the package is available in
Xenial, and will be preferrred to the unstable version given the
cautious apt preferences we configure.
@conorsch conorsch requested review from kushaldas and rmol September 2, 2020 17:54
@conorsch conorsch requested a review from emkll as a code owner September 2, 2020 17:54
@conorsch
Copy link
Contributor Author

conorsch commented Sep 2, 2020

CI is failing on the make-debs command, which I ommitted from the test plan. Will update, marking WIP until CI is passing. @rmol if you've already started review, I welcome comments, but might be best to hold off until we confirm the packages are building correctly.

@rmol
Copy link
Contributor

rmol commented Sep 2, 2020

Was just about to comment. I've been seeing that same error locally:

invalid command 'bdist_wheel'
Install SecureDrop Python requirements in container...
      xenial-sd-app failed | msg: non-zero return code | stdout: Collecting alembic==0.9.9
      Downloading alembic-0.9.9.tar.gz (1.0 MB)
    Collecting argon2-cffi==20.1.0
      Downloading argon2-cffi-20.1.0.tar.gz (1.8 MB)
      Installing build dependencies: started
      Installing build dependencies: finished with status 'done'
      Getting requirements to build wheel: started
      Getting requirements to build wheel: finished with status 'done'
      Installing backend dependencies: started
      Installing backend dependencies: finished with status 'done'
        Preparing wheel metadata: started
        Preparing wheel metadata: finished with status 'error' | stderr:     ERROR: Command errored out with exit status 1:
         command: /usr/bin/python3 /tmp/tmprp5x3t24 prepare_metadata_for_build_wheel /tmp/tmpzcqyz2fk
             cwd: /tmp/pip-install-3r_amccn/argon2-cffi
        Complete output (20 lines):
        running dist_info
        creating /tmp/pip-modern-metadata-1h4jng6z/argon2_cffi.egg-info
        writing /tmp/pip-modern-metadata-1h4jng6z/argon2_cffi.egg-info/PKG-INFO
        writing dependency_links to /tmp/pip-modern-metadata-1h4jng6z/argon2_cffi.egg-info/dependency_links.txt
        writing requirements to /tmp/pip-modern-metadata-1h4jng6z/argon2_cffi.egg-info/requires.txt
        writing top-level names to /tmp/pip-modern-metadata-1h4jng6z/argon2_cffi.egg-info/top_level.txt
        writing manifest file '/tmp/pip-modern-metadata-1h4jng6z/argon2_cffi.egg-info/SOURCES.txt'
        reading manifest file '/tmp/pip-modern-metadata-1h4jng6z/argon2_cffi.egg-info/SOURCES.txt'
        reading manifest template 'MANIFEST.in'
        warning: no files found matching '*.txt'
        warning: no files found matching '.coveragerc'
        warning: no previously-included files found matching 'src/argon2/_ffi.py'
        warning: no previously-included files found matching '.gitmodules'
        warning: no previously-included files found matching 'extras/libargon2/.git'
        warning: no previously-included files found matching '*.yml'
        warning: no previously-included files matching '*.pyc' found under directory 'tests'
        no previously-included directories found matching 'docs/_build'
        writing manifest file '/tmp/pip-modern-metadata-1h4jng6z/argon2_cffi.egg-info/SOURCES.txt'
        creating '/tmp/pip-modern-metadata-1h4jng6z/argon2_cffi.dist-info'
        error: invalid command 'bdist_wheel'
        ----------------------------------------
    ERROR: Command errored out with exit status 1: /usr/bin/python3 /tmp/tmprp5x3t24 prepare_metadata_for_build_wheel /tmp/tmpzcqyz2fk Check the logs for full command output.

Which suggests we need python3-wheel or python-pip-whl installed, maybe? But while looking into this I found the new build image has Python 3.8 installed? I was still investigating that when I saw your comment. Ideas?

@rmol
Copy link
Contributor

rmol commented Sep 2, 2020

The installation of dh-virtualenv from Debian unstable was pulling in python3-virtualenv and newer Python 3 bits. Including virtualenv in the big package list installed earlier prevents that and solves the problem above.

@conorsch
Copy link
Contributor Author

conorsch commented Sep 2, 2020

Thanks, @rmol. I'll pull down the changes and confirm.

Conor Schaefer added 2 commits September 2, 2020 16:16
Fixes to the build logic were just added by @rmol, so I've rebuilt the
image and pushed it to facilitate testing by others.
We now test explicitly for expected versions of:

  * python3
  * dh-virtualenv

Since we're pulling from non-Ubuntu repositories. We'll reuse
the same test logic in the march toward Focal build support, as well.
@conorsch
Copy link
Contributor Author

conorsch commented Sep 3, 2020

Thanks for the collab, @rmol. CI is now passing. Take a gander at the most recent commits, which try to flesh out the tests a bit to make the py3 & dh-virtualenv required versions more obvious. Will port those over to Focal when reviewing #5465.

@conorsch conorsch mentioned this pull request Sep 3, 2020
9 tasks
Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

The diff looks good, tested with the following steps:

  • make dev results in a working container
  • cd molecule/builder-xenial && make builds successfully
  • run an interactive shell inside the latest xenial build container and confirm that apt-cache policy dh-virtualenv shows 1.2.1-1 (not 0.11-1 from Xenial)
  • make build-debs builds xenial packages without error

@kushaldas kushaldas merged commit 3990426 into develop Sep 3, 2020
conorsch pushed a commit that referenced this pull request Sep 3, 2020
The test suite for validating deb packages was duplicated between Xenial
& Focal. That's going to run a risk of tests only being updated in a
single location, as was done in #5477. We have the ability to customize
behavior of the tests via env vars, so let's keep the same test suite
for now.
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.

making new docker builder image is broken in xenial
3 participants