From 9ed0317cbcc32f3397b3db2324880b2e114bdf0f Mon Sep 17 00:00:00 2001 From: Olga Pudrovska Date: Tue, 19 Dec 2023 11:48:08 +0100 Subject: [PATCH] Review fixes. --- pkg/query/configuration/fluxobject_test.go | 211 --------------------- pkg/query/configuration/objectkind.go | 48 +++-- pkg/query/configuration/objectkind_test.go | 203 ++++++++++++++++++++ 3 files changed, 225 insertions(+), 237 deletions(-) delete mode 100644 pkg/query/configuration/fluxobject_test.go diff --git a/pkg/query/configuration/fluxobject_test.go b/pkg/query/configuration/fluxobject_test.go deleted file mode 100644 index 375f34d5e2..0000000000 --- a/pkg/query/configuration/fluxobject_test.go +++ /dev/null @@ -1,211 +0,0 @@ -package configuration - -import ( - "testing" - - "github.com/alecthomas/assert" - "github.com/fluxcd/helm-controller/api/v2beta1" - kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1" - clusterreflectorv1alpha1 "github.com/weaveworks/cluster-reflector-controller/api/v1alpha1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -func TestStatusAndMessage(t *testing.T) { - tests := []struct { - name string - desiredStatus ObjectStatus - desiredMessage string - obj client.Object - objectKind ObjectKind - }{ - { - name: "HelmRelease with Ready condition", - desiredStatus: Success, - desiredMessage: "Helm release sync succeeded", - obj: &v2beta1.HelmRelease{ - Status: v2beta1.HelmReleaseStatus{ - Conditions: []metav1.Condition{ - { - Type: "Ready", - Status: "True", - Message: "Helm release sync succeeded", - }, - }, - }, - }, - objectKind: HelmReleaseObjectKind, - }, - { - name: "Kustomization with Ready condition", - desiredStatus: Success, - desiredMessage: "Applied revision: main/1234567890", - obj: &kustomizev1.Kustomization{ - Status: kustomizev1.KustomizationStatus{ - Conditions: []metav1.Condition{ - {Type: "Ready", Status: "True", Message: "Applied revision: main/1234567890"}, - }, - }, - }, - objectKind: KustomizationObjectKind, - }, - { - name: "HelmRelease with failed Ready condition", - desiredStatus: Failed, - desiredMessage: "Helm release sync failed: failed to download \"fluxcd/flux\" (hint: running `helm repo update` may help)", - obj: &v2beta1.HelmRelease{ - Status: v2beta1.HelmReleaseStatus{ - Conditions: []metav1.Condition{ - {Type: "Ready", Status: "False", Message: "Helm release sync failed: failed to download \"fluxcd/flux\" (hint: running `helm repo update` may help)"}, - }, - }, - }, - objectKind: HelmReleaseObjectKind, - }, - { - name: "Kustomization with failed Ready condition", - desiredStatus: Failed, - desiredMessage: "Kustomization apply failed: failed to apply revision: main/1234567890", - obj: &kustomizev1.Kustomization{ - Status: kustomizev1.KustomizationStatus{ - Conditions: []metav1.Condition{ - {Type: "Ready", Status: "False", Message: "Kustomization apply failed: failed to apply revision: main/1234567890"}, - }, - }, - }, - objectKind: KustomizationObjectKind, - }, - { - name: "Kustomization with Suspended computed status", - desiredStatus: Suspended, - desiredMessage: "", - obj: &kustomizev1.Kustomization{ - Spec: kustomizev1.KustomizationSpec{ - Suspend: true, - }, - Status: kustomizev1.KustomizationStatus{ - Conditions: []metav1.Condition{ - {Type: "CustomCondition", Status: "CustomStatus", Message: "CustomMessage"}, - }, - }, - }, - objectKind: KustomizationObjectKind, - }, - { - name: "HelmRelease with NoStatus condition", - desiredStatus: NoStatus, - desiredMessage: "", - obj: &v2beta1.HelmRelease{ - Status: v2beta1.HelmReleaseStatus{ - Conditions: []metav1.Condition{ - {Type: "-", Status: "DoesNotMatter", Message: "CustomMessage"}, - }, - }, - }, - objectKind: HelmReleaseObjectKind, - }, - { - name: "Kustomization without Ready and without NoStatus conditions", - desiredStatus: Failed, - desiredMessage: "", - obj: &kustomizev1.Kustomization{ - Status: kustomizev1.KustomizationStatus{ - Conditions: []metav1.Condition{ - {Type: "CustomCondition", Status: "CustomStatus", Message: "CustomMessage"}, - }, - }, - }, - objectKind: KustomizationObjectKind, - }, - { - name: "HelmRelease with Ready condition and Reconciling computed status", - desiredStatus: Reconciling, - desiredMessage: "Reconciling message for HelmRelease", - obj: &v2beta1.HelmRelease{ - Status: v2beta1.HelmReleaseStatus{ - Conditions: []metav1.Condition{ - { - Type: "Ready", - Status: "Unknown", - Reason: "Progressing", - Message: "Reconciling message for HelmRelease", - }, - }, - }, - }, - objectKind: HelmReleaseObjectKind, - }, - { - name: "Kustomization with Available condition and Reconciling computed status", - desiredStatus: Reconciling, - desiredMessage: "Reconciling message for Kustomization", - obj: &kustomizev1.Kustomization{ - Status: kustomizev1.KustomizationStatus{ - Conditions: []metav1.Condition{ - { - Type: "Available", - Status: "Unknown", - Reason: "Progressing", - Message: "Reconciling message for Kustomization", - }, - }, - }, - }, - objectKind: KustomizationObjectKind, - }, - // TODO: Replace Kustomization with a Terraform object after Explorer starts supporting Terraform objects. - { - name: "Fake Terraform object with Ready condition and PendingAction computed status", - desiredStatus: PendingAction, - desiredMessage: "PendingAction message for Terraform object", - obj: &kustomizev1.Kustomization{ - Status: kustomizev1.KustomizationStatus{ - Conditions: []metav1.Condition{ - { - Type: "Ready", - Status: "Unknown", - Reason: "TerraformPlannedWithChanges", - Message: "PendingAction message for Terraform object", - }, - }, - }, - }, - objectKind: KustomizationObjectKind, - }, - { - name: "AutomatedClusterDiscovery with Ready condition", - desiredStatus: Success, - desiredMessage: "Applied revision: main/1234567890", - obj: &clusterreflectorv1alpha1.AutomatedClusterDiscovery{ - Status: clusterreflectorv1alpha1.AutomatedClusterDiscoveryStatus{ - Conditions: []metav1.Condition{ - { - Type: "Ready", - Status: "True", - Message: "Applied revision: main/1234567890", - }, - }, - }, - }, - objectKind: AutomatedClusterDiscoveryObjectKind, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := defaultFluxObjectStatusFunc(tt.obj, tt.objectKind) - assert.NoError(t, err) - - if got != tt.desiredStatus { - t.Errorf("Status() = %v, want %v", got, tt.desiredStatus) - } - - msg, err := defaultFluxObjectMessageFunc(tt.obj, tt.objectKind) - assert.NoError(t, err) - - if msg != tt.desiredMessage { - t.Errorf("Message() = %v, want %v", got, tt.desiredMessage) - } - }) - } -} diff --git a/pkg/query/configuration/objectkind.go b/pkg/query/configuration/objectkind.go index 6ff4c8b904..74f50ea168 100644 --- a/pkg/query/configuration/objectkind.go +++ b/pkg/query/configuration/objectkind.go @@ -108,10 +108,6 @@ func (o ObjectKind) Validate() error { return nil } -type FluxObject interface { - client.Object -} - var ( HelmReleaseObjectKind = ObjectKind{ Gvk: helmv2beta1.GroupVersion.WithKind(helmv2beta1.HelmReleaseKind), @@ -133,8 +129,8 @@ var ( } return hr.Spec.Suspend, nil }, - StatusFunc: defaultFluxObjectStatusFunc, - MessageFunc: defaultFluxObjectMessageFunc, + StatusFunc: defaultStatusFunc, + MessageFunc: defaultMessageFunc, Category: CategoryAutomation, } @@ -158,8 +154,8 @@ var ( } return ks.Spec.Suspend, nil }, - StatusFunc: defaultFluxObjectStatusFunc, - MessageFunc: defaultFluxObjectMessageFunc, + StatusFunc: defaultStatusFunc, + MessageFunc: defaultMessageFunc, Category: CategoryAutomation, } @@ -183,8 +179,8 @@ var ( } return hr.Spec.Suspend, nil }, - StatusFunc: defaultFluxObjectStatusFunc, - MessageFunc: defaultFluxObjectMessageFunc, + StatusFunc: defaultStatusFunc, + MessageFunc: defaultMessageFunc, Category: CategorySource, } @@ -208,8 +204,8 @@ var ( } return chart.Spec.Suspend, nil }, - StatusFunc: defaultFluxObjectStatusFunc, - MessageFunc: defaultFluxObjectMessageFunc, + StatusFunc: defaultStatusFunc, + MessageFunc: defaultMessageFunc, Category: CategorySource, } @@ -233,8 +229,8 @@ var ( } return repo.Spec.Suspend, nil }, - StatusFunc: defaultFluxObjectStatusFunc, - MessageFunc: defaultFluxObjectMessageFunc, + StatusFunc: defaultStatusFunc, + MessageFunc: defaultMessageFunc, Category: CategorySource, } @@ -258,8 +254,8 @@ var ( } return repo.Spec.Suspend, nil }, - StatusFunc: defaultFluxObjectStatusFunc, - MessageFunc: defaultFluxObjectMessageFunc, + StatusFunc: defaultStatusFunc, + MessageFunc: defaultMessageFunc, Category: CategorySource, } @@ -283,8 +279,8 @@ var ( } return b.Spec.Suspend, nil }, - StatusFunc: defaultFluxObjectStatusFunc, - MessageFunc: defaultFluxObjectMessageFunc, + StatusFunc: defaultStatusFunc, + MessageFunc: defaultMessageFunc, Category: CategorySource, } @@ -342,7 +338,7 @@ var ( StatusFunc: func(obj client.Object, _ ObjectKind) (ObjectStatus, error) { e, ok := obj.(*corev1.Event) if !ok { - return nil, fmt.Errorf("object is not an Event") + return "", fmt.Errorf("object is not an Event") } if e.Type == "Normal" { @@ -382,8 +378,8 @@ var ( } return gs.Spec.Suspend, nil }, - StatusFunc: defaultFluxObjectStatusFunc, - MessageFunc: defaultFluxObjectMessageFunc, + StatusFunc: defaultStatusFunc, + MessageFunc: defaultMessageFunc, Category: CategoryGitopsSet, } @@ -449,8 +445,8 @@ var ( } return acd.Spec.Suspend, nil }, - StatusFunc: defaultFluxObjectStatusFunc, - MessageFunc: defaultFluxObjectMessageFunc, + StatusFunc: defaultStatusFunc, + MessageFunc: defaultMessageFunc, Category: CategoryClusterDiscovery, } ) @@ -483,12 +479,12 @@ var SupportedRbacKinds = []ObjectKind{ ClusterRoleBindingObjectKind, } -// defaultFluxObjectStatusFunc is the default status function for Flux objects. +// defaultStatusFunc is the default status function for Flux objects. // The status of Flux objects is computed based on the values of the Conditions and Spec.Suspend fields, // so we can standardize on that, based on the current object statuses, computed in non-Explorer UI: // https://github.com/weaveworks/weave-gitops/blob/cc3c17632334ffa56838c4765e68ce388bde6b2f/ui/components/KubeStatusIndicator.tsx#L18-L25 // TODO: Remove the reference to non-Explorer UI logic once we move computing all object statuses to the backend. -func defaultFluxObjectStatusFunc(obj client.Object, objectKind ObjectKind) (ObjectStatus, error) { +func defaultStatusFunc(obj client.Object, objectKind ObjectKind) (ObjectStatus, error) { if objectKind.GetConditionsFunc == nil { return "", fmt.Errorf("missing get conditions func") } @@ -537,7 +533,7 @@ func defaultFluxObjectStatusFunc(obj client.Object, objectKind ObjectKind) (Obje return Failed, nil } -func defaultFluxObjectMessageFunc(obj client.Object, objectKind ObjectKind) (string, error) { +func defaultMessageFunc(obj client.Object, objectKind ObjectKind) (string, error) { if objectKind.GetConditionsFunc == nil { return "", fmt.Errorf("missing get conditions func") } diff --git a/pkg/query/configuration/objectkind_test.go b/pkg/query/configuration/objectkind_test.go index 756fb6c08c..2456fd4cc0 100644 --- a/pkg/query/configuration/objectkind_test.go +++ b/pkg/query/configuration/objectkind_test.go @@ -1,9 +1,14 @@ package configuration import ( + "github.com/alecthomas/assert" + "github.com/fluxcd/helm-controller/api/v2beta1" + kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1" v1 "github.com/fluxcd/kustomize-controller/api/v1" sourcev1 "github.com/fluxcd/source-controller/api/v1" . "github.com/onsi/gomega" + clusterreflectorv1alpha1 "github.com/weaveworks/cluster-reflector-controller/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" @@ -91,5 +96,203 @@ func TestObjectKind_Validate(t *testing.T) { } g.Expect(kind.Validate().Error()).To(Equal("missing message func")) }) +} + +func TestStatusAndMessage(t *testing.T) { + tests := []struct { + name string + desiredStatus ObjectStatus + desiredMessage string + obj client.Object + objectKind ObjectKind + }{ + { + name: "HelmRelease with Ready condition", + desiredStatus: Success, + desiredMessage: "Helm release sync succeeded", + obj: &v2beta1.HelmRelease{ + Status: v2beta1.HelmReleaseStatus{ + Conditions: []metav1.Condition{ + { + Type: "Ready", + Status: "True", + Message: "Helm release sync succeeded", + }, + }, + }, + }, + objectKind: HelmReleaseObjectKind, + }, + { + name: "Kustomization with Ready condition", + desiredStatus: Success, + desiredMessage: "Applied revision: main/1234567890", + obj: &kustomizev1.Kustomization{ + Status: kustomizev1.KustomizationStatus{ + Conditions: []metav1.Condition{ + {Type: "Ready", Status: "True", Message: "Applied revision: main/1234567890"}, + }, + }, + }, + objectKind: KustomizationObjectKind, + }, + { + name: "HelmRelease with failed Ready condition", + desiredStatus: Failed, + desiredMessage: "Helm release sync failed: failed to download \"fluxcd/flux\" (hint: running `helm repo update` may help)", + obj: &v2beta1.HelmRelease{ + Status: v2beta1.HelmReleaseStatus{ + Conditions: []metav1.Condition{ + {Type: "Ready", Status: "False", Message: "Helm release sync failed: failed to download \"fluxcd/flux\" (hint: running `helm repo update` may help)"}, + }, + }, + }, + objectKind: HelmReleaseObjectKind, + }, + { + name: "Kustomization with failed Ready condition", + desiredStatus: Failed, + desiredMessage: "Kustomization apply failed: failed to apply revision: main/1234567890", + obj: &kustomizev1.Kustomization{ + Status: kustomizev1.KustomizationStatus{ + Conditions: []metav1.Condition{ + {Type: "Ready", Status: "False", Message: "Kustomization apply failed: failed to apply revision: main/1234567890"}, + }, + }, + }, + objectKind: KustomizationObjectKind, + }, + { + name: "Kustomization with Suspended computed status", + desiredStatus: Suspended, + desiredMessage: "", + obj: &kustomizev1.Kustomization{ + Spec: kustomizev1.KustomizationSpec{ + Suspend: true, + }, + Status: kustomizev1.KustomizationStatus{ + Conditions: []metav1.Condition{ + {Type: "CustomCondition", Status: "CustomStatus", Message: "CustomMessage"}, + }, + }, + }, + objectKind: KustomizationObjectKind, + }, + { + name: "HelmRelease with NoStatus condition", + desiredStatus: NoStatus, + desiredMessage: "", + obj: &v2beta1.HelmRelease{ + Status: v2beta1.HelmReleaseStatus{ + Conditions: []metav1.Condition{ + {Type: "-", Status: "DoesNotMatter", Message: "CustomMessage"}, + }, + }, + }, + objectKind: HelmReleaseObjectKind, + }, + { + name: "Kustomization without Ready and without NoStatus conditions", + desiredStatus: Failed, + desiredMessage: "", + obj: &kustomizev1.Kustomization{ + Status: kustomizev1.KustomizationStatus{ + Conditions: []metav1.Condition{ + {Type: "CustomCondition", Status: "CustomStatus", Message: "CustomMessage"}, + }, + }, + }, + objectKind: KustomizationObjectKind, + }, + { + name: "HelmRelease with Ready condition and Reconciling computed status", + desiredStatus: Reconciling, + desiredMessage: "Reconciling message for HelmRelease", + obj: &v2beta1.HelmRelease{ + Status: v2beta1.HelmReleaseStatus{ + Conditions: []metav1.Condition{ + { + Type: "Ready", + Status: "Unknown", + Reason: "Progressing", + Message: "Reconciling message for HelmRelease", + }, + }, + }, + }, + objectKind: HelmReleaseObjectKind, + }, + { + name: "Kustomization with Available condition and Reconciling computed status", + desiredStatus: Reconciling, + desiredMessage: "Reconciling message for Kustomization", + obj: &kustomizev1.Kustomization{ + Status: kustomizev1.KustomizationStatus{ + Conditions: []metav1.Condition{ + { + Type: "Available", + Status: "Unknown", + Reason: "Progressing", + Message: "Reconciling message for Kustomization", + }, + }, + }, + }, + objectKind: KustomizationObjectKind, + }, + // TODO: Replace Kustomization with a Terraform object after Explorer starts supporting Terraform objects. + { + name: "Fake Terraform object with Ready condition and PendingAction computed status", + desiredStatus: PendingAction, + desiredMessage: "PendingAction message for Terraform object", + obj: &kustomizev1.Kustomization{ + Status: kustomizev1.KustomizationStatus{ + Conditions: []metav1.Condition{ + { + Type: "Ready", + Status: "Unknown", + Reason: "TerraformPlannedWithChanges", + Message: "PendingAction message for Terraform object", + }, + }, + }, + }, + objectKind: KustomizationObjectKind, + }, + { + name: "AutomatedClusterDiscovery with Ready condition", + desiredStatus: Success, + desiredMessage: "Applied revision: main/1234567890", + obj: &clusterreflectorv1alpha1.AutomatedClusterDiscovery{ + Status: clusterreflectorv1alpha1.AutomatedClusterDiscoveryStatus{ + Conditions: []metav1.Condition{ + { + Type: "Ready", + Status: "True", + Message: "Applied revision: main/1234567890", + }, + }, + }, + }, + objectKind: AutomatedClusterDiscoveryObjectKind, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := defaultStatusFunc(tt.obj, tt.objectKind) + assert.NoError(t, err) + + if got != tt.desiredStatus { + t.Errorf("Status() = %v, want %v", got, tt.desiredStatus) + } + + msg, err := defaultMessageFunc(tt.obj, tt.objectKind) + assert.NoError(t, err) + if msg != tt.desiredMessage { + t.Errorf("Message() = %v, want %v", got, tt.desiredMessage) + } + }) + } }