Skip to content

Commit

Permalink
Merge pull request #13220 from wallyworld/merge-2.9-20210806
Browse files Browse the repository at this point in the history 
#13220

Merge 2.9

#13211 Fix data race (accessing charmInfo) in TestV2CharmExitsApplicationWorker
#13212 Fix retry-provisioning command
#13210 Add note and todo
#13209 Improve readablity of 'no prdesc not found' error message
#13214 Adds remaining authorization objects to ignore
#13215 Use a key-based mutex lock to download resources/agent bins once
#13218 Show application data set by the remote app in show-unit output
#13219 Add impish to the ubuntu series list

Trivial conflicts in imports/code comments.

```
Conflicts:
# apiserver/facades/client/client/client.go
# apiserver/facades/schema.json
# resource/resourceadapters/opener.go
```
## QA steps

See PRs
  • Loading branch information
@jujubot
jujubot authored Aug 8, 2021
2 parents 016394b + 3ea57b3 commit acfa032
Show file tree
Hide file tree
Showing 31 changed files with 586 additions and 280 deletions.
69 changes: 0 additions & 69 deletions apiserver/common/setstatus.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,75 +187,6 @@ func (s *StatusSetter) SetStatus(args params.SetStatus) (params.ErrorResults, er
return result, nil
}

func (s *StatusSetter) updateEntityStatusData(tag names.Tag, data map[string]interface{}) error {
entity0, err := s.st.FindEntity(tag)
if err != nil {
return err
}
statusGetter, ok := entity0.(status.StatusGetter)
if !ok {
return apiservererrors.NotSupportedError(tag, "getting status")
}
existingStatusInfo, err := statusGetter.Status()
if err != nil {
return err
}
newData := existingStatusInfo.Data
if newData == nil {
newData = data
} else {
for k, v := range data {
newData[k] = v
}
}
entity, ok := entity0.(status.StatusSetter)
if !ok {
return apiservererrors.NotSupportedError(tag, "updating status")
}
if len(newData) > 0 && existingStatusInfo.Status != status.Error {
return fmt.Errorf("%s is not in an error state", names.ReadableString(tag))
}
// TODO(perrito666) 2016-05-02 lp:1558657
now := time.Now()
sInfo := status.StatusInfo{
Status: existingStatusInfo.Status,
Message: existingStatusInfo.Message,
Data: newData,
Since: &now,
}
return entity.SetStatus(sInfo)
}

// UpdateStatus updates the status data of each given entity.
// TODO(fwereade): WTF. This method exists *only* for the convenience of the
// *client* API -- and is itself completely broken -- but we still expose it
// in every facade with a StatusSetter? FFS.
func (s *StatusSetter) UpdateStatus(args params.SetStatus) (params.ErrorResults, error) {
result := params.ErrorResults{
Results: make([]params.ErrorResult, len(args.Entities)),
}
if len(args.Entities) == 0 {
return result, nil
}
canModify, err := s.getCanModify()
if err != nil {
return params.ErrorResults{}, errors.Trace(err)
}
for i, arg := range args.Entities {
tag, err := names.ParseTag(arg.Tag)
if err != nil {
result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm)
continue
}
err = apiservererrors.ErrPerm
if canModify(tag) {
err = s.updateEntityStatusData(tag, arg.Data)
}
result.Results[i].Error = apiservererrors.ServerError(err)
}
return result, nil
}

// UnitAgentFinder is a state.EntityFinder that finds unit agents.
type UnitAgentFinder struct {
state.EntityFinder
Expand Down
15 changes: 8 additions & 7 deletions apiserver/facades/client/application/application.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2748,13 +2748,6 @@ func (api *APIBase) relationData(app Application, myUnit Unit) ([]params.Endpoin
ApplicationData: make(map[string]interface{}),
UnitRelationData: make(map[string]params.RelationData),
}
appSettings, err := rel.ApplicationSettings(app.Name())
if err != nil {
return nil, errors.Trace(err)
}
for k, v := range appSettings {
erd.ApplicationData[k] = v
}
relatedEps, err := rel.RelatedEndpoints(app.Name())
if err != nil {
return nil, errors.Trace(err)
Expand All @@ -2763,6 +2756,14 @@ func (api *APIBase) relationData(app Application, myUnit Unit) ([]params.Endpoin
related := relatedEps[0]
erd.RelatedEndpoint = related.Name

appSettings, err := rel.ApplicationSettings(related.ApplicationName)
if err != nil {
return nil, errors.Trace(err)
}
for k, v := range appSettings {
erd.ApplicationData[k] = v
}

otherApp, err := api.backend.Application(related.ApplicationName)
if errors.IsNotFound(err) {
erd.CrossModel = true
Expand Down
2 changes: 1 addition & 1 deletion apiserver/facades/client/application/application_unit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2131,7 +2131,7 @@ func (s *ApplicationSuite) TestUnitsInfo(c *gc.C) {
Endpoint: "db",
CrossModel: true,
RelatedEndpoint: "server",
ApplicationData: map[string]interface{}{"app-postgresql": "setting"},
ApplicationData: map[string]interface{}{"app-gitlab": "setting"},
UnitRelationData: map[string]params.RelationData{
"gitlab/2": {
InScope: true,
Expand Down
77 changes: 62 additions & 15 deletions apiserver/facades/client/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/juju/juju/core/os"
"github.com/juju/juju/core/permission"
"github.com/juju/juju/core/series"
"github.com/juju/juju/core/status"
"github.com/juju/juju/environs"
"github.com/juju/juju/environs/config"
"github.com/juju/juju/environs/context"
Expand All @@ -51,9 +52,7 @@ type API struct {

multiwatcherFactory multiwatcher.Factory

client *Client
// statusSetter provides common methods for updating an entity's provisioning status.
statusSetter *common.StatusSetter
client *Client
toolsFinder *common.ToolsFinder
leadershipReader leadership.Reader
modelCache *cache.Model
Expand Down Expand Up @@ -177,7 +176,6 @@ func newFacade(ctx facade.Context) (*Client, error) {
modelUUID := model.UUID()

urlGetter := common.NewToolsURLGetter(modelUUID, ctx.StatePool().SystemState())
statusSetter := common.NewStatusSetter(st, common.AuthAlways())
toolsFinder := common.NewToolsFinder(configGetter, st, urlGetter, newEnviron)
blockChecker := common.NewBlockChecker(st)
backend := modelconfig.NewStateBackend(model)
Expand All @@ -204,7 +202,6 @@ func newFacade(ctx facade.Context) (*Client, error) {
resources,
authorizer,
presence,
statusSetter,
toolsFinder,
newEnviron,
blockChecker,
Expand All @@ -223,7 +220,6 @@ func NewClient(
resources facade.Resources,
authorizer facade.Authorizer,
presence facade.Presence,
statusSetter *common.StatusSetter,
toolsFinder *common.ToolsFinder,
newEnviron common.NewEnvironFunc,
blockChecker *common.BlockChecker,
Expand All @@ -243,7 +239,6 @@ func NewClient(
auth: authorizer,
resources: resources,
presence: presence,
statusSetter: statusSetter,
toolsFinder: toolsFinder,
leadershipReader: leadershipReader,
modelCache: modelCache,
Expand Down Expand Up @@ -820,8 +815,13 @@ func (c *Client) AddCharmWithAuthorization(args params.AddCharmWithAuthorization
return errors.NewNotSupported(nil, "Deprecated AddCharmWithAuthorization call has been removed in Juju 3.0; please use the charms facade instead")
}

// Method was deprecated as of juju 2.9 and removed in juju 3.0. Please use the
// client/charms facade instead.
// ResolveCharms resolves the best available charm URLs with series, for charm
// locations without a series specified.
//
// NOTE: ResolveCharms is deprecated as of juju 2.9 and charms facade version 3.
// Please discontinue use and move to the charms facade version.
//
// TODO: remove in juju 3.0
func (c *Client) ResolveCharms(args params.ResolveCharms) (params.ResolveCharmResults, error) {
return params.ResolveCharmResults{}, errors.NewNotSupported(nil, "Deprecated ResolveChamrs call has been removed in Juju 3.0; please use the charms facade instead")
}
Expand All @@ -835,13 +835,60 @@ func (c *Client) RetryProvisioning(p params.Entities) (params.ErrorResults, erro
if err := c.check.ChangeAllowed(); err != nil {
return params.ErrorResults{}, errors.Trace(err)
}
entityStatus := make([]params.EntityStatusArgs, len(p.Entities))
for i, entity := range p.Entities {
entityStatus[i] = params.EntityStatusArgs{Tag: entity.Tag, Data: map[string]interface{}{"transient": true}}
result := params.ErrorResults{
Results: make([]params.ErrorResult, len(p.Entities)),
}
for i, e := range p.Entities {
tag, err := names.ParseMachineTag(e.Tag)
if err != nil {
result.Results[i].Error = apiservererrors.ServerError(err)
continue
}
if err := c.updateInstanceStatus(tag, map[string]interface{}{"transient": true}); err != nil {
result.Results[i].Error = apiservererrors.ServerError(err)
}
}
return result, nil
}

type instanceStatus interface {
InstanceStatus() (status.StatusInfo, error)
SetInstanceStatus(sInfo status.StatusInfo) error
}

func (c *Client) updateInstanceStatus(tag names.Tag, data map[string]interface{}) error {
entity0, err := c.api.stateAccessor.FindEntity(tag)
if err != nil {
return err
}
statusGetterSetter, ok := entity0.(instanceStatus)
if !ok {
return apiservererrors.NotSupportedError(tag, "getting status")
}
existingStatusInfo, err := statusGetterSetter.InstanceStatus()
if err != nil {
return err
}
newData := existingStatusInfo.Data
if newData == nil {
newData = data
} else {
for k, v := range data {
newData[k] = v
}
}
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)
}
// TODO(perrito666) 2016-05-02 lp:1558657
now := time.Now()
sInfo := status.StatusInfo{
Status: existingStatusInfo.Status,
Message: existingStatusInfo.Message,
Data: newData,
Since: &now,
}
return c.api.statusSetter.UpdateStatus(params.SetStatus{
Entities: entityStatus,
})
return statusGetterSetter.SetInstanceStatus(sInfo)
}

// APIHostPorts returns the API host/port addresses stored in state.
Expand Down
12 changes: 7 additions & 5 deletions apiserver/facades/client/client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1663,12 +1663,14 @@ func (s *clientSuite) TestRetryProvisioning(c *gc.C) {
Message: "error",
Since: &now,
}
err = machine.SetStatus(sInfo)
err = machine.SetInstanceStatus(sInfo)
c.Assert(err, jc.ErrorIsNil)
_, err = s.APIState.Client().RetryProvisioning(machine.Tag().(names.MachineTag))
result, err := s.APIState.Client().RetryProvisioning(machine.Tag().(names.MachineTag))
c.Assert(err, jc.ErrorIsNil)
c.Assert(result, gc.HasLen, 1)
c.Assert(result[0].Error, gc.IsNil)

statusInfo, err := machine.Status()
statusInfo, err := machine.InstanceStatus()
c.Assert(err, jc.ErrorIsNil)
c.Assert(statusInfo.Status, gc.Equals, status.Error)
c.Assert(statusInfo.Message, gc.Equals, "error")
Expand All @@ -1684,15 +1686,15 @@ func (s *clientSuite) setupRetryProvisioning(c *gc.C) *state.Machine {
Message: "error",
Since: &now,
}
err = machine.SetStatus(sInfo)
err = machine.SetInstanceStatus(sInfo)
c.Assert(err, jc.ErrorIsNil)
return machine
}

func (s *clientSuite) assertRetryProvisioning(c *gc.C, machine *state.Machine) {
_, err := s.APIState.Client().RetryProvisioning(machine.Tag().(names.MachineTag))
c.Assert(err, jc.ErrorIsNil)
statusInfo, err := machine.Status()
statusInfo, err := machine.InstanceStatus()
c.Assert(err, jc.ErrorIsNil)
c.Assert(statusInfo.Status, gc.Equals, status.Error)
c.Assert(statusInfo.Message, gc.Equals, "error")
Expand Down
1 change: 0 additions & 1 deletion apiserver/facades/client/client/statushistory_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ func (s *statusHistoryTestSuite) SetUpTest(c *gc.C) {
nil, // resources
authorizer,
nil, // presence
nil, // statusSetter
nil, // toolsFinder
nil, // newEnviron
nil, // blockChecker
Expand Down
62 changes: 1 addition & 61 deletions apiserver/facades/schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15967,7 +15967,7 @@
"$ref": "#/definitions/ResolveCharmResults"
}
},
"description": "Method was deprecated as of juju 2.9 and removed in juju 3.0. Please use the\nclient/charms facade instead."
"description": "ResolveCharms resolves the best available charm URLs with series, for charm\nlocations without a series specified.\n\nNOTE: ResolveCharms is deprecated as of juju 2.9 and charms facade version 3.\nPlease discontinue use and move to the charms facade version.\n\nTODO: remove in juju 3.0"
},
"Resolved": {
"type": "object",
Expand Down Expand Up @@ -21717,18 +21717,6 @@
},
"description": "SetStatus sets the status of the specified entities."
},
"UpdateStatus": {
"type": "object",
"properties": {
"Params": {
"$ref": "#/definitions/SetStatus"
},
"Result": {
"$ref": "#/definitions/ErrorResults"
}
},
"description": "UpdateStatus updates the status data of each given entity.\nTODO(fwereade): WTF. This method exists *only* for the convenience of the\n*client* API -- and is itself completely broken -- but we still expose it\nin every facade with a StatusSetter? FFS."
},
"WatchAPIHostPorts": {
"type": "object",
"properties": {
Expand Down Expand Up @@ -28637,18 +28625,6 @@
},
"description": "SetStatus sets the status of each given entity."
},
"UpdateStatus": {
"type": "object",
"properties": {
"Params": {
"$ref": "#/definitions/SetStatus"
},
"Result": {
"$ref": "#/definitions/ErrorResults"
}
},
"description": "UpdateStatus updates the status data of each given entity.\nTODO(fwereade): WTF. This method exists *only* for the convenience of the\n*client* API -- and is itself completely broken -- but we still expose it\nin every facade with a StatusSetter? FFS."
},
"Watch": {
"type": "object",
"properties": {
Expand Down Expand Up @@ -34568,18 +34544,6 @@
},
"description": "Tools finds the tools necessary for the given agents."
},
"UpdateStatus": {
"type": "object",
"properties": {
"Params": {
"$ref": "#/definitions/SetStatus"
},
"Result": {
"$ref": "#/definitions/ErrorResults"
}
},
"description": "UpdateStatus updates the status data of each given entity.\nTODO(fwereade): WTF. This method exists *only* for the convenience of the\n*client* API -- and is itself completely broken -- but we still expose it\nin every facade with a StatusSetter? FFS."
},
"WatchAPIHostPorts": {
"type": "object",
"properties": {
Expand Down Expand Up @@ -41481,18 +41445,6 @@
},
"description": "SetVolumeInfo records the details of newly provisioned volumes."
},
"UpdateStatus": {
"type": "object",
"properties": {
"Params": {
"$ref": "#/definitions/SetStatus"
},
"Result": {
"$ref": "#/definitions/ErrorResults"
}
},
"description": "UpdateStatus updates the status data of each given entity.\nTODO(fwereade): WTF. This method exists *only* for the convenience of the\n*client* API -- and is itself completely broken -- but we still expose it\nin every facade with a StatusSetter? FFS."
},
"VolumeAttachmentParams": {
"type": "object",
"properties": {
Expand Down Expand Up @@ -43302,18 +43254,6 @@
},
"description": "SetStatus sets the status of each given entity."
},
"UpdateStatus": {
"type": "object",
"properties": {
"Params": {
"$ref": "#/definitions/SetStatus"
},
"Result": {
"$ref": "#/definitions/ErrorResults"
}
},
"description": "UpdateStatus updates the status data of each given entity.\nTODO(fwereade): WTF. This method exists *only* for the convenience of the\n*client* API -- and is itself completely broken -- but we still expose it\nin every facade with a StatusSetter? FFS."
},
"WatchModelResources": {
"type": "object",
"properties": {
Expand Down
Loading

0 comments on commit acfa032

Please sign in to comment.