Skip to content

services/introspection: support to show introspection grpc service plugin#5455

Closed
Iceber wants to merge 1 commit intocontainerd:mainfrom
Iceber:fix-introspection-service
Closed

services/introspection: support to show introspection grpc service plugin#5455
Iceber wants to merge 1 commit intocontainerd:mainfrom
Iceber:fix-introspection-service

Conversation

@Iceber
Copy link
Copy Markdown
Member

@Iceber Iceber commented May 6, 2021 •
edited
Loading

ctr plugins ls does not display the introspection grpc service plugin

# ctr plugins ls                                                                         
TYPE                            ID                       PLATFORMS      STATUS
io.containerd.content.v1        content                  -              ok
io.containerd.snapshotter.v1    aufs                     linux/amd64    error
io.containerd.snapshotter.v1    btrfs                    linux/amd64    error
io.containerd.snapshotter.v1    devmapper                linux/amd64    error
io.containerd.snapshotter.v1    native                   linux/amd64    ok
io.containerd.snapshotter.v1    overlayfs                linux/amd64    ok
io.containerd.snapshotter.v1    zfs                      linux/amd64    error
io.containerd.metadata.v1       bolt                     -              ok
io.containerd.differ.v1         walking                  linux/amd64    ok
io.containerd.gc.v1             scheduler                -              ok
io.containerd.service.v1        introspection-service    -              ok
io.containerd.service.v1        containers-service       -              ok
io.containerd.service.v1        content-service          -              ok
io.containerd.service.v1        diff-service             -              ok
io.containerd.service.v1        images-service           -              ok
io.containerd.service.v1        leases-service           -              ok
io.containerd.service.v1        namespaces-service       -              ok
io.containerd.service.v1        snapshots-service        -              ok
io.containerd.runtime.v1        linux                    linux/amd64    ok
io.containerd.runtime.v2        task                     linux/amd64    ok
io.containerd.monitor.v1        cgroups                  linux/amd64    ok
io.containerd.service.v1        tasks-service            -              ok
io.containerd.internal.v1       restart                  -              ok
io.containerd.grpc.v1           containers               -              ok
io.containerd.grpc.v1           content                  -              ok
io.containerd.grpc.v1           diff                     -              ok
io.containerd.grpc.v1           events                   -              ok
io.containerd.grpc.v1           healthcheck              -              ok
io.containerd.grpc.v1           images                   -              ok
io.containerd.grpc.v1           leases                   -              ok
io.containerd.grpc.v1           namespaces               -              ok
io.containerd.internal.v1       opt                      -              ok
io.containerd.grpc.v1           snapshots                -              ok
io.containerd.grpc.v1           tasks                    -              ok
io.containerd.grpc.v1           version                  -              ok
io.containerd.grpc.v1           cri                      linux/amd64    ok

analysis

The Plugins method of the introspection grpc service does not show itself and the plugins initialised after it.

When the introspection grpc service is initialised, it will get the list of currently registered plugins from the plugin context, which will not contain itself and plugins that have not yet been initialised.

Add to initialization list when initialization is complete:

containerd/services/server/server.go

Lines 168 to 173 in 1431679

result := p.Init(initContext)
if err := initialized.Add(result); err != nil {
return nil, errors.Wrapf(err, "could not add plugin result to plugin set")
}
instance, err := result.Instance()

When the introspection grpc service is initialised, it will get the list of currently registered plugins from the plugin context, which will not contain itself and plugins that have not yet been initialised.

containerd/services/introspection/service.go

Lines 53 to 64 in 1431679

allPluginsPB := pluginsToPB(ic.GetAll())
localClient, ok := i.(*Local)
if !ok {
return nil, errors.Errorf("Could not create a local client for introspection service")
}
localClient.UpdateLocal(ic.Root, allPluginsPB)
return &server{
local: localClient,
}, nil
},

Although the introspection grpc service requires itself to be initialized last(Requires: []plugin.Type{"*"}), it still allows Requests from other plugins to also be *, and if these plugins are registered by calling plugin.Register before the introspection grpc service, they will be initialized after the introspection grpc service when using plugin.Graph sorting.

output

# ctr plugin ls
TYPE                            ID                       PLATFORMS      STATUS
io.containerd.content.v1        content                  -              ok
io.containerd.snapshotter.v1    aufs                     linux/amd64    error
io.containerd.snapshotter.v1    btrfs                    linux/amd64    error
io.containerd.snapshotter.v1    devmapper                linux/amd64    error
io.containerd.snapshotter.v1    native                   linux/amd64    ok
io.containerd.snapshotter.v1    overlayfs                linux/amd64    ok
io.containerd.snapshotter.v1    zfs                      linux/amd64    error
io.containerd.metadata.v1       bolt                     -              ok
io.containerd.differ.v1         walking                  linux/amd64    ok
io.containerd.gc.v1             scheduler                -              ok
io.containerd.service.v1        introspection-service    -              ok
io.containerd.service.v1        containers-service       -              ok
io.containerd.service.v1        content-service          -              ok
io.containerd.service.v1        diff-service             -              ok
io.containerd.service.v1        images-service           -              ok
io.containerd.service.v1        leases-service           -              ok
io.containerd.service.v1        namespaces-service       -              ok
io.containerd.service.v1        snapshots-service        -              ok
io.containerd.runtime.v1        linux                    linux/amd64    ok
io.containerd.runtime.v2        task                     linux/amd64    ok
io.containerd.monitor.v1        cgroups                  linux/amd64    ok
io.containerd.service.v1        tasks-service            -              ok
io.containerd.grpc.v1           introspection            -              ok
io.containerd.internal.v1       restart                  -              ok
io.containerd.grpc.v1           containers               -              ok
io.containerd.grpc.v1           content                  -              ok
io.containerd.grpc.v1           diff                     -              ok
io.containerd.grpc.v1           events                   -              ok
io.containerd.grpc.v1           healthcheck              -              ok
io.containerd.grpc.v1           images                   -              ok
io.containerd.grpc.v1           leases                   -              ok
io.containerd.grpc.v1           namespaces               -              ok
io.containerd.internal.v1       opt                      -              ok
io.containerd.grpc.v1           snapshots                -              ok
io.containerd.grpc.v1           tasks                    -              ok
io.containerd.grpc.v1           version                  -              ok
io.containerd.grpc.v1           cri                      linux/amd64    ok

@k8s-ci-robot
Copy link
Copy Markdown

k8s-ci-robot commented May 6, 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.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented May 6, 2021

Build succeeded.

@Iceber Iceber force-pushed the fix-introspection-service branch from d191f9d to 0784f46 Compare May 6, 2021 07:23
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented May 6, 2021

Build succeeded.

@Iceber Iceber changed the title services/introspection: support to show introspection grpc service services/introspection: support to show introspection grpc service plugin May 6, 2021
@Iceber Iceber force-pushed the fix-introspection-service branch from 0784f46 to 350ad28 Compare May 7, 2021 13:06
@Iceber Iceber force-pushed the fix-introspection-service branch from 350ad28 to c3fe456 Compare May 7, 2021 13:07
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented May 7, 2021

Build succeeded.

@Iceber
Copy link
Copy Markdown
Member Author

Iceber commented May 8, 2021

@stevvooe @AkihiroSuda PTAL, Thanks

@kzys
Copy link
Copy Markdown
Member

kzys commented Jun 15, 2021

/ok-to-test

@kzys
Copy link
Copy Markdown
Member

kzys commented Jul 2, 2021

@containerd/committers Can someone take a look?

@dmcgowan dmcgowan added this to the 1.6 milestone Nov 17, 2021
dmcgowan
dmcgowan reviewed Nov 17, 2021
View reviewed changes
Comment thread plugin/plugin.go
return false
}
for _, reg := range register.r {
byID, typeok := plugins.byTypeAndID[reg.Type]
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan Nov 17, 2021

Choose a reason for hiding this comment

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

The race detector does not complain about this?

dmcgowan
dmcgowan reviewed Nov 17, 2021
View reviewed changes
Comment thread services/introspection/local.go
}

func (l *Local) getPlugins() []api.Plugin {
if !plugin.AreRegisteredPluginsInitialized(l.plugins) {
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan Nov 17, 2021

Choose a reason for hiding this comment

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

Instead of checking against registered, can you just check the size against what is already cached. If it changes, then update the cache.

@dmcgowan dmcgowan mentioned this pull request Jan 11, 2022
Merged
@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Jan 11, 2022

Carried in #6432

@dmcgowan dmcgowan closed this Jan 11, 2022
@Iceber
Copy link
Copy Markdown
Member Author

Iceber commented Jan 12, 2022

@dmcgowan I'm sorry, I forgot to continue processing this pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmcgowan dmcgowan dmcgowan left review comments

@kzys kzys kzys approved these changes

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

1.6

Development

Successfully merging this pull request may close these issues.

4 participants

@Iceber @k8s-ci-robot @kzys @dmcgowan