-
Notifications
You must be signed in to change notification settings - Fork 101
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
Make sure config/crds are always up to date for testing #977
Conversation
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.
That makes master so much better! 🎉 PTAL at my comments/suggestions
crds := cmdinit.CRDs() | ||
|
||
if false { | ||
// change this to true if you want to one time override the manifests with new values |
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.
Wouldn't it be nice to have a make test-crds
which would call into a main()
method (can be in this test file) that writes the manifests? Instead of manually changing false -> true
here? Just an idea
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.
I'll leave that as an exercise for someone who is not comfortable with the true/false situation 😛
What this PR does / why we need it:
In the past, we generated manifests using
/config/manifests
. Then we switched to kudo init. Nowkudo init
should be the ultimate source of truth but at the same time, we still have/config/manifests
file with usually obsolete manifests that are causing cryptic test failures.This PR removes this tech debt. Instead of removing
/config/crds
, I went the way of generating them from init in tests and then writing test that always ensures that these are up to date with what init provides. The manifests are right now used e.g. when test harness starts - the harness expects folder with CRDs so this was one solution of how to provide it (I find it the easier one).This is a replacement of #857