-
Notifications
You must be signed in to change notification settings - Fork 15
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
codegen: Fix the hack/update-codegen.sh template #564
Conversation
Signed-off-by: timflannagan <[email protected]>
Issues linked to changelog: |
Signed-off-by: timflannagan <[email protected]>
Makefile
Outdated
if [ "$(shell uname)" == "Darwin" ]; then \ | ||
if [ "$(shell uname)" = "Darwin" ]; then \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed to get install-protoc
working on a fresh fork.
Hmm I tried running diff --git a/pkg/api/v1/clients/kube/client/clientset/versioned/fake/clientset_generated.go b/pkg/api/v1/clients/kube/client/clientset/versioned/fake/clientset_generated.go
index e162c4a..b4c2d23 100644
--- a/pkg/api/v1/clients/kube/client/clientset/versioned/fake/clientset_generated.go
+++ b/pkg/api/v1/clients/kube/client/clientset/versioned/fake/clientset_generated.go
@@ -19,9 +19,7 @@ limitations under the License.
package fake
import (
- "github.com/solo-io/solo-kit/pkg/api/v1/clients/kube/crd"
clientset "github.com/solo-io/solo-kit/pkg/api/v1/clients/kube/client/clientset/versioned"
- realregisterfile "github.com/solo-io/solo-kit/pkg/api/v1/clients/kube/client/clientset/versioned/scheme"
resourcesv1 "github.com/solo-io/solo-kit/pkg/api/v1/clients/kube/client/clientset/versioned/typed/solo.io/v1"
fakeresourcesv1 "github.com/solo-io/solo-kit/pkg/api/v1/clients/kube/client/clientset/versioned/typed/solo.io/v1/fake"
"k8s.io/apimachinery/pkg/runtime"
@@ -33,23 +31,14 @@ import (
// NewSimpleClientset returns a clientset that will respond with the provided objects.
// It's backed by a very simple object tracker that processes creates, updates and deletions as-is,
-// without applying any validations and/or defaults. It shouldn't be considered a replacement
+// without applying any field management, validations and/or defaults. It shouldn't be considered a replacement
// for a real clientset and is mostly useful in simple unit tests.
-func NewSimpleClientset(crd crd.Crd, objects ...runtime.Object) *Clientset { // NOTE(marco): this line was updated
-
- // ###############################################################################################
- // ###############################################################################################
- // NOTE(marco): the following line was updated to reference the scheme our CRD objects register
- // with. Originally, this pointed to the scheme in the register.go file in the same package.
- //
- // The generated code expected pkg/api/v1/clients/kube/crd/solo.io/v1 to export a function named
- // "AddToScheme" that would be called from both fake/register.go and scheme/register.go to add
- // the custom types to the respective schemes. We moved the same logic to the "NewCrd" function
- // in pkg/api/v1/clients/kube/crd/crd.go, but it always writes to the scheme in scheme/register.go,
- // so we reference it here.
- // ###############################################################################################
- // ###############################################################################################
- o := testing.NewObjectTracker(realregisterfile.Scheme, codecs.UniversalDecoder())
+//
+// DEPRECATED: NewClientset replaces this with support for field management, which significantly improves
+// server side apply testing. NewClientset is only available when apply configurations are generated (e.g.
+// via --with-applyconfig).
+func NewSimpleClientset(objects ...runtime.Object) *Clientset { It seems like we're in a really awkward situation with some cruft with this repository right now. |
Created a tracking issue for this. Closing as I don't have the capacity to push this through right now. |
Description
This is a quick follow-up to #560 which updated the generate_script.go for Kubernetes clients.
Why is this needed?
When pulling the most recent of solo-kit into the Gloo repository, the following hack/update-codegen.sh bash script was produced:
When I try to run that file locally, the following script output is produced:
This no-op output is due to a couple of reasons. Let's replace the solo-kit dependency with a local copy to debug this further:
And then start with the first problem, which is the script attempting to
source
the kube_codegen.sh script and run a command that's defined within the same script in the same LOC. Let's manually fix that LOC in the script:Which produces a new error that we need to burn down:
After tinkering with this for a while, I refactored the template script to use the right directories with the gen_client sub-command and new client generated output was being copied back to the project's source. However, this client generation was incomplete, and lister & informer generated files were empty (i.e. no-op operations).
I wasn't able to determine the root cause for why the new gen_client-based approach didn't work, but after researching how repositories in the community were handling k8s 1.31 bumps, I saw a lot of repositories were referencing the gateway-api's implementation that runs k8s.io/code-generator binaries directly.
Refactoring the template to use this new approach fixed the no-op listener and informer issues observed with the singular gen_client command, and we were back in business:
And the following diff was produced:
Lastly, we can verify the script works as lister-gen generated lister code that uses the new generic implementation introduced to client-go in 1.31: