From 02da47a8a6fe22b29dfb42d1cccb9fc2983517be Mon Sep 17 00:00:00 2001 From: Ian Booth Date: Fri, 9 Feb 2018 15:50:49 +1000 Subject: [PATCH] Add caas operator presence, and status fixes --- apiserver/common/modelstatus.go | 33 +++++------ apiserver/common/modelstatus_test.go | 49 ++++++++++++++++ .../caasunitprovisioner/provisioner.go | 35 +++++++----- .../caasunitprovisioner/provisioner_test.go | 10 ++-- cmd/juju/status/output_tabular.go | 15 +++-- cmd/juju/status/status_test.go | 12 +++- state/application.go | 56 +++++++++++++++++++ state/application_test.go | 47 ++++++++++++++++ state/machine.go | 2 +- state/unit.go | 8 +-- 10 files changed, 214 insertions(+), 53 deletions(-) diff --git a/apiserver/common/modelstatus.go b/apiserver/common/modelstatus.go index 9de4f007583..d4ea4044672 100644 --- a/apiserver/common/modelstatus.go +++ b/apiserver/common/modelstatus.go @@ -93,28 +93,29 @@ func (c *ModelStatusAPI) modelStatus(tag string) (params.ModelStatus, error) { return status, errors.Trace(err) } - volumes, err := st.AllVolumes() - if err != nil { - return status, errors.Trace(err) - } - modelVolumes := ModelVolumeInfo(volumes) - - filesystems, err := st.AllFilesystems() - if err != nil { - return status, errors.Trace(err) - } - modelFilesystems := ModelFilesystemInfo(filesystems) - - return params.ModelStatus{ + result := params.ModelStatus{ ModelTag: tag, OwnerTag: model.Owner().String(), Life: params.Life(model.Life().String()), HostedMachineCount: len(hostedMachines), ApplicationCount: len(applications), Machines: modelMachines, - Volumes: modelVolumes, - Filesystems: modelFilesystems, - }, nil + } + + if model.Type() == state.ModelTypeIAAS { + volumes, err := st.AllVolumes() + if err != nil { + return status, errors.Trace(err) + } + result.Volumes = ModelVolumeInfo(volumes) + + filesystems, err := st.AllFilesystems() + if err != nil { + return status, errors.Trace(err) + } + result.Filesystems = ModelFilesystemInfo(filesystems) + } + return result, nil } // ModelFilesystemInfo returns information about filesystems in the model. diff --git a/apiserver/common/modelstatus_test.go b/apiserver/common/modelstatus_test.go index 5454a6fa7ad..4bb1ddbb827 100644 --- a/apiserver/common/modelstatus_test.go +++ b/apiserver/common/modelstatus_test.go @@ -17,6 +17,7 @@ import ( "github.com/juju/juju/constraints" "github.com/juju/juju/environs" "github.com/juju/juju/environs/config" + "github.com/juju/juju/feature" "github.com/juju/juju/instance" "github.com/juju/juju/provider/dummy" "github.com/juju/juju/state" @@ -218,6 +219,54 @@ func (s *modelStatusSuite) TestModelStatus(c *gc.C) { }) } +func (s *modelStatusSuite) TestModelStatusCAAS(c *gc.C) { + s.SetFeatureFlags(feature.CAAS) + otherModelOwner := s.Factory.MakeModelUser(c, nil) + otherSt := s.Factory.MakeModel(c, &factory.ModelParams{ + Name: "caas-model", + Type: state.ModelTypeCAAS, CloudRegion: "", + Owner: otherModelOwner.UserTag, + ConfigAttrs: testing.Attrs{ + "controller": false, + }, + StorageProviderRegistry: factory.NilStorageProviderRegistry{}}) + defer otherSt.Close() + + otherFactory := factory.NewFactory(otherSt) + otherFactory.MakeApplication(c, &factory.ApplicationParams{ + Charm: otherFactory.MakeCharm(c, nil), + }) + + otherModel, err := otherSt.Model() + c.Assert(err, jc.ErrorIsNil) + + controllerModelTag := s.IAASModel.ModelTag().String() + hostedModelTag := otherModel.ModelTag().String() + + req := params.Entities{ + Entities: []params.Entity{{Tag: controllerModelTag}, {Tag: hostedModelTag}}, + } + results, err := s.controller.ModelStatus(req) + c.Assert(err, jc.ErrorIsNil) + + c.Assert(results.Results, jc.DeepEquals, []params.ModelStatus{ + params.ModelStatus{ + ModelTag: controllerModelTag, + HostedMachineCount: 0, + ApplicationCount: 0, + OwnerTag: s.Owner.String(), + Life: params.Alive, + }, + params.ModelStatus{ + ModelTag: hostedModelTag, + HostedMachineCount: 0, + ApplicationCount: 1, + OwnerTag: otherModelOwner.UserTag.String(), + Life: params.Alive, + }, + }) +} + func (s *modelStatusSuite) TestModelStatusRunsForAllModels(c *gc.C) { req := params.Entities{ Entities: []params.Entity{ diff --git a/apiserver/facades/controller/caasunitprovisioner/provisioner.go b/apiserver/facades/controller/caasunitprovisioner/provisioner.go index 02c4b46d55e..0324abecb09 100644 --- a/apiserver/facades/controller/caasunitprovisioner/provisioner.go +++ b/apiserver/facades/controller/caasunitprovisioner/provisioner.go @@ -339,28 +339,36 @@ func (a *Facade) updateUnitsFromCloud(app Application, units []params.Applicatio unitUpdate.Deletes = append(unitUpdate.Deletes, u.DestroyOperation()) } - unitUpdateProperties := func(unitParams params.ApplicationUnitParams) state.UnitUpdateProperties { - return state.UnitUpdateProperties{ + unitUpdateProperties := func(unitParams params.ApplicationUnitParams, includeStatus bool) state.UnitUpdateProperties { + props := state.UnitUpdateProperties{ ProviderId: unitParams.ProviderId, Address: unitParams.Address, Ports: unitParams.Ports, - Status: &status.StatusInfo{ + } + if includeStatus { + props.Status = &status.StatusInfo{ Status: status.Status(unitParams.Status), Message: unitParams.Info, Data: unitParams.Data, - }, + } } + return props } - shouldUpdate := func(u Unit, params params.ApplicationUnitParams) (bool, error) { - if u.ProviderId() == "" { - return true, nil + shouldUpdateStatus := func(u Unit, params params.ApplicationUnitParams) (bool, error) { + // The container runtime can spam us with unimportant + // status updates, so ignore any irrelevant ones. + // TODO(caas) - the pods may get bounced but we don't model that yet + // so ignore allocating and running for now. + switch status.Status(params.Status) { + case status.Unknown, status.Allocating, status.Running: + return false, nil } existingStatus, err := u.AgentStatus() if err != nil { return false, errors.Trace(err) } - if string(existingStatus.Status) != params.Status || + if existingStatus.Status.String() != params.Status || existingStatus.Message != params.Info || len(existingStatus.Data) != len(params.Data) || reflect.DeepEqual(existingStatus.Data, params.Data) { @@ -380,15 +388,12 @@ func (a *Facade) updateUnitsFromCloud(app Application, units []params.Applicatio continue } // Check to see if any update is needed. - update, err := shouldUpdate(u, unitParams) + updateStatus, err := shouldUpdateStatus(u, unitParams) if err != nil { return errors.Trace(err) } - if !update { - continue - } unitUpdate.Updates = append(unitUpdate.Updates, - u.UpdateOperation(unitUpdateProperties(unitParams))) + u.UpdateOperation(unitUpdateProperties(unitParams, updateStatus))) } // For newly added units in the cloud, either update state units which @@ -399,13 +404,13 @@ func (a *Facade) updateUnitsFromCloud(app Application, units []params.Applicatio if idx < len(unassociatedUnits) { u := unassociatedUnits[idx] unitUpdate.Updates = append(unitUpdate.Updates, - u.UpdateOperation(unitUpdateProperties(unitParams))) + u.UpdateOperation(unitUpdateProperties(unitParams, true))) idx += 1 continue } unitUpdate.Adds = append(unitUpdate.Adds, - app.AddOperation(unitUpdateProperties(unitParams))) + app.AddOperation(unitUpdateProperties(unitParams, true))) } return app.UpdateUnits(&unitUpdate) } diff --git a/apiserver/facades/controller/caasunitprovisioner/provisioner_test.go b/apiserver/facades/controller/caasunitprovisioner/provisioner_test.go index 26eab7d12e5..15957b87954 100644 --- a/apiserver/facades/controller/caasunitprovisioner/provisioner_test.go +++ b/apiserver/facades/controller/caasunitprovisioner/provisioner_test.go @@ -204,7 +204,7 @@ func (s *CAASProvisionerSuite) TestUpdateApplicationsUnitsNoTags(c *gc.C) { {ProviderId: "another-uuid", Address: "another-address", Ports: []string{"another-port"}, Status: "running", Info: "another message"}, {ProviderId: "last-uuid", Address: "last-address", Ports: []string{"last-port"}, - Status: "running", Info: "last message"}, + Status: "error", Info: "last message"}, } args := params.UpdateApplicationUnitArgs{ Args: []params.UpdateApplicationUnits{ @@ -224,13 +224,12 @@ func (s *CAASProvisionerSuite) TestUpdateApplicationsUnitsNoTags(c *gc.C) { s.st.application.CheckCall(c, 0, "AddOperation", state.UnitUpdateProperties{ ProviderId: "last-uuid", Address: "last-address", Ports: []string{"last-port"}, - Status: &status.StatusInfo{Status: status.Running, Message: "last message"}, + Status: &status.StatusInfo{Status: status.Error, Message: "last message"}, }) s.st.application.units[0].(*mockUnit).CheckCallNames(c, "Life", "UpdateOperation") s.st.application.units[0].(*mockUnit).CheckCall(c, 1, "UpdateOperation", state.UnitUpdateProperties{ ProviderId: "uuid", Address: "address", Ports: []string{"port"}, - Status: &status.StatusInfo{Status: status.Running, Message: "message"}, }) s.st.application.units[1].(*mockUnit).CheckCallNames(c, "Life", "UpdateOperation") s.st.application.units[1].(*mockUnit).CheckCall(c, 1, "UpdateOperation", state.UnitUpdateProperties{ @@ -252,7 +251,7 @@ func (s *CAASProvisionerSuite) TestUpdateApplicationsUnitsWithTags(c *gc.C) { {ProviderId: "uuid", UnitTag: "unit-gitlab-0", Address: "address", Ports: []string{"port"}, Status: "running", Info: "message"}, {ProviderId: "another-uuid", UnitTag: "unit-gitlab-1", Address: "another-address", Ports: []string{"another-port"}, - Status: "running", Info: "another message"}, + Status: "error", Info: "another message"}, } args := params.UpdateApplicationUnitArgs{ Args: []params.UpdateApplicationUnits{ @@ -272,13 +271,12 @@ func (s *CAASProvisionerSuite) TestUpdateApplicationsUnitsWithTags(c *gc.C) { s.st.application.units[0].(*mockUnit).CheckCall(c, 1, "UpdateOperation", state.UnitUpdateProperties{ ProviderId: "uuid", Address: "address", Ports: []string{"port"}, - Status: &status.StatusInfo{Status: status.Running, Message: "message"}, }) s.st.application.units[1].(*mockUnit).CheckCallNames(c, "Life", "UpdateOperation") s.st.application.units[1].(*mockUnit).CheckCall(c, 1, "UpdateOperation", state.UnitUpdateProperties{ ProviderId: "another-uuid", Address: "another-address", Ports: []string{"another-port"}, - Status: &status.StatusInfo{Status: status.Running, Message: "another message"}, + Status: &status.StatusInfo{Status: status.Error, Message: "another message"}, }) s.st.application.units[2].(*mockUnit).CheckCallNames(c, "Life") } diff --git a/cmd/juju/status/output_tabular.go b/cmd/juju/status/output_tabular.go index 2a8e2ff8f05..33bc4047ecf 100644 --- a/cmd/juju/status/output_tabular.go +++ b/cmd/juju/status/output_tabular.go @@ -24,6 +24,8 @@ import ( "github.com/juju/juju/status" ) +const caasModelType = "caas" + // FormatTabular writes a tabular summary of machines, applications, and // units. Any subordinate items are indented by two spaces beneath // their superior. @@ -38,7 +40,7 @@ func FormatTabular(writer io.Writer, forceColor bool, value interface{}) error { } maxVersionWidth := iaasMaxVersionWidth - if fs.Model.Type == "caas" { + if fs.Model.Type == caasModelType { maxVersionWidth = caasMaxVersionWidth } truncatedWidth := maxVersionWidth - len(ellipsis) @@ -112,7 +114,7 @@ func FormatTabular(writer io.Writer, forceColor bool, value interface{}) error { app := fs.Applications[appName] version := app.Version // CAAS versions may have repo prefix we don't care about. - if fs.Model.Type == "caas" { + if fs.Model.Type == caasModelType { parts := strings.Split(version, "/") if len(parts) == 2 { version = parts[1] @@ -149,8 +151,6 @@ func FormatTabular(writer io.Writer, forceColor bool, value interface{}) error { } } - const caasModelType = "caas" - pUnit := func(name string, u unitStatus, level int) { message := u.WorkloadStatusInfo.Message agentDoing := agentDoing(u.JujuStatusInfo) @@ -161,8 +161,9 @@ func FormatTabular(writer io.Writer, forceColor bool, value interface{}) error { name += "*" } w.Print(indent("", level*2, name)) + w.PrintStatus(u.WorkloadStatusInfo.Current) + w.PrintStatus(u.JujuStatusInfo.Current) if fs.Model.Type == caasModelType { - w.PrintStatus(u.JujuStatusInfo.Current) p( u.Address, strings.Join(u.OpenedPorts, ","), @@ -170,8 +171,6 @@ func FormatTabular(writer io.Writer, forceColor bool, value interface{}) error { ) return } - w.PrintStatus(u.WorkloadStatusInfo.Current) - w.PrintStatus(u.JujuStatusInfo.Current) p( u.Machine, u.PublicAddress, @@ -181,7 +180,7 @@ func FormatTabular(writer io.Writer, forceColor bool, value interface{}) error { } if fs.Model.Type == caasModelType { - outputHeaders("Unit", "Status", "Address", "Ports", "Message") + outputHeaders("Unit", "Workload", "Agent", "Address", "Ports", "Message") } else { outputHeaders("Unit", "Workload", "Agent", "Machine", "Public address", "Ports", "Message") } diff --git a/cmd/juju/status/status_test.go b/cmd/juju/status/status_test.go index a9ed157b8bf..2153ffe133f 100644 --- a/cmd/juju/status/status_test.go +++ b/cmd/juju/status/status_test.go @@ -4186,6 +4186,9 @@ func (s *StatusSuite) TestFormatTabularCAASModel(c *gc.C) { JujuStatusInfo: statusInfoContents{ Current: status.Allocating, }, + WorkloadStatusInfo: statusInfoContents{ + Current: status.Active, + }, }, "foo/1": { Address: "10.0.0.1", @@ -4193,6 +4196,9 @@ func (s *StatusSuite) TestFormatTabularCAASModel(c *gc.C) { JujuStatusInfo: statusInfoContents{ Current: status.Running, }, + WorkloadStatusInfo: statusInfoContents{ + Current: status.Active, + }, }, }, }, @@ -4208,9 +4214,9 @@ Model Controller Cloud/Region Version App Version Status Scale Charm Store Rev OS Notes foo 1/2 0 -Unit Status Address Ports Message -foo/0 allocating -foo/1 running 10.0.0.1 80/TCP +Unit Workload Agent Address Ports Message +foo/0 active allocating +foo/1 active running 10.0.0.1 80/TCP `[1:]) } diff --git a/state/application.go b/state/application.go index 80a8064d841..9f9efe88647 100644 --- a/state/application.go +++ b/state/application.go @@ -9,6 +9,7 @@ import ( "sort" "strconv" "strings" + "time" "github.com/juju/errors" "github.com/juju/schema" @@ -27,6 +28,7 @@ import ( "github.com/juju/juju/constraints" "github.com/juju/juju/core/application" "github.com/juju/juju/core/leadership" + "github.com/juju/juju/state/presence" "github.com/juju/juju/status" ) @@ -2393,3 +2395,57 @@ func (op *AddUnitOperation) Done(err error) error { } return nil } + +// AgentPresence returns whether the respective remote agent is alive. +func (a *Application) AgentPresence() (bool, error) { + pwatcher := a.st.workers.presenceWatcher() + return pwatcher.Alive(a.globalKey()) +} + +// WaitAgentPresence blocks until the respective agent is alive. +// This should really only be used in the test suite. +func (a *Application) WaitAgentPresence(timeout time.Duration) (err error) { + defer errors.DeferredAnnotatef(&err, "waiting for agent of application %q", a) + ch := make(chan presence.Change) + pwatcher := a.st.workers.presenceWatcher() + pwatcher.Watch(a.globalKey(), ch) + defer pwatcher.Unwatch(a.globalKey(), ch) + pingBatcher := a.st.getPingBatcher() + if err := pingBatcher.Sync(); err != nil { + return err + } + for i := 0; i < 2; i++ { + select { + case change := <-ch: + if change.Alive { + return nil + } + case <-time.After(timeout): + // TODO(fwereade): 2016-03-17 lp:1558657 + return fmt.Errorf("still not alive after timeout") + case <-pwatcher.Dead(): + return pwatcher.Err() + } + } + panic(fmt.Sprintf("presence reported dead status twice in a row for application %q", a)) +} + +// SetAgentPresence signals that the agent for application a is alive. +// It returns the started pinger. +func (a *Application) SetAgentPresence() (*presence.Pinger, error) { + presenceCollection := a.st.getPresenceCollection() + recorder := a.st.getPingBatcher() + model, err := a.st.Model() + if err != nil { + return nil, errors.Trace(err) + } + p := presence.NewPinger(presenceCollection, model.ModelTag(), a.globalKey(), + func() presence.PingRecorder { return a.st.getPingBatcher() }) + err = p.Start() + if err != nil { + return nil, err + } + // Make sure this Agent status is written to the database before returning. + recorder.Sync() + return p, nil +} diff --git a/state/application_test.go b/state/application_test.go index 65f4521e1dc..7d1c63567f7 100644 --- a/state/application_test.go +++ b/state/application_test.go @@ -16,6 +16,7 @@ import ( gc "gopkg.in/check.v1" "gopkg.in/juju/charm.v6" "gopkg.in/juju/environschema.v1" + "gopkg.in/juju/worker.v1" "gopkg.in/mgo.v2/bson" "gopkg.in/mgo.v2/txn" @@ -3418,3 +3419,49 @@ func (s *ApplicationSuite) TestAddUnitWithProviderId(c *gc.C) { c.Assert(err, jc.ErrorIsNil) c.Assert(u.ProviderId(), gc.Equals, "provider-id") } + +func (s *ApplicationSuite) TestApplicationSetAgentPresence(c *gc.C) { + alive, err := s.mysql.AgentPresence() + c.Assert(err, jc.ErrorIsNil) + c.Assert(alive, jc.IsFalse) + + pinger, err := s.mysql.SetAgentPresence() + c.Assert(err, jc.ErrorIsNil) + c.Assert(pinger, gc.NotNil) + defer func() { + c.Assert(worker.Stop(pinger), jc.ErrorIsNil) + }() + s.State.StartSync() + alive, err = s.mysql.AgentPresence() + c.Assert(err, jc.ErrorIsNil) + c.Assert(alive, jc.IsTrue) +} + +func (s *ApplicationSuite) TestApplicationWaitAgentPresence(c *gc.C) { + alive, err := s.mysql.AgentPresence() + c.Assert(err, jc.ErrorIsNil) + c.Assert(alive, jc.IsFalse) + + err = s.mysql.WaitAgentPresence(coretesting.ShortWait) + c.Assert(err, gc.ErrorMatches, `waiting for agent of application "mysql": still not alive after timeout`) + + pinger, err := s.mysql.SetAgentPresence() + c.Assert(err, jc.ErrorIsNil) + + s.State.StartSync() + err = s.mysql.WaitAgentPresence(coretesting.LongWait) + c.Assert(err, jc.ErrorIsNil) + + alive, err = s.mysql.AgentPresence() + c.Assert(err, jc.ErrorIsNil) + c.Assert(alive, jc.IsTrue) + + err = pinger.KillForTesting() + c.Assert(err, jc.ErrorIsNil) + + s.State.StartSync() + + alive, err = s.mysql.AgentPresence() + c.Assert(err, jc.ErrorIsNil) + c.Assert(alive, jc.IsFalse) +} diff --git a/state/machine.go b/state/machine.go index f9eceb4a060..a056db8ed08 100644 --- a/state/machine.go +++ b/state/machine.go @@ -973,7 +973,7 @@ func (m *Machine) AgentPresence() (bool, error) { } // WaitAgentPresence blocks until the respective agent is alive. -// These should really only be used in the test suite. +// This should really only be used in the test suite. func (m *Machine) WaitAgentPresence(timeout time.Duration) (err error) { defer errors.DeferredAnnotatef(&err, "waiting for agent of machine %v", m) ch := make(chan presence.Change) diff --git a/state/unit.go b/state/unit.go index d0bc8c4c6b3..386acd56e20 100644 --- a/state/unit.go +++ b/state/unit.go @@ -1323,6 +1323,8 @@ func (u *Unit) AgentPresence() (bool, error) { if u.ShouldBeAssigned() { return pwatcher.Alive(u.globalAgentKey()) } + // Units in CAAS models rely on the operator pings. + // These are for the application itself. app, err := u.Application() if err != nil { return false, errors.Trace(err) @@ -1331,9 +1333,7 @@ func (u *Unit) AgentPresence() (bool, error) { if err != nil { return false, errors.Trace(err) } - // TODO(caas) - record presence for application agent - // We return true so that the agent doesn't appwar as lost. - return true || appAlive, nil + return appAlive, nil } // Tag returns a name identifying the unit. @@ -1350,7 +1350,7 @@ func (u *Unit) UnitTag() names.UnitTag { } // WaitAgentPresence blocks until the respective agent is alive. -// These should really only be used in the test suite. +// This should really only be used in the test suite. func (u *Unit) WaitAgentPresence(timeout time.Duration) (err error) { defer errors.DeferredAnnotatef(&err, "waiting for agent of unit %q", u) ch := make(chan presence.Change)