[pve-devel] [PATCH/RFC 0/2] Fix #1922 - run proxied pvesh calls as non-cli environment

[Fabian Grünbichler]

On Tue, Nov 27, 2018 at 05:46:03PM +0100, Stoiko Ivanov wrote:
> This patchset addresses issue #1922, where pvesh calls for vm-migrations
> yielded an error regarding invalid-json output, by creating a different
> RESTEnvironment type ('pub' instead of 'cli') when pvesh is called via
> ssh, with the --noproxy switch.

what does 'pub' stand for? :-P maybe it would make sense to call it
'pvesh'?

> The invalid json output is the result of the tasklog (which combines
> output of external commands, and internal prints), which gets output,
> when a fork_worker call is called via a cli environment.

thanks for tackling this, this behaviour is rather annoying indeed.

> Issues for discussion:
> * This way pvesh yields different output, when running an API call on the local
>   node vs. one on the remote node. It would be more consistent to always run as
>   a 'pub' environment, although it would yield fewer informations for the local
>   calls

consistent behaviour is preferable IMHO. I think pvesh simply returning
what the API would return (i.e., the UPID object/string in case of a an
async API call) would be the right thing, although it is yet another
change in the output of pvesh that might break existing usage..

we already briefly discussed last week that we might want to annotate
all those async calls properly with a new return object for the UPID (or
with a flag that automatically adds such an object by default). that
would also make it more obvious in the api-viewer if a call is async,
allow us to extend it more easily in the future (e.g., see below, but
also think about automatically linking the task resources in the
returned object, or similar stuff).

> * I put the helper function in common, although its currently only used in pvesh
> * In the longer run the issue will return, when/if we change our other CLI tools
>   (qm, pct,...) to use the CLIFormatter. An idea I had (but wouldn't want to
>   add before 5.3) would be to write the logoutput to stderr, and the result to
>   stdout. The downside being that we still would join actual stderr output (from
>   errors of external programs) with regular output of those programs and our
>   logging. Would be grateful for other ideas.

longterm I'd like to see something like structured task results that can
be retrieved alongside the log and status objects for a task. then for
CLI commands with --output-format, we could return

{
  upid: XXXX,
  status: {
    YYYY
  },
  log: {
    'ZZZZZZZZZZZZZZZZZZZZZZ'
  },
  result: {
    FOO: 'BAR"
  }
}

for json/yaml/scripting purposes, and the structured result would also
be nice for some API users (especially in contexts like ansible).

the regular CLI calls (some of which are interactive) can keep the
current stdout/stderr handling, but still print the task status and
optionally formatted result at the end?

it would require adding a helper for registering the task result, and of
course calling that with appropriate data before exiting the worker.

just some food for thought :)