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.

Sign up for GitHub

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

Jump to bottom

Cleanup dockerfile #183334

Merged
merged 5 commits into from
May 14, 2024
Merged

Cleanup dockerfile #183334

merged 5 commits into from
May 14, 2024

Conversation

jbudz
Copy link
Member

@jbudz jbudz commented May 13, 2024 •
edited by kibanamachine
Loading

  • Skips font install on serverless, pdf reporting is disabled
  • Moves tini and font download to stage 0
  • Removes curl install from Ubuntu stage 1

@jbudz jbudz added Team:Operations Team label for Operations Team release_note:skip Skip the PR/issue when compiling release notes ci:cloud-deploy Create or update a Cloud deployment ci:build-serverless-image labels May 13, 2024
@jbudz
Copy link
Member Author

jbudz commented May 13, 2024

buildkite test this

@jbudz
Copy link
Member Author

jbudz commented May 13, 2024

buildkite test this

jbudz
jbudz commented May 13, 2024
View reviewed changes
src/dev/build/tasks/os_packages/docker_generator/templates/base/Dockerfile
@@ -79,44 +104,25 @@ RUN for iter in {1..10}; do \
{{packageManager}} update && \
{{packageManager}} upgrade -y && \
{{packageManager}} install -y --no-install-recommends \
fontconfig fonts-liberation libnss3 libfontconfig1 ca-certificates curl && \
Copy link
Member Author

Choose a reason for hiding this comment

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

libfontconfig1 is already a dependency of fontconfig. fonts-liberation is superseded by the installed font NotoSansCJK.

@elastic/kibana-reporting-services any idea if I'm missing anything? I'm pretty sure we're okay, tests are passing and my manual checks lgtm.

Copy link
Member Author

Choose a reason for hiding this comment

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

docs at https://www.elastic.co/guide/en/kibana/current/secure-reporting.html#install-reporting-packages

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @tsullivan - we both don't spot anything missing in the Dockerfile. Thank you!

@jbudz jbudz marked this pull request as ready for review May 13, 2024 22:17
@jbudz jbudz requested a review from a team as a code owner May 13, 2024 22:17
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@jbudz jbudz added the ci:build-docker-fips Build Docker FIPS image label May 14, 2024
@kibana-ci
Copy link
Collaborator

kibana-ci commented May 14, 2024 •
edited
Loading

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: 782589a
  • Kibana UBI FIPS Image: docker.elastic.co/kibana-ci/kibana-ubi-fips:8.15.0-SNAPSHOT-782589a788abd316a75c60d1d62732cb70f20090
  • Cloud Deployment
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-183334-782589a788ab

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #97 / discover/esql discover esql view test should query an index pattern that doesnt translate to a dataview correctly

Metrics [docs]

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id before after diff
module count - 5407 +5407
total size - 8.8MB +8.8MB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@jbudz jbudz merged commit e2dd5ee into elastic:main May 14, 2024
23 checks passed
@jbudz jbudz added backport:all-open Backport to all branches that could still receive a release and removed v8.15.0 labels May 14, 2024
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
❌ 7.17 Backport failed because of merge conflicts

You might need to backport the following PRs to 7.17:
- [ci] Verify docker contexts (#122897)
❌ 8.14 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 183334

Questions ?

Please refer to the Backport tool documentation

jbudz added a commit to jbudz/kibana that referenced this pull request May 14, 2024
@jbudz
Cleanup dockerfile (elastic#183334)
b9de2f1 
- Skips font install on serverless, pdf reporting is disabled
- Moves tini and font download to stage 0
- Removes curl install from Ubuntu stage 1
@jbudz jbudz mentioned this pull request May 14, 2024
Merged
jbudz added a commit to jbudz/kibana that referenced this pull request May 14, 2024
@jbudz
Cleanup dockerfile (elastic#183334)
ee53b1a 
- Skips font install on serverless, pdf reporting is disabled
- Moves tini and font download to stage 0
- Removes curl install from Ubuntu stage 1
@jbudz jbudz mentioned this pull request May 14, 2024
Merged
@kibanamachine kibanamachine mentioned this pull request May 14, 2024
Merged
jbudz added a commit that referenced this pull request May 14, 2024
@jbudz
[8.14] Cleanup dockerfile (#183334) (#183438)
946e1b9 
#183334
@kibanamachine kibanamachine mentioned this pull request May 14, 2024
Merged
jbudz added a commit that referenced this pull request May 15, 2024
@jbudz
[7.17] Cleanup dockerfile (#183334) (#183440)
a8f15df 
#183334
@jsoriano
Copy link
Member

jsoriano commented May 16, 2024 •
edited
Loading

Hey @jbudz,

We use curl in elastic-package for healtchecks, and builds have started to fail since the latest snapshots. I wonder if the removal of curl can also cause issues to users.

These are our healthchecks in elastic-package: https://github.com/elastic/elastic-package/blob/cf8e1c9d33068a4daa63032a49e3e51006fe9171/internal/stack/_static/kibana-healthcheck.sh.tmpl

If the decision of removing curl is not going to be reconsidered, what would be your recommendation? Should we do the healthchecks in a different way? Or we should create our own custom images with curl?

cc @elastic/ecosystem

@jsoriano
Copy link
Member

Searching a bit in the Elastic org, it looks like curl is frequently used in healthchecks for Kibana: https://github.com/search?q=org%3Aelastic+curl+kibana+5601+health&type=code&p=1

jbudz added a commit that referenced this pull request May 16, 2024
@jbudz
Revert "[8.14] Cleanup dockerfile (#183334) (#183438)"
c65d1b4 
This reverts commit 946e1b9.
jbudz added a commit that referenced this pull request May 16, 2024
@jbudz
Revert "[7.17] Cleanup dockerfile (#183334) (#183440)"
f93d335 
This reverts commit a8f15df.
@jbudz jbudz mentioned this pull request May 16, 2024
Merged
@jbudz
Copy link
Member Author

jbudz commented May 16, 2024

Thanks for the ping. No problem, I reverted the backports and will re-add it to main. Linked issues are above.

jbudz added a commit that referenced this pull request May 16, 2024
@jbudz
[artifacts/container] Re-add curl (#183626)
b211842 
Partially reverts #183334. Some
services using kibana as a base expect it to exist for running health
checks.


cc @jsoriano
@jsoriano
Copy link
Member

Thanks @jbudz!

@jsoriano jsoriano mentioned this pull request May 17, 2024
Merged
7 tasks
@agithomas agithomas mentioned this pull request May 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rshen91 rshen91 rshen91 left review comments

@Ikuni17 Ikuni17 Ikuni17 approved these changes

Assignees
No one assigned
Labels
backport:all-open Backport to all branches that could still receive a release ci:build-docker-fips Build Docker FIPS image ci:build-serverless-image ci:cloud-deploy Create or update a Cloud deployment release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v8.15.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@jbudz @elasticmachine @kibana-ci @kibanamachine @jsoriano @Ikuni17 @rshen91