First draft

Signed-off-by: Manuel Kieweg <mkieweg@vectra.ai>

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"
@@ -401,6 +402,10 @@ 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)
 }
@@ -422,6 +427,15 @@ func createFakeApp(testApp string) *v1alpha1.Application {
 	return &app
 }
 
+func createFakeAppWithHealthAndTime(testApp string, status health.HealthStatusCode, timestamp metav1.Time) *v1alpha1.Application {
+	app := createFakeApp(testApp)
+	app.Status.Health = v1alpha1.HealthStatus{
+		Status:    status,
+		Timestamp: timestamp,
+	}
+	return app
+}
+
 func newFakeCM() map[string]interface{} {
 	var cm map[string]interface{}
 	err := yaml.Unmarshal([]byte(fakeStrayResource), &cm)

controller/health.go

@@ -8,17 +8,35 @@ 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 getOldTimestamp(statuses []appv1.ResourceStatus, i int) metav1.Time {
+	if len(statuses) == 0 {
+		return metav1.Now()
+	}
+
+	oldTimestamp := statuses[i].Health.Timestamp
+
+	if oldTimestamp.IsZero() {
+		oldTimestamp = metav1.Now()
+	}
+
+	return oldTimestamp
+}
+
 // 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
+	oldTimestamp := getOldTimestamp(statuses, 0)
 	appHealth := appv1.HealthStatus{Status: health.HealthStatusHealthy}
 	for i, res := range resources {
 		if res.Target != nil && hookutil.Skip(res.Target) {
@@ -54,7 +72,14 @@ func setApplicationHealth(resources []managedResource, statuses []appv1.Resource
 		}
 
 		if persistResourceHealth {
-			resHealth := appv1.HealthStatus{Status: healthStatus.Status, Message: healthStatus.Message}
+			timestamp := metav1.Now()
+
+			// If the status didn't change, we don't want to update the timestamp
+			if healthStatus.Status == statuses[i].Health.Status {
+				timestamp = getOldTimestamp(statuses, i)
+			}
+
+			resHealth := appv1.HealthStatus{Status: healthStatus.Status, Message: healthStatus.Message, Timestamp: timestamp}
 			statuses[i].Health = &resHealth
 		} else {
 			statuses[i].Health = nil
@@ -72,6 +97,9 @@ func setApplicationHealth(resources []managedResource, statuses []appv1.Resource
 
 		if health.IsWorse(appHealth.Status, healthStatus.Status) {
 			appHealth.Status = healthStatus.Status
+			appHealth.Timestamp = statuses[i].Health.Timestamp
+		} else if healthStatus.Status == health.HealthStatusHealthy {
+			appHealth.Timestamp = oldTimestamp
 		}
 	}
 	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{Timestamp: testTimestamp},
+		}
 	}
 	return statuses
 }
@@ -51,18 +60,30 @@ func TestSetApplicationHealth(t *testing.T) {
 	}}
 	resourceStatuses := initStatuses(resources)
 
+	// Populate health status
+	resourceStatuses[0].Health.Status = health.HealthStatusHealthy
+
 	healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true)
+	firstHealthStatusTimestamp := healthStatus.Timestamp
 	assert.NoError(t, err)
 	assert.Equal(t, health.HealthStatusDegraded, healthStatus.Status)
+	assert.NotEqual(t, testTimestamp, firstHealthStatusTimestamp)
 
 	assert.Equal(t, resourceStatuses[0].Health.Status, health.HealthStatusHealthy)
+	assert.Equal(t, testTimestamp, resourceStatuses[0].Health.Timestamp)
 	assert.Equal(t, resourceStatuses[1].Health.Status, health.HealthStatusDegraded)
+	assert.Equal(t, firstHealthStatusTimestamp, resourceStatuses[1].Health.Timestamp)
+
+	// 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, firstHealthStatusTimestamp, healthStatus.Timestamp)
 }
 
 func TestSetApplicationHealth_ResourceHealthNotPersisted(t *testing.T) {
@@ -78,6 +99,7 @@ func TestSetApplicationHealth_ResourceHealthNotPersisted(t *testing.T) {
 	assert.Equal(t, health.HealthStatusDegraded, healthStatus.Status)
 
 	assert.Nil(t, resourceStatuses[0].Health)
+	assert.False(t, healthStatus.Timestamp.IsZero())
 }
 
 func TestSetApplicationHealth_MissingResource(t *testing.T) {
@@ -90,6 +112,40 @@ 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.Timestamp.IsZero())
+}
+
+func TestSetApplicationHealth_HealthImproves(t *testing.T) {
+	overrides := lua.ResourceHealthOverrides{
+		lua.GetConfigMapKey(appv1.ApplicationSchemaGroupVersionKind): appv1.ResourceOverride{
+			HealthLua: `
+hs = {}
+hs.status = "Progressing"
+hs.message = ""
+if obj.status ~= nil then
+  if obj.status.health ~= nil then
+	hs.status = obj.status.health.status
+	if obj.status.health.message ~= nil then
+	  hs.message = obj.status.health.message
+	end
+  end
+end
+return hs`,
+		},
+	}
+
+	pod := resourceFromFile("./testdata/pod-running-restart-always.yaml")
+	timestamp := metav1.Now()
+	resources := []managedResource{{
+		Group: "", Version: "v1", Kind: "Pod", Target: &pod}, {}}
+	resourceStatuses := initStatuses(resources)
+	resourceStatuses[0].Health.Status = health.HealthStatusDegraded
+	resourceStatuses[0].Health.Timestamp = timestamp
+
+	healthStatus, err := setApplicationHealth(resources, resourceStatuses, overrides, app, true)
+	assert.NoError(t, err)
+	assert.Equal(t, health.HealthStatusProgressing, healthStatus.Status)
+	assert.NotEqual(t, healthStatus.Timestamp, timestamp)
 }
 
 func TestSetApplicationHealth_MissingResourceNoBuiltHealthCheck(t *testing.T) {
@@ -104,6 +160,7 @@ func TestSetApplicationHealth_MissingResourceNoBuiltHealthCheck(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, health.HealthStatusHealthy, healthStatus.Status)
 		assert.Equal(t, resourceStatuses[0].Health.Status, health.HealthStatusMissing)
+		assert.False(t, healthStatus.Timestamp.IsZero())
 	})
 
 	t.Run("HasOverride", func(t *testing.T) {
@@ -114,6 +171,7 @@ func TestSetApplicationHealth_MissingResourceNoBuiltHealthCheck(t *testing.T) {
 		}, app, true)
 		assert.NoError(t, err)
 		assert.Equal(t, health.HealthStatusMissing, healthStatus.Status)
+		assert.False(t, healthStatus.Timestamp.IsZero())
 	})
 }
 
@@ -164,6 +222,7 @@ return hs`,
 		healthStatus, err := setApplicationHealth(resources, resourceStatuses, overrides, app, true)
 		assert.NoError(t, err)
 		assert.Equal(t, health.HealthStatusDegraded, healthStatus.Status)
+		assert.False(t, healthStatus.Timestamp.IsZero())
 	})
 
 	t.Run("ChildAppMissing", func(t *testing.T) {
@@ -175,5 +234,6 @@ return hs`,
 		healthStatus, err := setApplicationHealth(resources, resourceStatuses, overrides, app, true)
 		assert.NoError(t, err)
 		assert.Equal(t, health.HealthStatusHealthy, healthStatus.Status)
+		assert.False(t, healthStatus.Timestamp.IsZero())
 	})
 }

controller/state.go

@@ -375,7 +375,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, Timestamp: metav1.Now()},
 			}, nil
 		} else {
 			return &comparisonResult{
@@ -384,7 +384,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, Timestamp: metav1.Now()},
 			}, nil
 		}
 	}
@@ -677,6 +677,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

@@ -683,6 +683,44 @@ func TestSetHealth(t *testing.T) {
 	assert.Nil(t, err)
 
 	assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status)
+	assert.False(t, compRes.healthStatus.Timestamp.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)
+	assert.Nil(t, err)
+
+	assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status)
+	assert.Equal(t, timestamp, compRes.healthStatus.Timestamp)
 }
 
 func TestSetHealthSelfReferencedApp(t *testing.T) {
@@ -720,6 +758,7 @@ func TestSetHealthSelfReferencedApp(t *testing.T) {
 	assert.Nil(t, err)
 
 	assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status)
+	assert.False(t, compRes.healthStatus.Timestamp.IsZero())
 }
 
 func TestSetManagedResourcesWithOrphanedResources(t *testing.T) {
@@ -795,6 +834,7 @@ func TestReturnUnknownComparisonStateOnSettingLoadError(t *testing.T) {
 	assert.Nil(t, err)
 
 	assert.Equal(t, health.HealthStatusUnknown, compRes.healthStatus.Status)
+	assert.False(t, compRes.healthStatus.Timestamp.IsZero())
 	assert.Equal(t, argoappv1.SyncStatusCodeUnknown, compRes.syncStatus.Status)
 }
 

pkg/apis/application/v1alpha1/types.go

@@ -1512,6 +1512,8 @@ type HealthStatus struct {
 	Status health.HealthStatusCode `json:"status,omitempty" protobuf:"bytes,1,opt,name=status"`
 	// Message is a human-readable informational message describing the health status
 	Message string `json:"message,omitempty" protobuf:"bytes,2,opt,name=message"`
+	// Timestamp is the time the HealthStatus was set
+	Timestamp metav1.Time `json:"timestamp,omitempty" protobuf:"bytes,3,opt,name=timestamp"`
 }
 
 // InfoItem contains arbitrary, human readable information about an application