juju-core

Merge lp:~thumper/juju-core/fix-uniter-proxy-env-vars into lp:~go-bot/juju-core/trunk

  1. fix-uniter-proxy-env-vars
  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: 2264
Proposed branch: lp:~thumper/juju-core/fix-uniter-proxy-env-vars
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 222 lines (+99/-36)
6 files modified
environs/config/config.go (+20/-0)
environs/config/config_test.go (+31/-0)
juju/osenv/proxy.go (+26/-18)
juju/osenv/proxy_test.go (+10/-6)
provider/local/environprovider_test.go (+6/-6)
worker/uniter/uniter_test.go (+6/-6)
To merge this branch: bzr merge lp:~thumper/juju-core/fix-uniter-proxy-env-vars
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+203459@code.launchpad.net

Commit message

Fix the proxy env vars in the uniter.

The shell variables exported in the uniter are
incorrect. They should have underscores not
dashes.

In an attempt to reduce the likelihood of this
happening again, moved the proxy code to use string
constants.

https://codereview.appspot.com/57590043/

Description of the change

Fix the proxy env vars in the uniter.

The shell variables exported in the uniter are
incorrect. They should have underscores not
dashes.

In an attempt to reduce the likelihood of this
happening again, moved the proxy code to use string
constants.

https://codereview.appspot.com/57590043/

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

Reviewers: mp+203459_code.launchpad.net,

Message:
Please take a look.

Description:
Fix the proxy env vars in the uniter.

The shell variables exported in the uniter are
incorrect. They should have underscores not
dashes.

In an attempt to reduce the likelihood of this
happening again, moved the proxy code to use string
constants.

https://code.launchpad.net/~thumper/juju-core/fix-uniter-proxy-env-vars/+merge/203459

(do not edit description out of merge proposal)

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

Affected files (+101, -36 lines):
   A [revision details]
   M environs/config/config.go
   M environs/config/config_test.go
   M juju/osenv/proxy.go
   M juju/osenv/proxy_test.go
   M provider/local/environprovider_test.go
   M worker/uniter/uniter_test.go

Revision history for this message
Ian Booth (wallyworld) wrote :

LGTM

https://codereview.appspot.com/57590043/

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (56.7 KiB)

The attempt to merge lp:~thumper/juju-core/fix-uniter-proxy-env-vars into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 3.101s
ok launchpad.net/juju-core/agent/tools 0.397s
ok launchpad.net/juju-core/bzr 10.755s
ok launchpad.net/juju-core/cert 3.425s
ok launchpad.net/juju-core/charm 1.796s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.033s
ok launchpad.net/juju-core/cloudinit/sshinit 1.092s
ok launchpad.net/juju-core/cmd 0.234s
ok launchpad.net/juju-core/cmd/charm-admin 0.892s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 266.615s

----------------------------------------------------------------------
FAIL: machine_test.go:329: MachineSuite.TestManageEnvironRunsInstancePoller

[LOG] 72.49412 DEBUG juju.environs.configstore Making /tmp/gocheck-4751997750760398084/64/home/ubuntu/.juju/environments
[LOG] 72.66601 DEBUG juju.environs.tools reading v1.* tools
[LOG] 72.66606 INFO juju environs/testing: uploading FAKE tools 1.17.1-precise-amd64
[LOG] 72.68618 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 72.68621 DEBUG juju.environs.tools no series specified when finding tools, looking for any
[LOG] 72.68651 DEBUG juju.environs.simplestreams fetchData failed for "http://127.0.0.1:36547/dummyenv/private/tools/streams/v1/index.sjson": file "tools/streams/v1/index.sjson" not found not found
[LOG] 72.68654 DEBUG juju.environs.simplestreams cannot load index "http://127.0.0.1:36547/dummyenv/private/tools/streams/v1/index.sjson": invalid URL "http://127.0.0.1:36547/dummyenv/private/tools/streams/v1/index.sjson" not found
[LOG] 72.68671 DEBUG juju.environs.simplestreams fetchData failed for "http://127.0.0.1:36547/dummyenv/private/tools/streams/v1/index.json": file "tools/streams/v1/index.json" not found not found
[LOG] 72.68673 DEBUG juju.environs.simplestreams cannot load index "http://127.0.0.1:36547/dummyenv/private/tools/streams/v1/index.json": invalid URL "http://127.0.0.1:36547/dummyenv/private/tools/streams/v1/index.json" not found
[LOG] 72.68771 INFO juju.environs.tools Writing tools/streams/v1/index.json
[LOG] 72.68786 INFO juju.environs.tools Writing tools/streams/v1/com.ubuntu.juju:released:tools.json
[LOG] 72.68802 INFO juju.environs.bootstrap bootstrapping environment "dummyenv"
[LOG] 72.68806 DEBUG juju.environs.bootstrap looking for bootstrap tools: series="precise", arch=<nil>, version=1.17.1
[LOG] 72.68807 INFO juju.environs.tools reading tools with major.minor version 1.17
[LOG] 72.68809 INFO juju.environs.tools filtering tools by version: 1.17.1
[LOG] 72.68809 INFO juju.environs.tools filtering tools by series: precise
[LOG] 72.68811 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 72.68816 DEBUG juju.environs.simplestreams fetchData failed for "http://127.0.0.1:36547/dummyenv/private/tools/streams/v1/index.sjson": file "tools/streams/v1/index.sjson" not found not ...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'environs/config/config.go'
--- environs/config/config.go 2014-01-22 23:36:28 +0000
+++ environs/config/config.go 2014-01-28 03:57:22 +0000
@@ -788,3 +788,23 @@
788 }788 }
789 return repo789 return repo
790}790}
791
792// ProxyConfigMap returns a map suitable to be applied to a Config to update
793// proxy settings.
794func ProxyConfigMap(proxy osenv.ProxySettings) map[string]interface{} {
795 return map[string]interface{}{
796 "http-proxy": proxy.Http,
797 "https-proxy": proxy.Https,
798 "ftp-proxy": proxy.Ftp,
799 }
800}
801
802// AptProxyConfigMap returns a map suitable to be applied to a Config to update
803// proxy settings.
804func AptProxyConfigMap(proxy osenv.ProxySettings) map[string]interface{} {
805 return map[string]interface{}{
806 "apt-http-proxy": proxy.Http,
807 "apt-https-proxy": proxy.Https,
808 "apt-ftp-proxy": proxy.Ftp,
809 }
810}
791811
=== modified file 'environs/config/config_test.go'
--- environs/config/config_test.go 2014-01-24 16:56:17 +0000
+++ environs/config/config_test.go 2014-01-28 03:57:22 +0000
@@ -1137,6 +1137,37 @@
1137 c.Assert(config.AptFtpProxy(), gc.Equals, "")1137 c.Assert(config.AptFtpProxy(), gc.Equals, "")
1138}1138}
11391139
1140func (*ConfigSuite) TestProxyConfigMap(c *gc.C) {
1141 defer makeFakeHome(c).Restore()
1142
1143 cfg := newTestConfig(c, testing.Attrs{})
1144 proxy := osenv.ProxySettings{
1145 Http: "http proxy",
1146 Https: "https proxy",
1147 Ftp: "ftp proxy",
1148 }
1149 cfg, err := cfg.Apply(config.ProxyConfigMap(proxy))
1150 c.Assert(err, gc.IsNil)
1151 c.Assert(cfg.ProxySettings(), gc.DeepEquals, proxy)
1152 c.Assert(cfg.AptProxySettings(), gc.DeepEquals, proxy)
1153}
1154
1155func (*ConfigSuite) TestAptProxyConfigMap(c *gc.C) {
1156 defer makeFakeHome(c).Restore()
1157
1158 cfg := newTestConfig(c, testing.Attrs{})
1159 proxy := osenv.ProxySettings{
1160 Http: "http proxy",
1161 Https: "https proxy",
1162 Ftp: "ftp proxy",
1163 }
1164 cfg, err := cfg.Apply(config.AptProxyConfigMap(proxy))
1165 c.Assert(err, gc.IsNil)
1166 // The default proxy settings should still be empty.
1167 c.Assert(cfg.ProxySettings(), gc.DeepEquals, osenv.ProxySettings{})
1168 c.Assert(cfg.AptProxySettings(), gc.DeepEquals, proxy)
1169}
1170
1140func (*ConfigSuite) TestGenerateStateServerCertAndKey(c *gc.C) {1171func (*ConfigSuite) TestGenerateStateServerCertAndKey(c *gc.C) {
1141 // In order to test missing certs, it checks the JUJU_HOME dir, so we need1172 // In order to test missing certs, it checks the JUJU_HOME dir, so we need
1142 // a fake home.1173 // a fake home.
11431174
=== modified file 'juju/osenv/proxy.go'
--- juju/osenv/proxy.go 2014-01-26 20:07:50 +0000
+++ juju/osenv/proxy.go 2014-01-28 03:57:22 +0000
@@ -9,6 +9,13 @@
9 "strings"9 "strings"
10)10)
1111
12const (
13 // Remove the likelihood of errors by mistyping string values.
14 http_proxy = "http_proxy"
15 https_proxy = "https_proxy"
16 ftp_proxy = "ftp_proxy"
17)
18
12// ProxySettings holds the values for the http, https and ftp proxies found by19// ProxySettings holds the values for the http, https and ftp proxies found by
13// Detect Proxies.20// Detect Proxies.
14type ProxySettings struct {21type ProxySettings struct {
@@ -28,9 +35,9 @@
28// DetectProxies returns the proxy settings found the environment.35// DetectProxies returns the proxy settings found the environment.
29func DetectProxies() ProxySettings {36func DetectProxies() ProxySettings {
30 return ProxySettings{37 return ProxySettings{
31 Http: getProxySetting("http_proxy"),38 Http: getProxySetting(http_proxy),
32 Https: getProxySetting("https_proxy"),39 Https: getProxySetting(https_proxy),
33 Ftp: getProxySetting("ftp_proxy"),40 Ftp: getProxySetting(ftp_proxy),
34 }41 }
35}42}
3643
@@ -47,9 +54,9 @@
47 fmt.Sprintf("export %s=%s", strings.ToUpper(proxy), value))54 fmt.Sprintf("export %s=%s", strings.ToUpper(proxy), value))
48 }55 }
49 }56 }
50 addLine("http_proxy", s.Http)57 addLine(http_proxy, s.Http)
51 addLine("https_proxy", s.Https)58 addLine(https_proxy, s.Https)
52 addLine("ftp_proxy", s.Ftp)59 addLine(ftp_proxy, s.Ftp)
53 return strings.Join(lines, "\n")60 return strings.Join(lines, "\n")
54}61}
5562
@@ -66,9 +73,9 @@
66 fmt.Sprintf("%s=%s", strings.ToUpper(proxy), value))73 fmt.Sprintf("%s=%s", strings.ToUpper(proxy), value))
67 }74 }
68 }75 }
69 addLine("http_proxy", s.Http)76 addLine(http_proxy, s.Http)
70 addLine("https_proxy", s.Https)77 addLine(https_proxy, s.Https)
71 addLine("ftp_proxy", s.Ftp)78 addLine(ftp_proxy, s.Ftp)
72 return lines79 return lines
73}80}
7481
@@ -76,14 +83,15 @@
76// proxy values stored in the settings object. Both the lower-case83// proxy values stored in the settings object. Both the lower-case
77// and upper-case variants are set.84// and upper-case variants are set.
78//85//
79// http-proxy, HTTP_PROXY86// http_proxy, HTTP_PROXY
80// https-proxy, HTTPS_PROXY87// https_proxy, HTTPS_PROXY
81// ftp-proxy, FTP_PROXY88// ftp_proxy, FTP_PROXY
82func (s *ProxySettings) SetEnvironmentValues() {89func (s *ProxySettings) SetEnvironmentValues() {
83 os.Setenv("http-proxy", s.Http)90 setenv := func(proxy, value string) {
84 os.Setenv("HTTP-PROXY", s.Http)91 os.Setenv(proxy, value)
85 os.Setenv("https-proxy", s.Https)92 os.Setenv(strings.ToUpper(proxy), value)
86 os.Setenv("HTTPS-PROXY", s.Https)93 }
87 os.Setenv("ftp-proxy", s.Ftp)94 setenv(http_proxy, s.Http)
88 os.Setenv("FTP-PROXY", s.Ftp)95 setenv(https_proxy, s.Https)
96 setenv(ftp_proxy, s.Ftp)
89}97}
9098
=== modified file 'juju/osenv/proxy_test.go'
--- juju/osenv/proxy_test.go 2014-01-26 20:07:50 +0000
+++ juju/osenv/proxy_test.go 2014-01-28 03:57:22 +0000
@@ -169,10 +169,14 @@
169 }169 }
170 proxy.SetEnvironmentValues()170 proxy.SetEnvironmentValues()
171171
172 c.Assert(os.Getenv("http-proxy"), gc.Equals, "http proxy")172 obtained := osenv.DetectProxies()
173 c.Assert(os.Getenv("HTTP-PROXY"), gc.Equals, "http proxy")173
174 c.Assert(os.Getenv("https-proxy"), gc.Equals, "https proxy")174 c.Assert(obtained, gc.DeepEquals, proxy)
175 c.Assert(os.Getenv("HTTPS-PROXY"), gc.Equals, "https proxy")175
176 c.Assert(os.Getenv("ftp-proxy"), gc.Equals, "")176 c.Assert(os.Getenv("http_proxy"), gc.Equals, "http proxy")
177 c.Assert(os.Getenv("FTP-PROXY"), gc.Equals, "")177 c.Assert(os.Getenv("HTTP_PROXY"), gc.Equals, "http proxy")
178 c.Assert(os.Getenv("https_proxy"), gc.Equals, "https proxy")
179 c.Assert(os.Getenv("HTTPS_PROXY"), gc.Equals, "https proxy")
180 c.Assert(os.Getenv("ftp_proxy"), gc.Equals, "")
181 c.Assert(os.Getenv("FTP_PROXY"), gc.Equals, "")
178}182}
179183
=== modified file 'provider/local/environprovider_test.go'
--- provider/local/environprovider_test.go 2014-01-23 08:27:40 +0000
+++ provider/local/environprovider_test.go 2014-01-28 03:57:22 +0000
@@ -46,12 +46,12 @@
46func (s *prepareSuite) SetUpTest(c *gc.C) {46func (s *prepareSuite) SetUpTest(c *gc.C) {
47 s.FakeHomeSuite.SetUpTest(c)47 s.FakeHomeSuite.SetUpTest(c)
48 loggo.GetLogger("juju.provider.local").SetLogLevel(loggo.TRACE)48 loggo.GetLogger("juju.provider.local").SetLogLevel(loggo.TRACE)
49 s.PatchEnvironment("http-proxy", "")49 s.PatchEnvironment("http_proxy", "")
50 s.PatchEnvironment("HTTP-PROXY", "")50 s.PatchEnvironment("HTTP_PROXY", "")
51 s.PatchEnvironment("https-proxy", "")51 s.PatchEnvironment("https_proxy", "")
52 s.PatchEnvironment("HTTPS-PROXY", "")52 s.PatchEnvironment("HTTPS_PROXY", "")
53 s.PatchEnvironment("ftp-proxy", "")53 s.PatchEnvironment("ftp_proxy", "")
54 s.PatchEnvironment("FTP-PROXY", "")54 s.PatchEnvironment("FTP_PROXY", "")
55 s.HookCommandOutput(&utils.AptCommandOutput, nil, nil)55 s.HookCommandOutput(&utils.AptCommandOutput, nil, nil)
56}56}
5757
5858
=== modified file 'worker/uniter/uniter_test.go'
--- worker/uniter/uniter_test.go 2014-01-26 20:18:26 +0000
+++ worker/uniter/uniter_test.go 2014-01-28 03:57:22 +0000
@@ -1975,12 +1975,12 @@
1975 for attempt := coretesting.LongAttempt.Start(); attempt.Next(); {1975 for attempt := coretesting.LongAttempt.Start(); attempt.Next(); {
1976 if ctx.uniter.GetProxyValues() == expected {1976 if ctx.uniter.GetProxyValues() == expected {
1977 // Also confirm that the values were specified for the environment.1977 // Also confirm that the values were specified for the environment.
1978 c.Assert(os.Getenv("http-proxy"), gc.Equals, expected.Http)1978 c.Assert(os.Getenv("http_proxy"), gc.Equals, expected.Http)
1979 c.Assert(os.Getenv("HTTP-PROXY"), gc.Equals, expected.Http)1979 c.Assert(os.Getenv("HTTP_PROXY"), gc.Equals, expected.Http)
1980 c.Assert(os.Getenv("https-proxy"), gc.Equals, expected.Https)1980 c.Assert(os.Getenv("https_proxy"), gc.Equals, expected.Https)
1981 c.Assert(os.Getenv("HTTPS-PROXY"), gc.Equals, expected.Https)1981 c.Assert(os.Getenv("HTTPS_PROXY"), gc.Equals, expected.Https)
1982 c.Assert(os.Getenv("ftp-proxy"), gc.Equals, expected.Ftp)1982 c.Assert(os.Getenv("ftp_proxy"), gc.Equals, expected.Ftp)
1983 c.Assert(os.Getenv("FTP-PROXY"), gc.Equals, expected.Ftp)1983 c.Assert(os.Getenv("FTP_PROXY"), gc.Equals, expected.Ftp)
1984 return1984 return
1985 }1985 }
1986 }1986 }

Subscribers

People subscribed via source and target branches

to status/vote changes: