From 9f323b5230747a9c33d8b8cc7e989b0aefa04d26 Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Mon, 21 Sep 2026 10:03:14 +0100 Subject: [PATCH] PR comment fixes - Add comment to explain sql driver update behaviour when it comes to updating custom labels - Apply custom labels in memory driver Signed-off-by: Evans Mungai --- internal/storage/driver/records.go | 4 ++++ internal/storage/driver/sql.go | 14 ++++++++++++++ pkg/storage/driver/records.go | 4 ++++ pkg/storage/driver/sql.go | 14 ++++++++++++++ 4 files changed, 36 insertions(+) diff --git a/internal/storage/driver/records.go b/internal/storage/driver/records.go index f154d5bca..99368e263 100644 --- a/internal/storage/driver/records.go +++ b/internal/storage/driver/records.go @@ -114,6 +114,10 @@ func newRecord(key string, rls *rspb.Release) *record { var lbs labels lbs.init() + + // apply custom labels + lbs.fromMap(rls.Labels) + lbs.set("name", rls.Name) lbs.set("owner", "helm") lbs.set("status", rls.Info.Status.String()) diff --git a/internal/storage/driver/sql.go b/internal/storage/driver/sql.go index 6cb6725d7..653507ba9 100644 --- a/internal/storage/driver/sql.go +++ b/internal/storage/driver/sql.go @@ -576,6 +576,20 @@ func (s *SQL) Create(key string, rel release.Releaser) error { } // Update updates a release. +// +// Custom labels on an existing revision are meant to be preserved. A release's +// labels are set by Create, and a revision keeps the labels it was created with +// once it is superseded, rather than picking up the labels of the upgrade that +// superseded it. TestUpgradeRelease_Labels in pkg/action is what asserts this. +// +// The drivers arrive at that from opposite directions. Here labels live in a +// separate table written only by Create, so an update leaves them untouched. The +// configmaps, memory and secrets drivers instead store labels on the record +// itself and replace the record wholesale on update, so they have to re-apply +// the labels every time or the update would discard all of them. A side effect +// is that those drivers persist a label change where this driver silently would +// not. No caller changes labels between Create and Update, so the two agree in +// practice. func (s *SQL) Update(key string, rel release.Releaser) error { rls, err := releaserToV1Release(rel) if err != nil { diff --git a/pkg/storage/driver/records.go b/pkg/storage/driver/records.go index f78b76b8b..3393bb603 100644 --- a/pkg/storage/driver/records.go +++ b/pkg/storage/driver/records.go @@ -114,6 +114,10 @@ func newRecord(key string, rls *rspb.Release) *record { var lbs labels lbs.init() + + // apply custom labels + lbs.fromMap(rls.Labels) + lbs.set("name", rls.Name) lbs.set("owner", "helm") lbs.set("status", rls.Info.Status.String()) diff --git a/pkg/storage/driver/sql.go b/pkg/storage/driver/sql.go index 6602f2ee3..2b278f7cb 100644 --- a/pkg/storage/driver/sql.go +++ b/pkg/storage/driver/sql.go @@ -576,6 +576,20 @@ func (s *SQL) Create(key string, rel release.Releaser) error { } // Update updates a release. +// +// Custom labels on an existing revision are meant to be preserved. A release's +// labels are set by Create, and a revision keeps the labels it was created with +// once it is superseded, rather than picking up the labels of the upgrade that +// superseded it. TestUpgradeRelease_Labels in pkg/action is what asserts this. +// +// The drivers arrive at that from opposite directions. Here labels live in a +// separate table written only by Create, so an update leaves them untouched. The +// configmaps, memory and secrets drivers instead store labels on the record +// itself and replace the record wholesale on update, so they have to re-apply +// the labels every time or the update would discard all of them. A side effect +// is that those drivers persist a label change where this driver silently would +// not. No caller changes labels between Create and Update, so the two agree in +// practice. func (s *SQL) Update(key string, rel release.Releaser) error { rls, err := releaserToV1Release(rel) if err != nil {