Remove Factory and LinuxFactory #3373
Conversation
kolyshkin
force-pushed
the
less-interfaces
branch
from
40874b1 to
031efc5
Compare
kolyshkin
force-pushed
the
less-interfaces
branch
from
031efc5 to
845638c
Compare
kolyshkin
force-pushed
the
less-interfaces
branch
from
845638c to
0f79e80
Compare
kolyshkin
force-pushed
the
less-interfaces
branch
from
0f79e80 to
5f39797
Compare
kolyshkin
force-pushed
the
less-interfaces
branch
from
5f39797 to
822c9bc
Compare
|
No longer a draft; PTAL @opencontainers/runc-maintainers @thaJeztah @AkihiroSuda |
|
PTAL @opencontainers/runc-maintainers @thaJeztah @AkihiroSuda (I have a few more draft PRs on top of this). |
|
This is mostly code removal, and is organized to ease the review as much as possible. |
|
Anything I can do to move this forward, @opencontainers/runc-maintainers? I have a few more PRs based on top of this. |
kolyshkin
force-pushed
the
less-interfaces
branch
from
822c9bc to
314682f
Compare
libcontainer/factory_linux.go
Outdated
| // | ||
| // Returns the new container with a running process. | ||
| // | ||
| // On error, any partially created container parts are cleaned up (the |
There was a problem hiding this comment.
I don't see any clean-up code
libcontainer/factory_linux.go
Outdated
| // Returns the new container with a running process. | ||
| // | ||
| // On error, any partially created container parts are cleaned up (the | ||
| // operation is atomic). |
There was a problem hiding this comment.
"atomic" might not be the right word.
"atomic" means "atomic" as in sync/atomic to me.
There was a problem hiding this comment.
It's worse than that -- currently, Create does not even creates a container (with a process running inside it, as the doc says) -- it merely creates a container directory and returns some structure that can be used to actually start a container.
I'll try to modify the docs accordingly. These were just moved from factory to not lose them, but seems like it was not a good idea.
|
👍 but godoc might not be correct |
Is is not used by anyone Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It is not used by anyone. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The only implementation is LinuxFactory, let's use this directly. Move the piece of documentation about Create from removed factory.go to the factory_linux.go. The LinuxFactory is to be removed later. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
StartInitialization does not have to be a method of Factory (while it is clear why it was done that way initially, now we only have Linux containers so it does not make sense). Fix callers and docs accordingly. No change in functionality. Also, since this was the only user of libcontainer.New with the empty string as an argument, the corresponding check can now be removed from it. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
These do not have to be methods. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since LinuxFactory has become the means to specify containers state top directory (aka --root), and is only used by two methods (Create and Load), it is easier to pass root to them directly. Modify all the users and the docs accordingly. While at it, fix Create and Load docs (those that were originally moved from the Factory interface docs). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin
force-pushed
the
less-interfaces
branch
from
314682f to
6a3fe16
Compare
Based on and currently includes #3374. Draft until that one is merged.Continuing the spring cleaning started in #3354. This dismantles Factory and LinuxFactory, doing some cleanups along the way.
Loosely based on parts of #3316.