juju-core

Merge lp:~thumper/juju-core/uniter-set-osenv into lp:~go-bot/juju-core/trunk

  1. uniter-set-osenv
  2. Merge into trunk
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 2259
Proposed branch: lp:~thumper/juju-core/uniter-set-osenv
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 94 lines (+50/-0)
4 files modified
juju/osenv/proxy.go (+16/-0)
juju/osenv/proxy_test.go (+25/-0)
worker/uniter/uniter.go (+2/-0)
worker/uniter/uniter_test.go (+7/-0)
To merge this branch: bzr merge lp:~thumper/juju-core/uniter-set-osenv
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+203259@code.launchpad.net

Commit message

Update the proxy settings for the uniter process

Some parts of the standard library functions can take
advantage of proxy settings when downloading things.
We should make sure that the agent process gets these
values too.

https://codereview.appspot.com/57110043/

Description of the change

Update the proxy settings for the uniter process

Some parts of the standard library functions can take
advantage of proxy settings when downloading things.
We should make sure that the agent process gets these
values too.

https://codereview.appspot.com/57110043/

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :
Download full text (4.2 KiB)

Reviewers: mp+203259_code.launchpad.net,

Message:
Please take a look.

Description:
Update the proxy settings for the uniter process

Some parts of the standard library functions can take
advantage of proxy settings when downloading things.
We should make sure that the agent process gets these
values too.

https://code.launchpad.net/~thumper/juju-core/uniter-set-osenv/+merge/203259

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/57110043/

Affected files (+52, -0 lines):
   A [revision details]
   M juju/osenv/proxy.go
   M juju/osenv/proxy_test.go
   M worker/uniter/uniter.go
   M worker/uniter/uniter_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140125125332-eckpnh62bygibitl
+New revision: <email address hidden>

Index: juju/osenv/proxy.go
=== modified file 'juju/osenv/proxy.go'
--- juju/osenv/proxy.go 2014-01-23 03:51:10 +0000
+++ juju/osenv/proxy.go 2014-01-26 20:07:50 +0000
@@ -71,3 +71,19 @@
   addLine("ftp_proxy", s.Ftp)
   return lines
  }
+
+// SetEnvironmentValues updates the process environment with the
+// proxy values stored in the settings object. Both the lower-case
+// and upper-case variants are set.
+//
+// http-proxy, HTTP_PROXY
+// https-proxy, HTTPS_PROXY
+// ftp-proxy, FTP_PROXY
+func (s *ProxySettings) SetEnvironmentValues() {
+ os.Setenv("http-proxy", s.Http)
+ os.Setenv("HTTP-PROXY", s.Http)
+ os.Setenv("https-proxy", s.Https)
+ os.Setenv("HTTPS-PROXY", s.Https)
+ os.Setenv("ftp-proxy", s.Ftp)
+ os.Setenv("FTP-PROXY", s.Ftp)
+}

Index: juju/osenv/proxy_test.go
=== modified file 'juju/osenv/proxy_test.go'
--- juju/osenv/proxy_test.go 2014-01-23 03:51:10 +0000
+++ juju/osenv/proxy_test.go 2014-01-26 20:07:50 +0000
@@ -4,6 +4,8 @@
  package osenv_test

  import (
+ "os"
+
   gc "launchpad.net/gocheck"

   "launchpad.net/juju-core/juju/osenv"
@@ -151,3 +153,26 @@
   }
   c.Assert(proxies.AsEnvironmentValues(), gc.DeepEquals, expected)
  }
+
+func (s *proxySuite) TestSetEnvironmentValues(c *gc.C) {
+ s.PatchEnvironment("http_proxy", "initial")
+ s.PatchEnvironment("HTTP_PROXY", "initial")
+ s.PatchEnvironment("https_proxy", "initial")
+ s.PatchEnvironment("HTTPS_PROXY", "initial")
+ s.PatchEnvironment("ftp_proxy", "initial")
+ s.PatchEnvironment("FTP_PROXY", "initial")
+
+ proxy := osenv.ProxySettings{
+ Http: "http proxy",
+ Https: "https proxy",
+ // Ftp left blank to show clearing env.
+ }
+ proxy.SetEnvironmentValues()
+
+ c.Assert(os.Getenv("http-proxy"), gc.Equals, "http proxy")
+ c.Assert(os.Getenv("HTTP-PROXY"), gc.Equals, "http proxy")
+ c.Assert(os.Getenv("https-proxy"), gc.Equals, "https proxy")
+ c.Assert(os.Getenv("HTTPS-PROXY"), gc.Equals, "https proxy")
+ c.Assert(os.Getenv("ftp-proxy"), gc.Equals, "")
+ c.Assert(os.Getenv("FTP-PROXY"), gc.Equals, "")
+}

Index: worker/uniter/uniter.go
=== modified file 'worker/uniter/uniter.go'
--- worker/uniter/uniter.go 2014-01-23 21:57:42 +0000
+++ worker/uniter/uniter.go 2014-01-26 20:18:26 +0000
@@ -694,6 +694,8...

Read more...

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

LGTM. Is the uniter the only thing that might need the proxy settings?
How about the upgrader and deployer?

https://codereview.appspot.com/57110043/diff/1/juju/osenv/proxy.go
File juju/osenv/proxy.go (right):

https://codereview.appspot.com/57110043/diff/1/juju/osenv/proxy.go#newcode78
juju/osenv/proxy.go:78: //
Why both upper-case and lower-case?

https://codereview.appspot.com/57110043/

Revision history for this message
Tim Penhey (thumper) wrote :

On 2014/01/27 00:12:55, dimitern wrote:
> LGTM. Is the uniter the only thing that might need the proxy settings?
 How about
> the upgrader and deployer?

I have a branch following that will add an environment watcher to the
machine agent.
We need a place to keep the files on disk correct, and it will also make
sure that
the environment settings for the process are good.

The golang net module will use the proxy settings if they are set.

> https://codereview.appspot.com/57110043/diff/1/juju/osenv/proxy.go
> File juju/osenv/proxy.go (right):

https://codereview.appspot.com/57110043/diff/1/juju/osenv/proxy.go#newcode78
> juju/osenv/proxy.go:78: //
> Why both upper-case and lower-case?

It seems to be historical. To the best of my knowledge most processes
look for the
lower-case versions, but unfortunately not all. Better to be safe and
just set both.

I would love to be proved wrong, but at this stage, it seems like the
best approach.

https://codereview.appspot.com/57110043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'juju/osenv/proxy.go'
2--- juju/osenv/proxy.go 2014-01-23 03:51:10 +0000
3+++ juju/osenv/proxy.go 2014-01-26 20:22:40 +0000
4@@ -71,3 +71,19 @@
5 addLine("ftp_proxy", s.Ftp)
6 return lines
7 }
8+
9+// SetEnvironmentValues updates the process environment with the
10+// proxy values stored in the settings object. Both the lower-case
11+// and upper-case variants are set.
12+//
13+// http-proxy, HTTP_PROXY
14+// https-proxy, HTTPS_PROXY
15+// ftp-proxy, FTP_PROXY
16+func (s *ProxySettings) SetEnvironmentValues() {
17+ os.Setenv("http-proxy", s.Http)
18+ os.Setenv("HTTP-PROXY", s.Http)
19+ os.Setenv("https-proxy", s.Https)
20+ os.Setenv("HTTPS-PROXY", s.Https)
21+ os.Setenv("ftp-proxy", s.Ftp)
22+ os.Setenv("FTP-PROXY", s.Ftp)
23+}
24
25=== modified file 'juju/osenv/proxy_test.go'
26--- juju/osenv/proxy_test.go 2014-01-23 03:51:10 +0000
27+++ juju/osenv/proxy_test.go 2014-01-26 20:22:40 +0000
28@@ -4,6 +4,8 @@
29 package osenv_test
30
31 import (
32+ "os"
33+
34 gc "launchpad.net/gocheck"
35
36 "launchpad.net/juju-core/juju/osenv"
37@@ -151,3 +153,26 @@
38 }
39 c.Assert(proxies.AsEnvironmentValues(), gc.DeepEquals, expected)
40 }
41+
42+func (s *proxySuite) TestSetEnvironmentValues(c *gc.C) {
43+ s.PatchEnvironment("http_proxy", "initial")
44+ s.PatchEnvironment("HTTP_PROXY", "initial")
45+ s.PatchEnvironment("https_proxy", "initial")
46+ s.PatchEnvironment("HTTPS_PROXY", "initial")
47+ s.PatchEnvironment("ftp_proxy", "initial")
48+ s.PatchEnvironment("FTP_PROXY", "initial")
49+
50+ proxy := osenv.ProxySettings{
51+ Http: "http proxy",
52+ Https: "https proxy",
53+ // Ftp left blank to show clearing env.
54+ }
55+ proxy.SetEnvironmentValues()
56+
57+ c.Assert(os.Getenv("http-proxy"), gc.Equals, "http proxy")
58+ c.Assert(os.Getenv("HTTP-PROXY"), gc.Equals, "http proxy")
59+ c.Assert(os.Getenv("https-proxy"), gc.Equals, "https proxy")
60+ c.Assert(os.Getenv("HTTPS-PROXY"), gc.Equals, "https proxy")
61+ c.Assert(os.Getenv("ftp-proxy"), gc.Equals, "")
62+ c.Assert(os.Getenv("FTP-PROXY"), gc.Equals, "")
63+}
64
65=== modified file 'worker/uniter/uniter.go'
66--- worker/uniter/uniter.go 2014-01-23 21:57:42 +0000
67+++ worker/uniter/uniter.go 2014-01-26 20:22:40 +0000
68@@ -694,6 +694,8 @@
69 if u.proxy != newSettings {
70 u.proxy = newSettings
71 logger.Debugf("Updated proxy settings: %#v", u.proxy)
72+ // Update the environment values used by the process.
73+ u.proxy.SetEnvironmentValues()
74 }
75 }
76
77
78=== modified file 'worker/uniter/uniter_test.go'
79--- worker/uniter/uniter_test.go 2014-01-23 21:57:42 +0000
80+++ worker/uniter/uniter_test.go 2014-01-26 20:22:40 +0000
81@@ -1974,6 +1974,13 @@
82 expected := (osenv.ProxySettings)(s)
83 for attempt := coretesting.LongAttempt.Start(); attempt.Next(); {
84 if ctx.uniter.GetProxyValues() == expected {
85+ // Also confirm that the values were specified for the environment.
86+ c.Assert(os.Getenv("http-proxy"), gc.Equals, expected.Http)
87+ c.Assert(os.Getenv("HTTP-PROXY"), gc.Equals, expected.Http)
88+ c.Assert(os.Getenv("https-proxy"), gc.Equals, expected.Https)
89+ c.Assert(os.Getenv("HTTPS-PROXY"), gc.Equals, expected.Https)
90+ c.Assert(os.Getenv("ftp-proxy"), gc.Equals, expected.Ftp)
91+ c.Assert(os.Getenv("FTP-PROXY"), gc.Equals, expected.Ftp)
92 return
93 }
94 }

Subscribers

People subscribed via source and target branches

to status/vote changes: