feat(api): make HelmRelease Install/Upgrade timeout per-Application
Replace the hardcoded r.kindName == "Kubernetes" switch in rest.go with a config-driven path. The ApplicationDefinition CR now accepts a release.cozystack.io/helm-install-timeout annotation that is parsed at cozystack-api startup into config.ReleaseConfig.HelmInstallTimeout and applied to both Install.Timeout and Upgrade.Timeout on the rendered HelmRelease. Applications that leave the annotation unset keep flux defaults so their failed installs remediate on the normal cadence - only the Kubernetes kind carries the override and gets a 15m budget. New kinds with a similar race can opt in by setting the same annotation; no rest.go patch needed. Kubernetes-rd sets the annotation to 15m. Table-driven test in rest_timeout_test.go covers three cases: Kubernetes with 15m, Qdrant unset, and an arbitrary future kind with 20m - all of which pin the Remediation.Retries == -1 contract as well. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
This commit is contained in:
Aleksei Sviridkin
parent
1757567218
commit
7b146cbe56
5 changed files with 133 additions and 82 deletions
|
|
@ -2,6 +2,14 @@ apiVersion: cozystack.io/v1alpha1
|
|||
kind: ApplicationDefinition
|
||||
metadata:
|
||||
name: kubernetes
|
||||
annotations:
|
||||
# Kubernetes tenants boot a Kamaji control plane whose admin-kubeconfig
|
||||
# Secret is provisioned asynchronously. Cold Kamaji start (image pull +
|
||||
# etcd + apiserver Ready) plus admin-kubeconfig generation can exceed
|
||||
# Flux helm-controller's default wait budget, causing remediation loops
|
||||
# that uninstall the Cluster CR. This override applied by cozystack-api
|
||||
# to the HelmRelease Spec.Install.Timeout and Spec.Upgrade.Timeout.
|
||||
release.cozystack.io/helm-install-timeout: "15m"
|
||||
spec:
|
||||
application:
|
||||
kind: Kubernetes
|
||||
|
|
|
|||
|
|
@ -160,6 +160,31 @@ func (o *CozyServerOptions) Complete() error {
|
|||
// Convert to ResourceConfig
|
||||
o.ResourceConfig = &config.ResourceConfig{}
|
||||
for _, crd := range crdList.Items {
|
||||
release := config.ReleaseConfig{
|
||||
Prefix: crd.Spec.Release.Prefix,
|
||||
Labels: crd.Spec.Release.Labels,
|
||||
ChartRef: config.ChartRefConfig{
|
||||
Kind: crd.Spec.Release.ChartRef.Kind,
|
||||
Name: crd.Spec.Release.ChartRef.Name,
|
||||
Namespace: crd.Spec.Release.ChartRef.Namespace,
|
||||
},
|
||||
}
|
||||
// Per-Application HelmRelease Install/Upgrade timeout. Applications
|
||||
// whose parent chart contains asynchronously-provisioned resources
|
||||
// the chart itself depends on (for example, the Kamaji-provisioned
|
||||
// admin-kubeconfig Secret for Kubernetes tenants) need a longer
|
||||
// wait budget than the Flux default. Consumed by the REST storage
|
||||
// layer when building the HelmRelease Spec.
|
||||
if raw, ok := crd.Annotations["release.cozystack.io/helm-install-timeout"]; ok && raw != "" {
|
||||
d, err := time.ParseDuration(raw)
|
||||
if err != nil {
|
||||
return fmt.Errorf(
|
||||
"ApplicationDefinition %q has invalid release.cozystack.io/helm-install-timeout %q: %w",
|
||||
crd.Name, raw, err,
|
||||
)
|
||||
}
|
||||
release.HelmInstallTimeout = d
|
||||
}
|
||||
resource := config.Resource{
|
||||
Application: config.ApplicationConfig{
|
||||
Kind: crd.Spec.Application.Kind,
|
||||
|
|
@ -168,15 +193,7 @@ func (o *CozyServerOptions) Complete() error {
|
|||
ShortNames: []string{}, // TODO: implement shortnames
|
||||
OpenAPISchema: crd.Spec.Application.OpenAPISchema,
|
||||
},
|
||||
Release: config.ReleaseConfig{
|
||||
Prefix: crd.Spec.Release.Prefix,
|
||||
Labels: crd.Spec.Release.Labels,
|
||||
ChartRef: config.ChartRefConfig{
|
||||
Kind: crd.Spec.Release.ChartRef.Kind,
|
||||
Name: crd.Spec.Release.ChartRef.Name,
|
||||
Namespace: crd.Spec.Release.ChartRef.Namespace,
|
||||
},
|
||||
},
|
||||
Release: release,
|
||||
}
|
||||
o.ResourceConfig.Resources = append(o.ResourceConfig.Resources, resource)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,6 +16,8 @@ limitations under the License.
|
|||
|
||||
package config
|
||||
|
||||
import "time"
|
||||
|
||||
// ResourceConfig represents the structure of the configuration file.
|
||||
type ResourceConfig struct {
|
||||
Resources []Resource `yaml:"resources"`
|
||||
|
|
@ -41,6 +43,12 @@ type ReleaseConfig struct {
|
|||
Prefix string `yaml:"prefix"`
|
||||
Labels map[string]string `yaml:"labels"`
|
||||
ChartRef ChartRefConfig `yaml:"chartRef"`
|
||||
// HelmInstallTimeout overrides the Flux HelmRelease Install.Timeout and
|
||||
// Upgrade.Timeout for this Application kind. When zero, flux defaults
|
||||
// apply. Populated from the
|
||||
// release.cozystack.io/helm-install-timeout annotation on the
|
||||
// ApplicationDefinition at start-up.
|
||||
HelmInstallTimeout time.Duration `yaml:"helmInstallTimeout,omitempty"`
|
||||
}
|
||||
|
||||
// ChartRefConfig references a Flux source artifact for the Helm chart.
|
||||
|
|
|
|||
|
|
@ -1528,16 +1528,17 @@ func (r *REST) convertApplicationToHelmRelease(app *appsv1alpha1.Application) (*
|
|||
},
|
||||
}
|
||||
|
||||
// The Kubernetes Application's parent chart creates CAPI/Kamaji resources
|
||||
// whose admin-kubeconfig Secret is provisioned asynchronously and mounted
|
||||
// by Deployments in the same chart. On a cold node, Kamaji control-plane
|
||||
// bootstrap routinely exceeds flux helm-controller's default wait budget,
|
||||
// so install remediation loops uninstall the Cluster CR and churn. Extend
|
||||
// the wait budget to 15m for Kubernetes only - other Application kinds
|
||||
// without this race keep flux defaults, so their failed installs do not
|
||||
// linger unnecessarily before remediation fires.
|
||||
if r.kindName == "Kubernetes" {
|
||||
timeout := metav1.Duration{Duration: 15 * time.Minute}
|
||||
// Per-Application HelmRelease wait budget. When an ApplicationDefinition
|
||||
// sets release.cozystack.io/helm-install-timeout, the annotation is
|
||||
// parsed at startup into ReleaseConfig.HelmInstallTimeout and applied
|
||||
// to both Install and Upgrade here. Applications that leave it unset
|
||||
// (the common case) keep flux defaults, so their failed installs
|
||||
// remediate on the normal cadence. Needed for the Kubernetes kind
|
||||
// because its parent chart contains CAPI/Kamaji resources whose
|
||||
// admin-kubeconfig Secret is provisioned asynchronously and Kamaji
|
||||
// cold-start routinely exceeds flux's default wait budget.
|
||||
if r.releaseConfig.HelmInstallTimeout > 0 {
|
||||
timeout := metav1.Duration{Duration: r.releaseConfig.HelmInstallTimeout}
|
||||
helmRelease.Spec.Install.Timeout = &timeout
|
||||
helmRelease.Spec.Upgrade.Timeout = &timeout
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ import (
|
|||
"github.com/cozystack/cozystack/pkg/config"
|
||||
)
|
||||
|
||||
func newRESTForKind(kind, prefix string) *REST {
|
||||
func newRESTForTimeout(kind, prefix string, helmInstallTimeout time.Duration) *REST {
|
||||
return &REST{
|
||||
kindName: kind,
|
||||
releaseConfig: config.ReleaseConfig{
|
||||
|
|
@ -20,76 +20,93 @@ func newRESTForKind(kind, prefix string) *REST {
|
|||
Name: "x",
|
||||
Namespace: "cozy-system",
|
||||
},
|
||||
HelmInstallTimeout: helmInstallTimeout,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func TestConvertApplicationToHelmRelease_KubernetesKindGetsLongTimeout(t *testing.T) {
|
||||
r := newRESTForKind("Kubernetes", "kubernetes-")
|
||||
app := &appsv1alpha1.Application{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "example", Namespace: "tenant-root"},
|
||||
// Table-driven: every Application kind carries a per-CRD HelmRelease wait
|
||||
// budget. The Kubernetes kind's parent chart contains CAPI/Kamaji resources
|
||||
// whose admin-kubeconfig Secret is provisioned asynchronously, so its
|
||||
// ApplicationDefinition sets release.cozystack.io/helm-install-timeout=15m
|
||||
// (or longer). Other kinds leave the annotation unset and keep flux defaults
|
||||
// so their failed installs remediate on the normal cadence. The test must
|
||||
// cover both paths: a kind with the timeout set and one without.
|
||||
func TestConvertApplicationToHelmRelease_AppliesReleaseConfigTimeout(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
kind string
|
||||
prefix string
|
||||
configured time.Duration
|
||||
wantSet bool
|
||||
}{
|
||||
{
|
||||
name: "Kubernetes kind with 15m configured gets Install and Upgrade Timeout",
|
||||
kind: "Kubernetes",
|
||||
prefix: "kubernetes-",
|
||||
configured: 15 * time.Minute,
|
||||
wantSet: true,
|
||||
},
|
||||
{
|
||||
name: "Qdrant kind without configured timeout keeps flux defaults",
|
||||
kind: "Qdrant",
|
||||
prefix: "qdrant-",
|
||||
configured: 0,
|
||||
wantSet: false,
|
||||
},
|
||||
{
|
||||
name: "arbitrary future kind with 20m configured gets 20m",
|
||||
kind: "TalosCluster",
|
||||
prefix: "talos-",
|
||||
configured: 20 * time.Minute,
|
||||
wantSet: true,
|
||||
},
|
||||
}
|
||||
|
||||
hr, err := r.convertApplicationToHelmRelease(app)
|
||||
if err != nil {
|
||||
t.Fatalf("convertApplicationToHelmRelease returned error: %v", err)
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
r := newRESTForTimeout(tc.kind, tc.prefix, tc.configured)
|
||||
app := &appsv1alpha1.Application{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "example", Namespace: "tenant-root"},
|
||||
}
|
||||
|
||||
if hr.Spec.Install == nil {
|
||||
t.Fatal("Spec.Install must not be nil")
|
||||
}
|
||||
if hr.Spec.Install.Timeout == nil {
|
||||
t.Fatal("Spec.Install.Timeout must be set for Kubernetes kind")
|
||||
}
|
||||
if hr.Spec.Install.Timeout.Duration < 15*time.Minute {
|
||||
t.Errorf("Spec.Install.Timeout must be >= 15m for Kubernetes, got %v", hr.Spec.Install.Timeout.Duration)
|
||||
}
|
||||
hr, err := r.convertApplicationToHelmRelease(app)
|
||||
if err != nil {
|
||||
t.Fatalf("convertApplicationToHelmRelease returned error: %v", err)
|
||||
}
|
||||
|
||||
if hr.Spec.Upgrade == nil {
|
||||
t.Fatal("Spec.Upgrade must not be nil")
|
||||
}
|
||||
if hr.Spec.Upgrade.Timeout == nil {
|
||||
t.Fatal("Spec.Upgrade.Timeout must be set for Kubernetes kind")
|
||||
}
|
||||
if hr.Spec.Upgrade.Timeout.Duration < 15*time.Minute {
|
||||
t.Errorf("Spec.Upgrade.Timeout must be >= 15m for Kubernetes, got %v", hr.Spec.Upgrade.Timeout.Duration)
|
||||
}
|
||||
if hr.Spec.Install == nil || hr.Spec.Upgrade == nil {
|
||||
t.Fatalf("Spec.Install/Upgrade must be non-nil")
|
||||
}
|
||||
|
||||
if hr.Spec.Install.Remediation == nil || hr.Spec.Install.Remediation.Retries != -1 {
|
||||
t.Errorf("Spec.Install.Remediation.Retries must remain -1, got %+v", hr.Spec.Install.Remediation)
|
||||
}
|
||||
if hr.Spec.Upgrade.Remediation == nil || hr.Spec.Upgrade.Remediation.Retries != -1 {
|
||||
t.Errorf("Spec.Upgrade.Remediation.Retries must remain -1, got %+v", hr.Spec.Upgrade.Remediation)
|
||||
}
|
||||
}
|
||||
|
||||
func TestConvertApplicationToHelmRelease_NonKubernetesKindKeepsFluxDefaults(t *testing.T) {
|
||||
// For Applications whose parent chart has no admin-kubeconfig race
|
||||
// (Qdrant, MongoDB, Postgres, etc.), do NOT extend the helm-wait
|
||||
// budget - otherwise failed installs would block three times as long
|
||||
// before Flux starts remediating.
|
||||
r := newRESTForKind("Qdrant", "qdrant-")
|
||||
app := &appsv1alpha1.Application{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "example", Namespace: "tenant-root"},
|
||||
}
|
||||
|
||||
hr, err := r.convertApplicationToHelmRelease(app)
|
||||
if err != nil {
|
||||
t.Fatalf("convertApplicationToHelmRelease returned error: %v", err)
|
||||
}
|
||||
|
||||
if hr.Spec.Install != nil && hr.Spec.Install.Timeout != nil {
|
||||
t.Errorf("Spec.Install.Timeout must be unset for non-Kubernetes kinds, got %v", hr.Spec.Install.Timeout.Duration)
|
||||
}
|
||||
if hr.Spec.Upgrade != nil && hr.Spec.Upgrade.Timeout != nil {
|
||||
t.Errorf("Spec.Upgrade.Timeout must be unset for non-Kubernetes kinds, got %v", hr.Spec.Upgrade.Timeout.Duration)
|
||||
}
|
||||
|
||||
// But remediation must still be -1 across the board.
|
||||
if hr.Spec.Install.Remediation == nil || hr.Spec.Install.Remediation.Retries != -1 {
|
||||
t.Errorf("Spec.Install.Remediation.Retries must remain -1, got %+v", hr.Spec.Install.Remediation)
|
||||
}
|
||||
if hr.Spec.Upgrade.Remediation == nil || hr.Spec.Upgrade.Remediation.Retries != -1 {
|
||||
t.Errorf("Spec.Upgrade.Remediation.Retries must remain -1, got %+v", hr.Spec.Upgrade.Remediation)
|
||||
if tc.wantSet {
|
||||
if hr.Spec.Install.Timeout == nil {
|
||||
t.Fatalf("Spec.Install.Timeout must be set when HelmInstallTimeout=%v", tc.configured)
|
||||
}
|
||||
if hr.Spec.Install.Timeout.Duration != tc.configured {
|
||||
t.Errorf("Spec.Install.Timeout = %v, want %v", hr.Spec.Install.Timeout.Duration, tc.configured)
|
||||
}
|
||||
if hr.Spec.Upgrade.Timeout == nil {
|
||||
t.Fatalf("Spec.Upgrade.Timeout must be set when HelmInstallTimeout=%v", tc.configured)
|
||||
}
|
||||
if hr.Spec.Upgrade.Timeout.Duration != tc.configured {
|
||||
t.Errorf("Spec.Upgrade.Timeout = %v, want %v", hr.Spec.Upgrade.Timeout.Duration, tc.configured)
|
||||
}
|
||||
} else {
|
||||
if hr.Spec.Install.Timeout != nil {
|
||||
t.Errorf("Spec.Install.Timeout must be nil when HelmInstallTimeout=0, got %v", hr.Spec.Install.Timeout.Duration)
|
||||
}
|
||||
if hr.Spec.Upgrade.Timeout != nil {
|
||||
t.Errorf("Spec.Upgrade.Timeout must be nil when HelmInstallTimeout=0, got %v", hr.Spec.Upgrade.Timeout.Duration)
|
||||
}
|
||||
}
|
||||
|
||||
if hr.Spec.Install.Remediation == nil || hr.Spec.Install.Remediation.Retries != -1 {
|
||||
t.Errorf("Spec.Install.Remediation.Retries must remain -1, got %+v", hr.Spec.Install.Remediation)
|
||||
}
|
||||
if hr.Spec.Upgrade.Remediation == nil || hr.Spec.Upgrade.Remediation.Retries != -1 {
|
||||
t.Errorf("Spec.Upgrade.Remediation.Retries must remain -1, got %+v", hr.Spec.Upgrade.Remediation)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue