feat(spec): Task 11 - warning system for misconfigured annotations

Add two new helper functions to pkg/action/sequencing.go:
- warnIfPartialReadinessAnnotations: emits slog.Warn when a resource has
  only one of readiness-success/readiness-failure (both required)
- warnIfIsolatedGroups: emits slog.Warn when multiple groups exist but
  some have no connections to other groups (possible misconfiguration)

Both functions are called from deployResourceGroupBatches. The existing
slog.Warn for non-existent group references (from ParseResourceGroups)
was already present.
pull/31992/head
caretak3r 8 months ago
parent 3a3280d1e8
commit ab8097799e

@ -164,6 +164,56 @@ func (s *sequencedDeployment) deployChartLevel(ctx context.Context, chrt *chartv
return nil
}
// warnIfPartialReadinessAnnotations emits a slog.Warn for each resource that has
// only one of the helm.sh/readiness-success / helm.sh/readiness-failure annotations.
// Both annotations must be present for custom readiness evaluation to work; a resource
// with only one will fall back to kstatus.
func warnIfPartialReadinessAnnotations(manifests []releaseutil.Manifest) {
for _, m := range manifests {
if m.Head == nil || m.Head.Metadata == nil {
continue
}
ann := m.Head.Metadata.Annotations
_, hasSuccess := ann[kube.AnnotationReadinessSuccess]
_, hasFailure := ann[kube.AnnotationReadinessFailure]
if hasSuccess != hasFailure {
slog.Warn("resource has only one readiness annotation; both helm.sh/readiness-success and helm.sh/readiness-failure must be present for custom readiness evaluation; falling back to kstatus",
"resource", m.Head.Metadata.Name,
)
}
}
}
// warnIfIsolatedGroups emits a slog.Warn for each resource-group that has no
// connections to other groups (no depends-on edges and not depended on by any
// other group). Isolated groups are valid but may indicate a misconfiguration
// when multiple groups are present.
func warnIfIsolatedGroups(result releaseutil.ResourceGroupResult) {
if len(result.Groups) <= 1 {
return // A single group is not isolated in a meaningful sense
}
for groupName := range result.Groups {
deps := result.GroupDeps[groupName]
isDependedOn := false
for _, otherDeps := range result.GroupDeps {
for _, d := range otherDeps {
if d == groupName {
isDependedOn = true
break
}
}
if isDependedOn {
break
}
}
if len(deps) == 0 && !isDependedOn {
slog.Warn("resource-group is isolated with no connections to other groups; if sequencing is intended, add helm.sh/depends-on/resource-groups annotation to related groups",
"group", groupName,
)
}
}
}
// deployResourceGroupBatches deploys manifests for a single chart level using
// resource-group annotation DAG ordering. Resources without group annotations
// (or with invalid ones) are deployed last.
@ -172,6 +222,9 @@ func (s *sequencedDeployment) deployResourceGroupBatches(ctx context.Context, ma
return nil
}
// Emit warnings for misconfigured annotations before deploying.
warnIfPartialReadinessAnnotations(manifests)
result, warnings := releaseutil.ParseResourceGroups(manifests)
for _, w := range warnings {
slog.Warn("resource-group annotation warning", "warning", w)
@ -179,6 +232,9 @@ func (s *sequencedDeployment) deployResourceGroupBatches(ctx context.Context, ma
// If there are sequenced groups, build their DAG and deploy in order
if len(result.Groups) > 0 {
// Warn about groups that have no connections to other groups.
warnIfIsolatedGroups(result)
dag, err := releaseutil.BuildResourceGroupDAG(result)
if err != nil {
return fmt.Errorf("building resource-group DAG: %w", err)

@ -17,7 +17,10 @@ limitations under the License.
package action
import (
"bytes"
"context"
"log/slog"
"strings"
"testing"
"time"
@ -218,3 +221,113 @@ func manifestNames(ms []releaseutil.Manifest) []string {
}
return names
}
// captureWarnings redirects the default slog logger to a buffer for the duration
// of the test and returns a function to retrieve the captured output.
func captureWarnings(t *testing.T) *bytes.Buffer {
t.Helper()
var buf bytes.Buffer
handler := slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn})
oldLogger := slog.Default()
slog.SetDefault(slog.New(handler))
t.Cleanup(func() { slog.SetDefault(oldLogger) })
return &buf
}
func TestWarnIfPartialReadinessAnnotations_OnlySuccess(t *testing.T) {
buf := captureWarnings(t)
manifests := []releaseutil.Manifest{
makeTestManifest("cm", "chart/templates/cm.yaml", map[string]string{
kube.AnnotationReadinessSuccess: "{.status.ready} == true",
}),
}
warnIfPartialReadinessAnnotations(manifests)
if !strings.Contains(buf.String(), "readiness") {
t.Errorf("expected warning about partial readiness annotation, got: %q", buf.String())
}
}
func TestWarnIfPartialReadinessAnnotations_OnlyFailure(t *testing.T) {
buf := captureWarnings(t)
manifests := []releaseutil.Manifest{
makeTestManifest("cm", "chart/templates/cm.yaml", map[string]string{
kube.AnnotationReadinessFailure: "{.status.failed} == true",
}),
}
warnIfPartialReadinessAnnotations(manifests)
if !strings.Contains(buf.String(), "readiness") {
t.Errorf("expected warning about partial readiness annotation, got: %q", buf.String())
}
}
func TestWarnIfPartialReadinessAnnotations_BothPresent(t *testing.T) {
buf := captureWarnings(t)
manifests := []releaseutil.Manifest{
makeTestManifest("cm", "chart/templates/cm.yaml", map[string]string{
kube.AnnotationReadinessSuccess: "{.status.ready} == true",
kube.AnnotationReadinessFailure: "{.status.failed} == true",
}),
}
warnIfPartialReadinessAnnotations(manifests)
if buf.Len() > 0 {
t.Errorf("expected no warning when both annotations present, got: %q", buf.String())
}
}
func TestWarnIfPartialReadinessAnnotations_NeitherPresent(t *testing.T) {
buf := captureWarnings(t)
manifests := []releaseutil.Manifest{
makeTestManifest("cm", "chart/templates/cm.yaml", nil),
}
warnIfPartialReadinessAnnotations(manifests)
if buf.Len() > 0 {
t.Errorf("expected no warning when neither annotation present, got: %q", buf.String())
}
}
func TestWarnIfIsolatedGroups_TwoGroupsNoConnections(t *testing.T) {
buf := captureWarnings(t)
result := releaseutil.ResourceGroupResult{
Groups: map[string][]releaseutil.Manifest{
"groupA": {makeTestManifest("cmA", "chart/templates/cmA.yaml", nil)},
"groupB": {makeTestManifest("cmB", "chart/templates/cmB.yaml", nil)},
},
GroupDeps: map[string][]string{},
}
warnIfIsolatedGroups(result)
output := buf.String()
if !strings.Contains(output, "isolated") {
t.Errorf("expected warning about isolated groups, got: %q", output)
}
}
func TestWarnIfIsolatedGroups_ConnectedGroups(t *testing.T) {
buf := captureWarnings(t)
result := releaseutil.ResourceGroupResult{
Groups: map[string][]releaseutil.Manifest{
"groupA": {makeTestManifest("cmA", "chart/templates/cmA.yaml", nil)},
"groupB": {makeTestManifest("cmB", "chart/templates/cmB.yaml", nil)},
},
GroupDeps: map[string][]string{
"groupB": {"groupA"}, // groupB depends on groupA
},
}
warnIfIsolatedGroups(result)
if buf.Len() > 0 {
t.Errorf("expected no warning when groups are connected, got: %q", buf.String())
}
}
func TestWarnIfIsolatedGroups_SingleGroup(t *testing.T) {
buf := captureWarnings(t)
result := releaseutil.ResourceGroupResult{
Groups: map[string][]releaseutil.Manifest{
"groupA": {makeTestManifest("cmA", "chart/templates/cmA.yaml", nil)},
},
GroupDeps: map[string][]string{},
}
warnIfIsolatedGroups(result)
if buf.Len() > 0 {
t.Errorf("expected no warning for single group, got: %q", buf.String())
}
}

Loading…
Cancel
Save