is-primary-controller-flag is getting "permission denied" instead of "lease claim denied"

[John A Meinel]

Maybe this is expected behavior, but I saw this during an upgrade process:
2018-02-14 07:19:45 DEBUG juju.worker.dependency engine.go:504 "is-primary-controller-flag" manifold worker stopped: permission denied
2018-02-14 07:19:45 ERROR juju.worker.dependency engine.go:551 "is-primary-controller-flag" manifold worker returned unexpected error: permission denied
2018-02-14 07:19:45 DEBUG juju.worker.dependency engine.go:553 stack trace:
permission denied
github.com/juju/juju/api/singular/api.go:76:
github.com/juju/juju/worker/singular/flag.go:141:
github.com/juju/juju/worker/singular/flag.go:63:
github.com/juju/juju/worker/singular/shim.go:31:
github.com/juju/juju/worker/singular/manifold.go:53:

That seems that the 'is-primary-controller-flag' manifold is confused by getting a "permission denied" error.

The code in question seems to want to get ErrClaimDenied (from worker/singular/flag.go)
func claim(config FlagConfig) (bool, error) {
 err := config.Facade.Claim(config.Duration)
 cause := errors.Cause(err)
 switch cause {
 case nil:
  return true, nil
 case lease.ErrClaimDenied:
  return false, nil
 }
 return false, errors.Trace(err)
}

However, looking at the server-side code it appears to be just returning ErrPerm: (apiserver/facades/conttroller/singular/singular.go)
func (facade *Facade) Claim(args params.SingularClaims) (result params.ErrorResults) {
 result.Results = make([]params.ErrorResult, len(args.Claims))
 for i, claim := range args.Claims {
  err := facade.claim(claim)
  result.Results[i].Error = common.ServerError(err)
 }
 return result
}

func (facade *Facade) claim(claim params.SingularClaim) error {
 if !allowedDuration(claim.Duration) {
  return common.ErrPerm
 }
 leaseId, err := facade.tagLeaseId(claim.EntityTag)
 if err != nil {
  return errors.Trace(err)
 }
 holder := facade.auth.GetAuthTag().String()
 if claim.ClaimantTag != holder {
  return common.ErrPerm
 }
 return facade.claimer.Claim(leaseId, holder, claim.Duration)
}

claim() is likely returning ErrPerm, which is being translated into the generic ServerError:
 ErrPerm: params.CodeUnauthorized,

Now the client API does try to do a translation here (api/singular/api.go):

func (api *API) Claim(duration time.Duration) error {
 args := params.SingularClaims{
  Claims: []params.SingularClaim{{
   EntityTag: api.entity.String(),
   ClaimantTag: api.claimant.String(),
   Duration: duration,
  }},
 }
 var results params.ErrorResults
 err := api.facadeCaller.FacadeCall("Claim", args, &results)
 if err != nil {
  return errors.Trace(err)
 }

 err = results.OneError()
 if err != nil {
  if params.IsCodeLeaseClaimDenied(err) {
   return lease.ErrClaimDenied
  }
  return errors.Trace(err)
 }
 return nil
}

But that will *only* translate a CodeLeaseClaimDenied:
 return ErrCode(err) == CodeLeaseClaimDenied

And that seems like it could be translated server side (apiserver/common/errors.go):
 lease.ErrClaimDenied: params.CodeLeaseClaimDenied,

Maybe I'm misreading it. Its possible that the apiserver/facades/controller/singular/singular.go is only checking that the identity calling the Claim is the same entity as the holder. (eg, machine-1 can only ask for a lease for machine-1.)
So it may be that there is something else wrong that we are doing that we are getting a 'permission denied' instead of a "lease claim denied".

Either way, we should fix this. It seems very much to be working by-accident instead of by-purpose.

[Canonical Juju QA Bot (juju-qa-bot)]

This bug has not been updated in 2 years, so we're marking it Low importance. If you believe this is incorrect, please update the importance.