jenkinsci / docker-agent

Jenkins agent (base image) and inbound agent Docker images
https://hub.docker.com/r/jenkins/inbound-agent/
MIT License
282 stars 228 forks source link

chore: jlink improvements from jenkinsci/docker #799

Closed lemeurherve closed 4 months ago

lemeurherve commented 4 months ago

This PR integrates jlink improvement from https://github.com/jenkinsci/docker/pull/1857.

adapted it to be POSIX compliant as the Alpine ash shell doesn't support arrays

Notes:

Extracted from:

Testing done

Local builds + CI

### Submitter checklist
- [x] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!
- [x] Ensure that the pull request title represents the desired changelog entry
- [x] Please describe what you did
- [x] Link to relevant issues in GitHub or Jira
- [x] Link to relevant pull requests, esp. upstream and downstream changes
- [x] Ensure you have provided tests - that demonstrates feature works or fixes the issue
dduportal commented 4 months ago

I removed the TARGETPLATFORM check in Alpine images as we're not publishing any arm/v7 image of this kind.

That assertion is false as per https://github.com/jenkinsci/docker-agent/blob/6d7bd6b71460c10571ff628052d10fd726abfbe3/docker-bake.hcl#L248

We do publish arm/v7 images for the agents. However, the builds seems to have passed! Anyone with a 32 bits ARM to test (ping @gounthar as you have my Rpis ;) ) the resulting image? It would be really a good news if no need for this if anymore \o/

gounthar commented 4 months ago

The following tags are working for me on armv7:

@dduportal let me know if I should send back your Pis soon. 😉

lemeurherve commented 4 months ago

I removed the TARGETPLATFORM check in Alpine images as we're not publishing any arm/v7 image of this kind.

That assertion is false as per

https://github.com/jenkinsci/docker-agent/blob/6d7bd6b71460c10571ff628052d10fd726abfbe3/docker-bake.hcl#L248

We do publish arm/v7 images for the agents. However, the builds seems to have passed! Anyone with a 32 bits ARM to test (ping @gounthar as you have my Rpis ;) ) the resulting image? It would be really a good news if no need for this if anymore \o/

@dduportal This line concerns debian images, not the Alpine one, AFAICT the assertion is true:

None of the Alpine images specify arm/v7 architecture: https://github.com/jenkinsci/docker-agent/blob/6d7bd6b71460c10571ff628052d10fd726abfbe3/docker-bake.hcl#L171-L225

dduportal commented 4 months ago

I removed the TARGETPLATFORM check in Alpine images as we're not publishing any arm/v7 image of this kind.

That assertion is false as per https://github.com/jenkinsci/docker-agent/blob/6d7bd6b71460c10571ff628052d10fd726abfbe3/docker-bake.hcl#L248

We do publish arm/v7 images for the agents. However, the builds seems to have passed! Anyone with a 32 bits ARM to test (ping @gounthar as you have my Rpis ;) ) the resulting image? It would be really a good news if no need for this if anymore \o/

@dduportal This line concerns debian images, not the Alpine one.

None of the Alpine images specify arm/v7 architecture, AFAICT the assertion is true:

https://github.com/jenkinsci/docker-agent/blob/6d7bd6b71460c10571ff628052d10fd726abfbe3/docker-bake.hcl#L171-L225

🤦 I missed the `Alpine£ word in the original sentence. my apologies.

dduportal commented 4 months ago

@dduportal let me know if I should send back your Pis soon. 😉

Thanks for taking some time. The goal was to test the unreleased version but as underlined by Hervé above I was misguided: there are no arm/v7 Alpine (I mistook with Debian).

gounthar commented 4 months ago

My bad, I misunderstood the request. 🤷