Skip to content

metadata: modify NewLeaseManager to return leases.Manager#5465

Merged
fuweid merged 1 commit intocontainerd:masterfrom
Iceber:update-lease-manager
May 13, 2021
Merged

metadata: modify NewLeaseManager to return leases.Manager#5465
fuweid merged 1 commit intocontainerd:masterfrom
Iceber:update-lease-manager

Conversation

@Iceber
Copy link
Copy Markdown
Member

@Iceber Iceber commented May 7, 2021

metadata.LeaseManager is usually used as leases.Manager, and the purpose of the metadata.LeaseManager should alse be to implement leases.Manager

containerd/services/leases/local.go

Lines 29 to 60 in 087d03b

func init() {
plugin.Register(&plugin.Registration{
Type: plugin.ServicePlugin,
ID: services.LeasesService,
Requires: []plugin.Type{
plugin.MetadataPlugin,
},
InitFn: func(ic *plugin.InitContext) (interface{}, error) {
m, err := ic.Get(plugin.MetadataPlugin)
if err != nil {
return nil, err
}
g, err := ic.Get(plugin.GCPlugin)
if err != nil {
return nil, err
}
return &local{
Manager: metadata.NewLeaseManager(m.(*metadata.DB)),
gc: g.(gcScheduler),
}, nil
},
})
}
type gcScheduler interface {
ScheduleAndWait(context.Context) (gc.Stats, error)
}
type local struct {
leases.Manager
gc gcScheduler
}

@k8s-ci-robot
Copy link
Copy Markdown

k8s-ci-robot commented May 7, 2021

Hi @Iceber. 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.

@Iceber Iceber force-pushed the update-lease-manager branch 2 times, most recently from fec23de to c05b0a3 Compare May 7, 2021 13:24
@Iceber Iceber force-pushed the update-lease-manager branch from c05b0a3 to e37ddaf Compare May 7, 2021 13:26
@Iceber Iceber changed the title metadata: change the NewLeaseManager return value to leases.Manager metadata: modify NewLeaseManager to return leases.Manager May 7, 2021
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented May 7, 2021

Build succeeded.

dmcgowan
dmcgowan approved these changes May 12, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan left a comment

Choose a reason for hiding this comment

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

LGTM

fuweid
fuweid approved these changes May 13, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@fuweid fuweid merged commit 18f3458 into containerd:master May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmcgowan dmcgowan dmcgowan approved these changes

@fuweid fuweid fuweid approved these changes

Assignees

No one assigned

Labels

needs-ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Iceber @k8s-ci-robot @dmcgowan @fuweid