Update k8s toolchain to 1.19.0 #3166

meyskens · 2020-08-10T12:47:07Z

What this PR does / why we need it:

This updates all our dependancies to Kubernetes v1.19-rc.3.
This is a requirement to use klog v2 see #3143. This is why this is still the RC version, we will upgrade to stable as soon as possible which will be in time for our v1.0 release.

This PR is split out of #3143 as a lot was changed in the API machinery causing us to fix some issues,

Special notes for your reviewer:
This was cherry picked out of #3143 if you see changes that look weird please say so

Removed due to squashing but big thanks to @JoshVanL to help spit out the conversion bugs we found

Release note:

Bump Kubernetes dependencies to 1.19

cmd/ctl/pkg/convert/convert.go

meyskens · 2020-08-10T12:59:34Z

/kind cleanup

JoshVanL

Just a couple comments from me :)

/assign @meyskens

test/integration/framework/apiserver.go

go.mod

meyskens · 2020-08-10T16:10:22Z

Due to Go modules not a hard requirement any longer.

/hold till 1.19 is out

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

meyskens · 2020-08-27T08:57:50Z

/test pull-cert-manager-e2e-v1-17
/test pull-cert-manager-e2e-v1-16
/test pull-cert-manager-e2e-v1-15
/test pull-cert-manager-e2e-v1-14
/test pull-cert-manager-e2e-v1-13
/test pull-cert-manager-e2e-v1-12
/test pull-cert-manager-e2e-v1-11

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

JoshVanL

Couple things from me.

pkg/acme/webhook/cmd/server/start.go

pkg/internal/apis/certmanager/install/install.go

pkg/webhook/server/server.go

test/integration/certificates/issuing_controller_test.go

JoshVanL · 2020-08-27T09:12:48Z

/test pull-cert-manager-e2e-v1-19
/test pull-cert-manager-e2e-v1-17
/test pull-cert-manager-e2e-v1-16
/test pull-cert-manager-e2e-v1-15
/test pull-cert-manager-e2e-v1-14
/test pull-cert-manager-e2e-v1-13
/test pull-cert-manager-e2e-v1-12
/test pull-cert-manager-e2e-v1-11

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

meyskens · 2020-08-27T12:13:05Z

/test pull-cert-manager-e2e-v1-19
/test pull-cert-manager-e2e-v1-17
/test pull-cert-manager-e2e-v1-16
/test pull-cert-manager-e2e-v1-15
/test pull-cert-manager-e2e-v1-14
/test pull-cert-manager-e2e-v1-13
/test pull-cert-manager-e2e-v1-12
/test pull-cert-manager-e2e-v1-11

wallrj

Just a few nits which you can feel free to ignore if you prefer.

/lgtm

wallrj · 2020-08-27T12:17:43Z

cmd/controller/app/controller.go

@@ -186,7 +186,7 @@ func buildControllerContext(ctx context.Context, stopCh <-chan struct{}, opts *o
 	intscheme.AddToScheme(scheme.Scheme)
 	log.V(logf.DebugLevel).Info("creating event broadcaster")
 	eventBroadcaster := record.NewBroadcaster()
-	eventBroadcaster.StartLogging(log.V(logf.DebugLevel).Info)
+	eventBroadcaster.StartLogging(logf.WithInfof(log.V(logf.DebugLevel)).Infof)


This is the fix for the logging panic.

wallrj · 2020-08-27T12:18:33Z

go.mod

+	k8s.io/kube-openapi v0.0.0-20200805222855-6aeccd4b50c6
+	k8s.io/kubectl v0.19.0
+	k8s.io/utils v0.0.0-20200729134348-d5654de09c73
+	sigs.k8s.io/controller-runtime v0.6.1-0.20200728202347-cea989b02ed0


I think there's a 0.6.2 version available : https://github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.6.2

wallrj · 2020-08-27T12:20:03Z

pkg/ctl/scheme.go

-
-	acmeinstall "github.com/jetstack/cert-manager/pkg/internal/apis/acme/install"
-	cminstall "github.com/jetstack/cert-manager/pkg/internal/apis/certmanager/install"
-	metainstall "github.com/jetstack/cert-manager/pkg/internal/apis/meta/install"


Sorry to be a pain, but why did the cert-manager imports move to the top?
I thought convention was to put them in a separate block at the bottom.

wallrj · 2020-08-27T12:26:29Z

pkg/ctl/scheme.go

+		}
+		b.(*corev1.List).Items = metaList.Items
+		return nil
+	})


How weird that the core API package doesn't have an install and doesn't include the conversions.

wallrj · 2020-08-27T12:30:30Z

pkg/logs/logs.go

+
+func (l *LogWithFormat) Infof(format string, a ...interface{}) {
+	l.Info(fmt.Sprintf(format, a...))
+}


Maybe worth a comment or two explaining why this is here and whether in future we expect to be able to just use klog directly?

wallrj · 2020-08-27T12:30:56Z

pkg/webhook/server/server.go

@@ -26,6 +26,8 @@ import (
 	"net/http"
 	"time"

+	"github.com/jetstack/cert-manager/pkg/webhook/server/util"


Nit: move this with the other CM imports.

jetstack-bot · 2020-08-27T12:32:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: meyskens, wallrj

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [meyskens]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

meyskens · 2020-08-27T12:35:52Z

/hold

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

meyskens · 2020-08-27T12:46:43Z

/unhold

meyskens · 2020-08-27T13:03:14Z

Server terminated abruptly (error code: 14, error message: 'Connect Failed', log file: '/bazel-scratch/.cache/bazel/_bazel_root/e50ce42c8209d553b7e7491e69e67e89/server/jvm.out')

seems we're pushing our infra a bit...

/test pull-cert-manager-e2e-v1-18

wallrj · 2020-08-27T13:40:45Z

/lgtm

jetstack-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. labels Aug 10, 2020

jetstack-bot requested a review from JoshVanL August 10, 2020 12:47

meyskens commented Aug 10, 2020

View reviewed changes

cmd/ctl/pkg/convert/convert.go Outdated Show resolved Hide resolved

meyskens force-pushed the k8s-119-rc branch from a2d666b to 6f5cc59 Compare August 10, 2020 12:57

jetstack-bot added the area/deploy Indicates a PR modifies deployment configuration label Aug 10, 2020

jetstack-bot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Aug 10, 2020

meyskens mentioned this pull request Aug 10, 2020

Improve logging #3143

Merged

JoshVanL requested changes Aug 10, 2020

View reviewed changes

test/integration/framework/apiserver.go Outdated Show resolved Hide resolved

go.mod Outdated Show resolved Hide resolved

jetstack-bot assigned meyskens Aug 10, 2020

meyskens requested a review from JoshVanL August 10, 2020 15:11

jetstack-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 10, 2020

meyskens added this to the v1.0 milestone Aug 12, 2020

jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 12, 2020

meyskens added 2 commits August 26, 2020 14:15

Update k8s toolchain to 0.19.0-rc.3

14ea7c3

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

Move NS creation to individual tests

0f6bc5e

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

meyskens force-pushed the k8s-119-rc branch from 88dcb32 to 0f6bc5e Compare August 26, 2020 12:18

jetstack-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 26, 2020

Use v0.19.0-rc.3

7b85a72

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

meyskens changed the title ~~Update k8s toolchain to 0.19.0-rc.3~~ Update k8s toolchain to 0.19.0-rc.4 Aug 26, 2020

meyskens added 2 commits August 26, 2020 14:24

Fix mismerge

93855b7

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

Run bazel updates

3a814e8

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

meyskens added 4 commits August 26, 2020 19:03

Fix conversion (insert facepalm here)

136085a

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

fix CRDs

3ec4a1e

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

Try and fail to fix list conversion

1914ecf

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

Use k8s GA

a160ba3

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

meyskens changed the title ~~Update k8s toolchain to 0.19.0-rc.4~~ Update k8s toolchain to 1.19.0 Aug 27, 2020

meyskens and others added 2 commits August 27, 2020 11:02

revert scheme changes

74929e6

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

Fixes client-go logs to use infof

c83ef5e

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

JoshVanL requested changes Aug 27, 2020

View reviewed changes

meyskens added 2 commits August 27, 2020 12:49

Fix ctl

b0a80d4

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

Implement feedback

e0749ad

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

meyskens mentioned this pull request Aug 27, 2020

Updates k8s.io dependency to 1.19 #3226

Closed

meyskens added 3 commits August 27, 2020 13:10

Update bazel

71c3d2f

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

Add boilerplate

db0a321

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

Fix boilerplate on scheme.go

a562ccd

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

wallrj approved these changes Aug 27, 2020

View reviewed changes

jetstack-bot assigned wallrj Aug 27, 2020

jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 27, 2020

Implement feedback from review

47266ff

Signed-off-by: Maartje Eyskens <maartje@eyskens.me>

jetstack-bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 27, 2020

jetstack-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 27, 2020

jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 27, 2020

jetstack-bot merged commit 6f2fc8e into cert-manager:master Aug 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update k8s toolchain to 1.19.0 #3166

Update k8s toolchain to 1.19.0 #3166

meyskens commented Aug 10, 2020 •

edited

meyskens commented Aug 10, 2020

JoshVanL left a comment

meyskens commented Aug 10, 2020

meyskens commented Aug 27, 2020

JoshVanL left a comment

JoshVanL commented Aug 27, 2020

meyskens commented Aug 27, 2020

wallrj left a comment

wallrj Aug 27, 2020

wallrj Aug 27, 2020

wallrj Aug 27, 2020

meyskens Aug 27, 2020

wallrj Aug 27, 2020

wallrj Aug 27, 2020

meyskens Aug 27, 2020

wallrj Aug 27, 2020

meyskens Aug 27, 2020

jetstack-bot commented Aug 27, 2020

meyskens commented Aug 27, 2020

meyskens commented Aug 27, 2020

meyskens commented Aug 27, 2020

wallrj commented Aug 27, 2020

Update k8s toolchain to 1.19.0 #3166

Update k8s toolchain to 1.19.0 #3166

Conversation

meyskens commented Aug 10, 2020 • edited

meyskens commented Aug 10, 2020

JoshVanL left a comment

Choose a reason for hiding this comment

meyskens commented Aug 10, 2020

meyskens commented Aug 27, 2020

JoshVanL left a comment

Choose a reason for hiding this comment

JoshVanL commented Aug 27, 2020

meyskens commented Aug 27, 2020

wallrj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jetstack-bot commented Aug 27, 2020

meyskens commented Aug 27, 2020

meyskens commented Aug 27, 2020

meyskens commented Aug 27, 2020

wallrj commented Aug 27, 2020

meyskens commented Aug 10, 2020 •

edited