feat(operator): add per-package upgradeCRDs policy for HelmRelease (#2427)
## What this PR does Add an opt-in `upgradeCRDs` field to `ComponentInstall` in `PackageSource`. The field maps directly to `HelmRelease.Spec.Upgrade.CRDs` so a component author can declare how Flux should handle CRDs from the chart's `crds/` directory when the release is upgraded. The helm-controller default on upgrade is `Skip`, which means CRDs added by a chart bump never reach clusters that already have the release installed — they must be applied manually with `kubectl apply --filename charts/.../crds/`. This surfaces on every upgrade of an operator whose CRD set expands between versions (etcd-operator, cnpg, kubevirt, kamaji, etc.). Setting `upgradeCRDs: CreateReplace` lets Flux apply new CRDs declaratively with the chart. Values are restricted to `Skip`, `Create`, `CreateReplace` via a kubebuilder enum marker. Empty / unset preserves the existing helm-controller default, so every current `PackageSource` keeps working unchanged. Migration is out of scope here — follow-ups will opt individual packages in case-by-case. ### Relation to existing CRD management approach The project convention (per #377) is to extract CRDs into a dedicated Helm chart that reconciles ahead of the operator chart. This PR does not replace that pattern — it complements it for charts that keep CRDs inline under `charts/<name>/crds/` where extraction isn't practical (vendored upstream charts with tightly coupled CRDs). Packages that already split CRDs out can leave `upgradeCRDs` unset and keep using their existing separate chart. ### Release note ```release-note feat(operator): add opt-in `upgradeCRDs` field to `PackageSource` component `install` block to control how CRDs from the chart's `crds/` directory are applied on HelmRelease upgrades (`Skip` by default; use `CreateReplace` for operators whose CRD set expands between versions). ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Configurable CRD upgrade policy for component installs: Skip, Create, CreateReplace — controls CRD handling during package upgrades and preserves controller default when unset. * **Documentation** * Guidance on CRD upgrade semantics, advice to use CreateReplace for specific operators, and warning about potential data-loss risks; clarified contributor scope examples and PR template guidance. * **Tests** * Added tests validating CRD policy parsing and presence of the CRD policy enum in the published schema. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
commit
62d2516525
7 changed files with 189 additions and 7 deletions
7
.github/PULL_REQUEST_TEMPLATE.md
vendored
7
.github/PULL_REQUEST_TEMPLATE.md
vendored
|
|
@ -1,9 +1,10 @@
|
|||
<!-- Thank you for making a contribution! Here are some tips for you:
|
||||
- Use Conventional Commits for the PR title: `type(scope): description`
|
||||
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
|
||||
- Scopes for system components: dashboard, platform, cilium, kube-ovn, linstor, fluxcd, cluster-api
|
||||
- Scopes for managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes
|
||||
- Scopes for development and maintenance: api, hack, tests, ci, docs, maintenance
|
||||
- Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples:
|
||||
- System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api
|
||||
- Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes
|
||||
- Development and maintenance: api, hack, tests, ci, docs, maintenance
|
||||
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer
|
||||
- If it's a work in progress, consider creating this PR as a draft.
|
||||
- Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft.
|
||||
|
|
|
|||
|
|
@ -132,6 +132,16 @@ type ComponentInstall struct {
|
|||
// DependsOn is a list of component names that must be installed before this component
|
||||
// +optional
|
||||
DependsOn []string `json:"dependsOn,omitempty"`
|
||||
|
||||
// UpgradeCRDs controls how CRDs from the chart's crds/ directory are
|
||||
// handled on HelmRelease upgrades. Maps to HelmRelease.Spec.Upgrade.CRDs.
|
||||
// Empty string (default) preserves the helm-controller default (Skip).
|
||||
// Use "CreateReplace" for operators that evolve their CRD set between
|
||||
// versions. Warning: CreateReplace overwrites CRDs and may cause data
|
||||
// loss if upstream drops fields from a CRD with live objects.
|
||||
// +optional
|
||||
// +kubebuilder:validation:Enum=Skip;Create;CreateReplace
|
||||
UpgradeCRDs string `json:"upgradeCRDs,omitempty"`
|
||||
}
|
||||
|
||||
// Component defines a single Helm release component within a package source
|
||||
|
|
|
|||
|
|
@ -43,10 +43,11 @@ git commit --signoff -m "type(scope): brief description"
|
|||
|
||||
**Types:** `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`
|
||||
|
||||
**Scopes** (e.g., not exhaustive — use any scope that names the component you are touching):
|
||||
- System: `dashboard`, `platform`, `cilium`, `kube-ovn`, `linstor`, `fluxcd`, `cluster-api`
|
||||
- Apps: `postgres`, `mariadb`, `redis`, `kafka`, `clickhouse`, `virtual-machine`, `kubernetes`
|
||||
- Other: `api`, `hack`, `tests`, `ci`, `docs`, `agents`, `maintenance`
|
||||
**Scopes** (examples — not an exhaustive list; pick the most specific scope that describes the change, and introduce a new one if a genuinely new area needs its own):
|
||||
|
||||
- System, e.g.: `dashboard`, `platform`, `operator`, `cilium`, `kube-ovn`, `linstor`, `fluxcd`, `cluster-api`
|
||||
- Apps, e.g.: `postgres`, `mariadb`, `redis`, `kafka`, `clickhouse`, `virtual-machine`, `kubernetes`
|
||||
- Other, e.g.: `api`, `hack`, `tests`, `ci`, `docs`, `agents`, `maintenance`
|
||||
|
||||
Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer.
|
||||
|
||||
|
|
|
|||
|
|
@ -83,6 +83,14 @@ packages/<category>/<package-name>/
|
|||
- Reference PR numbers when available
|
||||
- Keep commits atomic and focused
|
||||
|
||||
### PackageSource CRD upgrade policy
|
||||
|
||||
Each component in a `PackageSource` may set `install.upgradeCRDs` to control how CRDs from the chart's `crds/` directory are handled on `HelmRelease` upgrades. Allowed values: `Skip` (default — helm-controller does not touch CRDs on upgrade), `Create` (create new CRDs only), `CreateReplace` (create new and overwrite existing).
|
||||
|
||||
Set `upgradeCRDs: CreateReplace` for operators whose upstream regularly adds new CRDs between versions (etcd-operator, cnpg, kubevirt, kamaji). Without it, new CRDs from a chart bump do not land on existing clusters — only fresh installs get them.
|
||||
|
||||
Do **not** set `CreateReplace` blindly: it overwrites every CRD in `crds/` and can cause silent data loss if upstream drops a field from a CRD that has live objects. Only enable it for operators whose schema evolution is additive-only. When in doubt, leave it unset and apply new CRDs manually.
|
||||
|
||||
### Documentation
|
||||
|
||||
Documentation is organized as follows:
|
||||
|
|
|
|||
|
|
@ -118,6 +118,19 @@ spec:
|
|||
ReleaseName is the name of the HelmRelease resource that will be created
|
||||
If not specified, defaults to the component Name field
|
||||
type: string
|
||||
upgradeCRDs:
|
||||
description: |-
|
||||
UpgradeCRDs controls how CRDs from the chart's crds/ directory are
|
||||
handled on HelmRelease upgrades. Maps to HelmRelease.Spec.Upgrade.CRDs.
|
||||
Empty string (default) preserves the helm-controller default (Skip).
|
||||
Use "CreateReplace" for operators that evolve their CRD set between
|
||||
versions. Warning: CreateReplace overwrites CRDs and may cause data
|
||||
loss if upstream drops fields from a CRD with live objects.
|
||||
enum:
|
||||
- Skip
|
||||
- Create
|
||||
- CreateReplace
|
||||
type: string
|
||||
type: object
|
||||
libraries:
|
||||
description: |-
|
||||
|
|
|
|||
|
|
@ -45,6 +45,16 @@ const (
|
|||
SecretCozystackValues = "cozystack-values"
|
||||
)
|
||||
|
||||
// parseCRDPolicy maps ComponentInstall.UpgradeCRDs to a helmv2.CRDsPolicy.
|
||||
// Empty / nil preserves the helm-controller default (Skip on upgrade);
|
||||
// the CRD enum marker restricts the string to Skip/Create/CreateReplace.
|
||||
func parseCRDPolicy(install *cozyv1alpha1.ComponentInstall) helmv2.CRDsPolicy {
|
||||
if install == nil || install.UpgradeCRDs == "" {
|
||||
return ""
|
||||
}
|
||||
return helmv2.CRDsPolicy(install.UpgradeCRDs)
|
||||
}
|
||||
|
||||
// PackageReconciler reconciles Package resources
|
||||
type PackageReconciler struct {
|
||||
client.Client
|
||||
|
|
@ -221,6 +231,7 @@ func (r *PackageReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
|
|||
Remediation: &helmv2.UpgradeRemediation{
|
||||
Retries: -1,
|
||||
},
|
||||
CRDs: parseCRDPolicy(component.Install),
|
||||
},
|
||||
},
|
||||
}
|
||||
|
|
|
|||
138
internal/operator/package_reconciler_test.go
Normal file
138
internal/operator/package_reconciler_test.go
Normal file
|
|
@ -0,0 +1,138 @@
|
|||
/*
|
||||
Copyright 2025 The Cozystack Authors.
|
||||
|
||||
Licensed under the Apache License, Version 2.0 (the "License");
|
||||
you may not use this file except in compliance with the License.
|
||||
You may obtain a copy of the License at
|
||||
|
||||
http://www.apache.org/licenses/LICENSE-2.0
|
||||
|
||||
Unless required by applicable law or agreed to in writing, software
|
||||
distributed under the License is distributed on an "AS IS" BASIS,
|
||||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
See the License for the specific language governing permissions and
|
||||
limitations under the License.
|
||||
*/
|
||||
|
||||
package operator
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1"
|
||||
helmv2 "github.com/fluxcd/helm-controller/api/v2"
|
||||
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
|
||||
"sigs.k8s.io/yaml"
|
||||
)
|
||||
|
||||
func TestParseCRDPolicy(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
install *cozyv1alpha1.ComponentInstall
|
||||
want helmv2.CRDsPolicy
|
||||
}{
|
||||
{
|
||||
name: "nil install leaves flux default",
|
||||
install: nil,
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "empty upgradeCRDs leaves flux default",
|
||||
install: &cozyv1alpha1.ComponentInstall{},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "Skip is passed through",
|
||||
install: &cozyv1alpha1.ComponentInstall{UpgradeCRDs: "Skip"},
|
||||
want: helmv2.Skip,
|
||||
},
|
||||
{
|
||||
name: "Create is passed through",
|
||||
install: &cozyv1alpha1.ComponentInstall{UpgradeCRDs: "Create"},
|
||||
want: helmv2.Create,
|
||||
},
|
||||
{
|
||||
name: "CreateReplace is passed through",
|
||||
install: &cozyv1alpha1.ComponentInstall{UpgradeCRDs: "CreateReplace"},
|
||||
want: helmv2.CreateReplace,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := parseCRDPolicy(tc.install)
|
||||
if got != tc.want {
|
||||
t.Errorf("parseCRDPolicy() = %q, want %q", got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestPackageSourceCRDHasUpgradeCRDsEnum guards the generated CRD schema: the
|
||||
// invalid-value case from the spec is enforced at the API server via a
|
||||
// kubebuilder enum marker, not in the reconciler. If someone drops the marker
|
||||
// and forgets to regenerate, this test catches it.
|
||||
func TestPackageSourceCRDHasUpgradeCRDsEnum(t *testing.T) {
|
||||
path := filepath.Join("..", "crdinstall", "manifests", "cozystack.io_packagesources.yaml")
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
t.Fatalf("read %s: %v", path, err)
|
||||
}
|
||||
|
||||
var crd apiextensionsv1.CustomResourceDefinition
|
||||
if err := yaml.Unmarshal(data, &crd); err != nil {
|
||||
t.Fatalf("unmarshal CRD: %v", err)
|
||||
}
|
||||
|
||||
var field *apiextensionsv1.JSONSchemaProps
|
||||
for i := range crd.Spec.Versions {
|
||||
v := &crd.Spec.Versions[i]
|
||||
if v.Schema == nil || v.Schema.OpenAPIV3Schema == nil {
|
||||
continue
|
||||
}
|
||||
spec, ok := v.Schema.OpenAPIV3Schema.Properties["spec"]
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
variants, ok := spec.Properties["variants"]
|
||||
if !ok || variants.Items == nil || variants.Items.Schema == nil {
|
||||
continue
|
||||
}
|
||||
components, ok := variants.Items.Schema.Properties["components"]
|
||||
if !ok || components.Items == nil || components.Items.Schema == nil {
|
||||
continue
|
||||
}
|
||||
install, ok := components.Items.Schema.Properties["install"]
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
f, ok := install.Properties["upgradeCRDs"]
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
field = &f
|
||||
break
|
||||
}
|
||||
|
||||
if field == nil {
|
||||
t.Fatal("upgradeCRDs field not found in PackageSource CRD schema")
|
||||
}
|
||||
|
||||
got := map[string]bool{}
|
||||
for _, e := range field.Enum {
|
||||
var s string
|
||||
if err := json.Unmarshal(e.Raw, &s); err != nil {
|
||||
t.Fatalf("unmarshal enum value %q: %v", e.Raw, err)
|
||||
}
|
||||
got[s] = true
|
||||
}
|
||||
|
||||
for _, want := range []string{"Skip", "Create", "CreateReplace"} {
|
||||
if !got[want] {
|
||||
t.Errorf("enum value %q missing from upgradeCRDs; got %v", want, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue