Add support for additional groups ids on attach.

lxc / go-lxc

Go bindings for liblxc

https://linuxcontainers.org/lxc

Other

430 stars 76 forks source link

Add support for additional groups ids on attach. #144

Closed r10r closed 3 years ago

r10r commented 4 years ago

This is the counterpart for https://github.com/lxc/lxc/pull/3575.

stgraber commented 4 years ago

This is going to need some ifdef logic in there so go-lxc can still build with older liblxc

r10r commented 4 years ago

hi @stgraber - I wonder how to do this correctly.

What do you recommend ?
Can you point me to a similar implementation ?
How to handle the conditional implementation within the go code (struct_lxc_groups_t) ?

stgraber commented 4 years ago

It's really just the attach_options.groups assignments in the C binding that need to be under ifdef, the rest should be fine.

r10r commented 4 years ago

@stgraber This should be it.

r10r commented 4 years ago

Hmm, the build did not pick up my last commit. I had a look at the build history .. seems that the build job must be fixed. Last successful build was two years ago ?

stgraber commented 4 years ago

Usually the way we do this is with #if VERSION_AT_LEAST(4, 1, 0) which will then only compile this code when built against liblxc master or the upcoming 4.1 release.

r10r commented 4 years ago

Ok - didn't know that. But how do I then build with this enabled against my development branch / upstream master?

r10r commented 4 years ago

What about the CI builds - did I break it or was it broken before?

r10r commented 4 years ago

So 4.1.x is the next stable branch ? Is it based on the stable-4.0 branch ?

stgraber commented 4 years ago

4.1 should be the next version outside of the LTS branch. stable-4.0 is only bugfixes so wouldn't get this change.

stgraber commented 4 years ago

Travis was broken before, don't worry about it. We'll just want the liblxc side to be reviewed by @brauner and merged, then this can be merged.

r10r commented 4 years ago

Ok - I'm relieved ;) Is there an approximate release date for 4.1 ?

r10r commented 4 years ago

Ok - didn't know that. But how do I then build with this enabled against my development branch / upstream master?

Ok I got it .

r10r commented 3 years ago

Just a note to myself: The build should work once https://github.com/lxc/lxc/pull/3575 is merged.

brauner commented 3 years ago

Pulling this now to tweak a few things.

brauner commented 3 years ago

Pulling this now to tweak a few things.

Whoops. wrong repo. :)

brauner commented 3 years ago

re-kicking build now

r10r commented 3 years ago

hmm, i don't get it - trying locally instead of bugging Travis.

r10r commented 3 years ago

@brauner can you please have a look at the build output - apparently compilation is fixed now but the build still fails.
Looking at the previous builds indicates that it was already broken.

stgraber commented 3 years ago

The tests are known to be complete crap on go-lxc sadly...

I've got a task to both move them to Github Actions and make sure they're vaguely useful but it's very very far down the priority list unfortunately.

r10r commented 3 years ago

Ok. I plan to up CI/CD for crio-lxc as well and will take a look at Github Actions.

Thanks for merging!