diff --git a/acceptancetests/repository/charms/mysql/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb b/acceptancetests/repository/charms/mysql/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb deleted file mode 100644 index d4486da0ea7..00000000000 Binary files a/acceptancetests/repository/charms/mysql/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb and /dev/null differ diff --git a/acceptancetests/repository/charms/wordpress/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb b/acceptancetests/repository/charms/wordpress/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb deleted file mode 100644 index d4486da0ea7..00000000000 Binary files a/acceptancetests/repository/charms/wordpress/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb and /dev/null differ diff --git a/acceptancetests/repository/trusty/jenkins-slave/files/jenkins-slave_1.424.6+dfsg-1ubuntu0.2_all.deb b/acceptancetests/repository/trusty/jenkins-slave/files/jenkins-slave_1.424.6+dfsg-1ubuntu0.2_all.deb deleted file mode 100644 index 1518fe09f10..00000000000 Binary files a/acceptancetests/repository/trusty/jenkins-slave/files/jenkins-slave_1.424.6+dfsg-1ubuntu0.2_all.deb and /dev/null differ diff --git a/acceptancetests/repository/trusty/mysql/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb b/acceptancetests/repository/trusty/mysql/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb deleted file mode 100644 index d4486da0ea7..00000000000 Binary files a/acceptancetests/repository/trusty/mysql/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb and /dev/null differ diff --git a/acceptancetests/repository/trusty/wordpress/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb b/acceptancetests/repository/trusty/wordpress/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb deleted file mode 100644 index d4486da0ea7..00000000000 Binary files a/acceptancetests/repository/trusty/wordpress/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb and /dev/null differ diff --git a/acceptancetests/repository/xenial/jenkins-slave/files/jenkins-slave_1.424.6+dfsg-1ubuntu0.2_all.deb b/acceptancetests/repository/xenial/jenkins-slave/files/jenkins-slave_1.424.6+dfsg-1ubuntu0.2_all.deb deleted file mode 100644 index 1518fe09f10..00000000000 Binary files a/acceptancetests/repository/xenial/jenkins-slave/files/jenkins-slave_1.424.6+dfsg-1ubuntu0.2_all.deb and /dev/null differ diff --git a/acceptancetests/repository/xenial/mysql/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb b/acceptancetests/repository/xenial/mysql/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb deleted file mode 100644 index d4486da0ea7..00000000000 Binary files a/acceptancetests/repository/xenial/mysql/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb and /dev/null differ diff --git a/acceptancetests/repository/xenial/wordpress/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb b/acceptancetests/repository/xenial/wordpress/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb deleted file mode 100644 index d4486da0ea7..00000000000 Binary files a/acceptancetests/repository/xenial/wordpress/exec.d/charm-helper/charm-helper-sh_0.3+bzr179-3~precise1_all.deb and /dev/null differ diff --git a/api/client/machinemanager/machinemanager.go b/api/client/machinemanager/machinemanager.go index 5e216bf3b7c..f8816a37ae3 100644 --- a/api/client/machinemanager/machinemanager.go +++ b/api/client/machinemanager/machinemanager.go @@ -142,11 +142,13 @@ func (c *Client) ProvisioningScript(args params.ProvisioningScriptParams) (scrip // RetryProvisioning updates the provisioning status of a machine allowing the // provisioner to retry. -func (c *Client) RetryProvisioning(machines ...names.MachineTag) ([]params.ErrorResult, error) { - p := params.Entities{} - p.Entities = make([]params.Entity, len(machines)) +func (c *Client) RetryProvisioning(all bool, machines ...names.MachineTag) ([]params.ErrorResult, error) { + p := params.RetryProvisioningArgs{ + All: all, + } + p.Machines = make([]string, len(machines)) for i, machine := range machines { - p.Entities[i] = params.Entity{Tag: machine.String()} + p.Machines[i] = machine.String() } var results params.ErrorResults err := c.facade.FacadeCall("RetryProvisioning", p, &results) diff --git a/api/client/machinemanager/machinemanager_test.go b/api/client/machinemanager/machinemanager_test.go index e47c10f7dcf..e0466b57329 100644 --- a/api/client/machinemanager/machinemanager_test.go +++ b/api/client/machinemanager/machinemanager_test.go @@ -126,10 +126,10 @@ func (s *MachinemanagerSuite) TestRetryProvisioning(c *gc.C) { APICallerFunc: basetesting.APICallerFunc(func(objType string, version int, id, request string, a, response interface{}) error { c.Assert(request, gc.Equals, "RetryProvisioning") c.Assert(version, gc.Equals, 7) - c.Assert(a, jc.DeepEquals, params.Entities{ - Entities: []params.Entity{ - {Tag: "machine-0"}, - {Tag: "machine-1"}, + c.Assert(a, jc.DeepEquals, params.RetryProvisioningArgs{ + Machines: []string{ + "machine-0", + "machine-1", }, }) c.Assert(response, gc.FitsTypeOf, ¶ms.ErrorResults{}) @@ -140,7 +140,33 @@ func (s *MachinemanagerSuite) TestRetryProvisioning(c *gc.C) { } return nil })}) - result, err := client.RetryProvisioning(names.NewMachineTag("0"), names.NewMachineTag("1")) + result, err := client.RetryProvisioning(false, names.NewMachineTag("0"), names.NewMachineTag("1")) + c.Assert(err, jc.ErrorIsNil) + c.Assert(result, jc.DeepEquals, []params.ErrorResult{ + {¶ms.Error{Code: "boom"}}, + {}, + }) +} + +func (s *MachinemanagerSuite) TestRetryProvisioningAll(c *gc.C) { + client := machinemanager.NewClient( + basetesting.BestVersionCaller{ + BestVersion: 7, + APICallerFunc: basetesting.APICallerFunc(func(objType string, version int, id, request string, a, response interface{}) error { + c.Assert(request, gc.Equals, "RetryProvisioning") + c.Assert(version, gc.Equals, 7) + c.Assert(a, jc.DeepEquals, params.RetryProvisioningArgs{ + All: true, + }) + c.Assert(response, gc.FitsTypeOf, ¶ms.ErrorResults{}) + out := response.(*params.ErrorResults) + *out = params.ErrorResults{Results: []params.ErrorResult{ + {Error: ¶ms.Error{Code: "boom"}}, + {}}, + } + return nil + })}) + result, err := client.RetryProvisioning(true) c.Assert(err, jc.ErrorIsNil) c.Assert(result, jc.DeepEquals, []params.ErrorResult{ {¶ms.Error{Code: "boom"}}, diff --git a/apiserver/admin_test.go b/apiserver/admin_test.go index ecfa70254ac..878c6832e57 100644 --- a/apiserver/admin_test.go +++ b/apiserver/admin_test.go @@ -1496,7 +1496,7 @@ func (s *loginV3Suite) TestClientLoginToController(c *gc.C) { defer apiState.Close() client := machineclient.NewClient(apiState) - _, err = client.RetryProvisioning(names.NewMachineTag("machine-0")) + _, err = client.RetryProvisioning(false, names.NewMachineTag("machine-0")) c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{ Message: `facade "MachineManager" not supported for controller API connection`, Code: "not supported", diff --git a/apiserver/facades/agent/instancemutater/interface.go b/apiserver/facades/agent/instancemutater/interface.go index 6c0afa3b6d6..cf5d7e2b7df 100644 --- a/apiserver/facades/agent/instancemutater/interface.go +++ b/apiserver/facades/agent/instancemutater/interface.go @@ -54,7 +54,7 @@ type Unit interface { Application() (Application, error) PrincipalName() (string, bool) AssignedMachineId() (string, error) - CharmURL() (*charm.URL, error) + CharmURL() *string } // Charm represents point of use methods from the state Charm object. diff --git a/apiserver/facades/agent/instancemutater/lxdprofilewatcher.go b/apiserver/facades/agent/instancemutater/lxdprofilewatcher.go index 02935d57514..7f23fedce9e 100644 --- a/apiserver/facades/agent/instancemutater/lxdprofilewatcher.go +++ b/apiserver/facades/agent/instancemutater/lxdprofilewatcher.go @@ -449,12 +449,8 @@ func (w *machineLXDProfileWatcher) add(unit Unit) (bool, error) { _, ok := w.applications[appName] if !ok { - curl, err := unit.CharmURL() - if err != nil { - return false, errors.Trace(err) - } - - if curl == nil { + curlStr := unit.CharmURL() + if curlStr == nil { // this happens for new units to existing machines. app, err := unit.Application() if errors.IsNotFound(err) { @@ -463,22 +459,22 @@ func (w *machineLXDProfileWatcher) add(unit Unit) (bool, error) { } else if err != nil { return false, errors.Annotatef(err, "failed to get application %s for machine-%s", appName, w.machine.Id()) } - cURL := app.CharmURL() - curl, err = charm.ParseURL(*cURL) - if err != nil { - return false, errors.Annotatef(err, "application charm url") - } + curlStr = app.CharmURL() } + curl, err := charm.ParseURL(*curlStr) + if err != nil { + return false, errors.Annotatef(err, "application charm url") + } ch, err := w.backend.Charm(curl) if errors.IsNotFound(err) { - logger.Debugf("charm %s removed for %s on machine-%s", curl, unitName, w.machine.Id()) + logger.Debugf("charm %s removed for %s on machine-%s", *curlStr, unitName, w.machine.Id()) return false, nil } else if err != nil { - return false, errors.Annotatef(err, "failed to get charm %q for %s on machine-%s", curl, appName, w.machine.Id()) + return false, errors.Annotatef(err, "failed to get charm %q for %s on machine-%s", *curlStr, appName, w.machine.Id()) } info := appInfo{ - charmURL: curl.String(), + charmURL: *curlStr, units: set.NewStrings(unitName), } diff --git a/apiserver/facades/agent/instancemutater/lxdprofilewatcher_test.go b/apiserver/facades/agent/instancemutater/lxdprofilewatcher_test.go index e64593e6be2..1e7d40aaa0f 100644 --- a/apiserver/facades/agent/instancemutater/lxdprofilewatcher_test.go +++ b/apiserver/facades/agent/instancemutater/lxdprofilewatcher_test.go @@ -91,8 +91,8 @@ func (s *lxdProfileWatcherSuite) TestMachineLXDProfileWatcherProfile(c *gc.C) { defer workertest.CleanKill(c, s.assertStartLxdProfileWatcher(c)) s.setupPrincipalUnit() - curl := charm.MustParseURL("ch:name-me") - s.unit.EXPECT().CharmURL().Return(curl, nil) + curlStr := "ch:name-me" + s.unit.EXPECT().CharmURL().Return(&curlStr) s.unitChanges <- []string{"foo/0"} s.wc0.AssertOneChange() } @@ -151,8 +151,8 @@ func (s *lxdProfileWatcherSuite) TestMachineLXDProfileWatcherAddUnit(c *gc.C) { s.unit.EXPECT().Life().Return(state.Alive) s.unit.EXPECT().PrincipalName().Return("", false) s.unit.EXPECT().AssignedMachineId().Return("0", nil) - curl := charm.MustParseURL("ch:name-me") - s.unit.EXPECT().CharmURL().Return(curl, nil) + curlStr := "ch:name-me" + s.unit.EXPECT().CharmURL().Return(&curlStr) s.unitChanges <- []string{"bar/0"} s.wc0.AssertOneChange() } @@ -194,8 +194,8 @@ func (s *lxdProfileWatcherSuite) assertAddSubordinate() { s.unit.EXPECT().PrincipalName().Return("principal/0", true) s.unit.EXPECT().AssignedMachineId().Return("0", nil) - curl := charm.MustParseURL("ch:name-me") - s.unit.EXPECT().CharmURL().Return(curl, nil) + curlStr := "ch:name-me" + s.unit.EXPECT().CharmURL().Return(&curlStr) s.unitChanges <- []string{"foo/0"} } @@ -290,8 +290,8 @@ func (s *lxdProfileWatcherSuite) TestMachineLXDProfileWatcherRemoveOnlyUnit(c *g s.wc0.AssertNoChange() s.setupPrincipalUnit() - curl := charm.MustParseURL("ch:name-me") - s.unit.EXPECT().CharmURL().Return(curl, nil) + curlStr := "ch:name-me" + s.unit.EXPECT().CharmURL().Return(&curlStr) s.unitChanges <- []string{"foo/0"} s.wc0.AssertOneChange() @@ -346,7 +346,7 @@ func (s *lxdProfileWatcherSuite) TestMachineLXDProfileWatcherUnitChangeAppNotFou s.machine0.EXPECT().Units().Return(nil, nil) s.setupPrincipalUnit() - s.unit.EXPECT().CharmURL().Return(nil, nil) + s.unit.EXPECT().CharmURL().Return(nil) s.unit.EXPECT().Application().Return(nil, errors.NotFoundf("")) defer workertest.CleanKill(c, s.assertStartLxdProfileWatcher(c)) @@ -363,8 +363,9 @@ func (s *lxdProfileWatcherSuite) TestMachineLXDProfileWatcherUnitChangeCharmURLN s.machine0.EXPECT().Units().Return(nil, nil) s.setupPrincipalUnit() - curl := charm.MustParseURL("ch:name-me") - s.unit.EXPECT().CharmURL().Return(curl, nil) + curlStr := "ch:name-me" + s.unit.EXPECT().CharmURL().Return(&curlStr) + curl := charm.MustParseURL(curlStr) s.state.EXPECT().Charm(curl).Return(nil, errors.NotFoundf("")) defer workertest.CleanKill(c, s.assertStartLxdProfileWatcher(c)) diff --git a/apiserver/facades/agent/instancemutater/mocks/instancemutater_mock.go b/apiserver/facades/agent/instancemutater/mocks/instancemutater_mock.go index 8969a23a00a..0e11c6532a2 100644 --- a/apiserver/facades/agent/instancemutater/mocks/instancemutater_mock.go +++ b/apiserver/facades/agent/instancemutater/mocks/instancemutater_mock.go @@ -489,12 +489,11 @@ func (mr *MockUnitMockRecorder) AssignedMachineId() *gomock.Call { } // CharmURL mocks base method. -func (m *MockUnit) CharmURL() (*charm.URL, error) { +func (m *MockUnit) CharmURL() *string { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CharmURL") - ret0, _ := ret[0].(*charm.URL) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret0, _ := ret[0].(*string) + return ret0 } // CharmURL indicates an expected call of CharmURL. diff --git a/apiserver/facades/agent/metricsender/sender_test.go b/apiserver/facades/agent/metricsender/sender_test.go index 4eb21b39c6d..6459eb0273d 100644 --- a/apiserver/facades/agent/metricsender/sender_test.go +++ b/apiserver/facades/agent/metricsender/sender_test.go @@ -56,7 +56,8 @@ var _ metricsender.MetricSender = (*metricsender.HTTPSender)(nil) // is in use metrics get sent func (s *SenderSuite) TestHTTPSender(c *gc.C) { metricCount := 3 - expectedCharmURL, _ := s.unit.CharmURL() + expectedCharmURL := s.unit.CharmURL() + c.Assert(expectedCharmURL, gc.NotNil) receiverChan := make(chan wireformat.MetricBatch, metricCount) cleanup := s.startServer(c, testHandler(c, receiverChan, nil, 0)) @@ -74,7 +75,7 @@ func (s *SenderSuite) TestHTTPSender(c *gc.C) { c.Assert(receiverChan, gc.HasLen, metricCount) close(receiverChan) for batch := range receiverChan { - c.Assert(batch.CharmUrl, gc.Equals, expectedCharmURL.String()) + c.Assert(batch.CharmUrl, gc.Equals, *expectedCharmURL) } for _, metric := range metrics { diff --git a/apiserver/facades/agent/uniter/mocks/newlxdprofile.go b/apiserver/facades/agent/uniter/mocks/newlxdprofile.go index d1d7cbcf5ca..1156437d5dc 100644 --- a/apiserver/facades/agent/uniter/mocks/newlxdprofile.go +++ b/apiserver/facades/agent/uniter/mocks/newlxdprofile.go @@ -234,12 +234,11 @@ func (mr *MockLXDProfileUnitV2MockRecorder) AssignedMachineId() *gomock.Call { } // CharmURL mocks base method. -func (m *MockLXDProfileUnitV2) CharmURL() (*charm.URL, error) { +func (m *MockLXDProfileUnitV2) CharmURL() *string { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CharmURL") - ret0, _ := ret[0].(*charm.URL) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret0, _ := ret[0].(*string) + return ret0 } // CharmURL indicates an expected call of CharmURL. diff --git a/apiserver/facades/agent/uniter/newlxdprofile.go b/apiserver/facades/agent/uniter/newlxdprofile.go index 0f445db3546..12ef98d6f09 100644 --- a/apiserver/facades/agent/uniter/newlxdprofile.go +++ b/apiserver/facades/agent/uniter/newlxdprofile.go @@ -47,7 +47,7 @@ type LXDProfileMachineV2 interface { type LXDProfileUnitV2 interface { ApplicationName() string AssignedMachineId() (string, error) - CharmURL() (*charm.URL, error) + CharmURL() *string Name() string Tag() names.Tag } diff --git a/apiserver/facades/agent/uniter/uniter.go b/apiserver/facades/agent/uniter/uniter.go index f6e4d420161..827ff425f57 100644 --- a/apiserver/facades/agent/uniter/uniter.go +++ b/apiserver/facades/agent/uniter/uniter.go @@ -540,19 +540,14 @@ func (u *UniterAPI) CharmURL(args params.Entities) (params.StringBoolResults, er var unitOrApplication state.Entity unitOrApplication, err = u.st.FindEntity(tag) if err == nil { - // TODO (hmlanigan) 2022-06-08 - // cURL can be a string pointer once unit.CharmURL() - // returns a string pointer as well. - var cURL *charm.URL + var cURL *string var force bool switch entity := unitOrApplication.(type) { case *state.Application: - var cURLStr *string - cURLStr, force = entity.CharmURL() - cURL, err = charm.ParseURL(*cURLStr) + cURL, force = entity.CharmURL() case *state.Unit: - cURL, err = entity.CharmURL() + cURL = entity.CharmURL() // The force value is not actually used on the uniter's unit api. if cURL != nil { force = true @@ -560,7 +555,7 @@ func (u *UniterAPI) CharmURL(args params.Entities) (params.StringBoolResults, er } if cURL != nil { - result.Results[i].Result = cURL.String() + result.Results[i].Result = *cURL result.Results[i].Ok = force } } diff --git a/apiserver/facades/agent/uniter/uniter_test.go b/apiserver/facades/agent/uniter/uniter_test.go index 7527fc8538e..33f0db35ef8 100644 --- a/apiserver/facades/agent/uniter/uniter_test.go +++ b/apiserver/facades/agent/uniter/uniter_test.go @@ -875,9 +875,9 @@ func (s *uniterSuite) TestCharmURL(c *gc.C) { // Set wordpressUnit's charm URL first. err := s.wordpressUnit.SetCharmURL(s.wpCharm.URL()) c.Assert(err, jc.ErrorIsNil) - curl, err := s.wordpressUnit.CharmURL() - c.Assert(err, jc.ErrorIsNil) - c.Assert(curl, gc.DeepEquals, s.wpCharm.URL()) + curl := s.wordpressUnit.CharmURL() + c.Assert(curl, gc.NotNil) + c.Assert(*curl, gc.Equals, s.wpCharm.URL().String()) // Make sure wordpress application's charm is what we expect. curlStr, force := s.wordpress.CharmURL() @@ -913,8 +913,8 @@ func (s *uniterSuite) TestCharmURL(c *gc.C) { } func (s *uniterSuite) TestSetCharmURL(c *gc.C) { - _, ok := s.wordpressUnit.CharmURL() - c.Assert(ok, jc.IsFalse) + charmURL := s.wordpressUnit.CharmURL() + c.Assert(charmURL, gc.IsNil) args := params.EntitiesCharmURL{Entities: []params.EntityCharmURL{ {Tag: "unit-mysql-0", CharmURL: "cs:quantal/application-42"}, @@ -935,10 +935,9 @@ func (s *uniterSuite) TestSetCharmURL(c *gc.C) { err = s.wordpressUnit.Refresh() c.Assert(err, jc.ErrorIsNil) - charmURL, err := s.wordpressUnit.CharmURL() - c.Assert(err, jc.ErrorIsNil) + charmURL = s.wordpressUnit.CharmURL() c.Assert(charmURL, gc.NotNil) - c.Assert(charmURL.String(), gc.Equals, s.wpCharm.String()) + c.Assert(*charmURL, gc.Equals, s.wpCharm.String()) } func (s *uniterSuite) TestWorkloadVersion(c *gc.C) { diff --git a/apiserver/facades/client/client/status.go b/apiserver/facades/client/client/status.go index de02606c1ae..5a91ad834ed 100644 --- a/apiserver/facades/client/client/status.go +++ b/apiserver/facades/client/client/status.go @@ -1531,9 +1531,9 @@ func (context *statusContext) processUnit(unit *state.Unit, applicationCharm str if unit.IsPrincipal() { result.Machine, _ = unit.AssignedMachineId() } - curl, _ := unit.CharmURL() - if applicationCharm != "" && curl != nil && curl.String() != applicationCharm { - result.Charm = curl.String() + unitCharm := unit.CharmURL() + if applicationCharm != "" && unitCharm != nil && *unitCharm != applicationCharm { + result.Charm = *unitCharm } workloadVersion, err := context.status.UnitWorkloadVersion(unit.Name()) if err == nil { @@ -1550,7 +1550,18 @@ func (context *statusContext) processUnit(unit *state.Unit, applicationCharm str subUnit := context.unitByName(name) // subUnit may be nil if subordinate was filtered out. if subUnit != nil { - result.Subordinates[name] = context.processUnit(subUnit, applicationCharm, true) + subUnitAppCharm := "" + subUnitApp, err := subUnit.Application() + if err != nil { + logger.Debugf("error fetching subordinate application for %q", subUnit.ApplicationName()) + } + subUnitAppCh, _, err := subUnitApp.Charm() + if err == nil { + subUnitAppCharm = subUnitAppCh.String() + } else { + logger.Debugf("error fetching subordinate application charm for %q", subUnit.ApplicationName()) + } + result.Subordinates[name] = context.processUnit(subUnit, subUnitAppCharm, true) } } } diff --git a/apiserver/facades/client/client/status_test.go b/apiserver/facades/client/client/status_test.go index eb8232cf14d..15501552873 100644 --- a/apiserver/facades/client/client/status_test.go +++ b/apiserver/facades/client/client/status_test.go @@ -332,14 +332,14 @@ func (s *statusUnitTestSuite) TestModelMeterStatus(c *gc.C) { func (s *statusUnitTestSuite) TestMeterStatus(c *gc.C) { meteredCharm := s.Factory.MakeCharm(c, &factory.CharmParams{Name: "metered", URL: "cs:quantal/metered"}) - service := s.Factory.MakeApplication(c, &factory.ApplicationParams{Charm: meteredCharm}) + app := s.Factory.MakeApplication(c, &factory.ApplicationParams{Charm: meteredCharm}) - units, err := service.AllUnits() + units, err := app.AllUnits() c.Assert(err, jc.ErrorIsNil) c.Assert(units, gc.HasLen, 0) for i, unit := range testUnits { - u, err := service.AddUnit(state.AddUnitParams{}) + u, err := app.AddUnit(state.AddUnitParams{}) testUnits[i].unitName = u.Name() c.Assert(err, jc.ErrorIsNil) if unit.setStatus != nil { @@ -352,12 +352,12 @@ func (s *statusUnitTestSuite) TestMeterStatus(c *gc.C) { status, err := client.Status(nil) c.Assert(err, jc.ErrorIsNil) c.Assert(status, gc.NotNil) - serviceStatus, ok := status.Applications[service.Name()] + appStatus, ok := status.Applications[app.Name()] c.Assert(ok, gc.Equals, true) - c.Assert(serviceStatus.MeterStatuses, gc.HasLen, len(testUnits)-1) + c.Assert(appStatus.MeterStatuses, gc.HasLen, len(testUnits)-1) for _, unit := range testUnits { - unitStatus, ok := serviceStatus.MeterStatuses[unit.unitName] + unitStatus, ok := appStatus.MeterStatuses[unit.unitName] if unit.expectedStatus != nil { c.Assert(ok, gc.Equals, true) @@ -369,14 +369,14 @@ func (s *statusUnitTestSuite) TestMeterStatus(c *gc.C) { } func (s *statusUnitTestSuite) TestNoMeterStatusWhenNotRequired(c *gc.C) { - service := s.Factory.MakeApplication(c, nil) + app := s.Factory.MakeApplication(c, nil) - units, err := service.AllUnits() + units, err := app.AllUnits() c.Assert(err, jc.ErrorIsNil) c.Assert(units, gc.HasLen, 0) for i, unit := range testUnits { - u, err := service.AddUnit(state.AddUnitParams{}) + u, err := app.AddUnit(state.AddUnitParams{}) testUnits[i].unitName = u.Name() c.Assert(err, jc.ErrorIsNil) if unit.setStatus != nil { @@ -389,22 +389,22 @@ func (s *statusUnitTestSuite) TestNoMeterStatusWhenNotRequired(c *gc.C) { status, err := client.Status(nil) c.Assert(err, jc.ErrorIsNil) c.Assert(status, gc.NotNil) - serviceStatus, ok := status.Applications[service.Name()] + appStatus, ok := status.Applications[app.Name()] c.Assert(ok, gc.Equals, true) - c.Assert(serviceStatus.MeterStatuses, gc.HasLen, 0) + c.Assert(appStatus.MeterStatuses, gc.HasLen, 0) } func (s *statusUnitTestSuite) TestMeterStatusWithCredentials(c *gc.C) { - service := s.Factory.MakeApplication(c, nil) - c.Assert(service.SetMetricCredentials([]byte("magic-ticket")), jc.ErrorIsNil) + app := s.Factory.MakeApplication(c, nil) + c.Assert(app.SetMetricCredentials([]byte("magic-ticket")), jc.ErrorIsNil) - units, err := service.AllUnits() + units, err := app.AllUnits() c.Assert(err, jc.ErrorIsNil) c.Assert(units, gc.HasLen, 0) for i, unit := range testUnits { - u, err := service.AddUnit(state.AddUnitParams{}) + u, err := app.AddUnit(state.AddUnitParams{}) testUnits[i].unitName = u.Name() c.Assert(err, jc.ErrorIsNil) if unit.setStatus != nil { @@ -417,12 +417,12 @@ func (s *statusUnitTestSuite) TestMeterStatusWithCredentials(c *gc.C) { status, err := client.Status(nil) c.Assert(err, jc.ErrorIsNil) c.Assert(status, gc.NotNil) - serviceStatus, ok := status.Applications[service.Name()] + appStatus, ok := status.Applications[app.Name()] c.Assert(ok, gc.Equals, true) - c.Assert(serviceStatus.MeterStatuses, gc.HasLen, len(testUnits)-1) + c.Assert(appStatus.MeterStatuses, gc.HasLen, len(testUnits)-1) for _, unit := range testUnits { - unitStatus, ok := serviceStatus.MeterStatuses[unit.unitName] + unitStatus, ok := appStatus.MeterStatuses[unit.unitName] if unit.expectedStatus != nil { c.Assert(ok, gc.Equals, true) @@ -435,8 +435,8 @@ func (s *statusUnitTestSuite) TestMeterStatusWithCredentials(c *gc.C) { func (s *statusUnitTestSuite) TestApplicationWithExposedEndpoints(c *gc.C) { meteredCharm := s.Factory.MakeCharm(c, &factory.CharmParams{Name: "metered", URL: "cs:quantal/metered"}) - service := s.Factory.MakeApplication(c, &factory.ApplicationParams{Charm: meteredCharm}) - err := service.MergeExposeSettings(map[string]state.ExposedEndpoint{ + app := s.Factory.MakeApplication(c, &factory.ApplicationParams{Charm: meteredCharm}) + err := app.MergeExposeSettings(map[string]state.ExposedEndpoint{ "": { ExposeToSpaceIDs: []string{network.AlphaSpaceId}, ExposeToCIDRs: []string{"10.0.0.0/24", "192.168.0.0/24"}, @@ -448,10 +448,10 @@ func (s *statusUnitTestSuite) TestApplicationWithExposedEndpoints(c *gc.C) { status, err := client.Status(nil) c.Assert(err, jc.ErrorIsNil) c.Assert(status, gc.NotNil) - serviceStatus, ok := status.Applications[service.Name()] + appStatus, ok := status.Applications[app.Name()] c.Assert(ok, gc.Equals, true) - c.Assert(serviceStatus.ExposedEndpoints, gc.DeepEquals, map[string]params.ExposedEndpoint{ + c.Assert(appStatus.ExposedEndpoints, gc.DeepEquals, map[string]params.ExposedEndpoint{ "": { ExposeToSpaces: []string{network.AlphaSpaceName}, ExposeToCIDRs: []string{"10.0.0.0/24", "192.168.0.0/24"}, @@ -459,6 +459,87 @@ func (s *statusUnitTestSuite) TestApplicationWithExposedEndpoints(c *gc.C) { }) } +func (s *statusUnitTestSuite) TestPrincipalUpgradingFrom(c *gc.C) { + meteredCharm := s.Factory.MakeCharm(c, &factory.CharmParams{Name: "metered", URL: "cs:quantal/metered-3"}) + meteredCharmNew := s.Factory.MakeCharm(c, &factory.CharmParams{Name: "metered", URL: "cs:quantal/metered-5"}) + app := s.Factory.MakeApplication(c, &factory.ApplicationParams{Charm: meteredCharm}) + u := s.Factory.MakeUnit(c, &factory.UnitParams{ + Application: app, + SetCharmURL: true, + }) + client := apiclient.NewClient(s.APIState) + status, err := client.Status(nil) + c.Assert(err, jc.ErrorIsNil) + c.Assert(status, gc.NotNil) + unitStatus, ok := status.Applications[app.Name()].Units[u.Name()] + c.Assert(ok, gc.Equals, true) + c.Assert(unitStatus.Charm, gc.Equals, "") + + err = app.SetCharm(state.SetCharmConfig{ + Charm: meteredCharmNew, + }) + c.Assert(err, jc.ErrorIsNil) + + status, err = client.Status(nil) + c.Assert(err, jc.ErrorIsNil) + c.Assert(status, gc.NotNil) + unitStatus, ok = status.Applications[app.Name()].Units[u.Name()] + c.Assert(ok, gc.Equals, true) + c.Assert(unitStatus.Charm, gc.Equals, "cs:quantal/metered-3") +} + +func (s *statusUnitTestSuite) TestSubordinateUpgradingFrom(c *gc.C) { + principalCharm := s.Factory.MakeCharm(c, &factory.CharmParams{Name: "mysql", URL: "cs:quantal/mysql"}) + subordCharm := s.Factory.MakeCharm(c, &factory.CharmParams{Name: "logging", URL: "cs:quantal/logging-1"}) + subordCharmNew := s.Factory.MakeCharm(c, &factory.CharmParams{Name: "logging", URL: "cs:quantal/logging-2"}) + app := s.Factory.MakeApplication(c, &factory.ApplicationParams{ + Charm: principalCharm, + Name: "principal", + }) + pu := s.Factory.MakeUnit(c, &factory.UnitParams{ + Application: app, + }) + subordApp := s.Factory.MakeApplication(c, &factory.ApplicationParams{ + Charm: subordCharm, + Name: "subord", + }) + + subEndpoint, err := subordApp.Endpoint("info") + c.Assert(err, jc.ErrorIsNil) + principalEndpoint, err := app.Endpoint("juju-info") + c.Assert(err, jc.ErrorIsNil) + rel, err := s.State.AddRelation(subEndpoint, principalEndpoint) + c.Assert(err, jc.ErrorIsNil) + ru, err := rel.Unit(pu) + c.Assert(err, jc.ErrorIsNil) + err = ru.EnterScope(nil) + c.Assert(err, jc.ErrorIsNil) + subordUnit, err := s.State.Unit("subord/0") + c.Assert(err, jc.ErrorIsNil) + err = subordUnit.SetCharmURL(subordCharm.URL()) + c.Assert(err, jc.ErrorIsNil) + + client := apiclient.NewClient(s.APIState) + status, err := client.Status(nil) + c.Assert(err, jc.ErrorIsNil) + c.Assert(status, gc.NotNil) + unitStatus, ok := status.Applications["principal"].Units["principal/0"].Subordinates["subord/0"] + c.Assert(ok, gc.Equals, true) + c.Assert(unitStatus.Charm, gc.Equals, "") + + err = subordApp.SetCharm(state.SetCharmConfig{ + Charm: subordCharmNew, + }) + c.Assert(err, jc.ErrorIsNil) + + status, err = client.Status(nil) + c.Assert(err, jc.ErrorIsNil) + c.Assert(status, gc.NotNil) + unitStatus, ok = status.Applications["principal"].Units["principal/0"].Subordinates["subord/0"] + c.Assert(ok, gc.Equals, true) + c.Assert(unitStatus.Charm, gc.Equals, "cs:quantal/logging-1") +} + func addUnitWithVersion(c *gc.C, application *state.Application, version string) *state.Unit { unit, err := application.AddUnit(state.AddUnitParams{}) c.Assert(err, jc.ErrorIsNil) @@ -851,9 +932,9 @@ func (s *statusUpgradeUnitSuite) TestUpdateRevisionsCharmstore(c *gc.C) { client := apiclient.NewClient(s.APIState) status, _ := client.Status(nil) - serviceStatus, ok := status.Applications["mysql"] + appStatus, ok := status.Applications["mysql"] c.Assert(ok, gc.Equals, true) - c.Assert(serviceStatus.CanUpgradeTo, gc.Equals, "") + c.Assert(appStatus.CanUpgradeTo, gc.Equals, "") // Update to the latest available charm revision. result, err := s.charmrevisionupdater.UpdateLatestRevisions() @@ -862,9 +943,9 @@ func (s *statusUpgradeUnitSuite) TestUpdateRevisionsCharmstore(c *gc.C) { // Check if CanUpgradeTo suggests the latest revision. status, _ = client.Status(nil) - serviceStatus, ok = status.Applications["mysql"] + appStatus, ok = status.Applications["mysql"] c.Assert(ok, gc.Equals, true) - c.Assert(serviceStatus.CanUpgradeTo, gc.Equals, "cs:quantal/mysql-23") + c.Assert(appStatus.CanUpgradeTo, gc.Equals, "cs:quantal/mysql-23") } func (s *statusUpgradeUnitSuite) TestUpdateRevisionsCharmhub(c *gc.C) { @@ -877,9 +958,9 @@ func (s *statusUpgradeUnitSuite) TestUpdateRevisionsCharmhub(c *gc.C) { client := apiclient.NewClient(s.APIState) status, _ := client.Status(nil) - serviceStatus, ok := status.Applications["charmhubby"] + appStatus, ok := status.Applications["charmhubby"] c.Assert(ok, gc.Equals, true) - c.Assert(serviceStatus.CanUpgradeTo, gc.Equals, "") + c.Assert(appStatus.CanUpgradeTo, gc.Equals, "") // Update to the latest available charm revision. result, err := s.charmrevisionupdater.UpdateLatestRevisions() @@ -888,9 +969,9 @@ func (s *statusUpgradeUnitSuite) TestUpdateRevisionsCharmhub(c *gc.C) { // Check if CanUpgradeTo suggests the latest revision. status, _ = client.Status(nil) - serviceStatus, ok = status.Applications["charmhubby"] + appStatus, ok = status.Applications["charmhubby"] c.Assert(ok, gc.Equals, true) - c.Assert(serviceStatus.CanUpgradeTo, gc.Equals, "ch:charmhubby-42") + c.Assert(appStatus.CanUpgradeTo, gc.Equals, "ch:charmhubby-42") } type CAASStatusSuite struct { diff --git a/apiserver/facades/client/machinemanager/machinemanager.go b/apiserver/facades/client/machinemanager/machinemanager.go index fa7b1f840b1..5bcfcfdae80 100644 --- a/apiserver/facades/client/machinemanager/machinemanager.go +++ b/apiserver/facades/client/machinemanager/machinemanager.go @@ -9,6 +9,7 @@ import ( "strconv" "time" + "github.com/juju/collections/set" "github.com/juju/errors" "github.com/juju/loggo" "github.com/juju/names/v4" @@ -341,7 +342,7 @@ func (mm *MachineManagerAPI) ProvisioningScript(args params.ProvisioningScriptPa } // RetryProvisioning marks a provisioning error as transient on the machines. -func (mm *MachineManagerAPI) RetryProvisioning(p params.Entities) (params.ErrorResults, error) { +func (mm *MachineManagerAPI) RetryProvisioning(p params.RetryProvisioningArgs) (params.ErrorResults, error) { if err := mm.authorizer.CanWrite(); err != nil { return params.ErrorResults{}, err } @@ -349,37 +350,33 @@ func (mm *MachineManagerAPI) RetryProvisioning(p params.Entities) (params.ErrorR if err := mm.check.ChangeAllowed(); err != nil { return params.ErrorResults{}, errors.Trace(err) } - result := params.ErrorResults{ - Results: make([]params.ErrorResult, len(p.Entities)), + result := params.ErrorResults{} + machines, err := mm.st.AllMachines() + if err != nil { + return params.ErrorResults{}, errors.Trace(err) } - for i, e := range p.Entities { - tag, err := names.ParseMachineTag(e.Tag) + wanted := set.NewStrings() + for _, tagStr := range p.Machines { + tag, err := names.ParseMachineTag(tagStr) if err != nil { - result.Results[i].Error = apiservererrors.ServerError(err) + result.Results = append(result.Results, params.ErrorResult{Error: apiservererrors.ServerError(err)}) continue } - if err := mm.updateInstanceStatus(tag, map[string]interface{}{"transient": true}); err != nil { - result.Results[i].Error = apiservererrors.ServerError(err) + wanted.Add(tag.Id()) + } + for _, m := range machines { + if !p.All && !wanted.Contains(m.Id()) { + continue + } + if err := mm.maybeUpdateInstanceStatus(p.All, m, map[string]interface{}{"transient": true}); err != nil { + result.Results = append(result.Results, params.ErrorResult{Error: apiservererrors.ServerError(err)}) } } return result, nil } -type instanceStatus interface { - InstanceStatus() (status.StatusInfo, error) - SetInstanceStatus(sInfo status.StatusInfo) error -} - -func (mm *MachineManagerAPI) updateInstanceStatus(tag names.Tag, data map[string]interface{}) error { - entity0, err := mm.st.FindEntity(tag) - if err != nil { - return err - } - statusGetterSetter, ok := entity0.(instanceStatus) - if !ok { - return apiservererrors.NotSupportedError(tag, "getting status") - } - existingStatusInfo, err := statusGetterSetter.InstanceStatus() +func (mm *MachineManagerAPI) maybeUpdateInstanceStatus(all bool, m Machine, data map[string]interface{}) error { + existingStatusInfo, err := m.InstanceStatus() if err != nil { return err } @@ -392,7 +389,12 @@ func (mm *MachineManagerAPI) updateInstanceStatus(tag names.Tag, data map[string } } if len(newData) > 0 && existingStatusInfo.Status != status.Error && existingStatusInfo.Status != status.ProvisioningError { - return fmt.Errorf("%s is not in an error state (%v)", names.ReadableString(tag), existingStatusInfo.Status) + // If a specifc machine has been asked for and it's not in error, that's a problem. + if !all { + return fmt.Errorf("machine %s is not in an error state (%v)", m.Id(), existingStatusInfo.Status) + } + // Otherwise just skip it. + return nil } // TODO(perrito666) 2016-05-02 lp:1558657 now := time.Now() @@ -402,7 +404,7 @@ func (mm *MachineManagerAPI) updateInstanceStatus(tag names.Tag, data map[string Data: newData, Since: &now, } - return statusGetterSetter.SetInstanceStatus(sInfo) + return m.SetInstanceStatus(sInfo) } // DestroyMachineWithParams removes a set of machines from the model. diff --git a/apiserver/facades/client/machinemanager/machinemanager_test.go b/apiserver/facades/client/machinemanager/machinemanager_test.go index 97e23ee580b..883f3f27a50 100644 --- a/apiserver/facades/client/machinemanager/machinemanager_test.go +++ b/apiserver/facades/client/machinemanager/machinemanager_test.go @@ -629,7 +629,7 @@ func (s *ProvisioningMachineManagerSuite) setup(c *gc.C) *gomock.Controller { s.ctrlSt.EXPECT().ControllerTag().Return(coretesting.ControllerTag).AnyTimes() s.pool = mocks.NewMockPool(ctrl) - s.pool.EXPECT().SystemState().Return(s.ctrlSt, nil) + s.pool.EXPECT().SystemState().Return(s.ctrlSt, nil).AnyTimes() s.model = mocks.NewMockModel(ctrl) s.model.EXPECT().UUID().Return("uuid").AnyTimes() @@ -740,6 +740,75 @@ func (s *ProvisioningMachineManagerSuite) TestProvisioningScriptDisablePackageCo c.Assert(result.Script, gc.Not(jc.Contains), "apt-get upgrade") } +type statusMatcher struct { + c *gc.C + expected status.StatusInfo +} + +func (m statusMatcher) Matches(x interface{}) bool { + obtained, ok := x.(status.StatusInfo) + m.c.Assert(ok, jc.IsTrue) + if !ok { + return false + } + + m.c.Assert(obtained.Since, gc.NotNil) + obtained.Since = nil + m.c.Assert(obtained, jc.DeepEquals, m.expected) + return true +} + +func (m statusMatcher) String() string { + return "Match the status.StatusInfo value" +} + +func (s *ProvisioningMachineManagerSuite) TestRetryProvisioning(c *gc.C) { + ctrl := s.setup(c) + defer ctrl.Finish() + + s.st.EXPECT().GetBlockForType(state.ChangeBlock).Return(nil, false, nil).AnyTimes() + + machine0 := mocks.NewMockMachine(ctrl) + machine0.EXPECT().Id().Return("0") + machine0.EXPECT().InstanceStatus().Return(status.StatusInfo{Status: "provisioning error"}, nil) + machine0.EXPECT().SetInstanceStatus(statusMatcher{c: c, expected: status.StatusInfo{ + Status: status.ProvisioningError, + Data: map[string]interface{}{"transient": true}, + }}).Return(nil) + machine1 := mocks.NewMockMachine(ctrl) + machine1.EXPECT().Id().Return("1") + s.st.EXPECT().AllMachines().Return([]machinemanager.Machine{machine0, machine1}, nil) + + results, err := s.api.RetryProvisioning(params.RetryProvisioningArgs{ + Machines: []string{"machine-0"}, + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(results, jc.DeepEquals, params.ErrorResults{}) +} + +func (s *ProvisioningMachineManagerSuite) TestRetryProvisioningAll(c *gc.C) { + ctrl := s.setup(c) + defer ctrl.Finish() + + s.st.EXPECT().GetBlockForType(state.ChangeBlock).Return(nil, false, nil).AnyTimes() + + machine0 := mocks.NewMockMachine(ctrl) + machine0.EXPECT().InstanceStatus().Return(status.StatusInfo{Status: "provisioning error"}, nil) + machine0.EXPECT().SetInstanceStatus(statusMatcher{c: c, expected: status.StatusInfo{ + Status: status.ProvisioningError, + Data: map[string]interface{}{"transient": true}, + }}).Return(nil) + machine1 := mocks.NewMockMachine(ctrl) + machine1.EXPECT().InstanceStatus().Return(status.StatusInfo{Status: "pending"}, nil) + s.st.EXPECT().AllMachines().Return([]machinemanager.Machine{machine0, machine1}, nil) + + results, err := s.api.RetryProvisioning(params.RetryProvisioningArgs{ + All: true, + }) + c.Assert(err, jc.ErrorIsNil) + c.Assert(results, jc.DeepEquals, params.ErrorResults{}) +} + type UpgradeSeriesMachineManagerSuite struct{} func (s *UpgradeSeriesMachineManagerSuite) expectValidateMachine(ctrl *gomock.Controller, series string, isManager, isLockedForSeriesUpgrade bool) *mocks.MockMachine { diff --git a/apiserver/facades/client/machinemanager/mocks/types_mock.go b/apiserver/facades/client/machinemanager/mocks/types_mock.go index 37682677954..284087284eb 100644 --- a/apiserver/facades/client/machinemanager/mocks/types_mock.go +++ b/apiserver/facades/client/machinemanager/mocks/types_mock.go @@ -94,6 +94,21 @@ func (mr *MockBackendMockRecorder) AddOneMachine(arg0 interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddOneMachine", reflect.TypeOf((*MockBackend)(nil).AddOneMachine), arg0) } +// AllMachines mocks base method. +func (m *MockBackend) AllMachines() ([]machinemanager.Machine, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AllMachines") + ret0, _ := ret[0].([]machinemanager.Machine) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AllMachines indicates an expected call of AllMachines. +func (mr *MockBackendMockRecorder) AllMachines() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AllMachines", reflect.TypeOf((*MockBackend)(nil).AllMachines)) +} + // AllSpaceInfos mocks base method. func (m *MockBackend) AllSpaceInfos() (network.SpaceInfos, error) { m.ctrl.T.Helper() @@ -124,21 +139,6 @@ func (mr *MockBackendMockRecorder) Application(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Application", reflect.TypeOf((*MockBackend)(nil).Application), arg0) } -// FindEntity mocks base method. -func (m *MockBackend) FindEntity(arg0 names.Tag) (state.Entity, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "FindEntity", arg0) - ret0, _ := ret[0].(state.Entity) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// FindEntity indicates an expected call of FindEntity. -func (mr *MockBackendMockRecorder) FindEntity(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FindEntity", reflect.TypeOf((*MockBackend)(nil).FindEntity), arg0) -} - // GetBlockForType mocks base method. func (m *MockBackend) GetBlockForType(arg0 state.BlockType) (state.Block, bool, error) { m.ctrl.T.Helper() @@ -657,6 +657,21 @@ func (mr *MockMachineMockRecorder) Id() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Id", reflect.TypeOf((*MockMachine)(nil).Id)) } +// InstanceStatus mocks base method. +func (m *MockMachine) InstanceStatus() (status.StatusInfo, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "InstanceStatus") + ret0, _ := ret[0].(status.StatusInfo) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// InstanceStatus indicates an expected call of InstanceStatus. +func (mr *MockMachineMockRecorder) InstanceStatus() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InstanceStatus", reflect.TypeOf((*MockMachine)(nil).InstanceStatus)) +} + // IsLockedForSeriesUpgrade mocks base method. func (m *MockMachine) IsLockedForSeriesUpgrade() (bool, error) { m.ctrl.T.Helper() @@ -728,6 +743,20 @@ func (mr *MockMachineMockRecorder) Series() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Series", reflect.TypeOf((*MockMachine)(nil).Series)) } +// SetInstanceStatus mocks base method. +func (m *MockMachine) SetInstanceStatus(arg0 status.StatusInfo) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetInstanceStatus", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// SetInstanceStatus indicates an expected call of SetInstanceStatus. +func (mr *MockMachineMockRecorder) SetInstanceStatus(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetInstanceStatus", reflect.TypeOf((*MockMachine)(nil).SetInstanceStatus), arg0) +} + // SetKeepInstance mocks base method. func (m *MockMachine) SetKeepInstance(arg0 bool) error { m.ctrl.T.Helper() diff --git a/apiserver/facades/client/machinemanager/state.go b/apiserver/facades/client/machinemanager/state.go index f5320342d2c..4d517c078ab 100644 --- a/apiserver/facades/client/machinemanager/state.go +++ b/apiserver/facades/client/machinemanager/state.go @@ -37,13 +37,13 @@ type Backend interface { // Application returns a application state by name. Application(string) (Application, error) Machine(string) (Machine, error) + AllMachines() ([]Machine, error) Unit(string) (Unit, error) Model() (Model, error) GetBlockForType(t state.BlockType) (state.Block, bool, error) AddOneMachine(template state.MachineTemplate) (*state.Machine, error) AddMachineInsideNewMachine(template, parentTemplate state.MachineTemplate, containerType instance.ContainerType) (*state.Machine, error) AddMachineInsideMachine(template state.MachineTemplate, parentId string, containerType instance.ContainerType) (*state.Machine, error) - FindEntity(names.Tag) (state.Entity, error) ToolsStorage() (binarystorage.StorageCloser, error) } @@ -97,6 +97,8 @@ type Machine interface { UpgradeSeriesStatus() (model.UpgradeSeriesStatus, error) SetUpgradeSeriesStatus(model.UpgradeSeriesStatus, string) error ApplicationNames() ([]string, error) + InstanceStatus() (status.StatusInfo, error) + SetInstanceStatus(sInfo status.StatusInfo) error } type Application interface { @@ -136,6 +138,18 @@ func (s stateShim) Machine(name string) (Machine, error) { }, nil } +func (s stateShim) AllMachines() ([]Machine, error) { + all, err := s.State.AllMachines() + if err != nil { + return nil, errors.Trace(err) + } + result := make([]Machine, len(all)) + for i, m := range all { + result[i] = machineShim{Machine: m} + } + return result, nil +} + func (s stateShim) Unit(name string) (Unit, error) { u, err := s.State.Unit(name) if err != nil { diff --git a/apiserver/facades/client/metricsdebug/metricsdebug.go b/apiserver/facades/client/metricsdebug/metricsdebug.go index 9dece5184a8..561060ce85c 100644 --- a/apiserver/facades/client/metricsdebug/metricsdebug.go +++ b/apiserver/facades/client/metricsdebug/metricsdebug.go @@ -181,13 +181,14 @@ func (api *MetricsDebugAPI) setEntityMeterStatus(entity names.Tag, status state. if err != nil { return errors.Trace(err) } - chURL, err := unit.CharmURL() + chURLStr := unit.CharmURL() + if chURLStr == nil { + return errors.New("no charm url") + } + chURL, err := charm.ParseURL(*chURLStr) if err != nil { return errors.Trace(err) } - if chURL == nil { - return errors.New("no charm url") - } if !charm.Local.Matches(chURL.Schema) { return errors.New("not a local charm") } diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index 801545d2a14..1a0975753ce 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -26491,7 +26491,7 @@ "type": "object", "properties": { "Params": { - "$ref": "#/definitions/Entities" + "$ref": "#/definitions/RetryProvisioningArgs" }, "Result": { "$ref": "#/definitions/ErrorResults" @@ -27084,6 +27084,24 @@ "script" ] }, + "RetryProvisioningArgs": { + "type": "object", + "properties": { + "all": { + "type": "boolean" + }, + "machines": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "additionalProperties": false, + "required": [ + "all" + ] + }, "StringsResult": { "type": "object", "properties": { diff --git a/cmd/juju/model/retryprovisioning.go b/cmd/juju/model/retryprovisioning.go index 1e193ac3a60..b07eaa35d65 100644 --- a/cmd/juju/model/retryprovisioning.go +++ b/cmd/juju/model/retryprovisioning.go @@ -8,6 +8,7 @@ import ( "github.com/juju/cmd/v3" "github.com/juju/errors" + "github.com/juju/gnuflag" "github.com/juju/names/v4" "github.com/juju/juju/api/client/machinemanager" @@ -28,13 +29,15 @@ type retryProvisioningCommand struct { modelcmd.IAASOnlyCommand Machines []names.MachineTag api RetryProvisioningAPI + + all bool } // RetryProvisioningAPI defines methods on the client API // that the retry-provisioning command calls. type RetryProvisioningAPI interface { Close() error - RetryProvisioning(machines ...names.MachineTag) ([]params.ErrorResult, error) + RetryProvisioning(all bool, machines ...names.MachineTag) ([]params.ErrorResult, error) } func (c *retryProvisioningCommand) Info() *cmd.Info { @@ -45,10 +48,17 @@ func (c *retryProvisioningCommand) Info() *cmd.Info { }) } +func (c *retryProvisioningCommand) SetFlags(f *gnuflag.FlagSet) { + f.BoolVar(&c.all, "all", false, "retry provisioning all failed machines") +} + func (c *retryProvisioningCommand) Init(args []string) error { - if len(args) == 0 { + if !c.all && len(args) == 0 { return errors.Errorf("no machine specified") } + if c.all && len(args) > 0 { + return errors.Errorf("specify machines or --all but not both") + } c.Machines = make([]names.MachineTag, len(args)) for i, arg := range args { if !names.IsValidMachine(arg) { @@ -81,7 +91,7 @@ func (c *retryProvisioningCommand) Run(context *cmd.Context) error { } defer client.Close() - results, err := client.RetryProvisioning(c.Machines...) + results, err := client.RetryProvisioning(c.all, c.Machines...) if err != nil { return block.ProcessBlockedError(err, block.BlockChange) } diff --git a/cmd/juju/model/retryprovisioning_test.go b/cmd/juju/model/retryprovisioning_test.go index b84d624d7b5..1f2ea6bda0a 100644 --- a/cmd/juju/model/retryprovisioning_test.go +++ b/cmd/juju/model/retryprovisioning_test.go @@ -42,7 +42,7 @@ func (f *fakeRetryProvisioningClient) Close() error { return nil } -func (f *fakeRetryProvisioningClient) RetryProvisioning(machines ...names.MachineTag) ( +func (f *fakeRetryProvisioningClient) RetryProvisioning(all bool, machines ...names.MachineTag) ( []params.ErrorResult, error) { if f.err != nil { @@ -51,6 +51,9 @@ func (f *fakeRetryProvisioningClient) RetryProvisioning(machines ...names.Machin results := make([]params.ErrorResult, len(machines)) + if all { + machines = []names.MachineTag{names.NewMachineTag("0")} + } // For each of the machines passed in, verify that we have the // id and that the info string is "broken". for i, machine := range machines { @@ -102,11 +105,16 @@ var resolvedMachineTests = []struct { }, { args: []string{"42"}, stdErr: `machine 42 not found`, + }, { + args: []string{"42", "--all"}, + err: `specify machines or --all but not both`, }, { args: []string{"1"}, stdErr: `machine 1 is not in an error state`, }, { args: []string{"0"}, + }, { + args: []string{"--all"}, }, { args: []string{"0", "1"}, stdErr: `machine 1 is not in an error state`, @@ -133,7 +141,7 @@ func (s *retryProvisioningSuite) TestRetryProvisioning(c *gc.C) { output := cmdtesting.Stderr(context) stripped := strings.Replace(output, "\n", "", -1) c.Check(stripped, gc.Equals, t.stdErr) - if t.args[0] == "0" { + if t.args[0] == "0" || t.args[0] == "all" { m := s.fake.m["0"] c.Check(m.info, gc.Equals, "broken") c.Check(m.data["transient"], jc.IsTrue) diff --git a/core/cache/cachetest/state.go b/core/cache/cachetest/state.go index 8c59f94745a..092d44369f3 100644 --- a/core/cache/cachetest/state.go +++ b/core/cache/cachetest/state.go @@ -165,9 +165,9 @@ func UnitChange(c *gc.C, modelUUID string, unit *state.Unit) cache.UnitChange { } var charmURL string - cURL, err := unit.CharmURL() - c.Assert(err, jc.ErrorIsNil) - charmURL = cURL.String() + if cURL := unit.CharmURL(); cURL != nil { + charmURL = *cURL + } pr, err := unit.OpenedPortRanges() if !errors.IsNotAssigned(err) { diff --git a/migration/precheck.go b/migration/precheck.go index 21ad989cf20..b94e5ce2e8c 100644 --- a/migration/precheck.go +++ b/migration/precheck.go @@ -6,7 +6,6 @@ package migration import ( "fmt" - "github.com/juju/charm/v9" "github.com/juju/errors" "github.com/juju/names/v4" "github.com/juju/replicaset/v2" @@ -86,7 +85,7 @@ type PrecheckUnit interface { Name() string AgentTools() (*tools.Tools, error) Life() state.Life - CharmURL() (*charm.URL, error) + CharmURL() *string AgentStatus() (status.StatusInfo, error) Status() (status.StatusInfo, error) ShouldBeAssigned() bool @@ -421,8 +420,8 @@ func (ctx *precheckContext) checkUnits(app PrecheckApplication, units []Precheck } } - unitCharmURL, _ := unit.CharmURL() - if *appCharmURL != unitCharmURL.String() { + unitCharmURL := unit.CharmURL() + if unitCharmURL == nil || *appCharmURL != *unitCharmURL { return errors.Errorf("unit %s is upgrading", unit.Name()) } } diff --git a/migration/precheck_test.go b/migration/precheck_test.go index cd361186d38..745e4ead27a 100644 --- a/migration/precheck_test.go +++ b/migration/precheck_test.go @@ -6,7 +6,6 @@ package migration_test import ( "strings" - "github.com/juju/charm/v9" "github.com/juju/errors" "github.com/juju/names/v4" "github.com/juju/replicaset/v2" @@ -1071,12 +1070,12 @@ func (u *fakeUnit) ShouldBeAssigned() bool { return true } -func (u *fakeUnit) CharmURL() (*charm.URL, error) { +func (u *fakeUnit) CharmURL() *string { url := u.charmURL if url == "" { url = "cs:foo-1" } - return charm.MustParseURL(url), nil + return &url } func (u *fakeUnit) AgentStatus() (status.StatusInfo, error) { diff --git a/provider/azure/environ.go b/provider/azure/environ.go index 3d8f58fa825..6cd7638da98 100644 --- a/provider/azure/environ.go +++ b/provider/azure/environ.go @@ -1664,6 +1664,12 @@ func (env *azureEnviron) allQueuedInstances( if controllerOnly && !isControllerDeployment(deployment) { continue } + if len(deployment.Tags) == 0 { + continue + } + if toValue(deployment.Tags[tags.JujuModel]) != env.Config().UUID() { + continue + } provisioningState := armresources.ProvisioningStateCreating switch deployProvisioningState { case armresources.ProvisioningStateFailed, @@ -1751,6 +1757,12 @@ func (env *azureEnviron) allProvisionedInstances( if !isControllerInstance(vm, controllerUUID) { continue } + if len(vm.Tags) == 0 { + continue + } + if toValue(vm.Tags[tags.JujuModel]) != env.Config().UUID() { + continue + } inst := &azureInstance{ vmName: name, provisioningState: provisioningState, diff --git a/provider/azure/environ_test.go b/provider/azure/environ_test.go index 6ec89d39efd..6c8e096adf4 100644 --- a/provider/azure/environ_test.go +++ b/provider/azure/environ_test.go @@ -212,6 +212,9 @@ func (s *environSuite) SetUpTest(c *gc.C) { Properties: &armresources.DeploymentPropertiesExtended{ ProvisioningState: to.Ptr(armresources.ProvisioningStateSucceeded), }, + Tags: map[string]*string{ + "juju-model-uuid": to.Ptr(testing.ModelTag.Id()), + }, } s.deployment = nil diff --git a/provider/azure/instance_test.go b/provider/azure/instance_test.go index 12b9a2c9b4a..4343449c6b0 100644 --- a/provider/azure/instance_test.go +++ b/provider/azure/instance_test.go @@ -70,7 +70,8 @@ func (s *instanceSuite) SetUpTest(c *gc.C) { s.vms = []*armcompute.VirtualMachine{{ Name: to.Ptr("machine-0"), Tags: map[string]*string{ - "juju-controller-uuid": to.Ptr("foo"), + "juju-controller-uuid": to.Ptr(testing.ControllerTag.Id()), + "juju-model-uuid": to.Ptr(testing.ModelTag.Id()), "juju-is-controller": to.Ptr("true"), }, Properties: &armcompute.VirtualMachineProperties{ @@ -100,6 +101,9 @@ func makeDeployment(name string, provisioningState armresources.ProvisioningStat ProvisioningState: to.Ptr(provisioningState), Dependencies: dependencies, }, + Tags: map[string]*string{ + "juju-model-uuid": to.Ptr(testing.ModelTag.Id()), + }, } } @@ -647,7 +651,7 @@ func (s *instanceSuite) TestAllRunningInstances(c *gc.C) { func (s *instanceSuite) TestControllerInstancesSomePending(c *gc.C) { *((s.deployments[1].Properties.Dependencies)[0].DependsOn)[0].ResourceName = "juju-controller" s.sender = s.getInstancesSender() - ids, err := s.env.ControllerInstances(s.callCtx, "foo") + ids, err := s.env.ControllerInstances(s.callCtx, testing.ControllerTag.Id()) c.Assert(err, jc.ErrorIsNil) c.Assert(ids, gc.HasLen, 2) c.Assert(ids[0], gc.Equals, instance.Id("machine-0")) @@ -656,7 +660,7 @@ func (s *instanceSuite) TestControllerInstancesSomePending(c *gc.C) { func (s *instanceSuite) TestControllerInstances(c *gc.C) { s.sender = s.getInstancesSender() - ids, err := s.env.ControllerInstances(s.callCtx, "foo") + ids, err := s.env.ControllerInstances(s.callCtx, testing.ControllerTag.Id()) c.Assert(err, jc.ErrorIsNil) c.Assert(ids, gc.HasLen, 1) c.Assert(ids[0], gc.Equals, instance.Id("machine-0")) diff --git a/resource/opener.go b/resource/opener.go index 1701fa7ba79..2f7770c2aa7 100644 --- a/resource/opener.go +++ b/resource/opener.go @@ -74,15 +74,18 @@ func newInternalResourceOpener( NewClient() (*ResourceRetryClient, error) } - var charmURL *charm.URL + var chURLStr *string if unit != nil { - charmURL, _ = unit.CharmURL() + chURLStr = unit.CharmURL() } else { - cURL, _ := application.CharmURL() - charmURL, err = charm.ParseURL(*cURL) - if err != nil { - return nil, errors.Trace(err) - } + chURLStr, _ = application.CharmURL() + } + if chURLStr == nil { + return nil, errors.Errorf("missing charm URL for %q", appName) + } + charmURL, err := charm.ParseURL(*chURLStr) + if err != nil { + return nil, errors.Trace(err) } switch { case charm.CharmHub.Matches(charmURL.Schema): diff --git a/rpc/params/internal.go b/rpc/params/internal.go index 8f9382ae73b..b746b6e1445 100644 --- a/rpc/params/internal.go +++ b/rpc/params/internal.go @@ -768,6 +768,12 @@ type AgentVersionResult struct { Version version.Number `json:"version"` } +// RetryProvisioningArgs holds args for retrying machine provisioning. +type RetryProvisioningArgs struct { + Machines []string `json:"machines,omitempty"` + All bool `json:"all"` +} + // ProvisioningNetworkTopology holds a network topology that is based on // positive machine space constraints. // This is used for creating NICs on instances where the provider is not space diff --git a/scripts/juju-list-blobstore/main.go b/scripts/juju-list-blobstore/main.go index 7f47d4c6572..08df7bb2aa7 100644 --- a/scripts/juju-list-blobstore/main.go +++ b/scripts/juju-list-blobstore/main.go @@ -627,14 +627,12 @@ func (checker *ModelChecker) readApplicationsAndUnits() { units, err := app.AllUnits() checkErr(err, "AllUnits") for _, unit := range units { - unitCharmURL, err := unit.CharmURL() - checkErr(err, "unit CharmURL") + unitCharmURL := unit.CharmURL() if unitCharmURL == nil { continue } - unitString := unitCharmURL.String() - if unitString != *charmURL { - checker.unitReferencedCharms.Add(unitString, unit.Name()) + if *unitCharmURL != *charmURL { + checker.unitReferencedCharms.Add(*unitCharmURL, unit.Name()) } tools, err := unit.AgentTools() checkErr(err, "unit AgentTools") diff --git a/state/action_test.go b/state/action_test.go index 797479a67e2..3b6234b21a4 100644 --- a/state/action_test.go +++ b/state/action_test.go @@ -164,7 +164,10 @@ func (s *ActionSuite) TestAddAction(c *gc.C) { if t.expectedErr == "" { c.Assert(err, jc.ErrorIsNil) - curl, _ := t.whichUnit.CharmURL() + curlStr := t.whichUnit.CharmURL() + c.Assert(curlStr, gc.NotNil) + curl, err := charm.ParseURL(*curlStr) + c.Assert(err, jc.ErrorIsNil) ch, _ := s.State.Charm(curl) schema := ch.Actions() c.Logf("Schema for unit %q:\n%#v", t.whichUnit.Name(), schema) diff --git a/state/application_test.go b/state/application_test.go index 89ab0153348..728029ce107 100644 --- a/state/application_test.go +++ b/state/application_test.go @@ -2024,8 +2024,8 @@ func (s *ApplicationSuite) TestSettingsRefCountWorks(c *gc.C) { // refcount. u, err := app.AddUnit(state.AddUnitParams{}) c.Assert(err, jc.ErrorIsNil) - _, ok := u.CharmURL() - c.Assert(ok, jc.IsFalse) + charmURL := u.CharmURL() + c.Assert(charmURL, gc.IsNil) assertSettingsRef(c, s.State, appName, oldCh, 1) assertNoSettingsRef(c, s.State, appName, newCh) @@ -2033,7 +2033,9 @@ func (s *ApplicationSuite) TestSettingsRefCountWorks(c *gc.C) { // used by app as well, hence 2. err = u.SetCharmURL(oldCh.URL()) c.Assert(err, jc.ErrorIsNil) - curl, err := u.CharmURL() + charmURL = u.CharmURL() + c.Assert(charmURL, gc.NotNil) + curl, err := charm.ParseURL(*charmURL) c.Assert(err, jc.ErrorIsNil) c.Assert(curl, gc.DeepEquals, oldCh.URL()) assertSettingsRef(c, s.State, appName, oldCh, 2) diff --git a/state/machine.go b/state/machine.go index 9f794294c59..6255dad6f19 100644 --- a/state/machine.go +++ b/state/machine.go @@ -626,7 +626,11 @@ func (m *Machine) destroyControllerWithPrincipals() error { if !ok { continue } - curl, err := unit.CharmURL() + curlStr := unit.CharmURL() + if curlStr == nil { + continue + } + curl, err := charm.ParseURL(*curlStr) if err != nil { return errors.Trace(err) } @@ -2200,12 +2204,11 @@ func (m *Machine) UpdateMachineSeries(series string) error { Update: bson.D{{"$set", bson.D{{"series", series}}}}, }} for _, unit := range units { - curl, _ := unit.CharmURL() ops = append(ops, txn.Op{ C: unitsC, Id: unit.doc.DocID, Assert: bson.D{{"life", Alive}, - {"charmurl", curl}, + {"charmurl", unit.CharmURL()}, {"subordinates", unit.SubordinateNames()}}, Update: bson.D{{"$set", bson.D{{"series", series}}}}, }) diff --git a/state/metrics_test.go b/state/metrics_test.go index 4c73b911739..b332ce7107d 100644 --- a/state/metrics_test.go +++ b/state/metrics_test.go @@ -551,13 +551,13 @@ func (s *MetricSuite) TestMetricValidation(c *gc.C) { }} for i, t := range tests { c.Logf("test %d: %s", i, t.about) - chURL, err := t.unit.CharmURL() - c.Assert(err, jc.ErrorIsNil) + chURL := t.unit.CharmURL() + c.Assert(chURL, gc.NotNil) _, err = s.State.AddMetrics( state.BatchParam{ UUID: utils.MustNewUUID().String(), Created: now, - CharmURL: chURL.String(), + CharmURL: *chURL, Metrics: t.metrics, Unit: t.unit.UnitTag(), }, diff --git a/state/storage.go b/state/storage.go index 8ba740de491..73315d2f2bd 100644 --- a/state/storage.go +++ b/state/storage.go @@ -637,7 +637,7 @@ func validateRemoveOwnerStorageInstanceOps(si *storageInstance) ([]txn.Op, error Id: app.Name(), Assert: bson.D{ {"life", Alive}, - {"charmurl", ch.URL}, + {"charmurl", ch.String()}, }, }) case names.UnitTagKind: diff --git a/state/unit.go b/state/unit.go index f6e3df5ae8f..9cc142de484 100644 --- a/state/unit.go +++ b/state/unit.go @@ -1459,13 +1459,8 @@ func (u *Unit) OpenedPortRanges() (UnitPortRanges, error) { } // CharmURL returns the charm URL this unit is currently using. -func (u *Unit) CharmURL() (*charm.URL, error) { - if u.doc.CharmURL == nil { - return nil, nil - } - - cURL, err := charm.ParseURL(*u.doc.CharmURL) - return cURL, errors.Trace(err) +func (u *Unit) CharmURL() *string { + return u.doc.CharmURL } // SetCharmURL marks the unit as currently using the supplied charm URL. @@ -1495,7 +1490,7 @@ func (u *Unit) SetCharmURL(curl *charm.URL) error { return nil, stateerrors.ErrDead } } - sel := bson.D{{"_id", u.doc.DocID}, {"charmurl", curl}} + sel := bson.D{{"_id", u.doc.DocID}, {"charmurl", curl.String()}} if count, err := units.Find(sel).Count(); err != nil { return nil, errors.Trace(err) } else if count == 1 { @@ -1516,13 +1511,13 @@ func (u *Unit) SetCharmURL(curl *charm.URL) error { } // Set the new charm URL. - differentCharm := bson.D{{"charmurl", bson.D{{"$ne", curl}}}} + differentCharm := bson.D{{"charmurl", bson.D{{"$ne", curl.String()}}}} ops := append(incOps, txn.Op{ C: unitsC, Id: u.doc.DocID, Assert: append(notDeadDoc, differentCharm...), - Update: bson.D{{"$set", bson.D{{"charmurl", curl}}}}, + Update: bson.D{{"$set", bson.D{{"charmurl", curl.String()}}}}, }) unitCURL := u.doc.CharmURL @@ -1553,23 +1548,22 @@ func (u *Unit) SetCharmURL(curl *charm.URL) error { // charm returns the charm for the unit, or the application if the unit's charm // has not been set yet. func (u *Unit) charm() (*Charm, error) { - cURL, err := u.CharmURL() - if err != nil { - return nil, errors.Trace(err) - } - - if cURL == nil { + cURLStr := u.CharmURL() + if cURLStr == nil { app, err := u.Application() if err != nil { return nil, err } - appCURLStr, _ := app.CharmURL() - cURL, err = charm.ParseURL(*appCURLStr) - if err != nil { - return nil, errors.NotValidf("application charm url") - } + cURLStr, _ = app.CharmURL() } + if cURLStr == nil { + return nil, errors.Errorf("missing charm URL for %q", u.Name()) + } + cURL, err := charm.ParseURL(*cURLStr) + if err != nil { + return nil, errors.NotValidf("charm url %q", *cURLStr) + } ch, err := u.st.Charm(cURL) return ch, errors.Annotatef(err, "getting charm for %s", u) } diff --git a/state/unit_test.go b/state/unit_test.go index ff957357bf4..2235e239fa5 100644 --- a/state/unit_test.go +++ b/state/unit_test.go @@ -1262,22 +1262,20 @@ func (s *UnitSuite) TestRefresh(c *gc.C) { func (s *UnitSuite) TestSetCharmURLSuccess(c *gc.C) { preventUnitDestroyRemove(c, s.unit) - curl, ok := s.unit.CharmURL() - c.Assert(ok, jc.IsFalse) + curl := s.unit.CharmURL() c.Assert(curl, gc.IsNil) err := s.unit.SetCharmURL(s.charm.URL()) c.Assert(err, jc.ErrorIsNil) - curl, err = s.unit.CharmURL() - c.Assert(err, jc.ErrorIsNil) - c.Assert(curl, gc.DeepEquals, s.charm.URL()) + curl = s.unit.CharmURL() + c.Assert(curl, gc.NotNil) + c.Assert(*curl, gc.Equals, s.charm.URL().String()) } func (s *UnitSuite) TestSetCharmURLFailures(c *gc.C) { preventUnitDestroyRemove(c, s.unit) - curl, ok := s.unit.CharmURL() - c.Assert(ok, jc.IsFalse) + curl := s.unit.CharmURL() c.Assert(curl, gc.IsNil) err := s.unit.SetCharmURL(nil) @@ -1310,9 +1308,9 @@ func (s *UnitSuite) TestSetCharmURLWithDyingUnit(c *gc.C) { err = s.unit.SetCharmURL(s.charm.URL()) c.Assert(err, jc.ErrorIsNil) - curl, err := s.unit.CharmURL() - c.Assert(err, jc.ErrorIsNil) - c.Assert(curl, gc.DeepEquals, s.charm.URL()) + curl := s.unit.CharmURL() + c.Assert(curl, gc.NotNil) + c.Assert(*curl, gc.Equals, s.charm.URL().String()) } func (s *UnitSuite) TestSetCharmURLRetriesWithDeadUnit(c *gc.C) { @@ -1359,9 +1357,9 @@ func (s *UnitSuite) TestSetCharmURLRetriesWithDifferentURL(c *gc.C) { // Verify it worked after the second attempt. err := s.unit.Refresh() c.Assert(err, jc.ErrorIsNil) - currentURL, err := s.unit.CharmURL() - c.Assert(err, jc.ErrorIsNil) - c.Assert(currentURL, jc.DeepEquals, s.charm.URL()) + currentURL := s.unit.CharmURL() + c.Assert(currentURL, gc.NotNil) + c.Assert(*currentURL, gc.Equals, s.charm.URL().String()) }, }, ).Check() diff --git a/testing/factory/factory.go b/testing/factory/factory.go index 463b5b8277b..eb79573c447 100644 --- a/testing/factory/factory.go +++ b/testing/factory/factory.go @@ -679,14 +679,14 @@ func (factory *Factory) MakeMetric(c *gc.C, params *MetricParams) *state.MetricB }} } - chURL, err := params.Unit.CharmURL() - c.Assert(err, jc.ErrorIsNil) + chURL := params.Unit.CharmURL() + c.Assert(chURL, gc.NotNil) metric, err := factory.st.AddMetrics( state.BatchParam{ UUID: utils.MustNewUUID().String(), Created: *params.Time, - CharmURL: chURL.String(), + CharmURL: *chURL, Metrics: params.Metrics, Unit: params.Unit.UnitTag(), }) diff --git a/testing/factory/factory_test.go b/testing/factory/factory_test.go index 7b79a3716c7..7ff7acad702 100644 --- a/testing/factory/factory_test.go +++ b/testing/factory/factory_test.go @@ -401,8 +401,10 @@ func (s *factorySuite) TestMakeUnit(c *gc.C) { c.Assert(saved.Life(), gc.Equals, unit.Life()) applicationCharmURL, _ := application.CharmURL() - unitCharmURL, _ := saved.CharmURL() - c.Assert(unitCharmURL.String(), gc.Equals, *applicationCharmURL) + c.Assert(applicationCharmURL, gc.NotNil) + unitCharmURL := saved.CharmURL() + c.Assert(unitCharmURL, gc.NotNil) + c.Assert(*unitCharmURL, gc.Equals, *applicationCharmURL) } func (s *factorySuite) TestMakeRelationNil(c *gc.C) { diff --git a/tests/includes/check.sh b/tests/includes/check.sh index b11a495198d..51f88d109bb 100644 --- a/tests/includes/check.sh +++ b/tests/includes/check.sh @@ -85,6 +85,23 @@ check_gt() { fi } +check_ge() { + local input value chk + + input=${1} + shift + + value=${1} + shift + + if [[ ${input} > ${value} ]] || [ "${input}" == "${value}" ]; then + printf "Success: \"%s\" >= \"%s\"\n" "${input}" "${value}" >&2 + else + printf "Expected \"%s\" >= \"%s\"\n" "${input}" "${value}" >&2 + exit 1 + fi +} + check() { local want got diff --git a/tests/main.sh b/tests/main.sh index a73609c6a8b..9d4cee8ab63 100755 --- a/tests/main.sh +++ b/tests/main.sh @@ -44,6 +44,7 @@ TEST_NAMES="agents \ caasadmission \ charmhub \ cli \ + constraints \ controller \ ck \ deploy \ diff --git a/tests/suites/cli/local_charms.sh b/tests/suites/cli/local_charms.sh index c1233d24a06..191ad70168b 100644 --- a/tests/suites/cli/local_charms.sh +++ b/tests/suites/cli/local_charms.sh @@ -8,7 +8,7 @@ run_deploy_local_charm_revision() { TMP=$(mktemp -d -t ci-XXXXXXXXXX) cd "${TMP}" || exit 1 - git clone --depth=1 --quiet https://github.com/lampkicking/charm-ntp.git ntp + git clone --depth=1 --quiet https://git.launchpad.net/ntp-charm ntp cd "${TMP}/ntp" || exit 1 SHA_OF_NTP=\"$(git describe --dirty --always)\" @@ -38,11 +38,11 @@ run_deploy_local_charm_revision_no_vcs() { TMP=$(mktemp -d -t ci-XXXXXXXXXX) cd "${TMP}" || exit 1 - git clone --depth=1 --quiet https://github.com/lampkicking/charm-ntp.git ntp + git clone --depth=1 --quiet https://git.launchpad.net/ntp-charm ntp cd "${TMP}/ntp" || exit 1 rm -rf .git # make sure that no version file exists. - rm version + rm -f version OUTPUT=$(juju deploy --debug . 2>&1) @@ -61,7 +61,7 @@ run_deploy_local_charm_revision_no_vcs_but_version_file() { TMP=$(mktemp -d -t ci-XXXXXXXXXX) cd "${TMP}" || exit 1 - git clone --depth=1 --quiet https://github.com/lampkicking/charm-ntp.git ntp + git clone --depth=1 --quiet https://git.launchpad.net/ntp-charm ntp cd "${TMP}/ntp" || exit 1 rm -rf .git touch version @@ -101,7 +101,7 @@ run_deploy_local_charm_revision_relative_path() { create_local_git_folder SHA_OF_TMP=\"$(git describe --dirty --always)\" # create ${TMP}/ntp git folder - git clone --depth=1 --quiet https://github.com/lampkicking/charm-ntp.git ntp + git clone --depth=1 --quiet https://git.launchpad.net/ntp-charm ntp # state: ${TMP} is wrong git ${TMP}/ntp is correct git juju deploy ./ntp 2>&1 @@ -138,7 +138,7 @@ run_deploy_local_charm_revision_invalid_git() { TMP=$(mktemp -d -t ci-XXXXXXXXXX) cd "${TMP_CHARM_GIT}" || exit 1 - git clone --depth=1 --quiet https://github.com/lampkicking/charm-ntp.git ntp + git clone --depth=1 --quiet https://git.launchpad.net/ntp-charm ntp cd "${TMP_CHARM_GIT}/ntp" || exit 1 WANTED_CHARM_SHA=\"$(git describe --dirty --always)\" diff --git a/tests/suites/constraints/constraints.sh b/tests/suites/constraints/constraints.sh new file mode 100644 index 00000000000..4c1e666030d --- /dev/null +++ b/tests/suites/constraints/constraints.sh @@ -0,0 +1,88 @@ +run_constraints_lxd() { + # Echo out to ensure nice output to the test suite. + echo + + file="${TEST_DIR}/constraints-lxd.txt" + + ensure "constraints-lxd" "${file}" + + echo "Deploy 2 machines with different constraints" + juju add-machine --constraints "cores=2" + juju add-machine --constraints "cores=2 mem=2G" + + wait_for_machine_agent_status "0" "started" + wait_for_machine_agent_status "1" "started" + + echo "Ensure machine 0 has 2 cores" + machine0_hardware=$(juju machines --format json | jq -r '.["machines"]["0"]["hardware"]') + check_contains "${machine0_hardware}" "cores=2" + + echo "Ensure machine 1 has 2 cores and 2G memory" + machine1_hardware=$(juju machines --format json | jq -r '.["machines"]["1"]["hardware"]') + check_contains "${machine1_hardware}" "cores=2" + check_contains "${machine1_hardware}" "mem=2048M" + + destroy_model "constraints-lxd" +} + +run_constraints_aws() { + # Echo out to ensure nice output to the test suite. + echo + + file="${TEST_DIR}/constraints-aws.txt" + + ensure "constraints-aws" "${file}" + + echo "Deploy 3 machines with different constraints" + juju add-machine --constraints "root-disk=16G" + juju add-machine --constraints "cores=4 root-disk=16G" + juju add-machine --constraints "instance-type=t2.nano" + + wait_for_machine_agent_status "0" "started" + wait_for_machine_agent_status "1" "started" + wait_for_machine_agent_status "2" "started" + + echo "Ensure machine 0 has 16G root disk" + machine0_hardware=$(juju machines --format json | jq -r '.["machines"]["0"]["hardware"]') + machine0_rootdisk=$(echo "$machine0_hardware" | awk '{for(i=1;i<=NF;i++){if($i ~ /root-disk/){print $i}}}') + check_ge "${machine0_rootdisk}" "root-disk=16384M" + + echo "Ensure machine 1 has 4 cores and 16G root disk" + machine1_hardware=$(juju machines --format json | jq -r '.["machines"]["1"]["hardware"]') + machine1_cores=$(echo "$machine1_hardware" | awk '{for(i=1;i<=NF;i++){if($i ~ /cores/){print $i}}}') + machine1_rootdisk=$(echo "$machine1_hardware" | awk '{for(i=1;i<=NF;i++){if($i ~ /root-disk/){print $i}}}') + check_ge "${machine1_cores}" "cores=4" + check_ge "${machine1_rootdisk}" "root-disk=16384M" + + echo "Ensure machine 2 has t2.nano instance type" + machine2_constraints=$(juju machines --format json | jq -r '.["machines"]["2"]["constraints"]') + check_contains "${machine2_constraints}" "instance-type=t2.nano" + + destroy_model "constraints-aws" +} + +test_constraints_lxd() { + if [ "$(skip 'test_constraints_lxd')" ]; then + echo "==> TEST SKIPPED: constraints lxd" + return + fi + + ( + set_verbosity + cd .. || exit + run "run_constraints_lxd" + ) +} + +test_constraints_aws() { + if [ "$(skip 'test_constraints_aws')" ]; then + echo "==> TEST SKIPPED: constraints aws" + return + fi + + ( + set_verbosity + cd .. || exit + run "run_constraints_aws" + ) +} diff --git a/tests/suites/constraints/task.sh b/tests/suites/constraints/task.sh new file mode 100644 index 00000000000..90297ee7730 --- /dev/null +++ b/tests/suites/constraints/task.sh @@ -0,0 +1,27 @@ +test_constraints() { + if [ "$(skip 'test_constraints')" ]; then + echo "==> TEST SKIPPED: constraints tests" + return + fi + + set_verbosity + + echo "==> Checking for dependencies" + check_dependencies juju + + file="${TEST_DIR}/test-constraints.txt" + + bootstrap "test-constraints" "${file}" + + + case "${BOOTSTRAP_PROVIDER:-}" in + "aws") + test_constraints_aws + ;; + "lxd") + test_constraints_lxd + ;; + esac + + destroy_controller "test-constraints" +} diff --git a/tests/suites/deploy/deploy_revision.sh b/tests/suites/deploy/deploy_revision.sh index be491bb2aba..972ced32b05 100644 --- a/tests/suites/deploy/deploy_revision.sh +++ b/tests/suites/deploy/deploy_revision.sh @@ -11,7 +11,8 @@ run_deploy_revision() { wait_for "juju-qa-test" "$(charm_rev "juju-qa-test" 9)" # check resource revision per channel specified. - juju resources juju-qa-test --format json | jq -S '.resources[0] | .[ "revision"] == "1"' + got=$(juju resources juju-qa-test --format json | jq -S '.resources[0] | .["revision"] == "1"') + check_contains "${got}" "true" destroy_model "${model_name}" } @@ -29,7 +30,8 @@ run_deploy_revision_resource() { wait_for "juju-qa-test" "$(charm_rev "juju-qa-test" 9)" # check resource revision as specified in command. - juju resources juju-qa-test --format json | jq -S '.resources[0] | .[ "revision"] == "4"' + got=$(juju resources juju-qa-test --format json | jq -S '.resources[0] | .["revision"] == "4"') + check_contains "${got}" "true" destroy_model "${model_name}" } diff --git a/worker/uniter/util_test.go b/worker/uniter/util_test.go index fab7453f94d..a9ca3d6b87c 100644 --- a/worker/uniter/util_test.go +++ b/worker/uniter/util_test.go @@ -810,14 +810,15 @@ func (s waitUnitAgent) step(c *gc.C, ctx *testContext) { c.Logf("want resolved mode %q, got %q; still waiting", s.resolved, resolved) continue } - url, err := ctx.unit.CharmURL() - if err != nil { - c.Fatalf("cannot refresh unit: %v", err) - } - if url == nil { + urlStr := ctx.unit.CharmURL() + if urlStr == nil { c.Logf("want unit charm %q, got nil; still waiting", curl(s.charm)) continue } + url, err := corecharm.ParseURL(*urlStr) + if err != nil { + c.Fatalf("cannot refresh unit: %v", err) + } if *url != *curl(s.charm) { c.Logf("want unit charm %q, got %q; still waiting", curl(s.charm), url.String()) continue @@ -1045,7 +1046,9 @@ func (s verifyCharm) step(c *gc.C, ctx *testContext) { err = ctx.unit.Refresh() c.Assert(err, jc.ErrorIsNil) - url, err := ctx.unit.CharmURL() + urlStr := ctx.unit.CharmURL() + c.Assert(urlStr, gc.NotNil) + url, err := corecharm.ParseURL(*urlStr) c.Assert(err, jc.ErrorIsNil) c.Assert(url, gc.DeepEquals, curl(checkRevision)) }