remove-backup (client version 2.4) doesn't work on a 2.3 controller

[Junien F]

Hi,

Using a 2.4 client on a 2.3 controller, remove-backup produces the following :

$ juju backups
20180518-095455.85dd9bd5-78e5-498e-8ec6-2b7edf490dff
20180628-064906.85dd9bd5-78e5-498e-8ec6-2b7edf490dff

$ juju remove-backup --debug 20180518-095455.85dd9bd5-78e5-498e-8ec6-2b7edf490dff
10:20:44 INFO juju.cmd supercommand.go:56 running juju [2.4.1 gc go1.10]
10:20:44 DEBUG juju.cmd supercommand.go:57 args: []string{"juju", "remove-backup", "--debug", "20180518-095455.85dd9bd5-78e5-498e-8ec6-2b7edf490dff"}
[irrelevant stuff]
10:20:45 DEBUG juju.api monitor.go:35 RPC connection died
ERROR expected 1 result(s), got 0
10:20:45 DEBUG cmd supercommand.go:459 error stack:
github.com/juju/juju/api/backups/remove.go:20: expected 1 result(s), got 0
github.com/juju/juju/cmd/juju/backups/remove.go:117:

However, the same "remove-backup" above works fine with a 2.3 client.

Thanks

[John A Meinel (jameinel)]

in 2.4 it looks like we added API version 2 which allows you to pass >1 backup to remove.
However, it seems the 2.4 client always assumes a 2.4 server:
The client side code calls Remove, doesn't pay attention to the facade version and assumes len(results) == len(ids):
func (c *Client) Remove(ids ...string) ([]params.ErrorResult, error) {
 if len(ids) == 0 {
  return []params.ErrorResult{}, nil
 }
 args := params.BackupsRemoveArgs{IDs: ids}
 results := params.ErrorResults{}
 err := c.facade.FacadeCall("Remove", args, &results)
 if len(results.Results) != len(ids) {
  return nil, errors.Errorf(
   "expected %d result(s), got %d",
   len(ids), len(results.Results),
  )
 }
 return results.Results, errors.Trace(err)
}

However, it the 2.3 server it does:

func (a *API) Remove(args params.BackupsRemoveArgs) error {
 backups, closer := newBackups(a.backend)
 defer closer.Close()

 err := backups.Remove(args.ID)
 return errors.Trace(err)
}

(it only allows 1 Id in BackupRemoveArgs and it returns any error as the overall error for the function call.)

The server side implementing V2 is doing a good thing (allowing you to pass >1 argument, returning multiple possible error codes, etc.)

However, the new client needs to stay compatible with the old servers. It should have code something like:

 if len(ids) == 0 {
  return []params.ErrorResult{}, nil
 }
 if c.facade.BestAPIVersion() < 2 {
  // In version 2 we introduced multiple IDs that can be removed at the same time, prior versions only supported 1 backup to remove
  if len(ids) > 1 {
   return nil, errors.Errorf("server does not support removing >1 backup at a time, please pass only a single value")
  }
  args := params.BackupsRemoveArgs{ID: ids[0]}
  err := c.facade.FacadeCall("Remove", args, nil)
  // Do we turn this into a []params.ErrorResult?
  return nil, err
 }
 args := params.BackupsRemoveArgs{IDs: ids}
 results := params.ErrorResults{}
 err := c.facade.FacadeCall("Remove", args, &results)
 if len(results.Results) != len(ids) {
  return nil, errors.Errorf(
   "expected %d result(s), got %d",
   len(ids), len(results.Results),
  )
 }
 return results.Results, errors.Trace(err)

[John A Meinel (jameinel)]

This is a regression, as 2.4 is now passing the object to remove as a list in IDs, but the 2.3 server expects it as a single value in the ID field. So its fairly import to fix.

[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.