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

Bump golang to 1.16 for etcd #795

Merged
merged 1 commit into from
May 25, 2021
Merged

Bump golang to 1.16 for etcd #795

merged 1 commit into from
May 25, 2021

Conversation

gliptak
Copy link
Contributor

@gliptak gliptak commented Mar 18, 2021


name: Pull Request
about: Create a Pull Request
title: ''
labels: ''
assignees: ''


Issue
Fixes #{ issue number }

What this PR Includes
Bump golang to 1.15 for etcd

@gliptak gliptak requested a review from a team as a code owner March 18, 2021 19:55
@jasmingacic
Copy link
Contributor

@gliptak can you fix CI error before we proceed with review

@gliptak
Copy link
Contributor Author

gliptak commented Mar 22, 2021

@jasmingacic thank you for reviewing

https://github.com/k0sproject/k0s/pull/795/checks?check_run_id=2144826000#step:7:12809 shows

go: inconsistent vendoring in /etcd

It looks that https://github.com/k0sproject/k0s/blob/main/embedded-bins/etcd/Dockerfile#L7 is failing to build with 1.15.

https://travis-ci.com/github/etcd-io/etcd is successful

@jasmingacic
Copy link
Contributor

Funny locally it works just fine. I will trigger the tests again

jasmingacic
jasmingacic previously approved these changes Mar 24, 2021
@jasmingacic
Copy link
Contributor

We will keep this PR open until etcd 3.5.0 is released. We need more time to investigated whether this change will introduce any issues.

@jasmingacic jasmingacic added this to the 1.0.0 milestone Mar 24, 2021
@jasmingacic
Copy link
Contributor

image
So etcd v3.5 will be released sometimes in June

@jasmingacic jasmingacic added area/etcd need more info Information insufficient labels Apr 7, 2021
@mikhail-sakhnov mikhail-sakhnov marked this pull request as draft April 9, 2021 10:43
@mikhail-sakhnov
Copy link
Contributor

Since PR was approved already, I converted it to draft to avoid merging by accident

@ncopa
Copy link
Collaborator

ncopa commented Apr 13, 2021

I think we can merge this. Upstream uses golang 1.12, we currently golang 1.13. The tests passes. golang 1.15 generates better code. We can always revert it if it turns out that it breaks things (but I see no reason why it should).

So I see no reason to hold this back.

ncopa
ncopa previously approved these changes Apr 13, 2021
@gliptak gliptak dismissed stale reviews from ncopa and jasmingacic via 096763f May 2, 2021 15:26
@gliptak
Copy link
Contributor Author

gliptak commented May 2, 2021

@mviitane mviitane modified the milestones: 1.21+k0s.0, 1.21+k0s.1 May 3, 2021
@gliptak
Copy link
Contributor Author

gliptak commented May 22, 2021

etcd 3.5 targets Go 1.16+

@jasmingacic jasmingacic marked this pull request as ready for review May 24, 2021 12:34
@jasmingacic
Copy link
Contributor

I've change the PR from draft to ready for review.
I would suggest that we change dockerfile golang version to 1.16 and maybe remove -mod=mod

Signed-off-by: Gábor Lipták <gliptak@gmail.com>
@gliptak gliptak changed the title Bump golang to 1.15 for etcd Bump golang to 1.16 for etcd May 24, 2021
Copy link
Contributor

@jasmingacic jasmingacic left a comment

Choose a reason for hiding this comment

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

@ncopa can you take a look too?

Copy link
Collaborator

@ncopa ncopa left a comment

Choose a reason for hiding this comment

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

👍

@ncopa ncopa merged commit 0a7bbf5 into k0sproject:main May 25, 2021
@gliptak gliptak deleted the patch-3 branch May 25, 2021 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/etcd need more info Information insufficient
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants