add lastTransitionTime to health status

Signed-off-by: Manuel Kieweg <mail@manuelkieweg.de>

assets/swagger.json

@@ -6960,7 +6960,7 @@
         },
         "serverVersion": {
           "type": "string",
-          "title": "DEPRECATED: use Info.ServerVersion field instead.\nThe server version"
+          "title": "Deprecated: use Info.ServerVersion field instead.\nThe server version"
         },
         "shard": {
           "description": "Shard contains optional shard number. Calculated on the fly by the application controller if not specified.",
@@ -7325,6 +7325,9 @@
       "type": "object",
       "title": "HealthStatus contains information about the currently observed health state of an application or resource",
       "properties": {
+        "lastTransitionTime": {
+          "$ref": "#/definitions/v1Time"
+        },
         "message": {
           "type": "string",
           "title": "Message is a human-readable informational message describing the health status"

controller/appcontroller_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/argoproj/gitops-engine/pkg/health"
 	"github.com/argoproj/gitops-engine/pkg/utils/kube/kubetest"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
@@ -408,10 +409,23 @@ func newFakeApp() *v1alpha1.Application {
 	return createFakeApp(fakeApp)
 }
 
+func newFakeAppWithHealthAndTime(status health.HealthStatusCode, timestamp metav1.Time) *v1alpha1.Application {
+	return createFakeAppWithHealthAndTime(fakeApp, status, timestamp)
+}
+
 func newFakeMultiSourceApp() *v1alpha1.Application {
 	return createFakeApp(fakeMultiSourceApp)
 }
 
+func createFakeAppWithHealthAndTime(testApp string, status health.HealthStatusCode, timestamp metav1.Time) *v1alpha1.Application {
+	app := createFakeApp(testApp)
+	app.Status.Health = v1alpha1.HealthStatus{
+		Status:             status,
+		LastTransitionTime: timestamp,
+	}
+	return app
+}
+
 func newFakeAppWithDestMismatch() *v1alpha1.Application {
 	return createFakeApp(fakeAppWithDestMismatch)
 }

controller/health.go

@@ -8,19 +8,38 @@ import (
 	"github.com/argoproj/gitops-engine/pkg/sync/ignore"
 	kubeutil "github.com/argoproj/gitops-engine/pkg/utils/kube"
 	log "github.com/sirupsen/logrus"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application"
 	appv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	"github.com/argoproj/argo-cd/v2/util/lua"
 )
 
+func getLastTransitionTime(statuses []appv1.ResourceStatus, i int) metav1.Time {
+	if len(statuses) == 0 {
+		return metav1.Now()
+	}
+
+	lastTransitionTime := statuses[i].Health.LastTransitionTime
+
+	if lastTransitionTime.IsZero() {
+		lastTransitionTime = metav1.Now()
+	}
+
+	return lastTransitionTime
+}
+
 // setApplicationHealth updates the health statuses of all resources performed in the comparison
 func setApplicationHealth(resources []managedResource, statuses []appv1.ResourceStatus, resourceOverrides map[string]appv1.ResourceOverride, app *appv1.Application, persistResourceHealth bool) (*appv1.HealthStatus, error) {
 	var savedErr error
 	var errCount uint
+
+	// All statuses have the same timestamp, so we can safely get the first one
+	lastTransitionTime := getLastTransitionTime(statuses, 0)
 	appHealth := appv1.HealthStatus{Status: health.HealthStatusHealthy}
 	for i, res := range resources {
+		now := metav1.Now()
 		if res.Target != nil && hookutil.Skip(res.Target) {
 			continue
 		}
@@ -54,7 +73,12 @@ func setApplicationHealth(resources []managedResource, statuses []appv1.Resource
 		}
 
 		if persistResourceHealth {
-			resHealth := appv1.HealthStatus{Status: healthStatus.Status, Message: healthStatus.Message}
+			// If the status didn't change, we don't want to update the timestamp
+			if healthStatus.Status == statuses[i].Health.Status {
+				now = getLastTransitionTime(statuses, i)
+			}
+
+			resHealth := appv1.HealthStatus{Status: healthStatus.Status, Message: healthStatus.Message, LastTransitionTime: now}
 			statuses[i].Health = &resHealth
 		} else {
 			statuses[i].Health = nil
@@ -72,6 +96,13 @@ func setApplicationHealth(resources []managedResource, statuses []appv1.Resource
 
 		if health.IsWorse(appHealth.Status, healthStatus.Status) {
 			appHealth.Status = healthStatus.Status
+			if persistResourceHealth {
+				appHealth.LastTransitionTime = statuses[i].Health.LastTransitionTime
+			} else {
+				appHealth.LastTransitionTime = now
+			}
+		} else if healthStatus.Status == health.HealthStatusHealthy {
+			appHealth.LastTransitionTime = lastTransitionTime
 		}
 	}
 	if persistResourceHealth {

controller/health_test.go

@@ -3,6 +3,7 @@ package controller
 import (
 	"os"
 	"testing"
+	"time"
 
 	"github.com/argoproj/gitops-engine/pkg/health"
 	synccommon "github.com/argoproj/gitops-engine/pkg/sync/common"
@@ -18,12 +19,20 @@ import (
 	"github.com/argoproj/argo-cd/v2/util/lua"
 )
 
-var app = &appv1.Application{}
+var (
+	app           = &appv1.Application{}
+	testTimestamp = metav1.NewTime(time.Date(2020, time.January, 1, 12, 0, 0, 0, time.UTC))
+)
 
 func initStatuses(resources []managedResource) []appv1.ResourceStatus {
 	statuses := make([]appv1.ResourceStatus, len(resources))
 	for i := range resources {
-		statuses[i] = appv1.ResourceStatus{Group: resources[i].Group, Kind: resources[i].Kind, Version: resources[i].Version}
+		statuses[i] = appv1.ResourceStatus{
+			Group:   resources[i].Group,
+			Kind:    resources[i].Kind,
+			Version: resources[i].Version,
+			Health:  &appv1.HealthStatus{LastTransitionTime: testTimestamp},
+		}
 	}
 	return statuses
 }
@@ -51,19 +60,29 @@ func TestSetApplicationHealth(t *testing.T) {
 		Group: "batch", Version: "v1", Kind: "Job", Live: &failedJob,
 	}}
 	resourceStatuses := initStatuses(resources)
+	// Populate health status
+	resourceStatuses[0].Health.Status = health.HealthStatusHealthy
 
 	healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true)
+	firstHealthStatusTransitionTime := healthStatus.LastTransitionTime
 	assert.NoError(t, err)
 	assert.Equal(t, health.HealthStatusDegraded, healthStatus.Status)
+	assert.NotEqual(t, testTimestamp, firstHealthStatusTransitionTime)
 
 	assert.Equal(t, health.HealthStatusHealthy, resourceStatuses[0].Health.Status)
+	assert.Equal(t, testTimestamp, resourceStatuses[0].Health.LastTransitionTime)
 	assert.Equal(t, health.HealthStatusDegraded, resourceStatuses[1].Health.Status)
+	assert.Equal(t, firstHealthStatusTransitionTime, resourceStatuses[1].Health.LastTransitionTime)
+	// Mark both health statuses as degraded, as app is degraded.
+	resourceStatuses[0].Health.Status = health.HealthStatusDegraded
+	resourceStatuses[1].Health.Status = health.HealthStatusDegraded
 
 	// now mark the job as a hook and retry. it should ignore the hook and consider the app healthy
 	failedJob.SetAnnotations(map[string]string{synccommon.AnnotationKeyHook: "PreSync"})
 	healthStatus, err = setApplicationHealth(resources, resourceStatuses, nil, app, true)
 	assert.NoError(t, err)
 	assert.Equal(t, health.HealthStatusHealthy, healthStatus.Status)
+	assert.Equal(t, testTimestamp, healthStatus.LastTransitionTime)
 }
 
 func TestSetApplicationHealth_ResourceHealthNotPersisted(t *testing.T) {
@@ -79,6 +98,7 @@ func TestSetApplicationHealth_ResourceHealthNotPersisted(t *testing.T) {
 	assert.Equal(t, health.HealthStatusDegraded, healthStatus.Status)
 
 	assert.Nil(t, resourceStatuses[0].Health)
+	assert.False(t, healthStatus.LastTransitionTime.IsZero())
 }
 
 func TestSetApplicationHealth_MissingResource(t *testing.T) {
@@ -92,6 +112,33 @@ func TestSetApplicationHealth_MissingResource(t *testing.T) {
 	healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true)
 	assert.NoError(t, err)
 	assert.Equal(t, health.HealthStatusMissing, healthStatus.Status)
+	assert.False(t, healthStatus.LastTransitionTime.IsZero())
+}
+
+func TestSetApplicationHealth_HealthImproves(t *testing.T) {
+	overrides := lua.ResourceHealthOverrides{
+		lua.GetConfigMapKey(appv1.ApplicationSchemaGroupVersionKind): appv1.ResourceOverride{
+			HealthLua: `
+hs = {}
+hs.status = "Progressing"
+hs.message = ""
+return hs`,
+		},
+	}
+
+	degradedApp := newAppLiveObj(health.HealthStatusDegraded)
+	timestamp := metav1.Now()
+	resources := []managedResource{{
+		Group: application.Group, Version: "v1alpha1", Kind: application.ApplicationKind, Live: degradedApp,
+	}, {}}
+	resourceStatuses := initStatuses(resources)
+	resourceStatuses[0].Health.Status = health.HealthStatusDegraded
+	resourceStatuses[0].Health.LastTransitionTime = timestamp
+
+	healthStatus, err := setApplicationHealth(resources, resourceStatuses, overrides, app, true)
+	assert.NoError(t, err)
+	assert.Equal(t, health.HealthStatusProgressing, healthStatus.Status)
+	assert.NotEqual(t, healthStatus.LastTransitionTime, timestamp)
 }
 
 func TestSetApplicationHealth_MissingResourceNoBuiltHealthCheck(t *testing.T) {
@@ -117,6 +164,7 @@ func TestSetApplicationHealth_MissingResourceNoBuiltHealthCheck(t *testing.T) {
 		}, app, true)
 		assert.NoError(t, err)
 		assert.Equal(t, health.HealthStatusMissing, healthStatus.Status)
+		assert.False(t, healthStatus.LastTransitionTime.IsZero())
 	})
 }
 

controller/state.go

@@ -406,7 +406,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
 					Status:     v1alpha1.SyncStatusCodeUnknown,
 					Revisions:  revisions,
 				},
-				healthStatus: &v1alpha1.HealthStatus{Status: health.HealthStatusUnknown},
+				healthStatus: &v1alpha1.HealthStatus{Status: health.HealthStatusUnknown, LastTransitionTime: metav1.Now()},
 			}, nil
 		} else {
 			return &comparisonResult{
@@ -415,7 +415,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
 					Status:     v1alpha1.SyncStatusCodeUnknown,
 					Revision:   revisions[0],
 				},
-				healthStatus: &v1alpha1.HealthStatus{Status: health.HealthStatusUnknown},
+				healthStatus: &v1alpha1.HealthStatus{Status: health.HealthStatusUnknown, LastTransitionTime: metav1.Now()},
 			}, nil
 		}
 	}
@@ -706,6 +706,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
 			Kind:            gvk.Kind,
 			Version:         gvk.Version,
 			Group:           gvk.Group,
+			Health:          &app.Status.Health,
 			Hook:            isHook(obj),
 			RequiresPruning: targetObj == nil && liveObj != nil && isSelfReferencedObj,
 		}

controller/state_test.go

@@ -715,6 +715,44 @@ func TestSetHealth(t *testing.T) {
 	assert.NoError(t, err)
 
 	assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status)
+	assert.False(t, compRes.healthStatus.LastTransitionTime.IsZero())
+}
+
+func TestPreserveStatusTimestamp(t *testing.T) {
+	timestamp := metav1.Now()
+	app := newFakeAppWithHealthAndTime(health.HealthStatusHealthy, timestamp)
+	deployment := kube.MustToUnstructured(&v1.Deployment{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "apps/v1",
+			Kind:       "Deployment",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "demo",
+			Namespace: "default",
+		},
+	})
+	ctrl := newFakeController(&fakeData{
+		apps: []runtime.Object{app, &defaultProj},
+		manifestResponse: &apiclient.ManifestResponse{
+			Manifests: []string{},
+			Namespace: test.FakeDestNamespace,
+			Server:    test.FakeClusterURL,
+			Revision:  "abc123",
+		},
+		managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{
+			kube.GetResourceKey(deployment): deployment,
+		},
+	}, nil)
+
+	sources := make([]argoappv1.ApplicationSource, 0)
+	sources = append(sources, app.Spec.GetSource())
+	revisions := make([]string, 0)
+	revisions = append(revisions, "")
+	compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false)
+	assert.NoError(t, err)
+
+	assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status)
+	assert.Equal(t, timestamp, compRes.healthStatus.LastTransitionTime)
 }
 
 func TestSetHealthSelfReferencedApp(t *testing.T) {
@@ -752,6 +790,7 @@ func TestSetHealthSelfReferencedApp(t *testing.T) {
 	assert.NoError(t, err)
 
 	assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status)
+	assert.False(t, compRes.healthStatus.LastTransitionTime.IsZero())
 }
 
 func TestSetManagedResourcesWithOrphanedResources(t *testing.T) {
@@ -827,6 +866,7 @@ func TestReturnUnknownComparisonStateOnSettingLoadError(t *testing.T) {
 	assert.NoError(t, err)
 
 	assert.Equal(t, health.HealthStatusUnknown, compRes.healthStatus.Status)
+	assert.False(t, compRes.healthStatus.LastTransitionTime.IsZero())
 	assert.Equal(t, argoappv1.SyncStatusCodeUnknown, compRes.syncStatus.Status)
 }
 

manifests/core-install.yaml

@@ -1718,6 +1718,11 @@ spec:
                 description: Health contains information about the application's current
                   health status
                 properties:
+                  lastTransitionTime:
+                    description: LastTransitionTime is the time the HealthStatus was
+                      set
+                    format: date-time
+                    type: string
                   message:
                     description: Message is a human-readable informational message
                       describing the health status
@@ -4154,6 +4159,11 @@ spec:
                       description: HealthStatus contains information about the currently
                         observed health state of an application or resource
                       properties:
+                        lastTransitionTime:
+                          description: LastTransitionTime is the time the HealthStatus
+                            was set
+                          format: date-time
+                          type: string
                         message:
                           description: Message is a human-readable informational message
                             describing the health status
@@ -20345,6 +20355,9 @@ spec:
                       type: string
                     health:
                       properties:
+                        lastTransitionTime:
+                          format: date-time
+                          type: string
                         message:
                           type: string
                         status:

manifests/crds/application-crd.yaml

@@ -1717,6 +1717,11 @@ spec:
                 description: Health contains information about the application's current
                   health status
                 properties:
+                  lastTransitionTime:
+                    description: LastTransitionTime is the time the HealthStatus was
+                      set
+                    format: date-time
+                    type: string
                   message:
                     description: Message is a human-readable informational message
                       describing the health status
@@ -4153,6 +4158,11 @@ spec:
                       description: HealthStatus contains information about the currently
                         observed health state of an application or resource
                       properties:
+                        lastTransitionTime:
+                          description: LastTransitionTime is the time the HealthStatus
+                            was set
+                          format: date-time
+                          type: string
                         message:
                           description: Message is a human-readable informational message
                             describing the health status

manifests/crds/applicationset-crd.yaml

@@ -15344,6 +15344,9 @@ spec:
                       type: string
                     health:
                       properties:
+                        lastTransitionTime:
+                          format: date-time
+                          type: string
                         message:
                           type: string
                         status: