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 docker builds #4895

Merged
merged 2 commits into from
May 3, 2023
Merged

Fix docker builds #4895

merged 2 commits into from
May 3, 2023

Conversation

duncdrum
Copy link
Contributor

remove superfluous layers see experimental builds if it works

close #4894

@duncdrum duncdrum added the bug issue confirmed as bug label Apr 30, 2023
@duncdrum
Copy link
Contributor Author

build succeeds, container tests do too, push fails because this is from a PR from a fork, but attempt is visible.

@sonarcloud
Copy link

sonarcloud bot commented Apr 30, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@dizzzz
Copy link
Member

dizzzz commented May 3, 2023

@duncdrum @reinhapa ?

@adamretter
Copy link
Member

If I am not mistaken, whilst this is interesting, it removes support for Apache FOP, and therefore we may not want to merge this. Is that correct?

@duncdrum
Copy link
Contributor Author

duncdrum commented May 3, 2023

it removes support for Apache FOP

it does not. The layers are superfluous, not the deps

@reinhapa reinhapa merged commit 45790db into eXist-db:develop May 3, 2023
9 of 12 checks passed
@adamretter
Copy link
Member

it does not. The layers are superfluous, not the deps

@duncdrum Sorry can you explain that please? In your PR here, it appears that you have commented out the dependencies that are required for Apache FOP - https://github.com/eXist-db/exist/pull/4895/files#diff-e06009c91b03b0212493cef67386d831bde8dc3729db1c33707e1f24458c1ad2R31

Am I mistaken... or are those lines not now commented out?

@duncdrum
Copy link
Contributor Author

duncdrum commented May 3, 2023

@adamretter the dependencies are part of the java base image, no need to create new layers via COPY

@adamretter
Copy link
Member

adamretter commented May 3, 2023

@duncdrum So I think you are saying that the dependencies required for Apache FOP are available in gcr.io/distroless/java17:latest, and so we no longer need to copy them in from debian:bullseye-slim. Is that correct?

@duncdrum
Copy link
Contributor Author

duncdrum commented May 3, 2023

@adamretter yes

@adamretter
Copy link
Member

@duncdrum Okay great. Thank you :-)

@adamretter
Copy link
Member

@duncdrum I think you missed a file. The same changes will need to be made to Dockerfile-DEBUG too. Additionally can we just remove all the commented out stuff. There doesn't seem to be any value in leaving it there, no?

@duncdrum duncdrum deleted the fix-docker branch May 3, 2023 18:35
@duncdrum
Copy link
Contributor Author

duncdrum commented May 3, 2023

@adamretter i would wait to check the fallout from the default apps. We know that there are problems on develop, but as of now all our tests show green. Once we re back on solid footing we can delete things , clean up the docker Pom, and inline the experimental images

@reinhapa
Copy link
Member

reinhapa commented May 4, 2023

@duncdrum did I merge the quickly?

@duncdrum
Copy link
Contributor Author

duncdrum commented May 4, 2023

@reinhapa no the deploy for :latest is running again, and we have proper CI checks moving forward. That constituted the hotfix. Once this had time to percolate, i ll do a cleanup PR, with outstanding cosmetic changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issue confirmed as bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] docker latest tag hasn't been updated for 2 months
4 participants