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 k8s.io/client-go to v0.22.1 #705

Merged
merged 1 commit into from
Jan 13, 2022

Conversation

lrascao
Copy link
Contributor

@lrascao lrascao commented Jan 10, 2022

Description of the change

Bump k8s client api to v0.22.1, single change is propagating context across all functions in order to
reach k8s client api.

Benefits

Allows k8s controllers to use sealed secrets client API when running on newer more recent versions.

Possible drawbacks

Do not foresee any, single change is context propagation.

@mkmik
Copy link
Collaborator

mkmik commented Jan 10, 2022

we're currently running the tests against k8s: ["1.16.13", "1.17.11", "1.20.4"]

Probably we should also add version 1.22 to the build matrix.

@lrascao
Copy link
Contributor Author

lrascao commented Jan 10, 2022

we're currently running the tests against k8s: ["1.16.13", "1.17.11", "1.20.4"]

Probably we should also add version 1.22 to the build matrix.

roger, i'll add it here

@mkmik
Copy link
Collaborator

mkmik commented Jan 10, 2022

@lrascao you probably need to bump also the Go version in go.mod to 1.16

../../../go/pkg/mod/k8s.io/[email protected]/plugin/pkg/client/auth/exec/metrics.go:21:2: package io/fs is not in GOROOT (/opt/hostedtoolcache/go/1.15.15/x64/src/io/fs)
make: *** [Makefile:61: controller-static-linux-amd64] Error 1

@lrascao lrascao force-pushed the feature/bump_client_go branch 2 times, most recently from 3b82b8e to 7323f8d Compare January 10, 2022 14:05
@mkmik
Copy link
Collaborator

mkmik commented Jan 10, 2022

I think we can drop support for go <1.16

We can build go 1.16 and 1.17. The go 1.18 release is behind the corner so very soon this will cover the last three versions anyway.

Yeah, let's drop Go 1.14 and 1.15

@lrascao
Copy link
Contributor Author

lrascao commented Jan 10, 2022

I think we can drop support for go <1.16

We can build go 1.16 and 1.17. The go 1.18 release is behind the corner so very soon this will cover the last three versions anyway.

Yeah, let's drop Go 1.14 and 1.15

sounds good, i'll make the changes

@mkmik
Copy link
Collaborator

mkmik commented Jan 10, 2022

  container:
    name: Build Container
    runs-on: ubuntu-latest
    steps:
...

should we add a actions/setup-go@v2 step there too in order to make sure we get a recent version of go? (I'd use the latest 1.17 there)

@mkmik
Copy link
Collaborator

mkmik commented Jan 10, 2022

ok, the build is good now; now the problem appears to lie in the fact that the ingegration tests are behind the // +build integration tag; you have to update the API there too

@lrascao
Copy link
Contributor Author

lrascao commented Jan 10, 2022

ok, the build is good now; now the problem appears to lie in the fact that the ingegration tests are behind the // +build integration tag; you have to update the API there too

yeah, on it now

Copy link
Collaborator

@mkmik mkmik left a comment

Choose a reason for hiding this comment

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

LGTM;

@juan131 this PR also changes the CI job names and requires updating the list of required CI jobs (e.g. remove Build (1.14, macos-latest) and Build (1.15, macos-latest) and add the 1.6 and 1.7 ones)

@juan131 juan131 self-assigned this Jan 13, 2022
@juan131
Copy link
Collaborator

juan131 commented Jan 13, 2022

Awesome, thanks so much for this PR @lrascao

I think we can drop support for go <1.16
We can build go 1.16 and 1.17. The go 1.18 release is behind the corner so very soon this will cover the last three versions anyway.
Yeah, let's drop Go 1.14 and 1.15

I totally agree, it's time to drop it.

Probably we should also add version 1.22 to the build matrix.

+1

.github/workflows/ci.yml Show resolved Hide resolved
@@ -71,7 +77,7 @@ jobs:
needs: container
strategy:
matrix:
k8s: ["1.16.13", "1.17.11", "1.20.4"]
k8s: ["1.16.13", "1.17.11", "1.20.4", "1.22.5"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome thanks!

@juan131
Copy link
Collaborator

juan131 commented Jan 13, 2022

@lrascao the Go version needs to be updated in the release GH action too

Single change is propagating context across all functions in order to
reach k8s client api. Bump Go version to 1.16 due to io/fs import that
got introduced only at this version.
Copy link
Collaborator

@juan131 juan131 left a comment

Choose a reason for hiding this comment

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

LGTM

.github/workflows/ci.yml Show resolved Hide resolved
@juan131 juan131 merged commit c2ee5b3 into bitnami-labs:main Jan 13, 2022
@juan131
Copy link
Collaborator

juan131 commented Jan 13, 2022

@mkmik @lrascao I plan to create a new release (0.17.2) of the binaries including these changes + the changes we're working on at #648 once they're merged

@lrascao lrascao deleted the feature/bump_client_go branch January 13, 2022 09:59
@juan131 juan131 mentioned this pull request Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants