Removing clientset and upgrading scaffolding using kubebuilder v3.8.0 #31
Conversation
moved controller manifests to controller pkg as an asset
| @@ -1,480 +0,0 @@ | |||
| // Copyright 2022 Nokia | |||
There was a problem hiding this comment.
why delete this?
why not just make a test which loads the fake dynamic client and validates the operations?
There was a problem hiding this comment.
@marcushines My way of thinking was that if there is no clientset provided in this repo, there is no need to have tests that verify CRUD operations on custom resources, as this is now automatically done via controller-runtime.
As far as I see, from KNE perspective, there is no degradation in code coverage, and controller's operations are being tested (WIP) with ginkgo e2e tests https://github.com/srl-labs/srl-controller/blob/clientset-removal/controllers/srlinux_controller_test.go.
There was a problem hiding this comment.
so right now the srl node in kne depends on srlinux client api for create and delete - there should be clear tests in srlinux which test that api doesn't break - you could "run kne tests in an integration test" or you could just write your own api tests and leave them in the repo
There was a problem hiding this comment.
@marcushines I think in openconfig/kne#326 we removed "srlinux client API" in favor of a generic/dynamic controller-runtime client specifically to match the removal of a clientset in this PR.
Maybe I am missing something here?
There was a problem hiding this comment.
okay in kne - you are gonna now use this new client... how does kne know how to use that client and how can kne trust that api?
if you had a set of "unittests" showing hey when you create these objects this way they behave how you want it is very obvious how it is meant to be used however if you delete all the tests and the clientset it is not
There was a problem hiding this comment.
@marcushines I will create a test that
- creates Srlinux{} CR with that client and ensures the object is created
- creates Srlinux{} CR, then deletes Srlinux{} CR and ensures no more named object with that kind is available.
Codecov Report
@@ Coverage Diff @@
## main #31 +/- ##
===========================================
+ Coverage 19.81% 65.69% +45.87%
===========================================
Files 10 7 -3
Lines 772 548 -224
===========================================
+ Hits 153 360 +207
+ Misses 615 164 -451
- Partials 4 24 +20
|
Updating srl-controller to use recent version of
removing clientset in favor of using controller-runtime Client.
supersedes #30