Conversation
|
This is a blocker ATM. https://discuss.linuxcontainers.org/t/incus-6-11-mount-order-priority/23381 |
jochumdev
force-pushed
the
chore/ugly_patch
branch
from
22540f0 to
ada1fc4
Compare
|
Another note about this patch, it force users to use incus 6.11 which is the latest available atm. |
| for k, v := range s.Volumes { | ||
| fullVolName := p.Name + "_" + k | ||
| for _, v := range s.Volumes { | ||
| fullVolName := p.Name + "_" + v.Name |
There was a problem hiding this comment.
As I'm looking at my own patch - I think this is never true.
| // If no valid remote is found, an error is returned. | ||
| // | ||
| // UseProject overwrites the instance, that means this function will return the overwritten instance. | ||
| func (app *Compose) GetIncusInstance() (incus.InstanceServer, error) { |
There was a problem hiding this comment.
Needs to go away, this was added where I didn't know the code base.
| } | ||
|
|
||
| func (app *Compose) CreateProject(name string) error { | ||
| // get the project names while we're connected |
|
|
||
| // check for existing bind | ||
| d, err := app.getInstanceServer(containerName) | ||
| d, err := app.getInstanceServer(containerName, app.GetProject()) |
There was a problem hiding this comment.
I added this a lot, I made name == project and app.getInstance() selects the project now.
| // keep all the external commands in one place | ||
| func (app *Compose) Up() error { | ||
| err := app.SanityCheck() | ||
| err := app.CreateProject(app.GetProject()) |
There was a problem hiding this comment.
This was added to make a start of a docker compose show file possible.
| if volumes { | ||
| for volName, vol := range app.Services[service].Volumes { | ||
| slog.Info("Volume snapshot start", slog.String("volume", vol.CreateName(app.Name, service, volName))) | ||
| err := app.SnapshotVolume(vol.Pool, vol.CreateName(app.Name, service, volName), noexpiry, stateful, volumes) |
There was a problem hiding this comment.
A haevy duty breaking change volumes have been
Project-service-volume
Now they are:
Project-Volume
| Image string `yaml:"image" validate:"required"` | ||
| GPU bool `yaml:"gpu,omitempty"` | ||
| Volumes map[string]*Volume `yaml:"volumes,omitempty" validate:"dive,required"` | ||
| Volumes []*Volume `yaml:"volumes,omitempty" validate:"dive,required"` |
There was a problem hiding this comment.
Not needed anymore, still can stay maybe we once need to order Volumes?
|
@bketelsen may have a look at these changes? Can we discuss them before I split them up into nice commits? |
What's the reason for the 6.11 requirement? That will restrict a lot of deployments. |
|
@bketelsen many thanks for having a look at this preview patch, maybe I named it bad. The reason is https://discuss.linuxcontainers.org/t/incus-6-11-has-been-released/23322#p-80017-configurable-oci-entrypoint-8 In short it needs oci.uid and oci.gid for creating volumes with the uid and gid. Everything else can be done without Kind regards, Edit: I believe we can support pre-incus 6.11 users as well, we just need to make sure to set |
Oh wow, I missed this addition, very nice!
Yes, that should be OK as long as we have a path to support older versions. |
|
can you resolve the conflicts? Then I can do some testing. |
jochumdev
force-pushed
the
chore/ugly_patch
branch
from
ada1fc4 to
751008c
Compare
|
Done |
jochumdev
force-pushed
the
chore/ugly_patch
branch
from
751008c to
27a5aa2
Compare
|
I was hoping to get my systems updated to a newer incus but haven't. I tried testing on incus 6.9 and it failed with idmapping error. If we can resolve that for older incus versions we can move forward. |
|
What filesystem do you use for |
|
it was incus-in-incus, using What's next in this refactor/change? |
|
If your ok, with it overall. I would prepare some small commits to split this patch up. |
sounds good, let's get them in |
2 participants
Working on this for having full docker-compose support.
WIP.