Skip to content
This repository has been archived by the owner on Dec 11, 2023. It is now read-only.

Apply k8s 1.18.5 #1046

Merged
merged 47 commits into from
Nov 3, 2020
Merged

Apply k8s 1.18.5 #1046

merged 47 commits into from
Nov 3, 2020

Conversation

tfussell
Copy link
Contributor

@tfussell tfussell commented Sep 21, 2020

Towards https://github.com/giantswarm/giantswarm/issues/12716

Kubernetes client-go v1.18 introduced a breaking change by adding context.Context to all API calls. This meant we had to release new major versions of all core libraries like operatorkit and apiextensions that also used client-go. This PR updates all of these libraries in kvm-operator so we can run on controls planes with k8s 1.18 and get updates to GS libraries. 80% of the code changes are simply updating the library major versions and adding context.Context to all API calls. The rest is compatibility fixes for the updated libraries such as the removal of certain features in operatorkit and reading certificates differently. I will highlight other questions and important changes with GitHub comments in the code.

@tfussell tfussell self-assigned this Sep 21, 2020
@tfussell tfussell marked this pull request as ready for review October 19, 2020 17:53
@tfussell tfussell requested a review from a team as a code owner October 19, 2020 17:53
.nancy-ignore Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
Copy link
Contributor

@MarcelMue MarcelMue left a comment

Choose a reason for hiding this comment

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

Not sure if I missed something, the imports make the diff very large unfortunately. But nothing that we can do to fix it.

service/controller/cloudconfig/worker_template.go Outdated Show resolved Hide resolved
service/controller/cluster.go Outdated Show resolved Hide resolved
service/controller/cluster.go Outdated Show resolved Hide resolved
Selector: labels.SelectorFromSet(map[string]string{
key.PodWatcherLabel: project.Name(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes this seems incorrect. It should still be project.Name IMO. This chantge here could have unintended consequences for updates where the version might not be the project version version (while upgrading the nodes).

service/controller/key/key.go Show resolved Hide resolved
Copy link
Contributor

@stone-z stone-z left a comment

Choose a reason for hiding this comment

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

A couple small things, otherwise LGTM

go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
service/controller/cluster.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
Copy link

@brinker211 brinker211 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@jgsqware jgsqware left a comment

Choose a reason for hiding this comment

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

IGTLGTM

Copy link
Contributor

@rossf7 rossf7 left a comment

Choose a reason for hiding this comment

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

LGTM

tfussell and others added 5 commits October 30, 2020 07:27
REVERT ME: use k8scloudconfig from dockerhub-qps-v7 branch

go mod tidy

add secret with token

load secret as part of configuration

add dockerhubToken to CI values

base64 encode secret

make secret kubernetes-compliant

make secret safely quoted

fix CI defaults

fix secret mount

pass DockerhubToken to controllers

update test cases

update test cases (goconst)

fix configmap redefinition

bump k8scloudconfig to v7.2.0

fmt
@tfussell tfussell merged commit 45a89ff into master Nov 3, 2020
@tfussell tfussell deleted the k8s-1-18 branch November 3, 2020 23:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants