Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

Iceber
Copy link
Member

@Iceber Iceber commented May 6, 2021

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:

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.

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

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.

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

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

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
Signed-off-by: Iceber Gu <wei.cai-nat@daocloud.io>
@Iceber Iceber force-pushed the fix-introspection-service branch from 350ad28 to c3fe456 Compare May 7, 2021 13:07
@theopenlab-ci
Copy link

theopenlab-ci bot commented May 7, 2021

Build succeeded.

@Iceber
Copy link
Member Author

Iceber commented May 8, 2021

@stevvooe @AkihiroSuda PTAL, Thanks

@kzys
Copy link
Member

kzys commented Jun 15, 2021

/ok-to-test

@kzys
Copy link
Member

kzys commented Jul 2, 2021

@containerd/committers Can someone take a look?

@dmcgowan dmcgowan added this to Ready For Review in Code Review Jul 23, 2021
@dmcgowan dmcgowan added this to the 1.6 milestone Nov 17, 2021
return false
}
for _, reg := range register.r {
byID, typeok := plugins.byTypeAndID[reg.Type]
Copy link
Member

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?

@@ -94,9 +91,16 @@ func (l *Local) Plugins(ctx context.Context, req *api.PluginsRequest, _ ...grpc.
}

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

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.

@kzys kzys moved this from Ready For Review to Needs Contributor Update in Code Review Nov 30, 2021
@dmcgowan
Copy link
Member

Carried in #6432

@dmcgowan dmcgowan closed this Jan 11, 2022
Code Review automation moved this from Needs Contributor Update to Done Jan 11, 2022
@Iceber
Copy link
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
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants