Update and cleanup docker files #31

minosgalanakis · 2022-01-14T13:51:58Z

This PR is implementing the following:

Changes

Align 16.04 to 18.04 Docker images
Remove 16.04 related workarounds when possible
Updated the ARMLMD_LICENSE_FILE files and removed workarounds in groovy scripts
Created a 20.04 Docker Image file which is fully aligned to 18.04
Added the --security-opt seccomp=unconfined to the docker run parameter
Switched to distribution provided versions for abi-dumper, abi-compliance-checker libasn1

Testing

Image are tested locally using the build and run scripts, and isolated builds

Test job mbedtls-release-ci-testing 347 (RUN_BASIC_BUILD_TEST @ 16.04) -> Passed
Test job mbedtls-release-ci-testing 347 (RUN_BASIC_BUILD_TEST @ 18.04) -> Passed
Test job mbedtls-release-ci-testing 347 (RUN_BASIC_BUILD_TEST @ 20.04) -> Passed
Test job mbedtls-release-ci-testing 348 (RUN_ALL_SH@ 16.04) -> Passed
[x ] Test job mbedtls-release-ci-testing 348 (RUN_ALL_SH@ 20.04) -> Passed
Testing RUN_ABI_TEST in mbed-tls-pr-test-parametrized 225
Test job mbedtls-release-ci-testing 352 (RUN_ALL_SH@ 20.04) -> Failed
Test job mbedtls-release-ci-testing 352 (RUN_ALL_SH@ 20.04->18.04->16.04) -> Failed

Todo

Investigate why the following tests are failing at 20.04

all_u20-check_doxygen
all_u20-test_everest
all_u20-test_se_default

minosgalanakis · 2022-01-18T10:28:50Z

Failure investigations

The following code was added to the get_release_jobs method in order to test including 20.04 images

`def gen_release_jobs(label_prefix='', run_examples=true) {
def jobs = [:]

common.get_branch_information()

if (env.RUN_BASIC_BUILD_TEST == "true") {
    jobs = jobs + gen_code_coverage_job('ubuntu-16.04');
}

if (env.RUN_ALL_SH == "true") {
    for (component in (common.available_all_sh_components['ubuntu-20.04'])) {
        jobs = jobs + gen_all_sh_jobs('ubuntu-20.04', component, label_prefix)
    }

    for (component in (common.available_all_sh_components['ubuntu-16.04'] -
                       common.available_all_sh_components['ubuntu-20.04'])) {
        jobs = jobs + gen_all_sh_jobs('ubuntu-16.04', component, label_prefix)
     }

    for (component in (common.available_all_sh_components['ubuntu-18.04'] -
                       common.available_all_sh_components['ubuntu-20.04'] -
                       common.available_all_sh_components['ubuntu-16.04'])) {
        jobs = jobs + gen_all_sh_jobs('ubuntu-18.04', component, label_prefix)
     }
}`

Job 352 only tested 20.4 and 353 tested previous docker platforms as well

The following items are currently failing with 20.04 image

all_u20-check_doxygen
all_u20-test_everest
all_u20-test_se_default

all_u20-check_doxygen

This is a known issue with newer versions of doxygen as reported in mbedtls issue #3497

all_u20-test_everest

The test appears to fail by a runtime error full_log

PSA cipher encrypt: invalid key type .............................. /var/lib/build/library/psa_crypto.c:3595:16: runtime error: applying zero offset to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /var/lib/build/library/psa_crypto.c:3595:16 in

all_u20-test_se_default

The test also fails because of a runtime error full_log

test_suite_pkparse ................................................ PASS
test_suite_pkwrite ................................................ PASS
test_suite_poly1305 ............................................... PASS
test_suite_psa_crypto ............................................. psa_crypto.c:3595:16: runtime error: applying zero offset to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior psa_crypto.c:3595:16 in 
FAIL
--------------------- Begin test_suite_psa_crypto ---------------------

This patch updates docker's run.sh with the aforementioned parameter which disables the ASLR for zeroize test. The readme.md file has been updated accordingly

This patch updates the ubuntu-16.04 & ubuntu-18.04 Dockerfiles to use the up-to-date license license information, and removes the workaround in gen_jobs.groovy script.

This patch installs the official distribution selected packages for `abi-dumper` -> (version>=1.1-1) and `abi-compliance-checker` -> (version>=2.3-0.2)

This patch updates GnuTLS 3.7 configure command to use the distribution’s `libtasn1` -> (version>=4.13). Comments have been amended accordingly

mpg

This was just a first pass, I'll have a look at the test results tomorrow.

mpg · 2022-01-25T12:18:36Z

resources/docker_files/ubuntu-18.04/Dockerfile

-# - libtasn1: can use the Ubuntu version, except for GnuTLS 3.7 which needs
-# libtasn1 4.9 (Ubuntu 16.04 has 4.6); an config option
-# --with-included-libtasn1 is available, so use it for GnuTLS 3.7.
+# - libtasn1: can use the Ubuntu version


Please leave a note about --with-included-libtasn1 as a reminder, if in the future we need a bleeding-edge version of GnuTLS for which the Ubuntu version is not enough (like the comment on libunistring below).

mpg · 2022-01-25T12:29:21Z

resources/docker_files/ubuntu-18.04/Dockerfile

+    && rm -rf /var/lib/apt/lists/
+
+# Install abi-compliance-checker & abi-dumper
+RUN apt-get update -q && apt-get install -yq abi-compliance-checker abi-dumper


I think it would be better to add that to the big invocation of apt-get install that starts on line 39:

minimizing the number of times we call apt-get update reduces the time it takes to build the image

I think have everything in one place in a sorted list makes it easier to maintain

More importantly: the 16.04 says we want at least version 2.3 of abi-compliance-checker. Unless I'm mistaken, Ubuntu 18.04 only has 2.2. So, shouldn't we still compile 2.3?

(OTOH, abi-dumper seems OK as the Ubuntu repos have 1.1.)

mpg · 2022-01-25T12:31:09Z

resources/docker_files/ubuntu-20.04/Dockerfile

+ENV PATH=/opt/gcc-arm-none-eabi-5_4-2016q3/bin:$PATH
+
+# Install abi-compliance-checker
+RUN apt-get update -q && apt-get install -yq abi-compliance-checker


Same note as for the 18.04 file: let's keep all things installed with apt in the same place as much as possible.

This patch updates ubuntu18-04 Dockerfiles to compile abi-compliance-checker 2.3 from source. Both files are slightly modified to remove unessary apt-get updates when using the package manager. It also adds a note to OpenSSL comments, to serve as reminder of the optional `--with-included-libtasn1` parameter.

gilles-peskine-arm

I reviewed the code changes. I haven't inspected CI results. Have you run the CI with the latest version? If not, please update the PR description once you do. You might wait until you've handled the review feedback.

resources/docker_files/ubuntu-18.04/Dockerfile

resources/docker_files/ubuntu-20.04/Dockerfile

resources/docker_files/run.sh

resources/docker_files/README.md

This patch introduces the following 20.04 changes: * Added a note to link an existing known issue to abi-tools * Removed duplicate installation of abi-tools * Removed unnecessary pip upgrade operation * Reworded comments

This patch removes the afforementioned python dependencies in ubuntu-16.04, ubuntu-18.04, ubuntu-20.04. Test jobs will install dependencies as required.

… default" This reverts commit 20dd9eb.

minosgalanakis · 2022-02-08T22:11:49Z

I am quoting @gilles-peskine-arm from an slack channel thread

resources/docker_files/run.sh /some/directory ubuntu-16.04 no longer works out of the box because that results in HOME set to / which is not writable so installing python modules with scripts/min_requirements.py fails. Can we arrange to set HOME to the workspace directory mounted on /var/lib/ws

A new requirement/request is accepted to add an ENV entry for HOME->/var/lib/ws in all three images. It will be added in a single commit.

This patch introduces the aforementioned ENV setting in ubuntu-16.04, ubuntu-18.04, ubuntu-20.04. This fixes the home directory to a known location where scripts can access, to install and configure additional dependencies.

gilles-peskine-arm

Looks good to me except for HOME set to the wrong directory (sorry).

Please run at least a pr-merge test job and a release test job with the final version of the images.

gilles-peskine-arm · 2022-02-09T22:32:44Z

resources/docker_files/ubuntu-16.04/Dockerfile


+# Note: scripts/min_requirements.py need a writable
+# destination for installing python dependencies
+ENV HOME=/var/lib/ws


/var/lib/ws doesn't exist by default, and it's a directory outside the Docker image. /var/lib/builds is the one that's created by default and owned by user. So I think HOME should be set to that instead. Sorry, I wasn't thinking properly when I suggested /var/lib/ws.

This patch changes the HOME location from /var/lib/ws to /var/lib/builds

gilles-peskine-arm

Looks good to me on code reading.

mpg

Looks good to me.

I noticed an outdated comment, but we might choose to ignore it for now in order to merge this PR quickly and unblock work that depends on it.

mpg · 2022-02-21T11:15:46Z

resources/docker_files/ubuntu-18.04/Dockerfile

 # fails). The subsequent use of "pip config" (which requires pip >=10)
 # will however fail if the installation of a more recent pip failed.
-RUN python3 -m pip install 'pip<21' --upgrade | cat && \
+RUN python3 -m pip install pip --upgrade | cat && \


Minor: the comment above is outdated, as it says " Install pip<21" which is no longer what we're doing. (Non-blocker as this is only a comment.)

gilles-peskine-arm · 2022-02-21T12:35:12Z

@minosgalanakis I notice you've run at least https://jenkins-mbedtls.oss.arm.com/job/mbedtls-release-ci-testing/372/ on the latest commit. Can you please make a list of the jobs you've run?

minosgalanakis · 2022-02-21T16:59:30Z

As requested by @gilles-peskine-arm I am attaching the summary of the testing:

Test job mbedtls-release-ci-testing 372 (RUN_BASIC_BUILD_TEST @ 16.04) -> Passed
Test job mbedtls-release-ci-testing 372 (RUN_BASIC_BUILD_TEST @ 18.04) -> Passed
Test job mbedtls-release-ci-testing 372 (RUN_BASIC_BUILD_TEST @ 20.04) -> Passed
Test job mbedtls-release-ci-testing 348 (RUN_ALL_SH@ 16.04) -> Passed
Test job mbedtls-release-ci-testing 348 (RUN_ALL_SH@ 18.04) -> Passed
Test job mbedtls-release-ci-testing 348 (RUN_ALL_SH@ 20.04) -> Passed
Testing RUN_ABI_TEST in mbed-tls-pr-test-parametrized 327 (RUN_ABI_CHECK) --> Failed
Testing RUN_ALL in mbed-tls-pr-test-parametrized 327 (RUN_ALL_SH@ 16.04) --> Passed
Testing RUN_ABI_TEST in mbed-tls-pr-test-parametrized 327 (RUN_ALL_SH@ 18.04) --> Passed

The abi-check failure can be reviewed here

<problems_with_types severity="High">
  <header name="ssl.h">
    <type name="struct mbedtls_ssl_context">
      <problem id="Removed_Field">
        <change target="client_auth">Field @target has been removed from this type.</change>
        <effect target="client_auth" type_name="struct mbedtls_ssl_context">Recompilation of a client program may be broken with the error message: '@type_name' has no member named '@target'.</effect>
      </problem>
      </type>
  </header>
</problems_with_types>

I am investigating weather that issue is caused by the tools/versions or the codebase

gilles-peskine-arm · 2022-02-21T17:58:57Z

327 attempts to merge bff88ab into a7a1dea which is quite a bit older. So API/ABI differences are to be expected.

I'm happy with the positive testing from 327, 372 and 348 for development. We should also check the LTS branch and some jobs that are expected to report failures. We can do that with the mbed-tls-pr-ci-testing job after reconfiguring it to point to the desired branch of mbedtls-test, then kicking off some PR jobs.

mpg · 2022-02-22T09:29:09Z

Testing looks good to me, but I want to clarify something: what do you mean when you write RUN_BASIC_BUILD_TEST @ 20.04? Looking at the job, it seems to me that basic-build-test.sh was only run on 16.04 as usual. But the job built the 20.04 image successfully. (Same for all.sh: it ran mostly on 16.04, some components on 18.04, and none on 20.04 AFAICS.)

Said otherwise, I think we have sufficient testing to be confident that we can merge this PR without breaking anything which is the goal.

I don't think we've tested how all.sh and basic-build-test.sh would run on the 18.04 or 20.04 images, but that's IMO out of scope for this task will be done later.

gilles-peskine-arm · 2022-02-22T09:52:46Z

Looking at the job, it seems to me that basic-build-test.sh was only run on 16.04 as usual. But the job built the 20.04 image successfully. (Same for all.sh: it ran mostly on 16.04, some components on 18.04, and none on 20.04 AFAICS.)

I don't think we've tested how all.sh and basic-build-test.sh would run on the 18.04 or 20.04 images, but that's IMO out of scope for this task will be done later.

Right, the goal here was only to update the Docker images. We're still dispatching jobs to 16.04 if possible, otherwise 18.04, and now 20.04 added as a third option. But since all current jobs can run on either 16.04 or 20.04, nothing is getting run on 20.04 yet.

minosgalanakis mentioned this pull request Jan 14, 2022

Update and cleanup docker files #30

Closed

minosgalanakis added enhancement New feature or request needs: work labels Jan 14, 2022

minosgalanakis linked an issue Jan 14, 2022 that may be closed by this pull request

Update and clean up Ubuntu docker files #10

Closed

5 tasks

minosgalanakis added the size-s Estimated task size: small (~2d) label Jan 14, 2022

minosgalanakis force-pushed the dev/mg/update-docker-files branch from 58f1d00 to a3fcabb Compare January 17, 2022 17:09

minosgalanakis added 8 commits January 18, 2022 10:39

Added docker --security-opt seccomp=unconfined parameter as default

20dd9eb

This patch updates docker's run.sh with the aforementioned parameter which disables the ASLR for zeroize test. The readme.md file has been updated accordingly

Set ubuntu-16.04 Dockerfile as baseline for ubuntu-18.04 alignment

97b3ba3

Updated ARMLMD_LICENSE_FILE environment variable

9d8894f

This patch updates the ubuntu-16.04 & ubuntu-18.04 Dockerfiles to use the up-to-date license license information, and removes the workaround in gen_jobs.groovy script.

Updated ubuntu-18.04 Dockerfile for abi-dumper, abi-compliance-checker

ced6663

This patch installs the official distribution selected packages for `abi-dumper` -> (version>=1.1-1) and `abi-compliance-checker` -> (version>=2.3-0.2)

Updated ubuntu-18.04 Dockerfile to remove the workaround for GnuTLS 3.7

d61658c

This patch updates GnuTLS 3.7 configure command to use the distribution’s `libtasn1` -> (version>=4.13). Comments have been amended accordingly

Updated ubuntu-18.04 Dockerfile to use latest pip/setuptools

0f0e47d

Added ubuntu-20.04 Dockerfile

5c49542

Updated dockerfile_builder to incude 20.04 image

7e74103

minosgalanakis force-pushed the dev/mg/update-docker-files branch from 10beddb to 7e74103 Compare January 18, 2022 13:23

minosgalanakis added needs: review needs: reviewer and removed needs: work labels Jan 18, 2022

This was referenced Jan 18, 2022

Enabling RUN_ALL testing target to run on Ubuntu 20.04 image #32

Open

Add most recent Ubuntu LTS (20.04) to our automated testing #8

Open

Fix dockerfile fail on arm64 Mbed-TLS/mbedtls#4971

Closed

minosgalanakis added the needs: ci label Jan 20, 2022

mpg self-requested a review January 25, 2022 12:16

mpg reviewed Jan 25, 2022

View reviewed changes

mpg self-requested a review January 26, 2022 09:10

gilles-peskine-arm self-requested a review February 4, 2022 17:52

gilles-peskine-arm removed the needs: reviewer label Feb 4, 2022

gilles-peskine-arm mentioned this pull request Feb 4, 2022

Condition Open CI changes based on Jenkins URL #34

Closed

gilles-peskine-arm requested changes Feb 4, 2022

View reviewed changes

minosgalanakis added needs: work and removed needs: review labels Feb 7, 2022

gilles-peskine-arm mentioned this pull request Feb 7, 2022

Update and clean up Ubuntu docker files #10

Closed

5 tasks

minosgalanakis added 4 commits February 8, 2022 21:39

Ammended the removal of a valid comment from ubuntu-18.04 Dockerfile

58938a3

Updated ubuntu-20.04 Dockerfile

45a5fce

This patch introduces the following 20.04 changes: * Added a note to link an existing known issue to abi-tools * Removed duplicate installation of abi-tools * Removed unnecessary pip upgrade operation * Reworded comments

Removed mypy and pylint dependencies

ef1f053

This patch removes the afforementioned python dependencies in ubuntu-16.04, ubuntu-18.04, ubuntu-20.04. Test jobs will install dependencies as required.

Revert "Added docker --security-opt seccomp=unconfined parameter as…

74ea266

… default" This reverts commit 20dd9eb.

Dockerfiles: Set the HOME to /var/lib/ws

36b60be

This patch introduces the aforementioned ENV setting in ubuntu-16.04, ubuntu-18.04, ubuntu-20.04. This fixes the home directory to a known location where scripts can access, to install and configure additional dependencies.

minosgalanakis added needs: review and removed needs: work labels Feb 9, 2022

gilles-peskine-arm requested changes Feb 9, 2022

View reviewed changes

gilles-peskine-arm added the needs: work label Feb 11, 2022

Dockerfiles: Set the HOME to /var/lib/builds

a15b48d

This patch changes the HOME location from /var/lib/ws to /var/lib/builds

gilles-peskine-arm approved these changes Feb 14, 2022

View reviewed changes

minosgalanakis mentioned this pull request Feb 15, 2022

Driver dispatch Codegen 1.1 Mbed-TLS/mbedtls#5396

Merged

4 tasks

gilles-peskine-arm removed the needs: work label Feb 21, 2022

mpg approved these changes Feb 21, 2022

View reviewed changes

gilles-peskine-arm removed the needs: review label Feb 21, 2022

gilles-peskine-arm merged commit e1a6125 into master Feb 22, 2022

Update and cleanup docker files #31

Update and cleanup docker files #31

Uh oh!

Conversation

minosgalanakis commented Jan 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Testing

Todo

Uh oh!

minosgalanakis commented Jan 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Failure investigations

all_u20-check_doxygen

all_u20-test_everest

all_u20-test_se_default

Uh oh!

mpg left a comment

Choose a reason for hiding this comment

Uh oh!

mpg Jan 25, 2022

Choose a reason for hiding this comment

Uh oh!

mpg Jan 25, 2022

Choose a reason for hiding this comment

Uh oh!

mpg Jan 25, 2022

Choose a reason for hiding this comment

Uh oh!

mpg Jan 25, 2022

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

minosgalanakis commented Feb 8, 2022

Uh oh!

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm Feb 9, 2022

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Uh oh!

mpg left a comment

Choose a reason for hiding this comment

Uh oh!

mpg Feb 21, 2022

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm commented Feb 21, 2022

Uh oh!

minosgalanakis commented Feb 21, 2022

Uh oh!

gilles-peskine-arm commented Feb 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mpg commented Feb 22, 2022

Uh oh!

gilles-peskine-arm commented Feb 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

minosgalanakis commented Jan 14, 2022 •

edited

Loading

minosgalanakis commented Jan 18, 2022 •

edited

Loading

gilles-peskine-arm commented Feb 21, 2022 •

edited

Loading