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: do not fail on apt-get update for extra packages, for #5620 #5623

Merged

Conversation

stasadev
Copy link
Member

The Issue

We must not allow apt-get update to fail if some repository is down, because it can stop the webserver from starting.

How This PR Solves The Issue

Allows apt-get install to continue after a fail for apt-get update.

Manual Testing Instructions

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

@stasadev stasadev requested a review from a team as a code owner December 12, 2023 19:46
Copy link

@@ -1085,8 +1085,8 @@ RUN (groupadd --gid $gid "$username" || groupadd "$username" || true) && (userad
if extraPackages != nil {
contents = contents + `
### DDEV-injected from webimage_extra_packages or dbimage_extra_packages
RUN echo "Disk usage problems: " && df -h /tmp && ls -l /var/lib/apt/lists
RUN apt-get -qq update && DEBIAN_FRONTEND=noninteractive apt-get -qq install -y -o Dpkg::Options::="--force-confold" --no-install-recommends --no-install-suggests ` + strings.Join(extraPackages, " ") + "\n"
RUN echo "Disk usage problems: " && df -h / && ls -l /var/lib/apt/lists
Copy link
Member Author

Choose a reason for hiding this comment

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

@rfay I'm not sure if we still need this check, if we skip failures for apt-get update, but anyway:
I changed df -h /tmp to df -h / - we discussed this on Discord earlier and df -h /tmp produced less output.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed we probably won't need it at all, can remove. Install of PHP related packages can still fail, but we have no control. And this check didn't actually do anything but somehow seemed to result in less failures

@rfay
Copy link
Member

rfay commented Dec 13, 2023

Looks fine to me. I thought maybe there was a second place we do apt update.

@stasadev
Copy link
Member Author

There are several apt-get update added in Go files, all of which already have || true.
This is the last one without || true.

@stasadev stasadev merged commit 64e9958 into ddev:master Dec 13, 2023
9 checks passed
@stasadev stasadev deleted the 20231212_stasadev_do_not_fail_on_apt_get_update branch December 13, 2023 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants