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

Only upgrade go version in genmod, don't downgrade #1418

Merged
merged 2 commits into from
Apr 27, 2023

Conversation

dmathieu
Copy link
Member

We have some modules (such as apmotel) that don't support all the versions we provide support for.
For example, OpenTelemetry follows the go release policy, so versions lower than 1.19 aren't supported, and generics are used.

This means we can't build that module on go 1.15, and can't run make update-modules.

With this change, we will try to upgrade the go version if it's lower than the one specified. However, if it's higher, we consider that to be on purpose, and don't change it.

@dmathieu dmathieu mentioned this pull request Apr 26, 2023
@dmathieu dmathieu force-pushed the genmod-go-version branch from 6b6a19f to f134a1e Compare April 26, 2023 10:26
@dmathieu dmathieu force-pushed the genmod-go-version branch from f134a1e to a88e449 Compare April 26, 2023 10:29
@@ -138,7 +139,7 @@ func updateModule(dir string, gomod *GoMod, modules map[string]*GoMod) error {
"-require", require.Path + "@" + *versionFlag,
"-replace", require.Path + "=" + relDir,
}
if *goVersionFlag != "" {
if *goVersionFlag != "" && semver.Compare("v"+*goVersionFlag, "v"+gomod.Go) == 1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if this would be acceptable for all cases since it might break the compatibility of agents if go.mod for a specific module is unintentionally upgraded. @axw can probably clarify this.

Alternatively, we can add another option to exclude certain modules and execute 2 separate commands in the update-modules (might be more of a hack as the list might grow).

Copy link
Member Author

Choose a reason for hiding this comment

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

If go.mod is unintentionally upgraded, wouldn't PR reviews be there to prevent that?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine releasing new modules with a version greater than the agent's minimum, just not the other way around. If the version of a module is updated after release such that compatibility is broken, I would consider that a bug that we would fix by reverting.

@dmathieu dmathieu force-pushed the genmod-go-version branch from 28aa96e to ad2ae84 Compare April 26, 2023 12:38
@dmathieu dmathieu merged commit 932e751 into elastic:main Apr 27, 2023
@dmathieu dmathieu deleted the genmod-go-version branch April 27, 2023 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants