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

Support Kubernetes 1.26.0 #326

Closed
wants to merge 1 commit into from
Closed

Conversation

mamachanko
Copy link
Collaborator

Addresses #325

@codecov
Copy link

codecov bot commented Jan 3, 2023

Codecov Report

Base: 61.55% // Head: 61.11% // Decreases project coverage by -0.44% ⚠️

Coverage data is based on head (0a8cab0) compared to base (d9febe1).
Patch coverage: 37.03% of modified lines in pull request are covered.

❗ Current head 0a8cab0 differs from pull request most recent head e1edd1f. Consider uploading reports for the commit e1edd1f to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #326      +/-   ##
==========================================
- Coverage   61.55%   61.11%   -0.45%     
==========================================
  Files          17       17              
  Lines        2154     2173      +19     
==========================================
+ Hits         1326     1328       +2     
- Misses        751      768      +17     
  Partials       77       77              
Impacted Files Coverage Δ
testing/client.go 60.47% <37.03%> (-4.97%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mamachanko
Copy link
Collaborator Author

@scothis After naively upgrading sigs.k8s.io/controller-runtime@1.{13 → 14}.1 I realised there are compilation errors in reconciler-runtime. That's because of breaking changes to client-go as of sigs.k8s.io/controller-runtime@v0.14.0.

It all goes a little over my head, but I think to have understood that its about the new SubResourceClient.

This PR is taking a stab at #326. However, I might be doing the completely wrong thing. With these changes at least our downstream project compiles, passes tests and performs as desired.

Would you be able to take a look and tell if I am far off the target? Possibly you had something else in mind for supporting Kubernetes 1.26.0. I'd be happy to yield this PR in that case.

@scothis
Copy link
Contributor

scothis commented Jan 3, 2023

@mamachanko I had an alternate implementation in flight that I hadn't pushed up until now over at #327. The main difference is that I bride calls from the sub resource client for the status sub resource over to the status client so they are handled consistently. I also don't call the upstream fake sub resource client as most of the methods are unimplemented. Instead I depend on the reactor chain to handle the request.

@mamachanko
Copy link
Collaborator Author

Superseded by #327

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

Successfully merging this pull request may close these issues.

None yet

3 participants