Skip to content

Commit ca00511

Browse files
myasnikovdaniilmyasnikovdaniil
committed
feat(api): add helm-upgrade-timeout annotation for asymmetric budgets
release.cozystack.io/helm-install-timeout sets both Install.Timeout and Upgrade.Timeout to one value. Add release.cozystack.io/helm-upgrade-timeout so an ApplicationDefinition can override only Upgrade.Timeout — letting a kind keep a short install budget but a longer upgrade budget (or vice versa). When set it wins over the value the install annotation would otherwise apply to the upgrade side. - config: add HelmUpgradeTimeoutAnnotation + ReleaseConfig.HelmUpgradeTimeout; generalize ParseHelmInstallTimeoutAnnotation to ParseHelmTimeoutAnnotation since it now parses both annotations (identical Flux unit validation). - server: parse the new annotation at startup with the same loud-fail behavior; update --helmrelease-upgrade-timeout help. - rest: apply HelmUpgradeTimeout after the install-annotation copy so the upgrade-specific value wins; restore the accurate carrier comment. - tests: cover the upgrade-only override, the asymmetric precedence, and the install-sets-both regression guard. Behavior is unchanged for every existing ApplicationDefinition (none set the new annotation today). Address review feedback from coderabbitai on rest.go. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 896debc commit ca00511

5 files changed

Lines changed: 113 additions & 18 deletions

File tree

‎pkg/cmd/server/start.go‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,10 @@ func NewCommandStartCozyServer(ctx context.Context, defaults *CozyServerOptions)
137137
"same annotation also overrides --helmrelease-upgrade-timeout.")
138138
flags.StringVar(&o.HelmReleaseUpgradeTimeout, "helmrelease-upgrade-timeout", o.HelmReleaseUpgradeTimeout,
139139
"Default timeout for the Helm upgrade action of HelmReleases generated from Application "+
140-
"resources (Spec.Upgrade.Timeout). Overridden per-Application by the same "+
141-
"release.cozystack.io/helm-install-timeout annotation as --helmrelease-install-timeout: "+
142-
"one annotation overrides both install and upgrade timeouts together.")
140+
"resources (Spec.Upgrade.Timeout). Overridden per-Application by the "+
141+
"release.cozystack.io/helm-install-timeout annotation (which sets both install and "+
142+
"upgrade), or by release.cozystack.io/helm-upgrade-timeout to override only the upgrade "+
143+
"side for a kind that needs an asymmetric budget.")
143144
flags.IntVar(&o.HelmReleaseMaxHistory, "helmrelease-max-history", o.HelmReleaseMaxHistory,
144145
"Number of release revisions Helm keeps for HelmReleases generated from Application "+
145146
"resources (Spec.MaxHistory). 0 means unlimited; 5 matches Helm's default.")
@@ -273,8 +274,10 @@ func (o *CozyServerOptions) Complete() error {
273274
// layer when building the HelmRelease Spec. The parser rejects
274275
// units Flux would reject at webhook time, so a bad annotation
275276
// surfaces as a loud startup failure instead of a silent drop to
276-
// defaults.
277-
d, err := config.ParseHelmInstallTimeoutAnnotation(
277+
// defaults. helm-install-timeout covers both install and upgrade;
278+
// helm-upgrade-timeout overrides only the upgrade side for kinds
279+
// that need an asymmetric budget.
280+
installTimeout, err := config.ParseHelmTimeoutAnnotation(
278281
crd.Annotations[config.HelmInstallTimeoutAnnotation],
279282
)
280283
if err != nil {
@@ -283,7 +286,17 @@ func (o *CozyServerOptions) Complete() error {
283286
crd.Name, config.HelmInstallTimeoutAnnotation, err,
284287
)
285288
}
286-
release.HelmInstallTimeout = d
289+
release.HelmInstallTimeout = installTimeout
290+
upgradeTimeout, err := config.ParseHelmTimeoutAnnotation(
291+
crd.Annotations[config.HelmUpgradeTimeoutAnnotation],
292+
)
293+
if err != nil {
294+
return fmt.Errorf(
295+
"ApplicationDefinition %q has invalid %s annotation: %w",
296+
crd.Name, config.HelmUpgradeTimeoutAnnotation, err,
297+
)
298+
}
299+
release.HelmUpgradeTimeout = upgradeTimeout
287300
resource := config.Resource{
288301
Application: config.ApplicationConfig{
289302
Kind: crd.Spec.Application.Kind,

‎pkg/config/config.go‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ import (
2727
// Upgrade.Timeout for a given Application kind.
2828
const HelmInstallTimeoutAnnotation = "release.cozystack.io/helm-install-timeout"
2929

30+
// HelmUpgradeTimeoutAnnotation is the ApplicationDefinition metadata
31+
// annotation key that overrides only the Flux HelmRelease Upgrade.Timeout
32+
// for a given Application kind. When set it takes precedence over the
33+
// Upgrade.Timeout value HelmInstallTimeoutAnnotation would otherwise apply,
34+
// letting a kind keep a short install budget but a longer upgrade budget
35+
// (or vice versa).
36+
const HelmUpgradeTimeoutAnnotation = "release.cozystack.io/helm-upgrade-timeout"
37+
3038
// helmTimeoutPattern mirrors the CRD validation pattern used by Flux
3139
// helm-controller on HelmReleaseSpec.Install.Timeout (ms/s/m/h units only).
3240
// time.ParseDuration accepts ns/us/µs, but Flux rejects them - parsing here
@@ -36,14 +44,15 @@ const HelmInstallTimeoutAnnotation = "release.cozystack.io/helm-install-timeout"
3644
// in the go module cache.
3745
var helmTimeoutPattern = regexp.MustCompile(`^([0-9]+(\.[0-9]+)?(ms|s|m|h))+$`)
3846

39-
// ParseHelmInstallTimeoutAnnotation parses the value of the
40-
// release.cozystack.io/helm-install-timeout annotation. The empty string is
41-
// treated as "unset" and returns (0, nil) so callers can leave
42-
// HelmInstallTimeout zeroed and let flux defaults apply. Values accepted by
47+
// ParseHelmTimeoutAnnotation parses the value of a Flux HelmRelease timeout
48+
// annotation (release.cozystack.io/helm-install-timeout or
49+
// release.cozystack.io/helm-upgrade-timeout). The empty string is treated as
50+
// "unset" and returns (0, nil) so callers can leave the corresponding
51+
// ReleaseConfig field zeroed and let flux defaults apply. Values accepted by
4352
// time.ParseDuration but rejected by Flux (ns/us/µs) return a helpful
4453
// error instead of silently parsing and failing later at HelmRelease
4554
// admission.
46-
func ParseHelmInstallTimeoutAnnotation(raw string) (time.Duration, error) {
55+
func ParseHelmTimeoutAnnotation(raw string) (time.Duration, error) {
4756
if raw == "" {
4857
return 0, nil
4958
}
@@ -106,6 +115,13 @@ type ReleaseConfig struct {
106115
// Populated from the release.cozystack.io/helm-install-timeout
107116
// annotation on the ApplicationDefinition at start-up.
108117
HelmInstallTimeout time.Duration `yaml:"helmInstallTimeout,omitempty"`
118+
// HelmUpgradeTimeout is a per-Application override of Upgrade.Timeout
119+
// only. When non-zero, it wins over both HelmReleaseUpgradeTimeout and
120+
// the Upgrade.Timeout value HelmInstallTimeout would otherwise apply,
121+
// so a kind can carry an asymmetric install/upgrade budget. Populated
122+
// from the release.cozystack.io/helm-upgrade-timeout annotation on the
123+
// ApplicationDefinition at start-up.
124+
HelmUpgradeTimeout time.Duration `yaml:"helmUpgradeTimeout,omitempty"`
109125
// HelmReleaseInterval is the global default for Spec.Interval on
110126
// HelmReleases generated by cozystack-api. Set from the api server
111127
// --helmrelease-interval flag; mirrors the cozystack-operator flag of

‎pkg/config/config_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
// - compound forms (the CRD pattern accepts repeats),
1616
// - units time.ParseDuration accepts but Flux rejects (ns, us, µs),
1717
// - outright garbage.
18-
func TestParseHelmInstallTimeoutAnnotation(t *testing.T) {
18+
func TestParseHelmTimeoutAnnotation(t *testing.T) {
1919
cases := []struct {
2020
name string
2121
input string
@@ -98,7 +98,7 @@ func TestParseHelmInstallTimeoutAnnotation(t *testing.T) {
9898

9999
for _, tc := range cases {
100100
t.Run(tc.name, func(t *testing.T) {
101-
got, err := ParseHelmInstallTimeoutAnnotation(tc.input)
101+
got, err := ParseHelmTimeoutAnnotation(tc.input)
102102
if tc.wantErr {
103103
if err == nil {
104104
t.Fatalf("expected error, got duration=%v", got)

‎pkg/registry/apps/application/rest.go‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1504,11 +1504,14 @@ func latestMonitorTime(m *cozyv1alpha1.WorkloadMonitor) metav1.Time {
15041504
// from Spec.Interval — the previous coupling caused failed installs to
15051505
// retry only every 5m, exceeding E2E budgets.
15061506
func (r *REST) convertApplicationToHelmRelease(app *appsv1alpha1.Application) (*helmv2.HelmRelease, error) {
1507-
// Per-Application annotation override (HelmInstallTimeout, populated
1508-
// from release.cozystack.io/helm-install-timeout on the
1509-
// ApplicationDefinition at startup) wins over the global defaults
1510-
// (HelmReleaseInstallTimeout / HelmReleaseUpgradeTimeout).
1511-
// kubernetes-rd and tenant-rd carry the annotation today: the
1507+
// Per-Application annotation overrides win over the global defaults
1508+
// (HelmReleaseInstallTimeout / HelmReleaseUpgradeTimeout):
1509+
// - HelmInstallTimeout (release.cozystack.io/helm-install-timeout)
1510+
// sets both Install.Timeout and Upgrade.Timeout;
1511+
// - HelmUpgradeTimeout (release.cozystack.io/helm-upgrade-timeout)
1512+
// then overrides only Upgrade.Timeout, so a kind can carry an
1513+
// asymmetric budget (short install, long upgrade or vice versa).
1514+
// kubernetes-rd and tenant-rd carry helm-install-timeout today: the
15121515
// Kubernetes Application's parent chart contains CAPI/Kamaji
15131516
// resources whose admin-kubeconfig Secret is provisioned
15141517
// asynchronously and Kamaji cold-start routinely exceeds flux's
@@ -1522,6 +1525,9 @@ func (r *REST) convertApplicationToHelmRelease(app *appsv1alpha1.Application) (*
15221525
installTimeout = r.releaseConfig.HelmInstallTimeout
15231526
upgradeTimeout = r.releaseConfig.HelmInstallTimeout
15241527
}
1528+
if r.releaseConfig.HelmUpgradeTimeout > 0 {
1529+
upgradeTimeout = r.releaseConfig.HelmUpgradeTimeout
1530+
}
15251531

15261532
maxHistory := r.releaseConfig.HelmReleaseMaxHistory
15271533
helmRelease := &helmv2.HelmRelease{

‎pkg/registry/apps/application/rest_timeout_test.go‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,3 +136,63 @@ func TestConvertApplicationToHelmRelease_AppliesReleaseConfigTimeout(t *testing.
136136
})
137137
}
138138
}
139+
140+
// HelmUpgradeTimeout (the per-Application release.cozystack.io/helm-upgrade-timeout
141+
// annotation override) overrides only Upgrade.Timeout, and wins over the
142+
// Upgrade.Timeout value HelmInstallTimeout would otherwise apply. This lets a
143+
// kind carry an asymmetric budget — e.g. a short install but a long upgrade.
144+
func TestConvertApplicationToHelmRelease_UpgradeTimeoutAnnotation(t *testing.T) {
145+
cases := []struct {
146+
name string
147+
installTimeout time.Duration // helm-install-timeout annotation
148+
upgradeTimeout time.Duration // helm-upgrade-timeout annotation
149+
wantInstall time.Duration
150+
wantUpgrade time.Duration
151+
}{
152+
{
153+
name: "upgrade override alone leaves install on the global default",
154+
installTimeout: 0,
155+
upgradeTimeout: 20 * time.Minute,
156+
wantInstall: testGlobalInstallTimeout,
157+
wantUpgrade: 20 * time.Minute,
158+
},
159+
{
160+
name: "upgrade override wins over the install-annotation copy",
161+
installTimeout: 15 * time.Minute,
162+
upgradeTimeout: 25 * time.Minute,
163+
wantInstall: 15 * time.Minute,
164+
wantUpgrade: 25 * time.Minute,
165+
},
166+
{
167+
name: "install override alone still sets both (regression guard)",
168+
installTimeout: 15 * time.Minute,
169+
upgradeTimeout: 0,
170+
wantInstall: 15 * time.Minute,
171+
wantUpgrade: 15 * time.Minute,
172+
},
173+
}
174+
175+
for _, tc := range cases {
176+
t.Run(tc.name, func(t *testing.T) {
177+
r := newRESTForTimeout("Example", "example-", tc.installTimeout)
178+
r.releaseConfig.HelmUpgradeTimeout = tc.upgradeTimeout
179+
app := &appsv1alpha1.Application{
180+
ObjectMeta: metav1.ObjectMeta{Name: "example", Namespace: "tenant-root"},
181+
}
182+
183+
hr, err := r.convertApplicationToHelmRelease(app)
184+
if err != nil {
185+
t.Fatalf("convertApplicationToHelmRelease returned error: %v", err)
186+
}
187+
if hr.Spec.Install == nil || hr.Spec.Upgrade == nil {
188+
t.Fatalf("Spec.Install/Upgrade must be non-nil")
189+
}
190+
if hr.Spec.Install.Timeout == nil || hr.Spec.Install.Timeout.Duration != tc.wantInstall {
191+
t.Errorf("Spec.Install.Timeout = %v, want %v", hr.Spec.Install.Timeout, tc.wantInstall)
192+
}
193+
if hr.Spec.Upgrade.Timeout == nil || hr.Spec.Upgrade.Timeout.Duration != tc.wantUpgrade {
194+
t.Errorf("Spec.Upgrade.Timeout = %v, want %v", hr.Spec.Upgrade.Timeout, tc.wantUpgrade)
195+
}
196+
})
197+
}
198+
}

0 commit comments

Comments
 (0)