incus icon indicating copy to clipboard operation
incus copied to clipboard

Published 20 hours ago • verified publisher icon lxc

Add ability to read YAML from stdin to all `create` commands

Open stgraber opened this issue 10 months ago • 17 comments

Currently only incus create and incus launch will read YAML data from stdin to create a new instance (reading an InstancePut which gets embeded in the InstancePost struct).

The same logic should be supported for all other create commands:

  • [x] incus cluster group create
  • [x] incus config template create
  • [x] incus network acl create
  • [x] incus network forward create
  • [x] incus network integration create
  • [x] incus network load-balancer create
  • [x] incus network peer create
  • [x] incus network zone create
  • [x] incus network zone record create
  • [x] incus profile create
  • [x] incus project create
  • [x] incus snapshot create
  • [x] incus storage bucket create
  • [x] incus storage bucket key create
  • [x] incus storage create
  • [x] incus storage volume create
  • [ ] incus storage volume snapshot create

That list was based from a quick grep + some hand filtering, so there may be some entries where reading from stdin doesn't make sense.

stgraber avatar Apr 08 '24 16:04 stgraber

I'd like to work on this issue, can it be assigned to me please!

awalvie avatar Apr 08 '24 18:04 awalvie

sure, done!

stgraber avatar Apr 08 '24 18:04 stgraber

Where can I find valid example config.yaml files as reference for these?

awalvie avatar Apr 09 '24 11:04 awalvie

@awalvie Try to take a look at the --expanded flag that is used with incus config show <instance> --expanded

If i read the docs correctly, it displays all information there is about the entity. It needs to get its information from somewhere, hopefully from an object, that is/exists for all entities.

This object then could be the source of truth to base all possible config keys from.

From my testing, it looks like, it is not implemented in every show command. So, implementing it for every show command, would be a thing to do in the future anyways. @stgraber Correct me if i'm wrong.

rvveber avatar Apr 09 '24 13:04 rvveber

Maybe we could add minimal example configs for each resource to the docs? In much the same way the kubernetes docs do? For example:

  • https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#creating-a-deployment
  • https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/#example

awalvie avatar Apr 09 '24 14:04 awalvie

Where can I find valid example config.yaml files as reference for these?

In general, the same data as you get from the matching show command.

stgraber avatar Apr 09 '24 14:04 stgraber

Maybe we could add minimal example configs for each resource to the docs? In much the same way the kubernetes docs do? For example:

https://linuxcontainers.org/incus/docs/main/rest-api-spec/

This has example values for all the objects, it's not the best looking thing ever but it doesn't need to be manually updated.

stgraber avatar Apr 09 '24 14:04 stgraber

@stgraber incus snapshot create uses an exiting instance so does it make sense to allow creation of a snapshot via a config yaml?

awalvie avatar Apr 17 '24 17:04 awalvie

@awalvie the user would still provide the instance and snapshot names but there can still be modifiable fields within a snapshot (see InstanceSnapshotPut), currently that's just the expiry date (ExpiresAt).

stgraber avatar Apr 17 '24 17:04 stgraber

@stgraber are there any configuration options for incus config template create? I couldn't find a api PUT object or in the API spec.

PS. Could we check off other items from the original list please! Easier to keep track on what's been done :p

awalvie avatar Apr 23 '24 10:04 awalvie

That one is indeed a file API so no struct, stdin should be the file we upload instead.

stgraber avatar Apr 23 '24 10:04 stgraber

@stgraber Can you also mark off:

  • incus network acl create
  • incus profile create

Might've missed them with so many PRs!

awalvie avatar May 05 '24 16:05 awalvie

@stgraber For incus storage volume snapshot create, there doesn't seem to be a Put struct for api.StorageVolumeSnapshots

awalvie avatar May 13 '24 10:05 awalvie

api.StorageVolumeSnapshotPut should be it, no?

stgraber avatar May 13 '24 12:05 stgraber

ah, crap, ripgrep didn't show it in vim for some reason, I see it now!

awalvie avatar May 13 '24 12:05 awalvie

It doesn't seem to follow the same pattern of embedding though. In this case StorageVolumeSnapshot embeds StorageVolumeSnapshotPut instead of StorageVolumeSnapshotPost

awalvie avatar May 13 '24 12:05 awalvie

I'm confused, the exact same is true of InstanceSnapshot embedding InstanceSnapshotPut, am I missing something?

stgraber avatar May 14 '24 03:05 stgraber

@awalvie any luck with that last one?

stgraber avatar May 22 '24 18:05 stgraber

Sorry about the delay, was out on vacation the past week, I'll get back here shortly!

awalvie avatar May 23 '24 07:05 awalvie

Oh, yup, my bad, the way I phrased it was unclear.

In the current Run() function for cmdStorageVolumeSnapshotCreate:

https://github.com/lxc/incus/blob/73914201345e8763566aa1e2a1d3ebcc60480a19/cmd/incus/storage_volume.go#L2337

We create a req object of the type StorageVolumeSnapshotsPost (with an extra s) and use that during the creation of the volume snapshot in CreateStoragePoolVolumeSnapshot.

If I do read the data from stdin into an object of the type StorageVolumeSnapshotPut I'm having a hard time understanding where to embed that struct. There's only StorageVolumeSnapshot but we don't use that object in the function as far as I can tell.

awalvie avatar May 23 '24 08:05 awalvie

https://github.com/lxc/incus/pull/891 is basically a copy/paste from incus/snapshot.go and seems to work.

stgraber avatar May 23 '24 12:05 stgraber

https://github.com/lxc/incus/pull/891 is basically a copy/paste from incus/snapshot.go and seems to work.

Oh, yeah, I see it now. My brain got confused for some reason. Thank You!

awalvie avatar May 24 '24 05:05 awalvie