-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Apt proxy fallback #602
Apt proxy fallback #602
Conversation
Proxy test function added to scripts/common to prevent duplicate code
Test connectivity before exporting http_proxy environment variable
If the config variable is equals to 0, build fails in case of proxy-related errors.
Build will break only in case of APT_PROXY_FALLBACK option is not 1, continue otherwise
Proxy detection on chroot requires curl to be present to test connectivity with proxy server
If APT_PROXY_FALLBACK configured, 2 new files will be installed to rootfs temporarily. Do not break old behavior if not configured. export-image stage removes installed files.
build.sh
Outdated
if [[ -n "${APT_PROXY}" ]] && ! curl --silent "${APT_PROXY}" >/dev/null ; then | ||
echo "Could not reach APT_PROXY server: ${APT_PROXY}" | ||
exit 1 | ||
if [[ -n "${APT_PROXY}" && ! proxy_check ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
proxy_check
already has [[ -n "${APT_PROXY}" ]]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. Early implementations of proxy_check
were not checking the environment and I forgot to update build.sh
, probably. Since it is not breaking things, didn't notice it during tests.
At a glance, looks good. Might be a while before I can go over it fully, test and merge. |
Remove duplicate environment check. proxy_check already checks APT_PROXY variable.
@@ -7,13 +7,16 @@ bootstrap(){ | |||
local BOOTSTRAP_CMD=debootstrap | |||
local BOOTSTRAP_ARGS=() | |||
|
|||
export http_proxy=${APT_PROXY} | |||
|
|||
if proxy_check; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm missing something, if proxy_check fails, this seems to be disabling it regardless of whether APT_PROXY_FALLBACK is set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since bootstrap is one of the earliest operations that fetch files, checking proxy in build.sh shadowed the bug in my test environment, I think. Fixed now.
Apt proxy fallback option added as a config file parameter.
If configured to use direct connection as a fallback, the script will test connectivity with the proxy server each time before using it via a bash function for debootstrap and via apt.conf.d/52fallback file on chroot.
If fallback is configured, the old 51cache file will not be installed.
export-image stage removes all installed files related to this functionality.
Testing this script on Debian buster with the following config file in 4 different scenarios worked fine for me. I tried it while the proxy is down all the time, up all the time, changed state from up to down and down to up during build. Script used proxy whenever it is available in all of these scenarios.
IMG_NAME='ProxyTest' APT_PROXY=http://10.0.2.3:3142/ APT_PROXY_FALLBACK=1