Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incus: update to 6.6; reverse dependency. #52384

Closed
wants to merge 1 commit into from

Conversation

duskmoss
Copy link
Contributor

incus-client is intended to be installed without the daemon to use to control remove incus servers. incus the daemon can't me used without the client, at least to authorize another client's to access the API.

I moved incus-user to the daemon package because it's a daemon and not part of the client.

readme: I also removed the information about subuid and subgid because void sets these up out of the box.

readme: I changed "and" to "or" because the two groups grant you access to the daemons in different privelege levels.

Testing the changes

  • I tested the changes in this PR: briefly

Local build testing

  • I built this PR locally for my native architecture:
    x86_64-glibc
  • I built this PR locally for these architectures:
    x86_64-musl
    (cross) aarch64-musl
    (cross) armv7l
    (cross) armv6l-musl

@duskmoss
Copy link
Contributor Author

@dkwo

If you could take a look at let me know what you think I'd appreciate it, willing to make changes, especially to content of readme. I also have opened a PR for handbook docs void-linux/void-docs#815 and we can discuss what information should exist in readme vs handbook.

@dkwo
Copy link
Contributor

dkwo commented Sep 28, 2024

Hey, the changes to incus make sense to me, thanks!
About the readme, could you leave the subuid/subgid part?

I was also thinking it'd be useful to have man pages, something like

vtargetrun incus manpage --all --format=man /target/path
vman /target/path

Can you try adding this?

@duskmoss
Copy link
Contributor Author

Sure! I'll work on getting the manpages added this weekend.

For the subuid/subguid, I can leave it in. Maybe I should add something about checking current /etc/subuid /etc/subguid?

Might not be necessary since if people do run it it'll hopefully be a no op, since base-files matches the range the read me suggests.

@duskmoss duskmoss force-pushed the incus-updates branch 2 times, most recently from 9c2f674 to ad2d1f6 Compare September 29, 2024 00:27
@dkwo
Copy link
Contributor

dkwo commented Sep 29, 2024

Hey, this looks good to me.
I think it's better to move a discussion about readme into the docs pr, so i'd leave the readme as is here.

Should some man pages go into the tools subpkg?
Can you post the output of xls incus, xls incus-client etc,
to make sure they are going to the right place?

@duskmoss
Copy link
Contributor Author

There are no man pages for "fuidshift-incus lxc-to-incus lxd-to-incus incus-benchmark incus-migrate incus-simplestreams" So it looks like they all go in the incus-client to me.

incus-tools

/usr/bin/fuidshift-incus
/usr/bin/incus-benchmark
/usr/bin/incus-migrate
/usr/bin/incus-simplestreams
/usr/bin/lxc-to-incus
/usr/bin/lxd-to-incus

incus

/etc/sv/incus/check
/etc/sv/incus/log/run
/etc/sv/incus/run
/etc/sv/incus-user/log/run
/etc/sv/incus-user/run
/usr/bin/incus-agent
/usr/libexec/incus/incus-user
/usr/libexec/incus/incusd
/usr/share/doc/incus/README.voidlinux
/etc/sv/incus/log/supervise -> /run/runit/supervise.incus-log
/etc/sv/incus/supervise -> /run/runit/supervise.incus
/etc/sv/incus-user/log/supervise -> /run/runit/supervise.incus-user-log
/etc/sv/incus-user/supervise -> /run/runit/supervise.incus-user

incus-client
https://0x0.st/Xguv.txt

@dkwo
Copy link
Contributor

dkwo commented Sep 30, 2024

You're right, and this looks good to me. Thanks!

@duskmoss
Copy link
Contributor Author

yw, thanks for the feedback!

@ahesford
Copy link
Member

ahesford commented Oct 1, 2024

The commit message would more aptly be something along the lines of

incus: move incus-user service to main package, install man pages

@duskmoss duskmoss changed the title incus: enable installing client without the daemon incus: reverse dependency; add man pages Oct 1, 2024
srcpkgs/incus/template Outdated Show resolved Hide resolved
srcpkgs/incus/template Outdated Show resolved Hide resolved
srcpkgs/incus/template Outdated Show resolved Hide resolved
@duskmoss
Copy link
Contributor Author

duskmoss commented Oct 1, 2024

Summarizing a quick conversation on IRC:

-> maybe man page naming should be changed up stream
-> the man pages have the same information as --help so maybe it's not worth it. (per https://linuxcontainers.org/incus/docs/main/packaging/#manual-pages )

There seem to be a few ways to move forward.

  1. Merge with the man pages change
    a) I get them fixed upstream we can remove the renaming logic when we version bump
    b) We decide the renaming we're doing is fine for now
  2. Merge the original changes but leave the man pages out
    a) Decide not to care about including the man pages
    b) I work on getting the files renamed upstream, and we add them after that.

I don't mind whichever direction we want to take, but If there's a lot of debate I'd rather merge the original changes and decide what to do about man pages separately.

@ericonr @classabbyamp @dkwo

@classabbyamp
Copy link
Member

300+ manpages just for regurgitated --help seems like a bit much

@duskmoss
Copy link
Contributor Author

duskmoss commented Oct 1, 2024

I saved the other commit on a local branch, so can bring the man page changes back if we need it, but here's the PR without adding man pages

@dkwo
Copy link
Contributor

dkwo commented Oct 2, 2024

I'm fine either way about man pages.

@duskmoss duskmoss changed the title incus: reverse dependency; add man pages incus: reverse dependency; move incus-user service to daemon package Oct 2, 2024
    incus-client is intended to be installed without the daemon to use to
    control remote incus servers. incus the daemon can't be used without
    the client, at least to authorize another client to access the API.

    I moved incus-user to the daemon package because it's a daemon and
    not part of the client.

    readme: I changed "and" to "or" because the two groups grant you
    access to the daemons in different privelege levels.
@duskmoss
Copy link
Contributor Author

duskmoss commented Oct 3, 2024

@dkwo I'm running the 6.6 update now to test, and have added it to this PR, unless you'd rather have it in a separate one.

@duskmoss duskmoss changed the title incus: reverse dependency; move incus-user service to daemon package incus: update to 6.6; reverse dependency. Oct 3, 2024
@dkwo
Copy link
Contributor

dkwo commented Oct 5, 2024

v6.6 works fine for me.

@duskmoss
Copy link
Contributor Author

duskmoss commented Oct 6, 2024

It's also been solid for me. Tested some vm stuff too just in case.

@ahesford ahesford closed this in 2e911ee Oct 6, 2024
@duskmoss duskmoss deleted the incus-updates branch October 8, 2024 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants