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/arm preperations #527

Open
wants to merge 63 commits into
base: main
Choose a base branch
from
Open

Fix/arm preperations #527

wants to merge 63 commits into from

Conversation

Maleware
Copy link
Contributor

Description

This PR fixes some problems with the arm build of our products.

Notable mentions:

  • Stackable-Base image has an updated sha to the manifest list of ubi8-minimal
  • STATSD_EXPORTER is now a binary in nexus rahter then being pulled from their image
  • OPA binary now available in nexus with both architectures
  • ubi8-rust-builder has now a dependency to the corresponding manifest list
  • Testing_Tools has now a dependency to the corresponding manifest list

Definition of Done Checklist

  • Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
  • Please make sure all these things are done and tick the boxes

Tasks

Edit tasklist title
Beta Give feedback Tasklist Tasks, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. Changes are OpenShift compatible
    Options
  2. All added packages (via microdnf or otherwise) have a comment on why they are added
    Options
  3. Things not downloaded from Red Hat repositories should be mirrored in the Stackable repository and downloaded from there
    Options
  4. All packages should have (if available) signatures/hashes verified
    Options
  5. Does your change affect an SBOM? Make sure to update all SBOMs
    Options

CHANGELOG.md Outdated
@@ -4,6 +4,8 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

## [23.11.0] - 2023-11-23
Copy link
Member

@sbernauer sbernauer Dec 27, 2023

Choose a reason for hiding this comment

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

I have to admit this is a bit confusing to me. What do you think of removing it again?

Also we should add entries for this PR

airflow/Dockerfile Outdated Show resolved Hide resolved
airflow/Dockerfile Outdated Show resolved Hide resolved
opa/Dockerfile Outdated Show resolved Hide resolved
airflow/Dockerfile Outdated Show resolved Hide resolved
superset/Dockerfile Outdated Show resolved Hide resolved
testing-tools/Dockerfile Outdated Show resolved Hide resolved
ubi8-rust-builder/Dockerfile Outdated Show resolved Hide resolved
@@ -291,7 +291,7 @@ gcloud-aio-bigquery==7.0.0
gcloud-aio-storage==9.0.0
gcsfs==2023.9.2
geomet==0.2.1.post1
gevent==23.9.1
gevent==22.10.2
Copy link
Member

Choose a reason for hiding this comment

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

Same as with greenlet here

@@ -313,7 +313,7 @@ google-resumable-media==2.5.0
googleapis-common-protos==1.56.4
graphql-core==3.2.3
graphviz==0.20.1
greenlet==2.0.2
greenlet==3.0.2
Copy link
Member

@sbernauer sbernauer Dec 27, 2023

Choose a reason for hiding this comment

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

I'm no Python guy but I assume we need to be careful here making a breaking change with a Airflow dependency.
https://github.com/python-greenlet/greenlet/blob/master/CHANGES.rst#300a1-2023-06-21 sounds like Python 2.12 addition and 2.7, 3.5 and 3.6 removal as well as C++ bump.

WDYT of pulling in 3.0.2 only for the experimental ARM builds to not disturb the "production" x86 image?

Maybe we could also back-port the fixes (whatever they are) to greenlet 2.0.2

Options:

  1. Merge as-is
  2. Pull in 3.0.2 only for the experimental ARM builds to not disturb the "production" x86 image
  3. Back-port the fixes (whatever they are) to greenlet 2.0.2
  4. Try to bump greenlet upstream in Airflow. This does not work for gevent however, as this PR downgrades it. This is more of a long-term solution, as we have a dependency on upstream releases.

Arch meeting 2024-01-03 outcome:

  1. No
  2. Maybe, but only as long as arm support is experimental
  3. No
  4. Yeah, at least raise an issue in Airflow

gevent==23.9.1
gevent==22.10.2
Copy link
Member

Choose a reason for hiding this comment

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

Why downgrade?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because... although they claim it gevent 23.9.1 is not building together with greenlet 3.0.2 on arm64.

Copy link
Member

Choose a reason for hiding this comment

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

Fwiw I'm with Sebastian and Malte. We need to be careful with changing dependency versions

airflow/Dockerfile Outdated Show resolved Hide resolved
stackable-base/Dockerfile Outdated Show resolved Hide resolved
Comment on lines 98 to 99
&& pushd /stackable && curl --fail -L https://repo.stackable.tech/repository/packages/statsd_exporter/statsd_exporter-${STATSD_EXPORTER}.linux-${ARCH}.tar.gz | tar -xzC /stackable && ln -s statsd_exporter-${STATSD_EXPORTER}.linux-${ARCH}/statsd_exporter statsd_exporter && popd

Copy link
Member

Choose a reason for hiding this comment

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

Not a fan of push and pop d. Can we just use "curl -o .." or workdir or at least use wrapping (" && \") for better readability?

testing-tools/Dockerfile Outdated Show resolved Hide resolved
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.

None yet

4 participants