Rebase k8s dependencies to 1.19-rc.2 #337

damemi · 2020-07-09T19:27:41Z

This is an in-place PR to start updating our deps to 1.19 for the upcoming 1.19 k8s release

Fixes #353

k8s-ci-robot · 2020-07-09T19:28:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damemi

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~~ [damemi]

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

damemi · 2020-07-09T19:34:39Z

/retest

damemi · 2020-07-09T19:48:58Z

Weird that these checks are failing CI, they pass locally for me and I'm running go 1.14.4 (same as the prow job image. Tested in a container:

$ docker run -it --rm -v $(pwd):/go/src/sigs.k8s.io/descheduler/.:Z golang:1.14.4 sh
# go version
go version go1.14.4 linux/amd64
# make verify
./hack/verify-gofmt.sh
./hack/verify-vendor.sh
go: downloading k8s.io/apimachinery v0.19.0-beta.2
go: downloading k8s.io/component-base v0.19.0-beta.2
go: downloading k8s.io/client-go v0.19.0-beta.2
go: downloading k8s.io/klog/v2 v2.1.0
(.........)
go: downloading github.com/mailru/easyjson v0.7.0
go: downloading github.com/kr/text v0.1.0
go: downloading github.com/fsnotify/fsnotify v1.4.9
go: downloading github.com/emicklei/go-restful v2.9.5+incompatible
go: downloading github.com/PuerkitoBio/purell v1.1.1
go: downloading github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578
Vendor Verified.
Removing ./hack/../_tmp/kube-vendor.3dNkBy
./_output/bin/golangci-lint run
./_output/bin/golangci-lint: 1: ./_output/bin/golangci-lint: ????: not found
./_output/bin/golangci-lint: 1: ./_output/bin/golangci-lint: Syntax error: "|" unexpected
make: *** [Makefile:88: lint] Error 2
# make test-unit
./test/run-unit-tests.sh
?   	sigs.k8s.io/descheduler/cmd/descheduler	[no test files]
?   	sigs.k8s.io/descheduler/cmd/descheduler/app	[no test files]
?   	sigs.k8s.io/descheduler/cmd/descheduler/app/options	[no test files]
?   	sigs.k8s.io/descheduler/pkg/api	[no test files]
?   	sigs.k8s.io/descheduler/pkg/api/v1alpha1	[no test files]
?   	sigs.k8s.io/descheduler/pkg/apis/componentconfig	[no test files]
?   	sigs.k8s.io/descheduler/pkg/apis/componentconfig/v1alpha1	[no test files]
ok  	sigs.k8s.io/descheduler/pkg/descheduler	0.506s
?   	sigs.k8s.io/descheduler/pkg/descheduler/client	[no test files]
ok  	sigs.k8s.io/descheduler/pkg/descheduler/evictions	0.078s
?   	sigs.k8s.io/descheduler/pkg/descheduler/evictions/utils	[no test files]
ok  	sigs.k8s.io/descheduler/pkg/descheduler/node	0.042s
ok  	sigs.k8s.io/descheduler/pkg/descheduler/pod	0.063s
?   	sigs.k8s.io/descheduler/pkg/descheduler/scheme	[no test files]
ok  	sigs.k8s.io/descheduler/pkg/descheduler/strategies	0.055s
?   	sigs.k8s.io/descheduler/pkg/utils	[no test files]
?   	sigs.k8s.io/descheduler/test	[no test files]

And I do get some error with golangci-lintbut the vendor should verify and the tests should pass...

damemi · 2020-07-09T20:19:40Z

I think it's some assumption we're making in our script that's messed up... I can sometimes get it to reproduce the failure by switching to master and running verify-vendor, but then running update-vendor fixes it (and produces no noticeable code changes)

damemi · 2020-07-09T20:25:40Z

Ok I can consistently reproduce this by switching between master and my new branch rebase-1.19:

$ git checkout rebase-1.19 
Switched to branch 'rebase-1.19'

$ hack/verify-vendor.sh 
(fails with the weird non-diffs we see in CI)

$ hack/update-vendor.sh
(no changes)

$ hack/verify-vendor.sh 
Vendor Verified.
Removing hack/../_tmp/kube-vendor.1xgx7F

$ git checkout master
Switched to branch 'master'

$ hack/verify-vendor.sh 
(fails again)

$ hack/update-vendor.sh
(no diff)

$ hack/verify-vendor.sh 
Vendor Verified.
Removing hack/../_tmp/kube-vendor.XP4DV4

I can go back and forth with this pattern forever, but it seems like hack/update-vendor.sh specifically (which only calls go mod tidy && go mod vendor) makes some change that fixes verify-vendor, but is ignored by git

It seems to be something about update-vendor that breaks verify-vendor on other branches. Because if I run an update on one branch, the other branch won't verify until I run an update on that branch, and then the first branch doesn't verify.

damemi · 2020-07-09T21:37:37Z

The problem is only related to OpenAPIV2 packages, related to the lowercasing of the new imports (see kubernetes/client-go#741 and the linked kube-openapi PR in that issue). This would explain why the diff doesn't show up in git

go mod vendor keeps resetting the OpenAPIV2 package name to caps on an older branch:

mdame@ibm-p9b-04-bmc[rebase-1.19] ~/go/src/sigs.k8s.io/descheduler$ git checkout master
Switched to branch 'master'
mdame@ibm-p9b-04-bmc[master] ~/go/src/sigs.k8s.io/descheduler$ ls vendor/github.com/googleapis/gnostic/
total 12K
drwxr-xr-x 6 mdame staff 192 Jul  9 17:23 .
drwxr-xr-x 3 mdame staff  96 Jul  9 17:23 ..
drwxr-xr-x 9 mdame staff 288 Jul  9 17:26 compiler
drwxr-xr-x 7 mdame staff 224 Jul  9 17:26 extensions
drwxr-xr-x 7 mdame staff 224 Jul  9 17:26 openapiv2
-rw-r--r-- 1 mdame staff 12K Jul  9 17:23 LICENSE
mdame@ibm-p9b-04-bmc[master] ~/go/src/sigs.k8s.io/descheduler$ hack/update-vendor.sh 
mdame@ibm-p9b-04-bmc[master] ~/go/src/sigs.k8s.io/descheduler$ ls vendor/github.com/googleapis/gnostic/
total 12K
drwxr-xr-x 6 mdame staff 192 Jul  9 17:26 .
drwxr-xr-x 3 mdame staff  96 Jul  9 17:26 ..
drwxr-xr-x 7 mdame staff 224 Jul  9 17:26 OpenAPIv2
drwxr-xr-x 9 mdame staff 288 Jul  9 17:26 compiler
drwxr-xr-x 7 mdame staff 224 Jul  9 17:26 extensions
-rw-r--r-- 1 mdame staff 12K Jul  9 17:26 LICENSE

which carries over when switching to the newer branch, so that's why I could switch between them and reproduce the issue. The rebase-1.19 (this) branch bumps the openapiv2 dependency to its new lowercase form. However, since git doesn't see the case change, it doesn't get committed and that's why it fails CI.

damemi · 2020-07-17T18:24:37Z

Need to make changes to update our policy conversion since default conversions no longer take place in 1.19 after kubernetes/kubernetes#90018

damemi · 2020-07-27T17:59:31Z

@seanmalloy @ingvagabund I just updated this PR to 1.19-rc2. If we want to merge this to get our master branch up to a "pre-release" state I'm ok with it, that will let us make sure our other ongoing changes right now will work with 1.19 and lets us update our CI as well. Then the eventual bump to GA 1.19 should just be smooth. What do you think?

seanmalloy · 2020-07-27T18:33:04Z

pkg/api/register.go

@@ -35,12 +33,6 @@ const GroupName = "descheduler"
 // SchemeGroupVersion is group version used to register these objects
 var SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: runtime.APIVersionInternal}

-func init() {
-	if err := addKnownTypes(scheme.Scheme); err != nil {


It looks like the addKnownTypes function(lines 46 to 51) is no longer called. So should the addKnownTypes function just be removed from this file too?

I also noticed that some other files have their own addKnownTypes functions, so maybe these should be cleaned up too?

descheduler/pkg/apis/componentconfig/register.go

Line 38 in 4014eba

if err := addKnownTypes(scheme.Scheme); err != nil {

pkg/apis/componentconfig/v1alpha1/register.go

descheduler/pkg/apis/componentconfig/v1alpha1/register.go

Line 51 in 4014eba

localSchemeBuilder.Register(addKnownTypes, addDefaultingFuncs)

I'm honestly not sure if these need to be cleaned up or not, but just wanted to point this out just in case.

Good point, let me check these out. I think at least some of them can be removed if not all

This addKnownTypes is still used by the SchemeBuilder: https://github.com/kubernetes-sigs/descheduler/blob/72946602bbc51c3aee3fe438120040a04e53cbdb/pkg/api/register.go#L25f0456cede4d61f48e51015136685ba1dR25

The others in ComponentConfig aren't called, but I think I need to update those to manually invoke AddToScheme like I did here

ingvagabund · 2020-07-28T12:27:04Z

@seanmalloy @ingvagabund I just updated this PR to 1.19-rc2. If we want to merge this to get our master branch up to a "pre-release" state I'm ok with it, that will let us make sure our other ongoing changes right now will work with 1.19 and lets us update our CI as well. Then the eventual bump to GA 1.19 should just be smooth. What do you think?

Sounds good. As long as release-1.19 branch is cut after GA 1.19 rebase.

damemi · 2020-07-28T14:03:54Z

Sounds good. As long as release-1.19 branch is cut after GA 1.19 rebase.

Yeah that would be exactly the plan, to not cut release-1.19 until after 1.19 GA

seanmalloy · 2020-07-28T16:08:36Z

/lgtm
/hold

@damemi these changes look good tome. Please remove the hold when you are ready for this to merge.

ingvagabund · 2020-07-28T16:55:32Z

Change gnostic/openapiv2 to lowercase in git

This will be troublesome every time someone runs go mod .... Hopefully, it will get fixed upstream soon.

/lgtm

damemi · 2020-07-28T17:17:21Z

Change gnostic/openapiv2 to lowercase in git

This will be troublesome every time someone runs go mod .... Hopefully, it will get fixed upstream soon.

/lgtm

This shouldn't be a problem once it's fixed here. It's really only a problem for case-insensitive filesystems (like Mac) which don't see the filename change when the version was bumped (and so, don't commit the change).

For that same reason, if anyone else is on a Mac they shouldn't see any change running go.mod, so nothing to worry about there. Just a hitch but from this version on it should be good

damemi · 2020-07-28T17:39:25Z

CI looks good, so I'll release the hold. Planned 1.19 GA is August 25, so this will give us some good soak time to catch any bugs (like #353, which this fixes so that's another reason to get it in sooner).

Thanks @seanmalloy and @ingvagabund
/hold cancel

Rebase k8s dependencies to 1.19-rc.2

damemi requested review from seanmalloy and ingvagabund July 9, 2020 19:27

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 9, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 9, 2020

damemi mentioned this pull request Jul 9, 2020

Kubernetes 1.19 Release Cycle #284

Closed

4 tasks

damemi force-pushed the rebase-1.19 branch from 1d27e87 to 179438b Compare July 9, 2020 19:50

damemi force-pushed the rebase-1.19 branch from a4a65a0 to 8142ce6 Compare July 9, 2020 21:36

damemi force-pushed the rebase-1.19 branch from 7d202c6 to 0d2a5da Compare July 17, 2020 14:38

This was referenced Jul 17, 2020

Logging is broken with the switch to klog/v2 #353

Closed

"unknown conversion" with descheduler policy post-1.14 openshift/cluster-kube-descheduler-operator#116

Closed

damemi force-pushed the rebase-1.19 branch from 95cea35 to 84af398 Compare July 20, 2020 15:34

damemi added 3 commits July 27, 2020 13:55

Pin k8s deps to 1.19-rc.2

63603f3

go mod tidy && go mod vendor

b09932e

Change gnostic/openapiv2 to lowercase in git

92cb6a3

damemi force-pushed the rebase-1.19 branch from 84af398 to 7294660 Compare July 27, 2020 17:55

damemi changed the title ~~[wip] Rebase upstream 1.19~~ Rebase k8s dependencies to 1.19-rc.2 Jul 27, 2020

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 27, 2020

seanmalloy requested changes Jul 27, 2020

View reviewed changes

Update API scheme registration for 1.19

5bb0389

damemi force-pushed the rebase-1.19 branch from 7294660 to 5bb0389 Compare July 28, 2020 14:24

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 28, 2020

k8s-ci-robot assigned seanmalloy Jul 28, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 28, 2020

k8s-ci-robot assigned ingvagabund Jul 28, 2020

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 28, 2020

k8s-ci-robot merged commit d0fbebb into kubernetes-sigs:master Jul 28, 2020

seanmalloy mentioned this pull request Jul 31, 2020

Desheduler needs to check the number of replicas before evicting the pod. #271

Closed

briend pushed a commit to briend/descheduler that referenced this pull request Feb 11, 2022

Merge pull request kubernetes-sigs#337 from damemi/rebase-1.19

f1eb5b3

Rebase k8s dependencies to 1.19-rc.2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rebase k8s dependencies to 1.19-rc.2 #337

Rebase k8s dependencies to 1.19-rc.2 #337

damemi commented Jul 9, 2020 •

edited

Loading

k8s-ci-robot commented Jul 9, 2020

damemi commented Jul 9, 2020

damemi commented Jul 9, 2020 •

edited

Loading

damemi commented Jul 9, 2020

damemi commented Jul 9, 2020 •

edited

Loading

damemi commented Jul 9, 2020 •

edited

Loading

damemi commented Jul 17, 2020

damemi commented Jul 27, 2020

seanmalloy Jul 27, 2020

damemi Jul 28, 2020

damemi Jul 28, 2020 •

edited

Loading

ingvagabund commented Jul 28, 2020

damemi commented Jul 28, 2020

seanmalloy commented Jul 28, 2020

ingvagabund commented Jul 28, 2020

damemi commented Jul 28, 2020

damemi commented Jul 28, 2020

Rebase k8s dependencies to 1.19-rc.2 #337

Rebase k8s dependencies to 1.19-rc.2 #337

Conversation

damemi commented Jul 9, 2020 • edited Loading

k8s-ci-robot commented Jul 9, 2020

damemi commented Jul 9, 2020

damemi commented Jul 9, 2020 • edited Loading

damemi commented Jul 9, 2020

damemi commented Jul 9, 2020 • edited Loading

damemi commented Jul 9, 2020 • edited Loading

damemi commented Jul 17, 2020

damemi commented Jul 27, 2020

seanmalloy Jul 27, 2020

Choose a reason for hiding this comment

damemi Jul 28, 2020

Choose a reason for hiding this comment

damemi Jul 28, 2020 • edited Loading

Choose a reason for hiding this comment

ingvagabund commented Jul 28, 2020

damemi commented Jul 28, 2020

seanmalloy commented Jul 28, 2020

ingvagabund commented Jul 28, 2020

damemi commented Jul 28, 2020

damemi commented Jul 28, 2020

damemi commented Jul 9, 2020 •

edited

Loading

damemi commented Jul 9, 2020 •

edited

Loading

damemi commented Jul 9, 2020 •

edited

Loading

damemi commented Jul 9, 2020 •

edited

Loading

damemi Jul 28, 2020 •

edited

Loading