Improve ManagementCommand status conditions

api/v1/conditions.go

@@ -24,6 +24,7 @@ const (
 	ReasonCriteriaNotMatched      = "CriteriaNotMatched"
 	ReasonCriteriaExpressionError = "CriteriaExpressionError"
 	ReasonCriteriaMatched         = "CriteriaMatched"
+	ReasonProgressing             = "InProgress"
 )
 
 // ConditionedObject defines interface for objects that can have conditions

api/v1/criteria_conditions.go

@@ -0,0 +1,103 @@
+package v1
+
+import (
+	"fmt"
+	"sort"
+	"strings"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// ConditionedResource is a ConditionedObject that also exposes generation tracking for status updates.
+// +k8s:deepcopy-gen=false
+type ConditionedResource interface {
+	ConditionedObject
+	GetGeneration() int64
+}
+
+// FormatClusterLabels returns a stable, human-readable summary of cluster labels for status messages.
+// Keys are sorted to make comparisons deterministic.
+func FormatClusterLabels(labels map[string]string) string {
+	if len(labels) == 0 {
+		return "(none)"
+	}
+	keys := make([]string, 0, len(labels))
+	for k := range labels {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	parts := make([]string, 0, len(keys))
+	for _, k := range keys {
+		parts = append(parts, fmt.Sprintf("%s=%s", k, labels[k]))
+	}
+	return strings.Join(parts, ", ")
+}
+
+// NewCriteriaCondition constructs a metav1.Condition encapsulating the outcome of criteria expression evaluation.
+// The condition uses the shared Reason constants defined in conditions.go for consistent signaling across CRDs.
+func NewCriteriaCondition(conditionType string, generation int64, matched bool, expression string, evaluationErr error, clusterLabels map[string]string) metav1.Condition {
+	labelSummary := FormatClusterLabels(clusterLabels)
+
+	condition := metav1.Condition{
+		Type:               conditionType,
+		ObservedGeneration: generation,
+		LastTransitionTime: metav1.Now(),
+	}
+
+	switch {
+	case expression == "":
+		condition.Status = metav1.ConditionTrue
+		condition.Reason = ReasonCriteriaMatched
+		condition.Message = "No criteria expression configured; defaulting to match"
+	case evaluationErr != nil:
+		condition.Status = metav1.ConditionFalse
+		condition.Reason = ReasonCriteriaExpressionError
+		condition.Message = fmt.Sprintf("Criteria expression %q failed: %v (cluster labels: %s)", expression, evaluationErr, labelSummary)
+	case matched:
+		condition.Status = metav1.ConditionTrue
+		condition.Reason = ReasonCriteriaMatched
+		condition.Message = fmt.Sprintf("Criteria expression %q matched cluster labels: %s", expression, labelSummary)
+	default:
+		condition.Status = metav1.ConditionFalse
+		condition.Reason = ReasonCriteriaNotMatched
+		condition.Message = fmt.Sprintf("Criteria expression %q evaluated to false for cluster labels: %s", expression, labelSummary)
+	}
+
+	return condition
+}
+
+// NewOwnerCondition constructs a metav1.Condition for owner-type signaling (execution success/failure) with sensible defaults.
+func NewOwnerCondition(conditionType string, generation int64, status metav1.ConditionStatus, reason, message string) metav1.Condition {
+	condition := metav1.Condition{
+		Type:               conditionType,
+		Status:             status,
+		Message:            message,
+		ObservedGeneration: generation,
+		LastTransitionTime: metav1.Now(),
+	}
+
+	if reason != "" {
+		condition.Reason = reason
+	} else {
+		condition.Reason = defaultOwnerReason(status)
+	}
+
+	return condition
+}
+
+// SetOwnerCondition applies an owner-style condition to the provided resource using the supplied parameters.
+func SetOwnerCondition(resource ConditionedResource, conditionType string, status metav1.ConditionStatus, reason, message string) {
+	condition := NewOwnerCondition(conditionType, resource.GetGeneration(), status, reason, message)
+	resource.SetCondition(condition)
+}
+
+func defaultOwnerReason(status metav1.ConditionStatus) string {
+	switch status {
+	case metav1.ConditionTrue:
+		return ConditionCompleted
+	case metav1.ConditionFalse:
+		return ConditionFailed
+	default:
+		return ReasonProgressing
+	}
+}

api/v1/criteria_conditions_test.go

@@ -0,0 +1,57 @@
+package v1
+
+import (
+	"errors"
+	"testing"
+
+	meta "k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestFormatClusterLabels(t *testing.T) {
+	labels := map[string]string{"environment": "prod", "region": "eastus"}
+	formatted := FormatClusterLabels(labels)
+	require.Equal(t, "environment=prod, region=eastus", formatted)
+
+	require.Equal(t, "(none)", FormatClusterLabels(nil))
+}
+
+func TestNewCriteriaCondition(t *testing.T) {
+	labels := map[string]string{"region": "westus"}
+
+	cond := NewCriteriaCondition("test", 7, true, "region == 'westus'", nil, labels)
+	require.Equal(t, metav1.ConditionTrue, cond.Status)
+	require.Equal(t, ReasonCriteriaMatched, cond.Reason)
+	require.Contains(t, cond.Message, "region == 'westus'")
+	require.Equal(t, int64(7), cond.ObservedGeneration)
+
+	cond = NewCriteriaCondition("test", 7, false, "region == 'westus'", nil, labels)
+	require.Equal(t, metav1.ConditionFalse, cond.Status)
+	require.Equal(t, ReasonCriteriaNotMatched, cond.Reason)
+
+	cond = NewCriteriaCondition("test", 7, false, "", nil, labels)
+	require.Equal(t, metav1.ConditionTrue, cond.Status)
+	require.Equal(t, ReasonCriteriaMatched, cond.Reason)
+	require.Equal(t, "No criteria expression configured; defaulting to match", cond.Message)
+
+	cond = NewCriteriaCondition("test", 7, false, "region == 'westus'", errors.New("parse"), labels)
+	require.Equal(t, metav1.ConditionFalse, cond.Status)
+	require.Equal(t, ReasonCriteriaExpressionError, cond.Reason)
+	require.Contains(t, cond.Message, "parse")
+}
+
+func TestSetOwnerConditionHelper(t *testing.T) {
+	fn := &Function{}
+	fn.Generation = 3
+
+	SetOwnerCondition(fn, "demo", metav1.ConditionTrue, "", "completed")
+
+	cond := meta.FindStatusCondition(fn.Status.Conditions, "demo")
+	require.NotNil(t, cond)
+	require.Equal(t, metav1.ConditionTrue, cond.Status)
+	require.Equal(t, ConditionCompleted, cond.Reason)
+	require.Equal(t, "completed", cond.Message)
+	require.Equal(t, int64(3), cond.ObservedGeneration)
+}

api/v1/function_types.go

@@ -18,7 +18,6 @@ package v1
 
 import (
 	"fmt"
-	"sort"
 	"strings"
 
 	meta "k8s.io/apimachinery/pkg/api/meta"
@@ -209,48 +208,6 @@ func (f *Function) SetDatabaseMatchCondition(matched bool, configuredDB, availab
 // SetCriteriaMatchCondition records the outcome of CriteriaExpression evaluation. An optional clusterLabels map may be
 // provided to enrich messages for debugging. Passing nil is permitted and treated as "no labels".
 func (f *Function) SetCriteriaMatchCondition(matched bool, expression string, err error, clusterLabels map[string]string) {
-	labelSummary := FormatClusterLabels(clusterLabels)
-	reason := ReasonCriteriaMatched
-	status := metav1.ConditionTrue
-	message := fmt.Sprintf("Criteria expression %q matched cluster labels: %s", expression, labelSummary)
-	if expression == "" {
-		reason = ReasonCriteriaMatched
-		message = "No criteria expression configured; defaulting to match"
-	}
-	if err != nil {
-		reason = ReasonCriteriaExpressionError
-		status = metav1.ConditionFalse
-		message = fmt.Sprintf("Criteria expression %q failed: %v (cluster labels: %s)", expression, err, labelSummary)
-	} else if !matched {
-		reason = ReasonCriteriaNotMatched
-		status = metav1.ConditionFalse
-		message = fmt.Sprintf("Criteria expression %q evaluated to false for cluster labels: %s", expression, labelSummary)
-	}
-	condition := metav1.Condition{
-		Type:               FunctionCriteriaMatch,
-		Status:             status,
-		Reason:             reason,
-		Message:            message,
-		ObservedGeneration: f.GetGeneration(),
-		LastTransitionTime: metav1.Now(),
-	}
+	condition := NewCriteriaCondition(FunctionCriteriaMatch, f.GetGeneration(), matched, expression, err, clusterLabels)
 	meta.SetStatusCondition(&f.Status.Conditions, condition)
 }
-
-// FormatClusterLabels returns a stable, human-readable summary of cluster labels for status messages.
-// Keys are sorted to make comparisons deterministic.
-func FormatClusterLabels(labels map[string]string) string {
-	if len(labels) == 0 {
-		return "(none)"
-	}
-	keys := make([]string, 0, len(labels))
-	for k := range labels {
-		keys = append(keys, k)
-	}
-	sort.Strings(keys)
-	parts := make([]string, 0, len(keys))
-	for _, k := range keys {
-		parts = append(parts, fmt.Sprintf("%s=%s", k, labels[k]))
-	}
-	return strings.Join(parts, ", ")
-}

api/v1/managementcommand_types.go

@@ -5,9 +5,17 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
-// NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
-const ManagementCommandConditionOwner = "managementcommand.adx-mon.azure.com"
+const (
+	ManagementCommandConditionOwner = "managementcommand.adx-mon.azure.com"
+	ManagementCommandCriteriaMatch  = "managementcommand.adx-mon.azure.com/CriteriaMatch"
+	ManagementCommandExecuted       = "managementcommand.adx-mon.azure.com/Executed"
+)
+
+const (
+	ReasonManagementCommandExecutionSucceeded = "ExecutionSucceeded"
+	ReasonManagementCommandExecutionFailed    = "ExecutionFailed"
+	ReasonManagementCommandCriteriaError      = "CriteriaEvaluationFailed"
+)
 
 // ManagementCommandSpec defines the desired state of ManagementCommand
 type ManagementCommandSpec struct {
@@ -51,20 +59,47 @@ func (m *ManagementCommand) GetCondition() *metav1.Condition {
 }
 
 func (m *ManagementCommand) SetCondition(c metav1.Condition) {
+	if c.Type == "" {
+		c.Type = ManagementCommandConditionOwner
+	}
+
+	existing := meta.FindStatusCondition(m.Status.Conditions, c.Type)
+	if c.Reason == "" {
+		if existing != nil && existing.Reason != "" {
+			c.Reason = existing.Reason
+		} else {
+			c.Reason = defaultOwnerReason(c.Status)
+		}
+	}
+	if c.Message == "" && existing != nil {
+		c.Message = existing.Message
+	}
 	if c.ObservedGeneration == 0 {
-		c.Reason = "Created"
-	} else {
-		c.Reason = "Updated"
+		c.ObservedGeneration = m.GetGeneration()
 	}
-	if c.Status == metav1.ConditionFalse {
-		c.Reason = "Failed"
+	if c.LastTransitionTime.IsZero() {
+		c.LastTransitionTime = metav1.Now()
 	}
-	c.ObservedGeneration = m.GetGeneration()
-	c.Type = ManagementCommandConditionOwner
 
 	meta.SetStatusCondition(&m.Status.Conditions, c)
 }
 
+// SetCriteriaMatchCondition records the criteria evaluation state for the ManagementCommand.
+func (m *ManagementCommand) SetCriteriaMatchCondition(matched bool, expression string, evaluationErr error, clusterLabels map[string]string) {
+	condition := NewCriteriaCondition(ManagementCommandCriteriaMatch, m.GetGeneration(), matched, expression, evaluationErr, clusterLabels)
+	m.SetCondition(condition)
+}
+
+// SetExecutionCondition updates both the ManagementCommandExecuted condition and the owner condition to reflect execution state.
+func (m *ManagementCommand) SetExecutionCondition(status metav1.ConditionStatus, reason, message string) {
+	execution := NewOwnerCondition(ManagementCommandExecuted, m.GetGeneration(), status, reason, message)
+	m.SetCondition(execution)
+
+	owner := execution
+	owner.Type = ManagementCommandConditionOwner
+	m.SetCondition(owner)
+}
+
 //+kubebuilder:object:root=true
 //+kubebuilder:subresource:status
 

api/v1/managementcommand_types_test.go

@@ -0,0 +1,79 @@
+package v1
+
+import (
+	"testing"
+
+	meta "k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestManagementCommandSetConditionDefaults(t *testing.T) {
+	cmd := &ManagementCommand{}
+	cmd.Generation = 4
+
+	cmd.SetCondition(metav1.Condition{Status: metav1.ConditionTrue})
+	owner := meta.FindStatusCondition(cmd.Status.Conditions, ManagementCommandConditionOwner)
+	require.NotNil(t, owner)
+	require.Equal(t, metav1.ConditionTrue, owner.Status)
+	require.Equal(t, ConditionCompleted, owner.Reason)
+	require.Equal(t, int64(4), owner.ObservedGeneration)
+
+	// Simulate UpdateStatus call with empty reason/message should preserve existing reason/message
+	ownerReason := "custom"
+	ownerMessage := "custom message"
+	cmd.SetExecutionCondition(metav1.ConditionFalse, ownerReason, ownerMessage)
+
+	cmd.SetCondition(metav1.Condition{Status: metav1.ConditionFalse})
+	owner = meta.FindStatusCondition(cmd.Status.Conditions, ManagementCommandConditionOwner)
+	require.Equal(t, ownerReason, owner.Reason)
+	require.Equal(t, ownerMessage, owner.Message)
+}
+
+func TestManagementCommandCriteriaMatchCondition(t *testing.T) {
+	labels := map[string]string{"environment": "prod"}
+	cmd := &ManagementCommand{}
+	cmd.Generation = 2
+
+	cmd.SetCriteriaMatchCondition(true, "environment == 'prod'", nil, labels)
+	crit := meta.FindStatusCondition(cmd.Status.Conditions, ManagementCommandCriteriaMatch)
+	require.NotNil(t, crit)
+	require.Equal(t, metav1.ConditionTrue, crit.Status)
+	require.Equal(t, ReasonCriteriaMatched, crit.Reason)
+
+	cmd.SetCriteriaMatchCondition(false, "environment == 'prod'", assertiveErr("boom"), labels)
+	crit = meta.FindStatusCondition(cmd.Status.Conditions, ManagementCommandCriteriaMatch)
+	require.NotNil(t, crit)
+	require.Equal(t, metav1.ConditionFalse, crit.Status)
+	require.Equal(t, ReasonCriteriaExpressionError, crit.Reason)
+	require.Contains(t, crit.Message, "boom")
+}
+
+func TestManagementCommandExecutionCondition(t *testing.T) {
+	cmd := &ManagementCommand{}
+	cmd.Generation = 10
+
+	cmd.SetExecutionCondition(metav1.ConditionFalse, ReasonManagementCommandExecutionFailed, "failed")
+	exec := meta.FindStatusCondition(cmd.Status.Conditions, ManagementCommandExecuted)
+	require.NotNil(t, exec)
+	require.Equal(t, metav1.ConditionFalse, exec.Status)
+	require.Equal(t, ReasonManagementCommandExecutionFailed, exec.Reason)
+	require.Equal(t, "failed", exec.Message)
+
+	owner := meta.FindStatusCondition(cmd.Status.Conditions, ManagementCommandConditionOwner)
+	require.NotNil(t, owner)
+	require.Equal(t, metav1.ConditionFalse, owner.Status)
+	require.Equal(t, ReasonManagementCommandExecutionFailed, owner.Reason)
+	require.Equal(t, "failed", owner.Message)
+
+	cmd.SetExecutionCondition(metav1.ConditionTrue, ReasonManagementCommandExecutionSucceeded, "ok")
+	exec = meta.FindStatusCondition(cmd.Status.Conditions, ManagementCommandExecuted)
+	require.Equal(t, metav1.ConditionTrue, exec.Status)
+	require.Equal(t, ReasonManagementCommandExecutionSucceeded, exec.Reason)
+	require.Equal(t, "ok", exec.Message)
+
+	owner = meta.FindStatusCondition(cmd.Status.Conditions, ManagementCommandConditionOwner)
+	require.Equal(t, metav1.ConditionTrue, owner.Status)
+	require.Equal(t, ReasonManagementCommandExecutionSucceeded, owner.Reason)
+}