Skip to content

Commit eef01e7

Browse files
jujubotjujubot
committed
Merge pull request juju#5206 from ericsnowcurrently/fix-1566431-public-ip-address
Try both private *and* public API addresses (part 2). (fixes https://bugs.launchpad.net/juju-core/+bug/1566431) We have been trying only the private IP addresses (or falling back to public) when upgrading. We've been updating the agent config in that same way. In some situations the private address is not reachable so we must fall back to the public one. This patch makes that change. Note that this change makes a backward-incompatible API change. (reviews: http://reviews.vapour.ws/r/4576/)
2 parents 329f037 + e08ce48 commit eef01e7

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

62 files changed

+493
-269
lines changed

‎agent/agent.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ func (c *configInternal) SetAPIHostPorts(servers [][]network.HostPort) {
575575
}
576576
var addrs []string
577577
for _, serverHostPorts := range servers {
578-
hps := network.SelectInternalHostPorts(serverHostPorts, false)
578+
hps := network.PrioritizeInternalHostPorts(serverHostPorts, false)
579579
addrs = append(addrs, hps...)
580580
}
581581
c.apiDetails.addresses = addrs

‎agent/agent_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,7 @@ func (*suite) TestSetAPIHostPorts(c *gc.C) {
772772
c.Assert(addrs, gc.DeepEquals, []string{
773773
"0.1.0.1:1111",
774774
"0.1.0.2:1111",
775+
"host.com:1111",
775776
"0.2.0.1:2222",
776777
"0.2.0.2:2222",
777778
"0.4.0.1:4444",

‎api/client.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -426,14 +426,14 @@ func (c *Client) ResolveCharm(ref *charm.URL) (*charm.URL, error) {
426426
}
427427

428428
// UploadTools uploads tools at the specified location to the API server over HTTPS.
429-
func (c *Client) UploadTools(r io.ReadSeeker, vers version.Binary, additionalSeries ...string) (*tools.Tools, error) {
429+
func (c *Client) UploadTools(r io.ReadSeeker, vers version.Binary, additionalSeries ...string) (tools.List, error) {
430430
endpoint := fmt.Sprintf("/tools?binaryVersion=%s&series=%s", vers, strings.Join(additionalSeries, ","))
431431
contentType := "application/x-tar-gz"
432432
var resp params.ToolsResult
433433
if err := c.httpPost(r, endpoint, contentType, &resp); err != nil {
434434
return nil, errors.Trace(err)
435435
}
436-
return resp.Tools, nil
436+
return resp.ToolsList, nil
437437
}
438438

439439
func (c *Client) httpPost(content io.ReadSeeker, endpoint, contentType string, response interface{}) error {

‎api/upgrader/unitupgrader_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,10 @@ func (s *unitUpgraderSuite) TestTools(c *gc.C) {
114114
s.rawMachine.SetAgentVersion(current)
115115
// UnitUpgrader.Tools returns the *desired* set of tools, not the currently
116116
// running set. We want to be upgraded to cur.Version
117-
stateTools, err := s.st.Tools(s.rawUnit.Tag().String())
117+
stateToolsList, err := s.st.Tools(s.rawUnit.Tag().String())
118118
c.Assert(err, jc.ErrorIsNil)
119+
c.Assert(stateToolsList, gc.HasLen, 1)
120+
stateTools := stateToolsList[0]
119121
c.Check(stateTools.Version.Number, gc.DeepEquals, current.Number)
120122
c.Assert(stateTools.URL, gc.NotNil)
121123
}

‎api/upgrader/upgrader.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func (st *State) DesiredVersion(tag string) (version.Number, error) {
7272

7373
// Tools returns the agent tools that should run on the given entity,
7474
// along with a flag whether to disable SSL hostname verification.
75-
func (st *State) Tools(tag string) (*tools.Tools, error) {
75+
func (st *State) Tools(tag string) (tools.List, error) {
7676
var results params.ToolsResults
7777
args := params.Entities{
7878
Entities: []params.Entity{{Tag: tag}},
@@ -90,7 +90,7 @@ func (st *State) Tools(tag string) (*tools.Tools, error) {
9090
if err := result.Error; err != nil {
9191
return nil, err
9292
}
93-
return result.Tools, nil
93+
return result.ToolsList, nil
9494
}
9595

9696
func (st *State) WatchAPIVersion(agentTag string) (watcher.NotifyWatcher, error) {

‎api/upgrader/upgrader_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,10 @@ func (s *machineUpgraderSuite) TestTools(c *gc.C) {
9595
s.rawMachine.SetAgentVersion(current)
9696
// Upgrader.Tools returns the *desired* set of tools, not the currently
9797
// running set. We want to be upgraded to cur.Version
98-
stateTools, err := s.st.Tools(s.rawMachine.Tag().String())
98+
stateToolsList, err := s.st.Tools(s.rawMachine.Tag().String())
9999
c.Assert(err, jc.ErrorIsNil)
100+
c.Assert(stateToolsList, gc.HasLen, 1)
101+
stateTools := stateToolsList[0]
100102
c.Assert(stateTools.Version, gc.Equals, current)
101103
url := fmt.Sprintf("https://%s/model/%s/tools/%s",
102104
s.stateAPI.Addr(), coretesting.ModelTag.Id(), current)

‎apiserver/client/instanceconfig.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func InstanceConfig(st *state.State, machineId, nonce, dataDir string) (*instanc
6666
if findToolsResult.Error != nil {
6767
return nil, errors.Annotate(findToolsResult.Error, "finding tools")
6868
}
69-
tools := findToolsResult.List[0]
69+
toolsList := findToolsResult.List
7070

7171
// Get the API connection info; attempt all API addresses.
7272
apiHostPorts, err := st.APIHostPorts()
@@ -112,7 +112,9 @@ func InstanceConfig(st *state.State, machineId, nonce, dataDir string) (*instanc
112112
if dataDir != "" {
113113
icfg.DataDir = dataDir
114114
}
115-
icfg.Tools = tools
115+
if err := icfg.SetTools(toolsList); err != nil {
116+
return nil, errors.Trace(err)
117+
}
116118
err = instancecfg.FinishInstanceConfig(icfg, environConfig)
117119
if err != nil {
118120
return nil, errors.Annotate(err, "finishing instance config")

‎apiserver/client/instanceconfig_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strconv"
1010

1111
jc "github.com/juju/testing/checkers"
12+
"github.com/juju/version"
1213
gc "gopkg.in/check.v1"
1314

1415
"github.com/juju/juju/agent"
@@ -56,8 +57,11 @@ func (s *machineConfigSuite) TestMachineConfig(c *gc.C) {
5657
c.Check(instanceConfig.MongoInfo.Addrs, gc.DeepEquals, mongoAddrs)
5758
c.Check(instanceConfig.APIInfo.Addrs, gc.DeepEquals, apiAddrs)
5859
toolsURL := fmt.Sprintf("https://%s/model/%s/tools/%s",
59-
apiAddrs[0], jujutesting.ModelTag.Id(), instanceConfig.Tools.Version)
60-
c.Assert(instanceConfig.Tools.URL, gc.Equals, toolsURL)
60+
apiAddrs[0], jujutesting.ModelTag.Id(), instanceConfig.ToolsInfo().Version)
61+
c.Assert(instanceConfig.ToolsList().URLs(), jc.DeepEquals, map[version.Binary][]string{
62+
instanceConfig.ToolsInfo().Version: []string{toolsURL},
63+
})
64+
//c.Assert(instanceConfig.ToolsList()[0].URL, gc.Equals, toolsURL)
6165
c.Assert(instanceConfig.AgentEnvironment[agent.AllowsSecureConnection], gc.Equals, "true")
6266
}
6367

‎apiserver/common/addresses.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,20 @@ func (api *APIAddresser) WatchAPIHostPorts() (params.NotifyWatchResult, error) {
6060

6161
// APIAddresses returns the list of addresses used to connect to the API.
6262
func (api *APIAddresser) APIAddresses() (params.StringsResult, error) {
63-
apiHostPorts, err := api.getter.APIHostPorts()
63+
addrs, err := apiAddresses(api.getter)
6464
if err != nil {
6565
return params.StringsResult{}, err
6666
}
67+
return params.StringsResult{
68+
Result: addrs,
69+
}, nil
70+
}
71+
72+
func apiAddresses(getter APIHostPortsGetter) ([]string, error) {
73+
apiHostPorts, err := getter.APIHostPorts()
74+
if err != nil {
75+
return nil, err
76+
}
6777
var addrs = make([]string, 0, len(apiHostPorts))
6878
for _, hostPorts := range apiHostPorts {
6979
ordered := network.PrioritizeInternalHostPorts(hostPorts, false)
@@ -73,9 +83,7 @@ func (api *APIAddresser) APIAddresses() (params.StringsResult, error) {
7383
}
7484
}
7585
}
76-
return params.StringsResult{
77-
Result: addrs,
78-
}, nil
86+
return addrs, nil
7987
}
8088

8189
// CACert returns the certificate used to validate the state connection.

‎apiserver/common/tools.go

Lines changed: 30 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package common
55

66
import (
77
"fmt"
8-
"math/rand"
98
"sort"
109

1110
"github.com/juju/errors"
@@ -26,9 +25,9 @@ var envtoolsFindTools = envtools.FindTools
2625

2726
// ToolsURLGetter is an interface providing the ToolsURL method.
2827
type ToolsURLGetter interface {
29-
// ToolsURL returns a URL for the tools with
28+
// ToolsURLs returns URLs for the tools with
3029
// the specified binary version.
31-
ToolsURL(v version.Binary) (string, error)
30+
ToolsURLs(v version.Binary) ([]string, error)
3231
}
3332

3433
type ModelConfigGetter interface {
@@ -88,9 +87,9 @@ func (t *ToolsGetter) Tools(args params.Entities) (params.ToolsResults, error) {
8887
result.Results[i].Error = ServerError(ErrPerm)
8988
continue
9089
}
91-
agentTools, err := t.oneAgentTools(canRead, tag, agentVersion, toolsStorage)
90+
agentToolsList, err := t.oneAgentTools(canRead, tag, agentVersion, toolsStorage)
9291
if err == nil {
93-
result.Results[i].Tools = agentTools
92+
result.Results[i].ToolsList = agentToolsList
9493
// TODO(axw) Get rid of this in 1.22, when all upgraders
9594
// are known to ignore the flag.
9695
result.Results[i].DisableSSLHostnameVerification = true
@@ -114,7 +113,7 @@ func (t *ToolsGetter) getGlobalAgentVersion() (version.Number, error) {
114113
return agentVersion, nil
115114
}
116115

117-
func (t *ToolsGetter) oneAgentTools(canRead AuthFunc, tag names.Tag, agentVersion version.Number, storage binarystorage.Storage) (*coretools.Tools, error) {
116+
func (t *ToolsGetter) oneAgentTools(canRead AuthFunc, tag names.Tag, agentVersion version.Number, storage binarystorage.Storage) (coretools.List, error) {
118117
if !canRead(tag) {
119118
return nil, ErrPerm
120119
}
@@ -141,7 +140,7 @@ func (t *ToolsGetter) oneAgentTools(canRead AuthFunc, tag names.Tag, agentVersio
141140
if err != nil {
142141
return nil, err
143142
}
144-
return list[0], nil
143+
return list, nil
145144
}
146145

147146
// ToolsSetter implements a common Tools method for use by various
@@ -227,17 +226,22 @@ func (f *ToolsFinder) findTools(args params.FindToolsParams) (coretools.List, er
227226
if err != nil {
228227
return nil, err
229228
}
230-
// Rewrite the URLs so they point at the API server. If the
229+
// Rewrite the URLs so they point at the API servers. If the
231230
// tools are not in tools storage, then the API server will
232231
// download and cache them if the client requests that version.
233-
for _, tools := range list {
234-
url, err := f.urlGetter.ToolsURL(tools.Version)
232+
var fullList coretools.List
233+
for _, baseTools := range list {
234+
urls, err := f.urlGetter.ToolsURLs(baseTools.Version)
235235
if err != nil {
236236
return nil, err
237237
}
238-
tools.URL = url
238+
for _, url := range urls {
239+
tools := *baseTools
240+
tools.URL = url
241+
fullList = append(fullList, &tools)
242+
}
239243
}
240-
return list, nil
244+
return fullList, nil
241245
}
242246

243247
// findMatchingTools searches tools storage and simplestreams for tools matching the
@@ -348,30 +352,21 @@ func NewToolsURLGetter(modelUUID string, a APIHostPortsGetter) *toolsURLGetter {
348352
return &toolsURLGetter{modelUUID, a}
349353
}
350354

351-
func (t *toolsURLGetter) ToolsURL(v version.Binary) (string, error) {
352-
apiHostPorts, err := t.apiHostPortsGetter.APIHostPorts()
355+
func (t *toolsURLGetter) ToolsURLs(v version.Binary) ([]string, error) {
356+
addrs, err := apiAddresses(t.apiHostPortsGetter)
353357
if err != nil {
354-
return "", err
355-
}
356-
if len(apiHostPorts) == 0 {
357-
return "", errors.New("no API host ports")
358-
}
359-
// TODO(axw) return all known URLs, so clients can try each one.
360-
//
361-
// The clients currently accept a single URL; we should change
362-
// the clients to disregard the URL, and have them download
363-
// straight from the API server.
364-
//
365-
// For now we choose a API server at random, and then select its
366-
// cloud-local address. The only user that will care about the URL
367-
// is the upgrader, and that is cloud-local.
368-
hostPorts := apiHostPorts[rand.Int()%len(apiHostPorts)]
369-
apiAddress := network.SelectInternalHostPort(hostPorts, false)
370-
if apiAddress == "" {
371-
return "", errors.Errorf("no suitable API server address to pick from %v", hostPorts)
372-
}
373-
serverRoot := fmt.Sprintf("https://%s/model/%s", apiAddress, t.modelUUID)
374-
return ToolsURL(serverRoot, v), nil
358+
return nil, err
359+
}
360+
if len(addrs) == 0 {
361+
return nil, errors.Errorf("no suitable API server address to pick from")
362+
}
363+
var urls []string
364+
for _, addr := range addrs {
365+
serverRoot := fmt.Sprintf("https://%s/model/%s", addr, t.modelUUID)
366+
url := ToolsURL(serverRoot, v)
367+
urls = append(urls, url)
368+
}
369+
return urls, nil
375370
}
376371

377372
// ToolsURL returns a tools URL pointing the API server

‎apiserver/common/tools_test.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,10 @@ func (s *toolsSuite) TestTools(c *gc.C) {
6969
c.Assert(err, jc.ErrorIsNil)
7070
c.Assert(result.Results, gc.HasLen, 3)
7171
c.Assert(result.Results[0].Error, gc.IsNil)
72-
c.Assert(result.Results[0].Tools, gc.NotNil)
73-
c.Assert(result.Results[0].Tools.Version, gc.DeepEquals, current)
74-
c.Assert(result.Results[0].Tools.URL, gc.Equals, "tools:"+current.String())
72+
c.Assert(result.Results[0].ToolsList, gc.HasLen, 1)
73+
tools := result.Results[0].ToolsList[0]
74+
c.Assert(tools.Version, gc.DeepEquals, current)
75+
c.Assert(tools.URL, gc.Equals, "tools:"+current.String())
7576
c.Assert(result.Results[0].DisableSSLHostnameVerification, jc.IsTrue)
7677
c.Assert(result.Results[1].Error, gc.DeepEquals, apiservertesting.ErrUnauthorized)
7778
c.Assert(result.Results[2].Error, gc.DeepEquals, apiservertesting.NotFoundError("machine 42"))
@@ -183,14 +184,17 @@ func (s *toolsSuite) TestFindTools(c *gc.C) {
183184
})
184185
c.Assert(err, jc.ErrorIsNil)
185186
c.Assert(result.Error, gc.IsNil)
186-
c.Assert(result.List, gc.DeepEquals, coretools.List{
187+
c.Check(result.List, jc.DeepEquals, coretools.List{
187188
&coretools.Tools{
188189
Version: version.MustParseBinary(storageMetadata[0].Version),
189190
Size: storageMetadata[0].Size,
190191
SHA256: storageMetadata[0].SHA256,
191192
URL: "tools:" + storageMetadata[0].Version,
192193
},
193-
envtoolsList[1],
194+
&coretools.Tools{
195+
Version: version.MustParseBinary("123.456.1-win81-alpha"),
196+
URL: "tools:123.456.1-win81-alpha",
197+
},
194198
})
195199
}
196200

@@ -283,13 +287,13 @@ func (s *toolsSuite) TestFindToolsToolsStorageError(c *gc.C) {
283287

284288
func (s *toolsSuite) TestToolsURLGetterNoAPIHostPorts(c *gc.C) {
285289
g := common.NewToolsURLGetter("my-uuid", mockAPIHostPortsGetter{})
286-
_, err := g.ToolsURL(current)
287-
c.Assert(err, gc.ErrorMatches, "no API host ports")
290+
_, err := g.ToolsURLs(current)
291+
c.Assert(err, gc.ErrorMatches, "no suitable API server address to pick from")
288292
}
289293

290294
func (s *toolsSuite) TestToolsURLGetterAPIHostPortsError(c *gc.C) {
291295
g := common.NewToolsURLGetter("my-uuid", mockAPIHostPortsGetter{err: errors.New("oh noes")})
292-
_, err := g.ToolsURL(current)
296+
_, err := g.ToolsURLs(current)
293297
c.Assert(err, gc.ErrorMatches, "oh noes")
294298
}
295299

@@ -299,15 +303,17 @@ func (s *toolsSuite) TestToolsURLGetter(c *gc.C) {
299303
network.NewHostPorts(1234, "0.1.2.3"),
300304
},
301305
})
302-
url, err := g.ToolsURL(current)
306+
urls, err := g.ToolsURLs(current)
303307
c.Assert(err, jc.ErrorIsNil)
304-
c.Assert(url, gc.Equals, "https://0.1.2.3:1234/model/my-uuid/tools/"+current.String())
308+
c.Check(urls, jc.DeepEquals, []string{
309+
"https://0.1.2.3:1234/model/my-uuid/tools/" + current.String(),
310+
})
305311
}
306312

307313
type sprintfURLGetter string
308314

309-
func (s sprintfURLGetter) ToolsURL(v version.Binary) (string, error) {
310-
return fmt.Sprintf(string(s), v), nil
315+
func (s sprintfURLGetter) ToolsURLs(v version.Binary) ([]string, error) {
316+
return []string{fmt.Sprintf(string(s), v)}, nil
311317
}
312318

313319
type mockAPIHostPortsGetter struct {

‎apiserver/params/internal.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ type VersionResults struct {
397397
// ToolsResult holds the tools and possibly error for a given
398398
// Tools() API call.
399399
type ToolsResult struct {
400-
Tools *tools.Tools
400+
ToolsList tools.List
401401
DisableSSLHostnameVerification bool
402402
Error *Error
403403
}

‎apiserver/tools.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ func (h *toolsUploadHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
7474
sendError(w, err)
7575
return
7676
}
77-
sendStatusAndJSON(w, http.StatusOK, &params.ToolsResult{Tools: agentTools})
77+
sendStatusAndJSON(w, http.StatusOK, &params.ToolsResult{
78+
ToolsList: tools.List{agentTools},
79+
})
7880
default:
7981
sendError(w, errors.MethodNotAllowedf("unsupported method: %q", r.Method))
8082
}

‎apiserver/tools_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func (s *toolsCommonSuite) downloadRequest(c *gc.C, version version.Binary, uuid
7272
func (s *toolsCommonSuite) assertUploadResponse(c *gc.C, resp *http.Response, agentTools *coretools.Tools) {
7373
toolsResponse := s.assertResponse(c, resp, http.StatusOK)
7474
c.Check(toolsResponse.Error, gc.IsNil)
75-
c.Check(toolsResponse.Tools, gc.DeepEquals, agentTools)
75+
c.Check(toolsResponse.ToolsList, jc.DeepEquals, coretools.List{agentTools})
7676
}
7777

7878
func (s *toolsCommonSuite) assertGetFileResponse(c *gc.C, resp *http.Response, expBody, expContentType string) {

‎apiserver/upgrader/unitupgrader.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/juju/juju/apiserver/params"
1212
"github.com/juju/juju/state"
1313
"github.com/juju/juju/state/watcher"
14+
"github.com/juju/juju/tools"
1415
)
1516

1617
// UnitUpgraderAPI provides access to the UnitUpgrader API facade.
@@ -152,7 +153,10 @@ func (u *UnitUpgraderAPI) getMachineTools(tag names.Tag) params.ToolsResult {
152153
result.Error = common.ServerError(err)
153154
return result
154155
}
155-
result.Tools = machineTools
156+
// We are okay returning the tools for just the one API server
157+
// address since the unit agent won't try to download tools that
158+
// are already present on the machine.
159+
result.ToolsList = tools.List{machineTools}
156160
return result
157161
}
158162

0 commit comments

Comments
 (0)