-
Couldn't load subscription status.
- Fork 32
K8SPS-567: automatic yaml generation #1136
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
base: main
Are you sure you want to change the base?
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.
Pull Request Overview
This PR introduces automated generation of the example deploy/cr.yaml file for the PerconaServerMySQL operator. The solution consists of a Go binary (cmd/example-gen) that generates a populated resource and a bash script that formats, sorts, and comments the output. This automation ensures that new API fields automatically appear in the example configuration after running make manifests.
Key changes include:
- New generation tooling with custom marshaling to ignore
omitemptytags - Removal of deprecated
StorageClassfield from S3 backup specifications - Relocation of
VolumeSpecfromPodSpectoMySQLSpecto fix structural issues
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/example-gen/main.go | Entry point for CR generation binary |
| cmd/example-gen/generate.sh | Bash script to format, sort, and comment generated YAML |
| cmd/example-gen/defaults/*.go | Default value configuration for manual and preset fields |
| cmd/example-gen/internal/marshal/marshal.go | Custom JSON marshaler ignoring omitempty tags |
| cmd/example-gen/internal/fill/bytype.go | Type-based field filling logic |
| api/v1/perconaservermysql_types.go | API changes: VolumeSpec relocation and StorageClass removal |
| deploy/cr.yaml | Regenerated example configuration |
| Multiple CRD files | Updated schemas reflecting API changes |
| Test files | Updated test fixtures for VolumeSpec relocation |
| Makefile | Added generate-cr-yaml target to manifest generation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
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.
Pull Request Overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
deploy/cr.yaml
Outdated
| # proxy: false | ||
| # proxySize: false | ||
| # pause: false | ||
| # pause: true |
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.
@pooknull, we need to have default values in commented options
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.
Pull Request Overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull Request Overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull Request Overview
Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
cmd/example-gen/scripts/lib/util.sh:1
- This complex awk script that transforms diff hunks lacks inline comments explaining its logic. Add comments describing: (1) what 'has_lt', 'has_gt', 'has_sep' track, (2) why delete hunks are converted to change hunks, and (3) how lines are prefixed with '> #' for commenting. This would significantly improve maintainability.
#!/usr/bin/env bash
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull Request Overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull Request Overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull Request Overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I think this PR should also add a job to github actions to check if example cr.yaml is updated like we do for manifests. |
|
@egegunes it is already done as part of |
This reverts commit ed6a0a0.
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.
Pull Request Overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
https://perconadev.atlassian.net/browse/K8SPS-567
DESCRIPTION
This PR introduces automatic generation of the example
cr.yaml,backup.yaml,restore.yamlfiles.How it works:
cmd/example-gengenerates a populatedPerconaServerMySQL/PerconaServerMySQLBackup/PerconaServerMySQLRestoreresources.cmd/example-gen/generate.sh:When a new field is added to
PerconaServerMySQL/PerconaServerMySQLBackup/PerconaServerMySQLRestore, it will automatically appear in the example file after runningmake manifests.By default, the field will:
omitemptyis used).To adjust it:
sort_yamlfunction.example-gen/pkg/defaults/manual.go.example-gen/pkg/defaults/preset.go.Additional changes:
VolumeSpecwas moved fromPodSpectoMySQLSpecas it is only used in the MySQLStorageClassfromBackupStorageS3Specas it is not usedCHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
Config/Logging/Testability