orioledb icon indicating copy to clipboard operation
orioledb copied to clipboard

Improve OrioleDB Docker Setup and add Minimal Regression Testing (RUN_REGRESSION_TESTS)

Open ImreSamu opened this issue 1 year ago β€’ 4 comments

The purpose of this PR is to improve the OrioleDB Docker environment and testing workflow while maintaining compatibility. Consequently, the newly proposed Dockerfile build argument defaults RUN_REGRESSION_TESTS to no, and thus I do not expect changes in .github/workflows/docker.yml.

Long-term Goal:

My long-term goal is to successfully run the complete OrioleDB and PostgreSQL checks in both Ubuntu and Alpine (musl) Docker environments. Currently, this is only partially achieved as the OrioleDB testgrescheck is not executed. If the changes prove successful, some parts will need to be integrated into .github/workflows/docker.yml .

If there are any questions or suggestions for modifications, I am happy to respond.

Changes:

Makefile : Fix: Use fake COMMIT_HASH if no .git history (Docker environment, building from tar.gz)

In the Docker environment, the .git subdirectory is removed to optimize the final image size,
making it impossible to retrieve the actual COMMIT_HASH.
This fix provides a temporary fake COMMIT_HASH
with a value of 0000000000000000000000000000000000000000 to handle this situation.

This fix is also useful for building from orioledb (zip) or (tar.gz) source code
when there is no .git information.

With this fix, the regression tests related to `orioledb_commit_hash()`
now pass successfully even without git commit hash information.

Dockerfile , Dockerfile.ubuntu : Improve Dockerfiles (Alpine, Ubuntu) and Add Minimal Build Time Regression Testing

Main Changes:
- Introduced `RUN_REGRESSION_TESTS` build-arg option.
  Enables 'regresscheck' and 'isolationcheck' for contrib/orioledb.
  Tests do not increase the final Docker image size; all caches are carefully removed after tests.

Small Changes:
- Added support for new Alpine (v3.20) and Ubuntu distributions.
- Enhanced wget options with `--tries=10 --timeout=60` for downloading from git.savannah.gnu.org.
- Updated ENV statements to use key=value format instead of the legacy key value format.
  ref: https://docs.docker.com/reference/build-checks/legacy-key-value-format/

.github/workflows/dockertest.yml : Improve DockerTest Workflow

- Add official-Docker-postgres testing  with `RUN_REGRESSION_TESTS=yes` for running (contrib/orioledb) `regresscheck` and `isolationcheck`.
- Include extra Alpine edge distribution for testing.

ImreSamu avatar Jul 07 '24 20:07 ImreSamu

Testing with alpine:edge and using the latest versions of gcc(13.2.1) and clang(18.1.8) can reveal new compilation warnings.

Example: "docker 16-gcc-alpine-edge" gcc(13.2.1)

  • ( via workflow: https://github.com/ImreSamu/orioledb/actions/runs/9830514328/job/27136782931 )
024-07-07T21:48:19.4228585Z #11 307.3 gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -O2 -fPIC -DCOMMIT_HASH=0000000000000000000000000000000000000000 -Wno-error=deprecated-declarations -fvisibility=hidden -I../../contrib/orioledb/include -I. -I. -I../../src/include  -D_GNU_SOURCE -I/usr/include/libxml2   -c -o src/catalog/o_class_cache.o src/catalog/o_class_cache.c
2024-07-07T21:48:19.4233960Z #11 307.3 In file included from ../../src/include/c.h:61,
2024-07-07T21:48:19.4234810Z #11 307.3                  from ../../src/include/postgres.h:45,
2024-07-07T21:48:19.4235635Z #11 307.3                  from src/catalog/indices.c:13:
2024-07-07T21:48:19.4236442Z #11 307.3 In function 'memmove',
2024-07-07T21:48:19.4237485Z #11 307.3     inlined from '_o_index_begin_parallel.constprop' at src/catalog/indices.c:814:4:
2024-07-07T21:48:19.4239320Z #11 307.3 /usr/include/fortify/string.h:66:16: warning: 'old_o_table_serialized' may be used uninitialized [-Wmaybe-uninitialized]
2024-07-07T21:48:19.4240922Z #11 307.3    66 |         return __orig_memmove(__d, __s, __n);
2024-07-07T21:48:19.4241743Z #11 307.3       |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2024-07-07T21:48:19.4242896Z #11 307.3 src/catalog/indices.c: In function '_o_index_begin_parallel.constprop':
2024-07-07T21:48:19.4244335Z #11 307.3 src/catalog/indices.c:733:25: note: 'old_o_table_serialized' was declared here
2024-07-07T21:48:19.4245419Z #11 307.3   733 |         Pointer         old_o_table_serialized;
2024-07-07T21:48:19.4246221Z #11 307.3       |                         ^~~~~~~~~~~~~~~~~~~~~~
2024-07-07T21:48:19.4395794Z #11 307.4 gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -O2 -fPIC -DCOMMIT_HASH=0000000000000000000000000000000000000000 -Wno-error=deprecated-declarations -fvisibility=hidden -I.

ImreSamu avatar Jul 07 '24 22:07 ImreSamu

I have configured what I could in the Docker-testgres environment.

Unfortunately, in older Ubuntu environments, a few s3_* tests from 'testgrescheck_part_2' do not run successfully. Therefore, if Python < 3.11, I allow the build to continue with "|| true" You can view the current errors in the dockerTEST CI/CD log. If this approach is not acceptable, I can change the logic.

The issue is that some tests use features introduced in newer versions of Python. For example

"/usr/src/postgresql/contrib/orioledb/t/s3_test.py", line 138, in test_s3_checkpoint
path = path.removeprefix(node.data_dir).split('/')[1:]
AttributeError: 'str' object has no attribute 'removeprefix'

This is a problem because removesuffix is a method available only in Python 3.9+, while Ubuntu 20.04 uses Python 3.8 by default. ( https://stackoverflow.com/a/66683635 )

There are likely other issues as well.

I have mostly finished the modifications and testing. And the build in the ImreSamu GitHub DockerTEST workflow is successful.

  • https://github.com/ImreSamu/orioledb/actions/workflows/dockertest.yml

The code is ready for review. If any changes are needed in the logic or if more comments are required, I am happy to make them.

ImreSamu avatar Jul 10 '24 23:07 ImreSamu

@ImreSamu, thank you so much for your work! Could you please, check CI results?

akorotkov avatar Jul 22 '24 13:07 akorotkov

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 71.91%. Comparing base (466738f) to head (5078e78). Report is 13 commits behind head on main.

:exclamation: There is a different number of reports uploaded between BASE (466738f) and HEAD (5078e78). Click for more details.

HEAD has 31 uploads less than BASE
Flag BASE (466738f) HEAD (5078e78)
32 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #369       +/-   ##
===========================================
- Coverage   90.11%   71.91%   -18.20%     
===========================================
  Files          79       76        -3     
  Lines       33244    29998     -3246     
===========================================
- Hits        29957    21574     -8383     
- Misses       3287     8424     +5137     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jul 22 '24 14:07 codecov[bot]

New updated/improved/etc version --> https://github.com/orioledb/orioledb/pull/372 So I am closing this PR.

ImreSamu avatar Oct 10 '24 10:10 ImreSamu