plugins: hard code plugin names

[marquiz]

Drop the -name argument for plugins, except for the template plugin where the default is set to "template". Ensures consistent name accross environments.

An less invasive alternative would be to change the flag default values.

[marquiz]

@klihub @mikebrow @saschagrunert @chrishenzie

[klihub]

Drop the -name argument for plugins, except for the template plugin where the default is set to "template". Ensures consistent name accross environments.

An less invasive alternative would be to change the flag default values.

@marquiz Cannot we simply omit giving any name ? In that case the stub would use filepath.Base(os.Args[0])), which would be the same what we are hardcoding here. We'd then also need to fix the image generating Dockerfile to not expect all plugins to be called /bin/plugin within the image...

[marquiz]

Cannot we simply omit giving any name ? In that case the stub would use filepath.Base(os.Args[0])), which would be the same what we are hardcoding here. We'd then also need to fix the image generating Dockerfile to not expect all plugins to be called /bin/plugin within the image...

IMO this PR is far more simpler (and simplification). We cannot fix the dockerfile. Either need to have separate dockerfile for each plugin, separate docker target/stage for each plugin in the dockerfile or some ugly scripting (dockerfile mangling) in the CI.

Why is it so bad to give the name explicitly (like the differ plugin already does)?

[klihub]

Cannot we simply omit giving any name ? In that case the stub would use filepath.Base(os.Args[0])), which would be the same what we are hardcoding here. We'd then also need to fix the image generating Dockerfile to not expect all plugins to be called /bin/plugin within the image...

IMO this PR is far more simpler (and simplification). We cannot fix the dockerfile. Either need to have separate

We could add ENV NRI_PLUGIN_NAME=${PLUGIN} to the final image in the Dockerfile and omit setting the plugin name here. The stub defaults to taking it from the env first, then falling back to the binary name otherwise if nothing is given.

Why is it so bad to give the name explicitly (like the differ plugin already does)?

It is not a biggie and of course we can go with what you suggest. I just thought that it'd be nice to avoid hardcoding and rely on the defaults instead...

[marquiz]

We could add ENV NRI_PLUGIN_NAME=${PLUGIN} to the final image in the Dockerfile and omit setting the plugin name here. The stub defaults to taking it from the env first, then falling back to the binary name otherwise if nothing is given.

Ah, ok, we can do that.

Why is it so bad to give the name explicitly (like the differ plugin already does)?

It is not a biggie and of course we can go with what you suggest. I just thought that it'd be nice to avoid hardcoding and rely on the defaults instead...

We already have opts = append(opts, stub.WithPluginName("... in every plugin so I thought just riding that wave (with some simplification). TBH, I don't quite get the name business here /bin/device-injector -name logger. I get it for the stub, but not for the plugins.

EDIT: Feel free to close this PR. I'll then adjust #197 instead

[klihub]

We already have opts = append(opts, stub.WithPluginName("... in every plugin so I thought just riding that wave

Yes, but we only do that if there is an explicit name given on the command line, otherwise we go with the defaults and let the stub figure it out.

(with some simplification).

That's why I suggested the alternative. Since you are touching exactly those same pieces of code, we could as well remove those altogether and just rely on the defaults.

TBH, I don't quite get the name business here /bin/device-injector -name logger. I get it for the stub, but not for the plugins.

I agree. Does not make any sense for most of the plugins to be able to set it with a command line option.

EDIT: Feel free to close this PR. I'll then adjust #197 instead

If we go this way, I think the only thing we need to do for #197 is to remove the name given on the command line.

[klihub]

Closing. As discussed above, let's not do this.