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

Honestly, I wonder if anyone is actually using this and if it should've been deprecated for v2.0. I think the hopes were to allow this to be used, but their usefulness is very limited, and I think Go itself largely abandoned any effort to make it better (I don't think it's even supported on all platforms).

Perhaps it could even be considered to do the reverse, and make it "opt-in".

@thaJeztah

This is actually deprecated in https://github.com/containerd/containerd/blob/v2.0.1/RELEASES.md#deprecated-features

So I guess we can just safely remove this feature

This is actually deprecated in https://github.com/containerd/containerd/blob/v2.0.1/RELEASES.md#deprecated-features

Ah! Nice. Perhaps it would make sense to include this in existing release branches to already allow packagers to disable it (and then remove from main)

Please squash the commits and update main/BUILDING.md#build-containerd

Done 👍
I wasn't sure how detailed the build tag description in BUILDING.md should be so I kept it (very) short, let me know if you want me to change it !

Note that this PR targets release/1.7 directly, not the main branch.
If you want the build tag in v2 then I believe I would have to open a similar PR in github.com/containerd/plugin.

I checked out of curiosity, using the build tag reduces the size of containerd itself from ~56MB to ~43MB, a 24% reduction 😄

👋 Hello @AkihiroSuda @djdongjin @thaJeztah, is there anything you want me to change, or anything I can do to get this merged ?

Note that this PR targets release/1.7 directly, not the main branch.
If you want the build tag in v2 then I believe I would have to open a similar PR in github.com/containerd/plugin.

Yes, we would probably need that, so that builders can opt-out of this feature both for 1.7 and 2.0 until it's removed.

Perhaps the alternative would be to handle this on the containerd side; that way we don't need a build-tag in containerd/plugins; that module doesn't necessarily have to be updated because importing the dynamic package is not enforced by the module, so we can leave it up to the consumer (containerd in this case) to decide whether to use it.

It looks like the dynamic loading happens here;

So we can stub that out based on build-tag, e.g. having a plugins.go and plugins_nodynamic.go;

//go:build no_dynamic_plugins

package server

func loadDynamic(path string) (loaded int, _ error) {
	// dynamic plugins disabled through no_dynamic_plugins build-tag
	return 0, nil
}

And change the server code to call that instead;

func LoadPlugins(ctx context.Context, config *srvconfig.Config) ([]plugin.Registration, error) {
	// load all plugins into containerd
	path := config.PluginDir //nolint:staticcheck
	if path == "" {
		path = filepath.Join(config.Root, "plugins")
	}
	if count, err := loadDynamic(path); err != nil {

Or we could even move out the whole bit about loading the dynamic ones;

func loadDynamic(ctx context.Context, config *srvconfig.Config) error {
	path := config.PluginDir //nolint:staticcheck
	if path == "" {
		path = filepath.Join(config.Root, "plugins")
	}
	count, err := dynamic.Load(path)
	if err != nil {
		return err
	}
	if count > 0 || config.PluginDir != "" { //nolint:staticcheck
		config.PluginDir = path //nolint:staticcheck
		log.G(ctx).Warningf("loaded %d dynamic plugins. `go_plugin` is deprecated, please use `external plugins` instead", count)
	}
	return nil
}

and stub;

func loadDynamic(ctx context.Context, config *srvconfig.Config) error {
	// dynamic plugins disabled through no_dynamic_plugins build-tag
	return nil
}

I opened a PR with similar changes targeting release/2.0, let me know if there is anything else I can do.
#11213

PR targeting release/2.0 is merged, any chance you can also re-review this one @AkihiroSuda ?