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

Does not compile with apimachinery 1.31 #2927

Closed
cagataygurturk opened this issue Aug 14, 2024 · 8 comments
Closed

Does not compile with apimachinery 1.31 #2927

cagataygurturk opened this issue Aug 14, 2024 · 8 comments

Comments

@cagataygurturk
Copy link

It seems the changes introduced on https://github.com/kubernetes/kubernetes/pull/121970/files#diff-723a06b5a60646d0e6eeaf63b66f2672b579c72f7008c9ab8529446f20484014R39 broke controller-runtime.

# sigs.k8s.io/controller-runtime/pkg/internal/controller
/Users/cguertuerk/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.18.5/pkg/internal/controller/controller.go:103:9: not enough arguments in call to fn
	have (interface{})
	want (context.Context, interface{})
# sigs.k8s.io/controller-runtime/pkg/webhook/admission
/Users/cguertuerk/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.18.5/pkg/webhook/admission/webhook.go:158:9: not enough arguments in call to fn
	have (interface{})
	want (context.Context, interface{})
@sbueringer
Copy link
Member

Interesting, mostly the same answers from #2925 apply

(cc @dims fyi)

@cagataygurturk
Copy link
Author

cagataygurturk commented Aug 14, 2024

Interesting, mostly the same answers from #2925 apply

(cc @dims fyi)

Thanks, I figured the next release will bring 1.31 compatibility and I see that the offending code above got fixed and is waiting to be released. Let me close the issue.

@sbueringer
Copy link
Member

sbueringer commented Aug 14, 2024

@cagataygurturk Was this already changed back in k8s.io/apimachinery? We didn't change anything in controller-runtime and the controller-runtime main branch doesn't have an issue with k8s.io/apimachinery 0.31.0

EDIT: Oh nevermind, We made a change in CR. I wasn't aware (or rather I didn't realize at the time and forgot until now, xref: https://github.com/kubernetes-sigs/controller-runtime/pull/2798/files#diff-3f86588ef108e53ef346d3223ca106ccaa8efe72cb1055489665d7675f3da7b4). What do you mean with "the offending code above got fixed and is waiting to be released"? (do you mean controller-runtime or k8s.io/apimachinery?)

Are they going to roll this back in k8s.io/apimachinery 0.31.1?

(It's really important because the upcoming CR v0.19 release has to be compatible with 0.31.x)

@sbueringer
Copy link
Member

/reopen

@k8s-ci-robot k8s-ci-robot reopened this Aug 14, 2024
@k8s-ci-robot
Copy link
Contributor

@sbueringer: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@cagataygurturk
Copy link
Author

cagataygurturk commented Aug 14, 2024

@sbueringer

I've seen that #2798 made the necessary changes to cover the new function signature in 1.31. Once the changes in this PR is released, I guess the issue will be fixed.
"By offending code" I meant controller-runtime code which was fixed on #2798.

I believe this issue can be closed because of that.

@sbueringer
Copy link
Member

Sounds good. Thx for clarifying!

/close

@k8s-ci-robot
Copy link
Contributor

@sbueringer: Closing this issue.

In response to this:

Sounds good. Thx for clarifying!

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants