Skip to content

Commit

Permalink
isolate initial changes
Browse files Browse the repository at this point in the history 
Signed-off-by: Manuel Kieweg <mkieweg@vectra.ai>
  • Loading branch information
Manuel Kieweg committed Jun 11, 2024
1 parent dfe45f2 commit ed6f3c4
Show file tree
Hide file tree
Showing 6 changed files with 292 additions and 297 deletions.
79 changes: 29 additions & 50 deletions controller/appcontroller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -19,6 +20,7 @@ import (
statecache "github.com/argoproj/argo-cd/v2/controller/cache"
"github.com/argoproj/argo-cd/v2/controller/sharding"

dbmocks "github.com/argoproj/argo-cd/v2/util/db/mocks"
"github.com/argoproj/gitops-engine/pkg/cache/mocks"
synccommon "github.com/argoproj/gitops-engine/pkg/sync/common"
"github.com/argoproj/gitops-engine/pkg/utils/kube"
Expand All @@ -35,15 +37,12 @@ import (
"k8s.io/client-go/tools/cache"
"sigs.k8s.io/yaml"

dbmocks "github.com/argoproj/argo-cd/v2/util/db/mocks"

mockstatecache "github.com/argoproj/argo-cd/v2/controller/cache/mocks"
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
appclientset "github.com/argoproj/argo-cd/v2/pkg/client/clientset/versioned/fake"
"github.com/argoproj/argo-cd/v2/reposerver/apiclient"
mockrepoclient "github.com/argoproj/argo-cd/v2/reposerver/apiclient/mocks"
"github.com/argoproj/argo-cd/v2/test"
"github.com/argoproj/argo-cd/v2/util/argo/normalizers"
cacheutil "github.com/argoproj/argo-cd/v2/util/cache"
appstatecache "github.com/argoproj/argo-cd/v2/util/cache/appstate"
"github.com/argoproj/argo-cd/v2/util/settings"
Expand All @@ -55,15 +54,14 @@ type namespacedResource struct {
}

type fakeData struct {
apps []runtime.Object
manifestResponse *apiclient.ManifestResponse
manifestResponses []*apiclient.ManifestResponse
managedLiveObjs map[kube.ResourceKey]*unstructured.Unstructured
namespacedResources map[kube.ResourceKey]namespacedResource
configMapData map[string]string
metricsCacheExpiration time.Duration
applicationNamespaces []string
updateRevisionForPathsResponse *apiclient.UpdateRevisionForPathsResponse
apps []runtime.Object
manifestResponse *apiclient.ManifestResponse
manifestResponses []*apiclient.ManifestResponse
managedLiveObjs map[kube.ResourceKey]*unstructured.Unstructured
namespacedResources map[kube.ResourceKey]namespacedResource
configMapData map[string]string
metricsCacheExpiration time.Duration
applicationNamespaces []string
}

type MockKubectl struct {
Expand Down Expand Up @@ -109,8 +107,6 @@ func newFakeController(data *fakeData, repoErr error) *ApplicationController {
}
}

mockRepoClient.On("UpdateRevisionForPaths", mock.Anything, mock.Anything).Return(data.updateRevisionForPathsResponse, nil)

mockRepoClientset := mockrepoclient.Clientset{RepoServerServiceClient: &mockRepoClient}

secret := corev1.Secret{
Expand Down Expand Up @@ -160,9 +156,9 @@ func newFakeController(data *fakeData, repoErr error) *ApplicationController {
nil,
data.applicationNamespaces,
nil,

false,
false,
normalizers.IgnoreNormalizerOpts{},
)
db := &dbmocks.ArgoDB{}
db.On("GetApplicationControllerReplicas").Return(1)
Expand Down Expand Up @@ -407,6 +403,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)
}
Expand All @@ -428,6 +428,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)
Expand Down Expand Up @@ -990,7 +999,7 @@ func TestNormalizeApplication(t *testing.T) {
normalized := false
fakeAppCs.AddReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
if string(patchAction.GetPatch()) == `{"spec":{"project":"default"},"status":{"sync":{"comparedTo":{"destination":{},"source":{"repoURL":""}}}}}` {
if string(patchAction.GetPatch()) == `{"spec":{"project":"default"}}` {
normalized = true
}
}
Expand All @@ -1012,7 +1021,7 @@ func TestNormalizeApplication(t *testing.T) {
normalized := false
fakeAppCs.AddReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
if string(patchAction.GetPatch()) == `{"spec":{"project":"default"},"status":{"sync":{"comparedTo":{"destination":{},"source":{"repoURL":""}}}}}` {
if string(patchAction.GetPatch()) == `{"spec":{"project":"default"}}` {
normalized = true
}
}
Expand Down Expand Up @@ -1102,8 +1111,8 @@ func TestGetResourceTree_HasOrphanedResources(t *testing.T) {
}})

assert.NoError(t, err)
assert.Equal(t, []v1alpha1.ResourceNode{managedDeploy}, tree.Nodes)
assert.Equal(t, []v1alpha1.ResourceNode{orphanedDeploy1, orphanedDeploy2}, tree.OrphanedNodes)
assert.Equal(t, tree.Nodes, []v1alpha1.ResourceNode{managedDeploy})
assert.Equal(t, tree.OrphanedNodes, []v1alpha1.ResourceNode{orphanedDeploy1, orphanedDeploy2})
}

func TestSetOperationStateOnDeletedApp(t *testing.T) {
Expand Down Expand Up @@ -1410,7 +1419,7 @@ func TestRefreshAppConditions(t *testing.T) {

_, hasErrors := ctrl.refreshAppConditions(app)
assert.False(t, hasErrors)
assert.Empty(t, app.Status.Conditions)
assert.Len(t, app.Status.Conditions, 0)
})

t.Run("PreserveExistingWarningCondition", func(t *testing.T) {
Expand Down Expand Up @@ -1719,36 +1728,6 @@ func TestProcessRequestedAppOperation_HasRetriesTerminated(t *testing.T) {
assert.Equal(t, string(synccommon.OperationFailed), phase)
}

func TestProcessRequestedAppOperation_Successful(t *testing.T) {
app := newFakeApp()
app.Spec.Project = "default"
app.Operation = &v1alpha1.Operation{
Sync: &v1alpha1.SyncOperation{},
}
ctrl := newFakeController(&fakeData{
apps: []runtime.Object{app, &defaultProj},
manifestResponses: []*apiclient.ManifestResponse{{
Manifests: []string{},
}},
}, nil)
fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset)
receivedPatch := map[string]interface{}{}
fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if patchAction, ok := action.(kubetesting.PatchAction); ok {
assert.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch))
}
return true, &v1alpha1.Application{}, nil
})

ctrl.processRequestedAppOperation(app)

phase, _, _ := unstructured.NestedString(receivedPatch, "status", "operationState", "phase")
assert.Equal(t, string(synccommon.OperationSucceeded), phase)
ok, level := ctrl.isRefreshRequested(ctrl.toAppKey(app.Name))
assert.True(t, ok)
assert.Equal(t, CompareWithLatestForceResolve, level)
}

func TestGetAppHosts(t *testing.T) {
app := newFakeApp()
data := &fakeData{
Expand Down
36 changes: 34 additions & 2 deletions controller/health.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 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 {
timestamp := metav1.Now()
if res.Target != nil && hookutil.Skip(res.Target) {
continue
}
Expand Down Expand Up @@ -54,7 +73,13 @@ 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 {
timestamp = getOldTimestamp(statuses, i)
}

resHealth := appv1.HealthStatus{Status: healthStatus.Status, Message: healthStatus.Message, Timestamp: timestamp}
statuses[i].Health = &resHealth
} else {
statuses[i].Health = nil
Expand All @@ -72,6 +97,13 @@ func setApplicationHealth(resources []managedResource, statuses []appv1.Resource

if health.IsWorse(appHealth.Status, healthStatus.Status) {
appHealth.Status = healthStatus.Status
if persistResourceHealth {
appHealth.Timestamp = statuses[i].Health.Timestamp
} else {
appHealth.Timestamp = timestamp
}
} else if healthStatus.Status == health.HealthStatusHealthy {
appHealth.Timestamp = oldTimestamp
}
}
if persistResourceHealth {
Expand All @@ -80,7 +112,7 @@ func setApplicationHealth(resources []managedResource, statuses []appv1.Resource
app.Status.ResourceHealthSource = appv1.ResourceHealthLocationAppTree
}
if savedErr != nil && errCount > 1 {
savedErr = fmt.Errorf("see application-controller logs for %d other errors; most recent error was: %w", errCount-1, savedErr)
savedErr = fmt.Errorf("see applicaton-controller logs for %d other errors; most recent error was: %w", errCount-1, savedErr)
}
return &appHealth, savedErr
}
57 changes: 53 additions & 4 deletions controller/health_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}
Expand Down Expand Up @@ -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)

assert.Equal(t, health.HealthStatusHealthy, resourceStatuses[0].Health.Status)
assert.Equal(t, health.HealthStatusDegraded, resourceStatuses[1].Health.Status)
// 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.Timestamp)
}

func TestSetApplicationHealth_ResourceHealthNotPersisted(t *testing.T) {
Expand All @@ -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) {
Expand All @@ -90,6 +112,32 @@ 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 = ""
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.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) {
Expand All @@ -114,6 +162,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())
})
}

Expand Down
Loading

0 comments on commit ed6f3c4

Please sign in to comment.