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

Failure to load some plugins when built with Go 1.18 #670

Closed
dtrudg opened this issue Mar 25, 2022 · 5 comments · Fixed by #678
Closed

Failure to load some plugins when built with Go 1.18 #670

dtrudg opened this issue Mar 25, 2022 · 5 comments · Fixed by #678
Labels
bug Something isn't working

Comments

@dtrudg
Copy link
Member

dtrudg commented Mar 25, 2022

Version of Singularity

master / release 3.9

Describe the bug

E2E tests with go 1.18 will fail with...

FATAL:   Plugin compile failed with error: while generating plugin manifest: while loading plugin /tmp/plugin-3139335449/src/plugin.so: plugin.Open("/tmp/plugin-3139335449/src/plugin"): plugin was built with a different version of package github.com/sylabs/singularity/internal/pkg/util/user

The plugin compiles, but on load Go is finding a package ABI hash that doesn't match.

Confirmed there is no issue under Go 1.17.8.

Did a very quick look at 'strings' output for the plugin.so vs singularity under 1.17.8 and 1.18 and there isn't any gross difference in the diffs.

internal/pkg/util/user does contain CGO code... so am suspecting something related to that at present.

Will be trying to work up a small reproducer... no luck to date... and subtract from the singularity code base to isolate this further.

@dtrudg dtrudg added the bug Something isn't working label Mar 25, 2022
@dtrudg
Copy link
Member Author

dtrudg commented Mar 25, 2022

Simple main + plugin reproducer that brings in github.com/sylabs/singularity/internal/pkg/util/user compiles and loads fine with Go 1.18.

Going to concentrate on subtraction from the full singularity code base now, and likely patch some debug output into the package hash / symbol calculation in Go 1.18. Likely this will drag through next week.

@dtrudg
Copy link
Member Author

dtrudg commented Mar 25, 2022

Wrote up some detail in an issue on the golang/go repository here: golang/go#51955

It appears this is due to a path to a CGO related temporary /tmp/gobuildxxxxx directory making its way into the strings section for github.com/sylabs/singularity/internal/pkg/util/user in the main singularity build, which does not happen with the plugin build.

I think this should probably never happen? A tmpdir like this is going to be different by definition on each compilation.

Will keep 🤞 that there we are able to get some insight through the linked issue.

@dtrudg
Copy link
Member Author

dtrudg commented Mar 28, 2022

Added some 1.17.8 vs 1.18 context on the Go issue: golang/go#51955 (comment)

Going to begin looking at places referencing _cgo_gotypes.go in the Go 1.18 code to see if I can work out where the extra path prefix in non-plugin buildmodes is sneaking in.

@dtrudg
Copy link
Member Author

dtrudg commented Mar 29, 2022

As explored on the golang/go issue, the problem with plugin builds in Go 1.18 is that our -gcflags -trimpath results in the 'automatic' trimpath rules being lost for our _cgo_gotypes.go file... and it ends up with a /tmp/gobuildxxxx path still prefixed to it.

It looks like this change from 1.17 -> 1.18 is unlikely to be addressed, as even back in 2019 when our plugin handling that exploited the -gcflags -trimpath was added, this was not supported...

image

The reason we need the -gcflags trimpath is so the strings in the main singularity binaries reference @v0.0.0 of github.com/sylabs/singularity. A plugin's own go.mod always references a @v0.0.0 for github.com/sylabs/singularity, so the plugin symbols will include that:

module github.com/sylabs/singularity/e2e-cli-plugin

go 1.13

require github.com/sylabs/singularity v0.0.0

replace github.com/sylabs/singularity => ./singularity_source

Without the custom gcflags trimpath....

-gcflags=github.com/sylabs/singularity/...="-trimpath $(SOURCEDIR)=>github.com/sylabs/singularity@v0.0.0

... that forces the @v0.0.0 into the main binary, we move from a mismatch on internal/pkg/util/user that uses CGO, to more general issues that report a mismatch on pkg/plugin between main binary and plugin.

The main binary strings don't include the @v0.0.0 and there is a mismatch from main binary to plugin, so the plugin cannot load.

Plugin

;github.com/sylabs/singularity@v0.0.0/pkg/plugin/manifest.goName
json:"author"Versionjson:"version"                              json:"name"Author
PluginRootDirPluginpluginPluginRootDirSymbolnjson:"description"9git.luolix.top/sylabs/singularity@v0.0.0/pkg/plugin/plugin.gManifest        CallbackCallbackInstall
                                            PluginSymbol

Main Binary

4git.luolix.top/sylabs/singularity/pkg/plugin/manifest.goName
json:"author"Versionjson:"version"                       json:"name"Author
PluginRootDirPluginpluginPluginRootDirSymbolnjson:"description"2git.luolix.top/sylabs/singularity/pkg/plugin/plugin.gManifest       CallbackCallbackInstall
                                            PluginSymbol

Seems pretty clear that our approach to build plugins out of tree like this is really abusing the way go.mod versioning etc. is meant to operate.... and we're not going to get back the 1.17 and prior gcflags trimpath behavior that allows us to do this.

I suspect we will have to go back to building plugins within the main source tree, under the scope of the main singularity go.mod.

dtrudg added a commit to dtrudg/singularity that referenced this issue Mar 29, 2022
In Go 1.18 we cannot use a `-gcflags` `-trimpath` as doing so causes
an issue where automatic trimpath rewrites do not occur. This prevents
the compiled e2e runtime plugin (and others using CGO) from loading,
due to a mismatch in the package hash.

It has been made clear in the past that using a `-gcflags` `-trimpath`
is not supported by the Go authors.

Although it is inconvenient, switch back to:

* Requiring that plugins are built from source *within* the
  singularity source tree.
* Plugins use the singularity `go.mod`.

This reverts a significant portion of 2019a5f.

See sylabs#670 for discussion.

Fixes sylabs#670
@dtrudg
Copy link
Member Author

dtrudg commented Mar 29, 2022

The decision is to switch back to compiling plugins in-tree, which infers the plugin uses the singularity go.mod, for 3.10

For 3.9, whether to make the same (breaking) change, or just exclude the failing test and issue errata is under consultation:

https://groups.google.com/g/singularity-ce/c/jyrQXcqTG0I

@tri-adam and myself discussed possible approaches. We don't believe that there is a practical safe alternative going forward that preserves out of tree build for plugins. We need to be conservative as the Go developers are clear on the expectations of what should work with Go plugins, and this issue shows that if we go beyond this we are likely to hit problems with new Go releases.

The 'proper' way to deal with this would be to split SingularityCE, so that the CLI is separate from a runtime module. Both the CLI and plugins could import this runtime module, of a specific and matching version from their own go.mod. CLI and This is the right kind of structure to avoid the -gcflags -trimpath that we have been using to 'patch' the package version strings in the main singularity executables when they are compiled, so that they match those in plugin builds.

dtrudg added a commit to dtrudg/singularity that referenced this issue Mar 29, 2022
In Go 1.18 we cannot use a `-gcflags` `-trimpath` as doing so causes
an issue where automatic trimpath rewrites do not occur. This prevents
the compiled e2e runtime plugin (and others using CGO) from loading,
due to a mismatch in the package hash.

It has been made clear in the past that using a `-gcflags` `-trimpath`
is not supported by the Go authors.

Although it is inconvenient, switch back to:

* Requiring that plugins are built from source *within* the
  singularity source tree.
* Plugins use the singularity `go.mod`.

This reverts a significant portion of 2019a5f.

See sylabs#670 for discussion.

Fixes sylabs#670
dtrudg added a commit to dtrudg/singularity that referenced this issue Mar 29, 2022
In Go 1.18 we cannot use a `-gcflags` `-trimpath` as doing so causes
an issue where automatic trimpath rewrites do not occur. This prevents
the compiled e2e runtime plugin (and others using CGO) from loading,
due to a mismatch in the package hash.

It has been made clear in the past that using a `-gcflags` `-trimpath`
is not supported by the Go authors.

Although it is inconvenient, switch back to:

* Requiring that plugins are built from source *within* the
  singularity source tree.
* Plugins use the singularity `go.mod`.

This reverts a significant portion of 2019a5f.

See sylabs#670 for discussion.

Fixes sylabs#670
dtrudg added a commit to dtrudg/singularity that referenced this issue Mar 29, 2022
In Go 1.18 we cannot use a `-gcflags` `-trimpath` as doing so causes
an issue where automatic trimpath rewrites do not occur. This prevents
the compiled e2e runtime plugin (and others using CGO) from loading,
due to a mismatch in the package hash.

It has been made clear in the past that using a `-gcflags` `-trimpath`
is not supported by the Go authors.

Although it is inconvenient, switch back to:

* Requiring that plugins are built from source *within* the
  singularity source tree.
* Plugins use the singularity `go.mod`.

This reverts a significant portion of 2019a5f.

See sylabs#670 for discussion.

Fixes sylabs#670
edytuk pushed a commit to vzokay/apptainer that referenced this issue Apr 21, 2022
In Go 1.18 we cannot use a `-gcflags` `-trimpath` as doing so causes
an issue where automatic trimpath rewrites do not occur. This prevents
the compiled e2e runtime plugin (and others using CGO) from loading,
due to a mismatch in the package hash.

It has been made clear in the past that using a `-gcflags` `-trimpath`
is not supported by the Go authors.

Although it is inconvenient, switch back to:

* Requiring that plugins are built from source *within* the
  singularity source tree.
* Plugins use the singularity `go.mod`.

This reverts a significant portion of 2019a5f.

See sylabs/singularity#670 for discussion.

Fixes sylabs/singularity#670

Signed-off-by: Edita Kizinevic <edita.kizinevic@cern.ch>

Conflicts:
	CHANGELOG.md
	cmd/internal/cli/plugin_compile_linux.go
	e2e/plugin/plugin.go
	e2e/plugin/testdata/cli/go.mod
	examples/plugins/cli-plugin/go.mod
	examples/plugins/config-plugin/go.mod
	examples/plugins/log-plugin/go.mod
	examples/plugins/ubuntu-userns-overlay-plugin/go.mod
	internal/app/apptainer/plugin_compile_linux.go
	internal/pkg/plugin/create.go
	mlocal/frags/build_cli.mk
	mlocal/frags/go_common_opts.mk
edytuk pushed a commit to vzokay/apptainer that referenced this issue Apr 26, 2022
In Go 1.18 we cannot use a `-gcflags` `-trimpath` as doing so causes
an issue where automatic trimpath rewrites do not occur. This prevents
the compiled e2e runtime plugin (and others using CGO) from loading,
due to a mismatch in the package hash.

It has been made clear in the past that using a `-gcflags` `-trimpath`
is not supported by the Go authors.

Although it is inconvenient, switch back to:

* Requiring that plugins are built from source *within* the
  singularity source tree.
* Plugins use the singularity `go.mod`.

This reverts a significant portion of 2019a5f.

See sylabs/singularity#670 for discussion.

Fixes sylabs/singularity#670

Signed-off-by: Edita Kizinevic <edita.kizinevic@cern.ch>

Conflicts:
	CHANGELOG.md
	cmd/internal/cli/plugin_compile_linux.go
	e2e/plugin/plugin.go
	e2e/plugin/testdata/cli/go.mod
	examples/plugins/cli-plugin/go.mod
	examples/plugins/config-plugin/go.mod
	examples/plugins/log-plugin/go.mod
	examples/plugins/ubuntu-userns-overlay-plugin/go.mod
	internal/app/apptainer/plugin_compile_linux.go
	internal/pkg/plugin/create.go
	mlocal/frags/build_cli.mk
	mlocal/frags/go_common_opts.mk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant