daemon/graphdriver: split, internalize packages to separate snapshotters and graphdrivers#48092
Merged
thaJeztah merged 4 commits intomoby:masterfrom Jul 8, 2024
Merged
Conversation
thaJeztah
added
area/storage
thaJeztah
force-pushed
the
fsmagic_internal
branch
2 times, most recently
from
63b9d18 to
a11a386
Compare
thaJeztah
changed the title
thaJeztah
changed the title
thaJeztah
force-pushed
the
fsmagic_internal
branch
3 times, most recently
from
5bdc9c5 to
32d7a70
Compare
thaJeztah
added
the
containerd-integration
These utilities were used in both graphdrivers and snapshotters. Move them to a separate package, to help decoupling snapshotters and graphdrivers, and make it internal, as it's not intended to be used as a generic utility package (we can still make it public if there would be a need). Signed-off-by: Sebastiaan van Stijn <[email protected]>
While detection of filesystem-types may not be supported on all platforms, it should be ok to define the types; this would allow for these types to be used to print names (e.g.) in cross-platform situations. Signed-off-by: Sebastiaan van Stijn <[email protected]>
…Checker The Checker interface was introduced in 1ba05cd as an optimization to allow passing a simplified check for situations that don't require mountinfo.Mounted to be executed (as that may result in parsing `/proc/self/mountinfo`). The Checker was defined as an interface with a single `IsMounted` method, possibly with the intent to allow for additional kind of checks to be added. No new additions were made since its inception 9 Years ago, and if a need would arrive, could probably be implemented as part of the check. This patch simplifies the definition to a function, removing the need to implement a wrapper struct just to satisfy the interface. The `Checker` type definition is somewhat redundant, but is kept to have a place to provide GoDoc. The `NewFsChecker` and `NewDefaultChecker` utilities are removed as part of this change, favoring a local definition for storage-drivers that used them. Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
fsmagic_internal
branch
from
32d7a70 to
6e60e0e
Compare
Member
Author
|
@cpuguy83 this one LGTY? |
cpuguy83
requested changes
Jul 1, 2024
| @@ -1,4 +1,4 @@ | |||
| package graphdriver // import "github.com/docker/docker/daemon/graphdriver" | |||
| package refcounter | |||
Member
There was a problem hiding this comment.
Seems like this should be called mountref with a type called Counter
Member
There was a problem hiding this comment.
Or we can have something like fstype and the refcounter thing in one package.
Member
Author
There was a problem hiding this comment.
Mountref works for me; I tried to find a proper name for it, but was indeed considering if it should include "mount" in the name.
Let me try updating and see how that looks.
I prefer keeping the FSType bits separate; I think they're somewhat separate concerns, and it's nice to have a small focussed package.
The RefCounter is used in both graphdrivers and snapshotters. Move it to a separate package to help decoupling snapshotters and graphdrivers, and make it internal, as it's not intended to be used as a generic utility package (we can still make it public if there would be a need). Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
fsmagic_internal
branch
from
6e60e0e to
0f3273e
Compare
Member
Author
|
@cpuguy83 updated; PTAL |
Member
Author
|
@cpuguy83 ptal 🤗 |
cpuguy83
approved these changes
Jul 8, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
/pkg#32989daemon/graphdriver: move FsMagic utilities to an internal package
These utilities were used in both graphdrivers and snapshotters. Move them
to a separate package, to help decoupling snapshotters and graphdrivers,
and make it internal, as it's not intended to be used as a generic utility
package (we can still make it public if there would be a need).
daemon/internal/fstype: make FsMagic values not platform-dependent
While detection of filesystem-types may not be supported on all platforms,
it should be ok to define the types; this would allow for these types to
be used to print names (e.g.) in cross-platform situations.
daemon/graphdriver: simplify Checker, remove NewFsChecker, NewDefaultChecker
The Checker interface was introduced in 1ba05cd
as an optimization to allow passing a simplified check for situations that
don't require mountinfo.Mounted to be executed (as that may result in parsing
/proc/self/mountinfo).The Checker was defined as an interface with a single
IsMountedmethod,possibly with the intent to allow for additional kind of checks to be added.
No new additions were made since its inception 9 Years ago, and if a need would
arrive, could probably be implemented as part of the check.
This patch simplifies the definition to a function, removing the need to
implement a wrapper struct just to satisfy the interface. The
Checkertype definition is somewhat redundant, but is kept to have a place to
provide GoDoc.
The
NewFsCheckerandNewDefaultCheckerutilities are removed as partof this change, favoring a local definition for storage-drivers that
used them.
daemon/graphdriver: move RefCounter to an internal package
The RefCounter is used in both graphdrivers and snapshotters. Move it
to a separate package to help decoupling snapshotters and graphdrivers,
and make it internal, as it's not intended to be used as a generic utility
package (we can still make it public if there would be a need).
- A picture of a cute animal (not mandatory but encouraged)