Write proxy env files for new proxy settings #11713
Conversation
/etc/juju-proxy.conf and /etc/juju-proxy-systemd.conf are only written for legacy proxy settings. This change adds the same behavior for the new-style proxy settings. Note: Those files can be relied upon in charms or scripts they render but do not by themselves modify the environment unless explicit action is taken by a charm author. LP: #1883656
|
Can one of the admins verify this patch? |
4 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
cloudconfig/userdatacfg_unix.go
Outdated
| exportedProxyEnv := proxySettings.AsScriptEnvironment() | ||
| w.conf.AddScripts(strings.Split(exportedProxyEnv, "\n")...) |
There was a problem hiding this comment.
We only want to do this for legacy settings I think, right?
There was a problem hiding this comment.
If I am not mistaken, this affects the cloud-init shell only (runcmd) on first boot.
exportedProxyEnv := proxySettings.AsScriptEnvironment()
w.conf.AddScripts(strings.Split(exportedProxyEnv, "\n")...)
About this one I am not sure since there are potential preruncmd and postruncmd commands added by a user via model-config.
If we follow the same principle here, I think we could:
- expose JUJU_CHARM_{HTTP, HTTPS, NO}_PROXY in the cloud-init shell environment and document that those should be used explicitly;
- tell users that /etc/juju-proxy.conf can be sourced in any custom
postruncmds.- wouldn't work on Windows since we don't write a file there.
cloudconfig/userdatacfg_unix.go
Outdated
| if w.icfg.JujuProxySettings.HasProxySet() { | ||
| proxySettings = w.icfg.JujuProxySettings | ||
| } else if w.icfg.LegacyProxySettings.HasProxySet() { | ||
| proxySettings = w.icfg.LegacyProxySettings |
There was a problem hiding this comment.
Need to cater for the case where neither juju nor legacy settings are set
There was a problem hiding this comment.
If none are set we can just rely on empty strings in the proxy.Settings struct.
https://github.com/juju/proxy/blob/master/proxy.go#L58-L64
I think I need to have a unit test for that though.
* only write to /etc/profile.d/juju-proxy.sh if legacy settings are used * modify the proxyupdater behavior as well as the userdata generator.
|
I have updated the PR (ca712e5), although further updates are required to unit tests for the proxyupdater. Before adding those, I would like to have a further discussion about:
|
|
I converted this to a draft and will likely close it given the above comment and the fact that we cat steal proxy settings from https://bugs.launchpad.net/charm-glance-simplestreams-sync/+bug/1883656/comments/9 |
|
Closing to clean up hanging PRs. Feel free to reopen if this is still fresh. |
Description of change
Write /etc/juju-proxy.conf and /etc/juju-proxy-systemd.conf when
juju-* proxy settings are set, not only when legacy proxy settings are
set.
QA steps
Documentation changes
None, maybe worth a release note.
Bug reference
https://bugs.launchpad.net/juju/+bug/1883656