[SPARK-52702] Support using latest generated CRD yaml in Helm charts #279
Conversation
### What changes were proposed in this pull request? This PR allows the generated yaml as latest version to be used by Helm charts, after merging with older versions ### Why are the changes needed? To avoid manually updating staged CRD yaml files when adding new fields to CRD ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? CIs ### Was this patch authored or co-authored using generative AI tooling? No
| yq eval-all ' | ||
| select(fileIndex == 1) as $source | ||
| | select(fileIndex == 0) | ||
| | .spec.versions += [$source.spec.versions[0]] |
There was a problem hiding this comment.
Oh, is this general enough to handle all cases when we change the APIs?
There was a problem hiding this comment.
The intention was to:
- keep old CRD version(s) that do not need new features in a template file, and
- append new version at the end of the template, then move the full generated YAML to the proper location of the chart.
As we still want supporting v1alpha1 & v1beta1, I can remove them from the template. In the next commit we can do
- keep a template file that includes an empty list of versions
- deprecated version(s) can be move to it in future
- keep a list of supported versions, generate latest YAML for them, then apply them to the template.
How does this sound ?
There was a problem hiding this comment.
Is it a little over-engineering without much clear benefits, isn't it. If we leave it in the existing way,
- It's logically simple because only a PR (which changes the specs) will add minimal and required lines.
- For all CRDs (including the latest CRD), we can have a full traceability with Git too.
There was a problem hiding this comment.
I understand there's still concern about this auto-generation. The automation was intended to reduce potential human error in maintaining the large yaml file.
While this PR itself can be a bit big for that purpose, can we at least reduce this to a test scenario validating the automated generated yaml in build is fully contained in the checked-in CRD yaml?
There was a problem hiding this comment.
According to your other PR, we are changing the APIs at v1.
+ resourceRetainDurationMillis:
+ type: integerGiven that, let's not delete the old files. They should be identical always for v1alpha and v1beta. And I'm not sure this PR guarantees that. For me, this PR seems to hide those kind of changes.
WDYT, @jiangzho ?
|
cc @peter-toth |
|
please refer the lightweight test at #283 |
2 participants
What changes were proposed in this pull request?
This PR allows the generated yaml as latest version to be used by Helm charts, after merging with older versions
Why are the changes needed?
To avoid manually updating staged CRD yaml files when updating spark-operator-api
Does this PR introduce any user-facing change?
No
How was this patch tested?
CIs
Was this patch authored or co-authored using generative AI tooling?
No