Skip to content

Add WithMetaStore to overlay snapshotter to allow bringing your own#8945

Merged
dmcgowan merged 1 commit intocontainerd:mainfrom
pdtpartners:feature/make-overlay-snapshotter-embeddable
Aug 21, 2023
Merged

Add WithMetaStore to overlay snapshotter to allow bringing your own#8945
dmcgowan merged 1 commit intocontainerd:mainfrom
pdtpartners:feature/make-overlay-snapshotter-embeddable

Conversation

@rbpdt
Copy link
Copy Markdown

@rbpdt rbpdt commented Aug 10, 2023

Added the option to pass a metastore to the overlay snapshotter as an opt to allow for easier embedding in other overlay-based snapshotter projects like stargz
#8959

@k8s-ci-robot
Copy link
Copy Markdown

Hi @rbpdt. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@rbpdt rbpdt force-pushed the feature/make-overlay-snapshotter-embeddable branch from 5dc9146 to c34a818 Compare August 10, 2023 16:49
@rbpdt rbpdt force-pushed the feature/make-overlay-snapshotter-embeddable branch 2 times, most recently from 3813f14 to 6f90c25 Compare August 11, 2023 17:35
@rbpdt rbpdt changed the title WIP Add WithMetaStore to overlay snapshotter to allow bringing your own Aug 11, 2023

// WithMetaStore allows the MetaStore to be created outside the snapshotter
// and passed in.
func WithMetaStore(ms *storage.MetaStore) Opt {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Directory structure managed by overlayfs snapshotter (e.g. upperPath()) should also be observed by remote snapshotter. Otherwize remote snapshotter needs to be implemented with assuming the directory structure and possibly can't notice the changes on the structure when bumping up the imports.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting we make upperPath and workPath public, which means we’ll also have to make the snapshotter struct public?


// WithMetaStore allows the MetaStore to be created outside the snapshotter
// and passed in.
func WithMetaStore(ms *storage.MetaStore) Opt {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid using a pointer to a struct here and pass in an interface. The interface is small and could just be added right before the function (to define the interface in package it is used in).

type MetaStore interface {
 TransactionContext(ctx context.Context, writable bool) (context.Context, storage.Transactor, error)
 Close() error
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the interface may also need withTransaction? But yes, I think this is a good idea.

Signed-off-by: Robbie Buxton <138501839+rbpdt@users.noreply.github.com>
@rbpdt rbpdt force-pushed the feature/make-overlay-snapshotter-embeddable branch from 6f90c25 to 23c9535 Compare August 17, 2023 18:29
@rbpdt rbpdt requested review from dmcgowan and ktock August 21, 2023 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants