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

update controller-runtime #1025

Closed
chuckha opened this issue Jun 14, 2019 · 22 comments · Fixed by #1054
Closed

update controller-runtime #1025

chuckha opened this issue Jun 14, 2019 · 22 comments · Fixed by #1054
Assignees
Labels
area/dependency Issues or PRs related to dependency changes kind/feature Categorizes issue or PR as related to a new feature. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor.

Comments

@chuckha
Copy link
Contributor

chuckha commented Jun 14, 2019

/kind feature

This is a tracking issue for people to pay attention to the controller-runtime dependency update.

Anything else you would like to add:
Add related issues below and why they are related:

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 14, 2019
@vincepri
Copy link
Member

/area dependency
/milestone v1alpha2

@k8s-ci-robot k8s-ci-robot added this to the v1alpha2 milestone Jun 14, 2019
@k8s-ci-robot k8s-ci-robot added the area/dependency Issues or PRs related to dependency changes label Jun 14, 2019
@alexeldeib
Copy link
Contributor

Are we saying migrating to v2 is dependent on v2 kubebuilder/controller runtime supporting webhooks fully? Or that we don’t have it today so we can punt to post-upgrade

@detiber
Copy link
Member

detiber commented Jun 17, 2019

I don't necessarily think that it is a blocker, but rather a nice to have.

@alexeldeib
Copy link
Contributor

I started looking at the cost of this w.r.t code-generation as well (they are fairly tightly linked, and have dependency interconnection problems). The CAPI side is a decent amount of code but mostly constrained to clusterclient and friends. I haven't looked at it from the provider side yet.

If the provider side isn't too crazy and most of the work is in clusterclient it might be possible to pull this off fairly smoothly even with ongoing data model changes.

@tamalsaha
Copy link

Folks, @tahsinrahman and @faem from our team would like to help with this process. We are also blocked on this, as we want to use the client-go 1.14 version for the cloud providers. If you can show us how you like to see this done for one the providers, we can send prs for other ones.

Are these projects planning to switch to the new v2 project structure and go.mod at the same time?

@detiber
Copy link
Member

detiber commented Jun 20, 2019

I don't have any objections to switching to the v2 project structure and go.mod.

@alexeldeib
Copy link
Contributor

alexeldeib commented Jun 20, 2019

I started work to move ClusterClient to use the controller-runtime clients and simultaneously bump to controller-runtime v0.2.0-beta.2. It's very WIP at this stage, I got the builds and type generation generally working. I attempted to directly rip out k8s.io/code-gen and anything that changed in v1 -> v2 while still supporting dep before doing any additional sort of clean up work around client instantiation, etc.

That said, I'm happy to see someone else go ahead with this instead, as I won't have time to finish it for at least ~a week.

@vincepri
Copy link
Member

Do Kubernetes and controller-runtime v2 use go.mod?

@tamalsaha
Copy link

@vincepri , Kubernetes 1.14 does not use go.mod. Kubernetes 1.15 uses go.mod.

kubebuilder's 2.0.0-alpha.X release uses go.mod and k8s 1.14.

I wonder if we could update kubebuilder and controller-runtime to k8s 1.15 and then update CAPI stuff to 1.15 at the same time. This might be a little ambitious.

@vincepri
Copy link
Member

That'd be all great, I know there has been a lot of work making sure go.mod works fine with upstream k/k. I only want to make sure that we don't duplicate the burden of supporting go.mod too early for CAPI and all the downstream providers.

@tamalsaha
Copy link

According to https://github.com/golang/go/wiki/Go-Release-Cycle, GO 1.13 is supposed to be released on Aug 1. My understanding is that it will have go mod on by default. This is probably a good time to switch to go.mod from that point.

@vincepri
Copy link
Member

Agreed!

@tamalsaha
Copy link

tamalsaha commented Jun 20, 2019

It seems controller-runtime can be upgraded to use k8s 1.1.5 kubernetes-sigs/controller-runtime#495 by just updating go.mod files. All tests are passing.

@vincepri
Copy link
Member

@tamalsaha @alexeldeib It seems that to introduce v1alpha2 types (#1035) we'd need to update controller-runtime and controller-tools. Would it be ok for me to open a PR later today to tackle these updates as they're blocking for all the other work items related to v1alpha2?

@alexeldeib
Copy link
Contributor

music to my ears. I am looking less at the k8s dependency side and more looking at cutting out the code-gen typed clients in favor of controller-runtime.

@alexeldeib
Copy link
Contributor

if anyone is interested, I'm working on that here: https://github.com/kubernetes-sigs/cluster-api/compare/master...alexeldeib:ace/ctrl?expand=1 (very messy WIP branch, but should be clear what I am trying to achieve from clusterclient.go)

@tamalsaha
Copy link

I have opened a pr against controller-tools to update it to 1.15.0 kubernetes-sigs/controller-tools#231 . One new thing is that instead of one Webhook struct, now there are separate MutatingWebhook and ValidatingWebhook.

@vincepri
Copy link
Member

Should I push the changes as part of the v1alpha2 types or a different PR? Given that both runtime and tools are beta we probably shouldn’t backport to v1a1 anyways

@detiber
Copy link
Member

detiber commented Jun 20, 2019

I'm partial to separating them to make reviewing the changes easier

@vincepri
Copy link
Member

vincepri commented Jun 20, 2019

Not a big deal either way, the update currently blocks v1alpha2 so takes priority

@vincepri
Copy link
Member

/lifecycle active

@k8s-ci-robot k8s-ci-robot added the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label Jun 21, 2019
@vincepri
Copy link
Member

PR is up, ptal

@vincepri vincepri self-assigned this Jun 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependency Issues or PRs related to dependency changes kind/feature Categorizes issue or PR as related to a new feature. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants