-
Notifications
You must be signed in to change notification settings - Fork 437
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
Do settings as a pre-install hook #564
Conversation
Issues linked to changelog: |
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.
Looks good!
/kick |
Only change since @marcogschmidt reviewed was rearranging some tests (removed tests that were broken and never run, added tests that are relatively fast to start testing install command). Not happy with coverage for the new functions but it is hard to test, regression tests should provide sufficient assurance that install still works however. |
Note: these clients were manually generated using a solo-kit that points to my local fork that implements solo-io#564. The gateway & gloo clients were updated to adopt recently support for generics throughout the 1.31 client-go release. Namely, listers and clients adopt this new approach. The nested extauth and graphql APIs have updated hack/update-codegen.sh bash scripts checked in with this commit, but I think we need to update the solo-kit.json configuration for those directories since we weren't previously committing their k8s clients. Similarly, the "gloosnapshot" API doesn't need k8s clients generated too. Signed-off-by: timflannagan <timflannagan@gmail.com>
Avoids race condition where settings gets created by the pod first, causing the install to fail. Also includes a small refactor to minimize code dupe.