From a3ebe19c5729e17569b6deaa2371445548414c34 Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 13:39:28 +0200 Subject: [PATCH 01/22] add health check for crd --- pkg/health/health.go | 5 ++ pkg/health/health_customresourcedefinition.go | 83 +++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 pkg/health/health_customresourcedefinition.go diff --git a/pkg/health/health.go b/pkg/health/health.go index b93d8c967..8047ca263 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -116,6 +116,11 @@ func GetHealthCheckFunc(gvk schema.GroupVersionKind) func(obj *unstructured.Unst case kube.IngressKind: return getIngressHealth } + case "apiextensions": + switch gvk.Kind { + case kube.CustomResourceDefinitionKind: + return getCustomResourceDefinitionHealth + } case "argoproj.io": switch gvk.Kind { case "Workflow": diff --git a/pkg/health/health_customresourcedefinition.go b/pkg/health/health_customresourcedefinition.go new file mode 100644 index 000000000..76e53dbd8 --- /dev/null +++ b/pkg/health/health_customresourcedefinition.go @@ -0,0 +1,83 @@ +package health + +import ( + "fmt" + + "github.com/argoproj/gitops-engine/pkg/utils/kube" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" +) + +func getCustomResourceDefinitionHealth(obj *unstructured.Unstructured) (*HealthStatus, error) { + gvk := obj.GroupVersionKind() + switch gvk { + case apiextensionsv1.SchemeGroupVersion.WithKind(kube.CustomResourceDefinitionKind): + var crd apiextensionsv1.CustomResourceDefinition + err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &crd) + if err != nil { + return nil, fmt.Errorf("failed to convert unstructured CustomResourceDefinition to typed: %v", err) + } + return getApiExtenstionsV1CustomResourceDefinitionHealth(&crd) + default: + return nil, fmt.Errorf("unsupported CustomResourceDefinition GVK: %s", gvk) + } +} + +func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.CustomResourceDefinition) (*HealthStatus, error) { + if crd.ObjectMeta.DeletionTimestamp != nil { + return &HealthStatus{ + Status: HealthStatusProgressing, + Message: "CRD is being terminated", + }, nil + } + if crd.Status.Conditions == nil { + return &HealthStatus{ + Status: HealthStatusProgressing, + Message: "Status conditions not found", + }, nil + } + + var ( + isEstablished bool + hasViolations bool + violationMsg string + establishedMsg string + ) + + // Check conditions + for _, condition := range crd.Status.Conditions { + switch condition.Type { + case apiextensionsv1.Established: + if condition.Status == apiextensionsv1.ConditionTrue { + isEstablished = true + } else { + establishedMsg = condition.Message + } + case "NonStructuralSchema": + if condition.Status == apiextensionsv1.ConditionTrue { + hasViolations = true + violationMsg = condition.Message + } + } + } + + // Return appropriate health status + switch { + case !isEstablished: + return &HealthStatus{ + Status: HealthStatusDegraded, + Message: fmt.Sprintf("CRD is not established: %s", establishedMsg), + }, nil + case hasViolations: + return &HealthStatus{ + Status: HealthStatusDegraded, + Message: fmt.Sprintf("Schema violations found: %s", violationMsg), + }, nil + default: + return &HealthStatus{ + Status: HealthStatusHealthy, + Message: "CRD is healthy", + }, nil + } +} From d3d35d93151aa3406b1b2e8ec2b19da632ee3dd2 Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 15:36:16 +0200 Subject: [PATCH 02/22] deleted handling of deletionTimestamp, added more conditions to handle and changed condition msg to a signle variable to be returned --- pkg/health/health.go | 4 +- pkg/health/health_customresourcedefinition.go | 45 ++++++++++++------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/pkg/health/health.go b/pkg/health/health.go index 8047ca263..f3a7c12c3 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -70,7 +70,6 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver Message: "Pending deletion", }, nil } - if healthOverride != nil { health, err := healthOverride.GetResourceHealth(obj) if err != nil { @@ -84,7 +83,6 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver return health, nil } } - if healthCheck := GetHealthCheckFunc(obj.GroupVersionKind()); healthCheck != nil { if health, err = healthCheck(obj); err != nil { health = &HealthStatus{ @@ -116,7 +114,7 @@ func GetHealthCheckFunc(gvk schema.GroupVersionKind) func(obj *unstructured.Unst case kube.IngressKind: return getIngressHealth } - case "apiextensions": + case "apiextensions.k8s.io": switch gvk.Kind { case kube.CustomResourceDefinitionKind: return getCustomResourceDefinitionHealth diff --git a/pkg/health/health_customresourcedefinition.go b/pkg/health/health_customresourcedefinition.go index 76e53dbd8..7a880a75d 100644 --- a/pkg/health/health_customresourcedefinition.go +++ b/pkg/health/health_customresourcedefinition.go @@ -25,12 +25,6 @@ func getCustomResourceDefinitionHealth(obj *unstructured.Unstructured) (*HealthS } func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.CustomResourceDefinition) (*HealthStatus, error) { - if crd.ObjectMeta.DeletionTimestamp != nil { - return &HealthStatus{ - Status: HealthStatusProgressing, - Message: "CRD is being terminated", - }, nil - } if crd.Status.Conditions == nil { return &HealthStatus{ Status: HealthStatusProgressing, @@ -39,40 +33,61 @@ func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.Cust } var ( - isEstablished bool - hasViolations bool - violationMsg string - establishedMsg string + isEstablished bool + isTerminating bool + namesNotAccepted bool + hasViolations bool + conditionMsg string ) // Check conditions for _, condition := range crd.Status.Conditions { switch condition.Type { + case apiextensionsv1.Terminating: + if condition.Status == apiextensionsv1.ConditionTrue { + isTerminating = true + conditionMsg = condition.Message + } + case apiextensionsv1.NamesAccepted: + if condition.Status == apiextensionsv1.ConditionTrue { + namesNotAccepted = true + conditionMsg = condition.Message + } case apiextensionsv1.Established: if condition.Status == apiextensionsv1.ConditionTrue { isEstablished = true } else { - establishedMsg = condition.Message + conditionMsg = condition.Message } - case "NonStructuralSchema": + case apiextensionsv1.NonStructuralSchema: if condition.Status == apiextensionsv1.ConditionTrue { hasViolations = true - violationMsg = condition.Message + conditionMsg = condition.Message } } } // Return appropriate health status switch { + case isTerminating: + return &HealthStatus{ + Status: HealthStatusProgressing, + Message: fmt.Sprintf("CRD is being terminated: %s", conditionMsg), + }, nil + case namesNotAccepted: + return &HealthStatus{ + Status: HealthStatusDegraded, + Message: fmt.Sprintf("CRD names have not been accepted: %s", conditionMsg), + }, nil case !isEstablished: return &HealthStatus{ Status: HealthStatusDegraded, - Message: fmt.Sprintf("CRD is not established: %s", establishedMsg), + Message: fmt.Sprintf("CRD is not established: %s", conditionMsg), }, nil case hasViolations: return &HealthStatus{ Status: HealthStatusDegraded, - Message: fmt.Sprintf("Schema violations found: %s", violationMsg), + Message: fmt.Sprintf("Schema violations found: %s", conditionMsg), }, nil default: return &HealthStatus{ From 040ccba7c8641dbcee294861637b5393d4120cf5 Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 16:07:41 +0200 Subject: [PATCH 03/22] adding tests --- pkg/health/health_customresourcedefinition.go | 5 +- pkg/health/health_test.go | 8 +++ pkg/health/testdata/crd-v1-healthy.yaml | 56 +++++++++++++++++ .../crd-v1-no-conditions-progressing.yaml | 46 ++++++++++++++ .../crd-v1-non-structual-degraded.yaml | 61 +++++++++++++++++++ .../crd-v1-not-established-degraded.yaml | 56 +++++++++++++++++ .../crd-v1-terminating-progressing.yaml | 57 +++++++++++++++++ 7 files changed, 287 insertions(+), 2 deletions(-) create mode 100644 pkg/health/testdata/crd-v1-healthy.yaml create mode 100644 pkg/health/testdata/crd-v1-no-conditions-progressing.yaml create mode 100644 pkg/health/testdata/crd-v1-non-structual-degraded.yaml create mode 100644 pkg/health/testdata/crd-v1-not-established-degraded.yaml create mode 100644 pkg/health/testdata/crd-v1-terminating-progressing.yaml diff --git a/pkg/health/health_customresourcedefinition.go b/pkg/health/health_customresourcedefinition.go index 7a880a75d..0e3fa7b73 100644 --- a/pkg/health/health_customresourcedefinition.go +++ b/pkg/health/health_customresourcedefinition.go @@ -25,7 +25,8 @@ func getCustomResourceDefinitionHealth(obj *unstructured.Unstructured) (*HealthS } func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.CustomResourceDefinition) (*HealthStatus, error) { - if crd.Status.Conditions == nil { + + if crd.Status.Conditions == nil || crd.Status.Conditions != nil && len(crd.Status.Conditions) == 0 { return &HealthStatus{ Status: HealthStatusProgressing, Message: "Status conditions not found", @@ -49,7 +50,7 @@ func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.Cust conditionMsg = condition.Message } case apiextensionsv1.NamesAccepted: - if condition.Status == apiextensionsv1.ConditionTrue { + if condition.Status == apiextensionsv1.ConditionFalse { namesNotAccepted = true conditionMsg = condition.Message } diff --git a/pkg/health/health_test.go b/pkg/health/health_test.go index 45ff74941..deb55342e 100644 --- a/pkg/health/health_test.go +++ b/pkg/health/health_test.go @@ -118,6 +118,14 @@ func TestAPIService(t *testing.T) { assertAppHealth(t, "./testdata/apiservice-v1beta1-false.yaml", HealthStatusProgressing) } +func TestCustomResourceDefinitionHealth(t *testing.T) { + assertAppHealth(t, "./testdata/crd-v1-healthy.yaml", HealthStatusHealthy) + assertAppHealth(t, "./testdata/crd-v1-non-structual-degraded.yaml", HealthStatusDegraded) + assertAppHealth(t, "./testdata/crd-v1-not-established-degraded.yaml", HealthStatusDegraded) + assertAppHealth(t, "./testdata/crd-v1-terminating-progressing.yaml", HealthStatusProgressing) + assertAppHealth(t, "./testdata/crd-v1-no-conditions-progressing.yaml", HealthStatusProgressing) +} + func TestGetArgoWorkflowHealth(t *testing.T) { sampleWorkflow := unstructured.Unstructured{Object: map[string]interface{}{ "spec": map[string]interface{}{ diff --git a/pkg/health/testdata/crd-v1-healthy.yaml b/pkg/health/testdata/crd-v1-healthy.yaml new file mode 100644 index 000000000..09e00f70b --- /dev/null +++ b/pkg/health/testdata/crd-v1-healthy.yaml @@ -0,0 +1,56 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: no conflicts found + reason: NoConflicts + status: 'True' + type: NamesAccepted + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: the initial names have been accepted + reason: InitialNamesAccepted + status: 'True' + type: Established + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/pkg/health/testdata/crd-v1-no-conditions-progressing.yaml b/pkg/health/testdata/crd-v1-no-conditions-progressing.yaml new file mode 100644 index 000000000..f12ce016b --- /dev/null +++ b/pkg/health/testdata/crd-v1-no-conditions-progressing.yaml @@ -0,0 +1,46 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: [] + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/pkg/health/testdata/crd-v1-non-structual-degraded.yaml b/pkg/health/testdata/crd-v1-non-structual-degraded.yaml new file mode 100644 index 000000000..3c7a07f3e --- /dev/null +++ b/pkg/health/testdata/crd-v1-non-structual-degraded.yaml @@ -0,0 +1,61 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: no conflicts found + reason: NoConflicts + status: 'True' + type: NamesAccepted + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: the initial names have been accepted + reason: InitialNamesAccepted + status: 'True' + type: Established + - lastTransitionTime: '2024-10-26T19:44:57Z' + message: 'spec.preserveUnknownFields: Invalid value: true: must be false' + reason: Violations + status: 'True' + type: NonStructuralSchema + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/pkg/health/testdata/crd-v1-not-established-degraded.yaml b/pkg/health/testdata/crd-v1-not-established-degraded.yaml new file mode 100644 index 000000000..5982bfa53 --- /dev/null +++ b/pkg/health/testdata/crd-v1-not-established-degraded.yaml @@ -0,0 +1,56 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: no conflicts found + reason: NoConflicts + status: 'False' + type: NamesAccepted + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: the initial names have been accepted + reason: InitialNamesAccepted + status: 'False' + type: Established + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/pkg/health/testdata/crd-v1-terminating-progressing.yaml b/pkg/health/testdata/crd-v1-terminating-progressing.yaml new file mode 100644 index 000000000..73d19e483 --- /dev/null +++ b/pkg/health/testdata/crd-v1-terminating-progressing.yaml @@ -0,0 +1,57 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io + deletionTimestamp: '2024-12-28T13:51:19Z' +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: no conflicts found + reason: NoConflicts + status: 'True' + type: NamesAccepted + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: the initial names have been accepted + reason: InitialNamesAccepted + status: 'True' + type: Established + storedVersions: + - v1alpha1 \ No newline at end of file From 394ad04f3f8c249b2e1e9b8057d6656e144b6f0f Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 16:10:38 +0200 Subject: [PATCH 04/22] reverting blank lines --- pkg/health/health.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/health/health.go b/pkg/health/health.go index f3a7c12c3..d5e2fcd65 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -70,6 +70,7 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver Message: "Pending deletion", }, nil } + if healthOverride != nil { health, err := healthOverride.GetResourceHealth(obj) if err != nil { @@ -83,6 +84,7 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver return health, nil } } + if healthCheck := GetHealthCheckFunc(obj.GroupVersionKind()); healthCheck != nil { if health, err = healthCheck(obj); err != nil { health = &HealthStatus{ From fb0f086a45b6e4094aae54bc188309665fc34501 Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 13:39:28 +0200 Subject: [PATCH 05/22] add health check for crd Signed-off-by: Almo Elda --- pkg/health/health.go | 5 ++ pkg/health/health_customresourcedefinition.go | 83 +++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 pkg/health/health_customresourcedefinition.go diff --git a/pkg/health/health.go b/pkg/health/health.go index b93d8c967..8047ca263 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -116,6 +116,11 @@ func GetHealthCheckFunc(gvk schema.GroupVersionKind) func(obj *unstructured.Unst case kube.IngressKind: return getIngressHealth } + case "apiextensions": + switch gvk.Kind { + case kube.CustomResourceDefinitionKind: + return getCustomResourceDefinitionHealth + } case "argoproj.io": switch gvk.Kind { case "Workflow": diff --git a/pkg/health/health_customresourcedefinition.go b/pkg/health/health_customresourcedefinition.go new file mode 100644 index 000000000..76e53dbd8 --- /dev/null +++ b/pkg/health/health_customresourcedefinition.go @@ -0,0 +1,83 @@ +package health + +import ( + "fmt" + + "github.com/argoproj/gitops-engine/pkg/utils/kube" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" +) + +func getCustomResourceDefinitionHealth(obj *unstructured.Unstructured) (*HealthStatus, error) { + gvk := obj.GroupVersionKind() + switch gvk { + case apiextensionsv1.SchemeGroupVersion.WithKind(kube.CustomResourceDefinitionKind): + var crd apiextensionsv1.CustomResourceDefinition + err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &crd) + if err != nil { + return nil, fmt.Errorf("failed to convert unstructured CustomResourceDefinition to typed: %v", err) + } + return getApiExtenstionsV1CustomResourceDefinitionHealth(&crd) + default: + return nil, fmt.Errorf("unsupported CustomResourceDefinition GVK: %s", gvk) + } +} + +func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.CustomResourceDefinition) (*HealthStatus, error) { + if crd.ObjectMeta.DeletionTimestamp != nil { + return &HealthStatus{ + Status: HealthStatusProgressing, + Message: "CRD is being terminated", + }, nil + } + if crd.Status.Conditions == nil { + return &HealthStatus{ + Status: HealthStatusProgressing, + Message: "Status conditions not found", + }, nil + } + + var ( + isEstablished bool + hasViolations bool + violationMsg string + establishedMsg string + ) + + // Check conditions + for _, condition := range crd.Status.Conditions { + switch condition.Type { + case apiextensionsv1.Established: + if condition.Status == apiextensionsv1.ConditionTrue { + isEstablished = true + } else { + establishedMsg = condition.Message + } + case "NonStructuralSchema": + if condition.Status == apiextensionsv1.ConditionTrue { + hasViolations = true + violationMsg = condition.Message + } + } + } + + // Return appropriate health status + switch { + case !isEstablished: + return &HealthStatus{ + Status: HealthStatusDegraded, + Message: fmt.Sprintf("CRD is not established: %s", establishedMsg), + }, nil + case hasViolations: + return &HealthStatus{ + Status: HealthStatusDegraded, + Message: fmt.Sprintf("Schema violations found: %s", violationMsg), + }, nil + default: + return &HealthStatus{ + Status: HealthStatusHealthy, + Message: "CRD is healthy", + }, nil + } +} From fac4caa171a322512197349e8e88a351e82e00f0 Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 15:36:16 +0200 Subject: [PATCH 06/22] deleted handling of deletionTimestamp, added more conditions to handle and changed condition msg to a signle variable to be returned Signed-off-by: Almo Elda --- pkg/health/health.go | 4 +- pkg/health/health_customresourcedefinition.go | 45 ++++++++++++------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/pkg/health/health.go b/pkg/health/health.go index 8047ca263..f3a7c12c3 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -70,7 +70,6 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver Message: "Pending deletion", }, nil } - if healthOverride != nil { health, err := healthOverride.GetResourceHealth(obj) if err != nil { @@ -84,7 +83,6 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver return health, nil } } - if healthCheck := GetHealthCheckFunc(obj.GroupVersionKind()); healthCheck != nil { if health, err = healthCheck(obj); err != nil { health = &HealthStatus{ @@ -116,7 +114,7 @@ func GetHealthCheckFunc(gvk schema.GroupVersionKind) func(obj *unstructured.Unst case kube.IngressKind: return getIngressHealth } - case "apiextensions": + case "apiextensions.k8s.io": switch gvk.Kind { case kube.CustomResourceDefinitionKind: return getCustomResourceDefinitionHealth diff --git a/pkg/health/health_customresourcedefinition.go b/pkg/health/health_customresourcedefinition.go index 76e53dbd8..7a880a75d 100644 --- a/pkg/health/health_customresourcedefinition.go +++ b/pkg/health/health_customresourcedefinition.go @@ -25,12 +25,6 @@ func getCustomResourceDefinitionHealth(obj *unstructured.Unstructured) (*HealthS } func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.CustomResourceDefinition) (*HealthStatus, error) { - if crd.ObjectMeta.DeletionTimestamp != nil { - return &HealthStatus{ - Status: HealthStatusProgressing, - Message: "CRD is being terminated", - }, nil - } if crd.Status.Conditions == nil { return &HealthStatus{ Status: HealthStatusProgressing, @@ -39,40 +33,61 @@ func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.Cust } var ( - isEstablished bool - hasViolations bool - violationMsg string - establishedMsg string + isEstablished bool + isTerminating bool + namesNotAccepted bool + hasViolations bool + conditionMsg string ) // Check conditions for _, condition := range crd.Status.Conditions { switch condition.Type { + case apiextensionsv1.Terminating: + if condition.Status == apiextensionsv1.ConditionTrue { + isTerminating = true + conditionMsg = condition.Message + } + case apiextensionsv1.NamesAccepted: + if condition.Status == apiextensionsv1.ConditionTrue { + namesNotAccepted = true + conditionMsg = condition.Message + } case apiextensionsv1.Established: if condition.Status == apiextensionsv1.ConditionTrue { isEstablished = true } else { - establishedMsg = condition.Message + conditionMsg = condition.Message } - case "NonStructuralSchema": + case apiextensionsv1.NonStructuralSchema: if condition.Status == apiextensionsv1.ConditionTrue { hasViolations = true - violationMsg = condition.Message + conditionMsg = condition.Message } } } // Return appropriate health status switch { + case isTerminating: + return &HealthStatus{ + Status: HealthStatusProgressing, + Message: fmt.Sprintf("CRD is being terminated: %s", conditionMsg), + }, nil + case namesNotAccepted: + return &HealthStatus{ + Status: HealthStatusDegraded, + Message: fmt.Sprintf("CRD names have not been accepted: %s", conditionMsg), + }, nil case !isEstablished: return &HealthStatus{ Status: HealthStatusDegraded, - Message: fmt.Sprintf("CRD is not established: %s", establishedMsg), + Message: fmt.Sprintf("CRD is not established: %s", conditionMsg), }, nil case hasViolations: return &HealthStatus{ Status: HealthStatusDegraded, - Message: fmt.Sprintf("Schema violations found: %s", violationMsg), + Message: fmt.Sprintf("Schema violations found: %s", conditionMsg), }, nil default: return &HealthStatus{ From b4b762ce8bd61f46c31d6643df60eebbd91f3378 Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 16:07:41 +0200 Subject: [PATCH 07/22] adding tests Signed-off-by: Almo Elda --- pkg/health/health_customresourcedefinition.go | 5 +- pkg/health/health_test.go | 8 +++ pkg/health/testdata/crd-v1-healthy.yaml | 56 +++++++++++++++++ .../crd-v1-no-conditions-progressing.yaml | 46 ++++++++++++++ .../crd-v1-non-structual-degraded.yaml | 61 +++++++++++++++++++ .../crd-v1-not-established-degraded.yaml | 56 +++++++++++++++++ .../crd-v1-terminating-progressing.yaml | 57 +++++++++++++++++ 7 files changed, 287 insertions(+), 2 deletions(-) create mode 100644 pkg/health/testdata/crd-v1-healthy.yaml create mode 100644 pkg/health/testdata/crd-v1-no-conditions-progressing.yaml create mode 100644 pkg/health/testdata/crd-v1-non-structual-degraded.yaml create mode 100644 pkg/health/testdata/crd-v1-not-established-degraded.yaml create mode 100644 pkg/health/testdata/crd-v1-terminating-progressing.yaml diff --git a/pkg/health/health_customresourcedefinition.go b/pkg/health/health_customresourcedefinition.go index 7a880a75d..0e3fa7b73 100644 --- a/pkg/health/health_customresourcedefinition.go +++ b/pkg/health/health_customresourcedefinition.go @@ -25,7 +25,8 @@ func getCustomResourceDefinitionHealth(obj *unstructured.Unstructured) (*HealthS } func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.CustomResourceDefinition) (*HealthStatus, error) { - if crd.Status.Conditions == nil { + + if crd.Status.Conditions == nil || crd.Status.Conditions != nil && len(crd.Status.Conditions) == 0 { return &HealthStatus{ Status: HealthStatusProgressing, Message: "Status conditions not found", @@ -49,7 +50,7 @@ func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.Cust conditionMsg = condition.Message } case apiextensionsv1.NamesAccepted: - if condition.Status == apiextensionsv1.ConditionTrue { + if condition.Status == apiextensionsv1.ConditionFalse { namesNotAccepted = true conditionMsg = condition.Message } diff --git a/pkg/health/health_test.go b/pkg/health/health_test.go index 45ff74941..deb55342e 100644 --- a/pkg/health/health_test.go +++ b/pkg/health/health_test.go @@ -118,6 +118,14 @@ func TestAPIService(t *testing.T) { assertAppHealth(t, "./testdata/apiservice-v1beta1-false.yaml", HealthStatusProgressing) } +func TestCustomResourceDefinitionHealth(t *testing.T) { + assertAppHealth(t, "./testdata/crd-v1-healthy.yaml", HealthStatusHealthy) + assertAppHealth(t, "./testdata/crd-v1-non-structual-degraded.yaml", HealthStatusDegraded) + assertAppHealth(t, "./testdata/crd-v1-not-established-degraded.yaml", HealthStatusDegraded) + assertAppHealth(t, "./testdata/crd-v1-terminating-progressing.yaml", HealthStatusProgressing) + assertAppHealth(t, "./testdata/crd-v1-no-conditions-progressing.yaml", HealthStatusProgressing) +} + func TestGetArgoWorkflowHealth(t *testing.T) { sampleWorkflow := unstructured.Unstructured{Object: map[string]interface{}{ "spec": map[string]interface{}{ diff --git a/pkg/health/testdata/crd-v1-healthy.yaml b/pkg/health/testdata/crd-v1-healthy.yaml new file mode 100644 index 000000000..09e00f70b --- /dev/null +++ b/pkg/health/testdata/crd-v1-healthy.yaml @@ -0,0 +1,56 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: no conflicts found + reason: NoConflicts + status: 'True' + type: NamesAccepted + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: the initial names have been accepted + reason: InitialNamesAccepted + status: 'True' + type: Established + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/pkg/health/testdata/crd-v1-no-conditions-progressing.yaml b/pkg/health/testdata/crd-v1-no-conditions-progressing.yaml new file mode 100644 index 000000000..f12ce016b --- /dev/null +++ b/pkg/health/testdata/crd-v1-no-conditions-progressing.yaml @@ -0,0 +1,46 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: [] + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/pkg/health/testdata/crd-v1-non-structual-degraded.yaml b/pkg/health/testdata/crd-v1-non-structual-degraded.yaml new file mode 100644 index 000000000..3c7a07f3e --- /dev/null +++ b/pkg/health/testdata/crd-v1-non-structual-degraded.yaml @@ -0,0 +1,61 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: no conflicts found + reason: NoConflicts + status: 'True' + type: NamesAccepted + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: the initial names have been accepted + reason: InitialNamesAccepted + status: 'True' + type: Established + - lastTransitionTime: '2024-10-26T19:44:57Z' + message: 'spec.preserveUnknownFields: Invalid value: true: must be false' + reason: Violations + status: 'True' + type: NonStructuralSchema + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/pkg/health/testdata/crd-v1-not-established-degraded.yaml b/pkg/health/testdata/crd-v1-not-established-degraded.yaml new file mode 100644 index 000000000..5982bfa53 --- /dev/null +++ b/pkg/health/testdata/crd-v1-not-established-degraded.yaml @@ -0,0 +1,56 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: no conflicts found + reason: NoConflicts + status: 'False' + type: NamesAccepted + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: the initial names have been accepted + reason: InitialNamesAccepted + status: 'False' + type: Established + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/pkg/health/testdata/crd-v1-terminating-progressing.yaml b/pkg/health/testdata/crd-v1-terminating-progressing.yaml new file mode 100644 index 000000000..73d19e483 --- /dev/null +++ b/pkg/health/testdata/crd-v1-terminating-progressing.yaml @@ -0,0 +1,57 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io + deletionTimestamp: '2024-12-28T13:51:19Z' +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: no conflicts found + reason: NoConflicts + status: 'True' + type: NamesAccepted + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: the initial names have been accepted + reason: InitialNamesAccepted + status: 'True' + type: Established + storedVersions: + - v1alpha1 \ No newline at end of file From 3b97a12048b1f95336327c468f060493750f8a0c Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 16:10:38 +0200 Subject: [PATCH 08/22] reverting blank lines Signed-off-by: Almo Elda --- pkg/health/health.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/health/health.go b/pkg/health/health.go index f3a7c12c3..d5e2fcd65 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -70,6 +70,7 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver Message: "Pending deletion", }, nil } + if healthOverride != nil { health, err := healthOverride.GetResourceHealth(obj) if err != nil { @@ -83,6 +84,7 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver return health, nil } } + if healthCheck := GetHealthCheckFunc(obj.GroupVersionKind()); healthCheck != nil { if health, err = healthCheck(obj); err != nil { health = &HealthStatus{ From a62ee8ffba52f87971a74651b186d42ab0a6238e Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 16:10:38 +0200 Subject: [PATCH 09/22] reverting blank lines Signed-off-by: Almo Elda --- pkg/health/health.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/health/health.go b/pkg/health/health.go index f3a7c12c3..d5e2fcd65 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -70,6 +70,7 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver Message: "Pending deletion", }, nil } + if healthOverride != nil { health, err := healthOverride.GetResourceHealth(obj) if err != nil { @@ -83,6 +84,7 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver return health, nil } } + if healthCheck := GetHealthCheckFunc(obj.GroupVersionKind()); healthCheck != nil { if health, err = healthCheck(obj); err != nil { health = &HealthStatus{ From 5d64820f8bb8bc9d861810543496576babbd0f5a Mon Sep 17 00:00:00 2001 From: "JM (Jason Meridth)" Date: Tue, 26 Nov 2024 13:45:57 -0600 Subject: [PATCH 10/22] chore(deps): upgrade go version in dockerfile (#638) - [x] fix warnings about case of `as` to `AS` in Dockerfile - `FromAsCasing: 'as' and 'FROM' keywords' casing do not match (line 1)` - [x] shorten go version in go.mod - [x] update Dockerfile Go version from 1.17 to 1.22 to match go.mod - [x] upgrade alipine/git image version to latest, current was 4 years old - -from alpine/git:v2.24.3 (4 years old) to alpine/git:v2.45.2 - [x] fix warning with linting - `WARN [config_reader] The configuration option 'run.skip-files' is deprecated, please use 'issues.exclude-files'` - [x] add .tool-versions (asdf) to .gitignore Signed-off-by: jmeridth Signed-off-by: Almo Elda --- .gitignore | 3 ++- .golangci.yaml | 6 +++--- Dockerfile | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index 1c66b8a42..72853bf32 100644 --- a/.gitignore +++ b/.gitignore @@ -3,4 +3,5 @@ .vscode .idea coverage.out -vendor/ \ No newline at end of file +vendor/ +.tool-versions diff --git a/.golangci.yaml b/.golangci.yaml index fbefc226d..c516a7c99 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,3 +1,3 @@ -run: - skip-files: - - "pkg/diff/internal/fieldmanager/borrowed_.+\\.go$" +issues: + exclude-files: + - "pkg/diff/internal/fieldmanager/borrowed_.*\\.go$" diff --git a/Dockerfile b/Dockerfile index 73140debb..64208ef0f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.17 as builder +FROM golang:1.22 AS builder WORKDIR /src @@ -12,5 +12,5 @@ COPY . . RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -ldflags="-w -s" -o /dist/gitops ./agent -FROM alpine/git:v2.24.3 +FROM alpine/git:v2.45.2 COPY --from=builder /dist/gitops /usr/local/bin/gitops From 14ea2656ac4cdf4ec06833b167b86bc1702172b5 Mon Sep 17 00:00:00 2001 From: Andrii Korotkov <137232734+andrii-korotkov-verkada@users.noreply.github.com> Date: Wed, 11 Dec 2024 12:28:47 -0800 Subject: [PATCH 11/22] fix: Server side diff now works correctly with fields removal (#640) * fix: Server side diff now works correctly with some fields removal Helps with https://github.com/argoproj/argo-cd/issues/20792 Removed and modified sets may only contain the fields that changed, not including key fields like "name". This can cause merge to fail, since it expects those fields to be present if they are present in the predicted live. Fortunately, we can inspect the set and derive the key fields necessary. Then they can be added to the set and used during a merge. Also, have a new test which fails before the fix, but passes now. Failure of the new test before the fix ``` Error: Received unexpected error: error removing non config mutations for resource Deployment/nginx-deployment: error reverting webhook removed fields in predicted live resource: .spec.template.spec.containers: element 0: associative list with keys has an element that omits key field "name" (and doesn't have default value) Test: TestServerSideDiff/will_test_removing_some_field_with_undoing_changes_done_by_webhook ``` Signed-off-by: Andrii Korotkov * Use new version of structured merge diff with a new option Signed-off-by: Andrii Korotkov * Add DCO Signed-off-by: Andrii Korotkov * Try to fix sonar exclusions config Signed-off-by: Andrii Korotkov --------- Signed-off-by: Andrii Korotkov Signed-off-by: Almo Elda --- go.mod | 2 +- go.sum | 4 +- pkg/diff/diff.go | 4 +- pkg/diff/diff_test.go | 25 +++ pkg/diff/testdata/data.go | 9 + pkg/diff/testdata/smd-deploy2-config.yaml | 36 ++++ pkg/diff/testdata/smd-deploy2-live.yaml | 161 ++++++++++++++++++ .../testdata/smd-deploy2-predicted-live.json | 124 ++++++++++++++ sonar-project.properties | 2 + 9 files changed, 362 insertions(+), 5 deletions(-) create mode 100644 pkg/diff/testdata/smd-deploy2-config.yaml create mode 100644 pkg/diff/testdata/smd-deploy2-live.yaml create mode 100644 pkg/diff/testdata/smd-deploy2-predicted-live.json create mode 100644 sonar-project.properties diff --git a/go.mod b/go.mod index 6b2f29ef5..9d7a5c4a9 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 k8s.io/kubectl v0.31.2 k8s.io/kubernetes v1.31.0 - sigs.k8s.io/structured-merge-diff/v4 v4.4.3 + sigs.k8s.io/structured-merge-diff/v4 v4.4.4-0.20241211184406-7bf59b3d70ee sigs.k8s.io/yaml v1.4.0 ) diff --git a/go.sum b/go.sum index 3f4885310..93736539b 100644 --- a/go.sum +++ b/go.sum @@ -328,7 +328,7 @@ sigs.k8s.io/kustomize/api v0.17.2 h1:E7/Fjk7V5fboiuijoZHgs4aHuexi5Y2loXlVOAVAG5g sigs.k8s.io/kustomize/api v0.17.2/go.mod h1:UWTz9Ct+MvoeQsHcJ5e+vziRRkwimm3HytpZgIYqye0= sigs.k8s.io/kustomize/kyaml v0.17.1 h1:TnxYQxFXzbmNG6gOINgGWQt09GghzgTP6mIurOgrLCQ= sigs.k8s.io/kustomize/kyaml v0.17.1/go.mod h1:9V0mCjIEYjlXuCdYsSXvyoy2BTsLESH7TlGV81S282U= -sigs.k8s.io/structured-merge-diff/v4 v4.4.3 h1:sCP7Vv3xx/CWIuTPVN38lUPx0uw0lcLfzaiDa8Ja01A= -sigs.k8s.io/structured-merge-diff/v4 v4.4.3/go.mod h1:N8f93tFZh9U6vpxwRArLiikrE5/2tiu1w1AGfACIGE4= +sigs.k8s.io/structured-merge-diff/v4 v4.4.4-0.20241211184406-7bf59b3d70ee h1:ipT2c6nEOdAfBwiwW1oI0mkrlPabbXEFmJBrg6B+OR8= +sigs.k8s.io/structured-merge-diff/v4 v4.4.4-0.20241211184406-7bf59b3d70ee/go.mod h1:N8f93tFZh9U6vpxwRArLiikrE5/2tiu1w1AGfACIGE4= sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E= sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY= diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 1b43ea0ce..1fceea0a2 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -258,7 +258,7 @@ func removeWebhookMutation(predictedLive, live *unstructured.Unstructured, gvkPa } if comparison.Modified != nil && !comparison.Modified.Empty() { - liveModValues := typedLive.ExtractItems(comparison.Modified) + liveModValues := typedLive.ExtractItems(comparison.Modified, typed.WithAppendKeyFields()) // revert modified fields not owned by any manager typedPredictedLive, err = typedPredictedLive.Merge(liveModValues) if err != nil { @@ -267,7 +267,7 @@ func removeWebhookMutation(predictedLive, live *unstructured.Unstructured, gvkPa } if comparison.Removed != nil && !comparison.Removed.Empty() { - liveRmValues := typedLive.ExtractItems(comparison.Removed) + liveRmValues := typedLive.ExtractItems(comparison.Removed, typed.WithAppendKeyFields()) // revert removed fields not owned by any manager typedPredictedLive, err = typedPredictedLive.Merge(liveRmValues) if err != nil { diff --git a/pkg/diff/diff_test.go b/pkg/diff/diff_test.go index 806919a1c..d3b5f32b2 100644 --- a/pkg/diff/diff_test.go +++ b/pkg/diff/diff_test.go @@ -933,6 +933,31 @@ func TestServerSideDiff(t *testing.T) { assert.Empty(t, liveSVC.Annotations[AnnotationLastAppliedConfig]) assert.Empty(t, predictedSVC.Labels["event"]) }) + + t.Run("will test removing some field with undoing changes done by webhook", func(t *testing.T) { + // given + t.Parallel() + liveState := StrToUnstructured(testdata.Deployment2LiveYAML) + desiredState := StrToUnstructured(testdata.Deployment2ConfigYAML) + opts := buildOpts(testdata.Deployment2PredictedLiveJSONSSD) + + // when + result, err := serverSideDiff(desiredState, liveState, opts...) + + // then + require.NoError(t, err) + assert.NotNil(t, result) + assert.True(t, result.Modified) + predictedDeploy := YamlToDeploy(t, result.PredictedLive) + liveDeploy := YamlToDeploy(t, result.NormalizedLive) + assert.Len(t, predictedDeploy.Spec.Template.Spec.Containers, 1) + assert.Len(t, liveDeploy.Spec.Template.Spec.Containers, 1) + assert.Equal(t, "500m", predictedDeploy.Spec.Template.Spec.Containers[0].Resources.Requests.Cpu().String()) + assert.Equal(t, "512Mi", predictedDeploy.Spec.Template.Spec.Containers[0].Resources.Requests.Memory().String()) + assert.Equal(t, "500m", liveDeploy.Spec.Template.Spec.Containers[0].Resources.Requests.Cpu().String()) + assert.Equal(t, "512Mi", liveDeploy.Spec.Template.Spec.Containers[0].Resources.Requests.Memory().String()) + }) + t.Run("will include mutation webhook modifications", func(t *testing.T) { // given t.Parallel() diff --git a/pkg/diff/testdata/data.go b/pkg/diff/testdata/data.go index 047f3190b..fd2a7ae96 100644 --- a/pkg/diff/testdata/data.go +++ b/pkg/diff/testdata/data.go @@ -24,6 +24,15 @@ var ( //go:embed smd-deploy-config.yaml DeploymentConfigYAML string + //go:embed smd-deploy2-live.yaml + Deployment2LiveYAML string + + //go:embed smd-deploy2-config.yaml + Deployment2ConfigYAML string + + //go:embed smd-deploy2-predicted-live.json + Deployment2PredictedLiveJSONSSD string + // OpenAPIV2Doc is a binary representation of the openapi // document available in a given k8s instance. To update // this file the following commands can be executed: diff --git a/pkg/diff/testdata/smd-deploy2-config.yaml b/pkg/diff/testdata/smd-deploy2-config.yaml new file mode 100644 index 000000000..1c141e3e1 --- /dev/null +++ b/pkg/diff/testdata/smd-deploy2-config.yaml @@ -0,0 +1,36 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: missing + applications.argoproj.io/app-name: nginx + something-else: bla + name: nginx-deployment + namespace: default +spec: + replicas: 2 + selector: + matchLabels: + app: nginx + template: + metadata: + labels: + app: nginx + applications.argoproj.io/app-name: nginx + spec: + containers: + - image: 'nginx:1.23.1' + imagePullPolicy: Never + livenessProbe: + exec: + command: + - cat + - non-existent-file + initialDelaySeconds: 5 + periodSeconds: 180 + name: nginx + ports: + - containerPort: 8081 + protocol: UDP + - containerPort: 80 + protocol: TCP diff --git a/pkg/diff/testdata/smd-deploy2-live.yaml b/pkg/diff/testdata/smd-deploy2-live.yaml new file mode 100644 index 000000000..94535a85d --- /dev/null +++ b/pkg/diff/testdata/smd-deploy2-live.yaml @@ -0,0 +1,161 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: '1' + creationTimestamp: '2022-09-18T23:50:25Z' + generation: 1 + labels: + app: missing + applications.argoproj.io/app-name: nginx + something-else: bla + managedFields: + - apiVersion: apps/v1 + fieldsType: FieldsV1 + fieldsV1: + 'f:metadata': + 'f:labels': + 'f:app': {} + 'f:applications.argoproj.io/app-name': {} + 'f:something-else': {} + 'f:spec': + 'f:replicas': {} + 'f:selector': {} + 'f:template': + 'f:metadata': + 'f:labels': + 'f:app': {} + 'f:applications.argoproj.io/app-name': {} + 'f:spec': + 'f:containers': + 'k:{"name":"nginx"}': + .: {} + 'f:image': {} + 'f:imagePullPolicy': {} + 'f:livenessProbe': + 'f:exec': + 'f:command': {} + 'f:initialDelaySeconds': {} + 'f:periodSeconds': {} + 'f:name': {} + 'f:ports': + 'k:{"containerPort":80,"protocol":"TCP"}': + .: {} + 'f:containerPort': {} + 'f:protocol': {} + 'f:resources': + 'f:requests': + 'f:cpu': {} + 'f:memory': {} + manager: argocd-controller + operation: Apply + time: '2022-09-18T23:50:25Z' + - apiVersion: apps/v1 + fieldsType: FieldsV1 + fieldsV1: + 'f:metadata': + 'f:annotations': + .: {} + 'f:deployment.kubernetes.io/revision': {} + 'f:status': + 'f:availableReplicas': {} + 'f:conditions': + .: {} + 'k:{"type":"Available"}': + .: {} + 'f:lastTransitionTime': {} + 'f:lastUpdateTime': {} + 'f:message': {} + 'f:reason': {} + 'f:status': {} + 'f:type': {} + 'k:{"type":"Progressing"}': + .: {} + 'f:lastTransitionTime': {} + 'f:lastUpdateTime': {} + 'f:message': {} + 'f:reason': {} + 'f:status': {} + 'f:type': {} + 'f:observedGeneration': {} + 'f:readyReplicas': {} + 'f:replicas': {} + 'f:updatedReplicas': {} + manager: kube-controller-manager + operation: Update + subresource: status + time: '2022-09-23T18:30:59Z' + name: nginx-deployment + namespace: default + resourceVersion: '7492752' + uid: 731f7434-d3d9-47fa-b179-d9368a84f7c9 +spec: + progressDeadlineSeconds: 600 + replicas: 2 + revisionHistoryLimit: 10 + selector: + matchLabels: + app: nginx + strategy: + rollingUpdate: + maxSurge: 25% + maxUnavailable: 25% + type: RollingUpdate + template: + metadata: + creationTimestamp: null + labels: + app: nginx + applications.argoproj.io/app-name: nginx + spec: + containers: + - image: 'nginx:1.23.1' + imagePullPolicy: Never + livenessProbe: + exec: + command: + - cat + - non-existent-file + failureThreshold: 3 + initialDelaySeconds: 5 + periodSeconds: 180 + successThreshold: 1 + timeoutSeconds: 1 + name: nginx + ports: + - containerPort: 80 + protocol: TCP + - containerPort: 8080 + protocol: TCP + - containerPort: 8081 + protocol: UDP + resources: + requests: + memory: 512Mi + cpu: 500m + terminationMessagePath: /dev/termination-log + terminationMessagePolicy: File + dnsPolicy: ClusterFirst + restartPolicy: Always + schedulerName: default-scheduler + securityContext: {} + terminationGracePeriodSeconds: 30 +status: + availableReplicas: 2 + conditions: + - lastTransitionTime: '2022-09-18T23:50:25Z' + lastUpdateTime: '2022-09-18T23:50:26Z' + message: ReplicaSet "nginx-deployment-6d68ff5f86" has successfully progressed. + reason: NewReplicaSetAvailable + status: 'True' + type: Progressing + - lastTransitionTime: '2022-09-23T18:30:59Z' + lastUpdateTime: '2022-09-23T18:30:59Z' + message: Deployment has minimum availability. + reason: MinimumReplicasAvailable + status: 'True' + type: Available + observedGeneration: 1 + readyReplicas: 2 + replicas: 2 + updatedReplicas: 2 diff --git a/pkg/diff/testdata/smd-deploy2-predicted-live.json b/pkg/diff/testdata/smd-deploy2-predicted-live.json new file mode 100644 index 000000000..8f6f26874 --- /dev/null +++ b/pkg/diff/testdata/smd-deploy2-predicted-live.json @@ -0,0 +1,124 @@ +{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "labels": { + "app": "missing", + "applications.argoproj.io/app-name": "nginx", + "something-else": "bla" + }, + "name": "nginx-deployment", + "namespace": "default", + "managedFields": [ + { + "apiVersion": "apps/v1", + "fieldsType": "FieldsV1", + "fieldsV1": { + "f:metadata": { + "f:labels": { + "f:app": {}, + "f:applications.argoproj.io/app-name": {}, + "f:something-else": {} + } + }, + "f:spec": { + "f:replicas": {}, + "f:selector": {}, + "f:template": { + "f:metadata": { + "f:labels": { + "f:app": {}, + "f:applications.argoproj.io/app-name": {} + } + }, + "f:spec": { + "f:containers": { + "k:{\"name\":\"nginx\"}": { + ".": {}, + "f:image": {}, + "f:imagePullPolicy": {}, + "f:livenessProbe": { + "f:exec": { + "f:command": {} + }, + "f:initialDelaySeconds": {}, + "f:periodSeconds": {} + }, + "f:name": {}, + "f:ports": { + "k:{\"containerPort\":80,\"protocol\":\"TCP\"}": { + ".": {}, + "f:containerPort": {}, + "f:protocol": {} + } + }, + "f:resources": { + "f:requests": { + "f:cpu": {}, + "f:memory": {} + } + } + } + } + } + } + } + }, + "manager": "argocd-controller", + "operation": "Apply", + "time": "2022-09-18T23:50:25Z" + } + ] + }, + "spec": { + "replicas": 2, + "selector": { + "matchLabels": { + "app": "nginx" + } + }, + "template": { + "metadata": { + "labels": { + "app": "nginx", + "applications.argoproj.io/app-name": "nginx" + } + }, + "spec": { + "containers": [ + { + "image": "nginx:1.23.1", + "imagePullPolicy": "Never", + "livenessProbe": { + "exec": { + "command": [ + "cat", + "non-existent-file" + ] + }, + "initialDelaySeconds": 5, + "periodSeconds": 180 + }, + "name": "nginx", + "ports": [ + { + "containerPort": 8081, + "protocol": "UDP" + }, + { + "containerPort": 80, + "protocol": "TCP" + } + ], + "resources": { + "requests": { + "memory": "512Mi", + "cpu": "500m" + } + } + } + ] + } + } + } +} diff --git a/sonar-project.properties b/sonar-project.properties new file mode 100644 index 000000000..c611b779d --- /dev/null +++ b/sonar-project.properties @@ -0,0 +1,2 @@ +# Exclude test manifests from analysis +sonar.exclusions=pkg/diff/testdata/** From 2045db166e533a72a88297dbb3eefdc23ebbcecd Mon Sep 17 00:00:00 2001 From: "JM (Jason Meridth)" Date: Thu, 12 Dec 2024 10:31:38 -0600 Subject: [PATCH 12/22] fix: github actions versions and warnings (#639) * fix: github actions versions and warnings - [x] upgrade github/actions/cache GitHub Action to latest - this fixings the following warnings (example list [here](https://github.com/argoproj/gitops-engine/actions/runs/11885468091)): - Your workflow is using a version of actions/cache that is scheduled for deprecation, actions/cache@v2.1.6. Please update your workflow to use the latest version of actions/cache to avoid interruptions. Learn more: https://github.blog/changelog/2024-09-16-notice-of-upcoming-deprecations-and-changes-in-github-actions-services/ - The `save-state` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/ - The `set-output` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/ - [x] update dependabot config - prefix PRs with `chore(deps)` - group non major version updates into 1 PR Signed-off-by: jmeridth * fix: switch to SHA from tags more secure, as tags are mutable Signed-off-by: jmeridth --------- Signed-off-by: jmeridth Signed-off-by: Almo Elda --- .github/dependabot.yml | 16 ++++++++++++++++ .github/workflows/ci.yaml | 10 +++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index d30f881ef..862bf6629 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,7 +4,23 @@ updates: directory: "/" schedule: interval: "daily" + commit-message: + prefix: "chore(deps)" + groups: + dependencies: + applies-to: version-updates + update-types: + - "minor" + - "patch" - package-ecosystem: "github-actions" directory: "/" schedule: interval: "daily" + commit-message: + prefix: "chore(deps)" + groups: + dependencies: + applies-to: version-updates + update-types: + - "minor" + - "patch" diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 19e7a7676..78b1a3c8b 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -12,23 +12,23 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@master - - uses: actions/cache@v2.1.6 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2 with: path: ~/go/pkg/mod key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} restore-keys: | ${{ runner.os }}-go- - - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.0 + - uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed # v5.1.0 with: go-version-file: go.mod - run: make test - name: Run golangci-lint - uses: golangci/golangci-lint-action@08e2f20817b15149a52b5b3ebe7de50aff2ba8c5 # v3.4.0 + uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 # v6.1.1 with: version: v1.58.2 args: --timeout 5m - - uses: codecov/codecov-action@v3.1.0 + - uses: codecov/codecov-action@a2f73fb6db51fcd2e0aa085dfb36dea90c5e3689 # v5.0.2 with: token: ${{ secrets.CODECOV_TOKEN }} #required file: ./coverage.out From 6d883a2b356c0c15243f516eff000b3086c40b63 Mon Sep 17 00:00:00 2001 From: "JM (Jason Meridth)" Date: Thu, 12 Dec 2024 10:56:51 -0600 Subject: [PATCH 13/22] fix: run go mod tidy in ci workflow (#652) Fixes issue that showed up in https://github.com/argoproj/gitops-engine/pull/650 [Error](https://github.com/argoproj/gitops-engine/actions/runs/12300709584/job/34329534904?pr=650#step:5:96) Signed-off-by: jmeridth Signed-off-by: Almo Elda --- .github/workflows/ci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 78b1a3c8b..d40d4dcf7 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -22,6 +22,7 @@ jobs: - uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed # v5.1.0 with: go-version-file: go.mod + - run: go mod tidy - run: make test - name: Run golangci-lint uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 # v6.1.1 From f932f1aca5afae564c335a2ac07a973b96eedf4c Mon Sep 17 00:00:00 2001 From: "JM (Jason Meridth)" Date: Thu, 12 Dec 2024 15:22:31 -0600 Subject: [PATCH 14/22] chore: add CODEOWNERS (#641) * chore: add CODEOWNERS and EMERITUS.md Setting up CODEOWNERS file so people are automatically notified about new PRs. Can eventually setup ruleset that requires at least one review before merging. I based currently list in CODEOWNERS on people who have recently merged PRs. I'm wondering if reviewers and approvers are a team/group instead of list of folks. Setup EMERITUS.md that contains list from OWNERS. Feedback on this PR will update this PR. Signed-off-by: jmeridth * chore: match this repo's CODEOWNERS to argoproj/argo-cd CODEOWNERS Signed-off-by: jmeridth --------- Signed-off-by: jmeridth Signed-off-by: Almo Elda --- .github/CODEOWNERS | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 000000000..40feb4e7e --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,10 @@ +# All +** @argoproj/argocd-approvers + +# Docs +/docs/** @argoproj/argocd-approvers @argoproj/argocd-approvers-docs +/README.md @argoproj/argocd-approvers @argoproj/argocd-approvers-docs + +# CI +/.codecov.yml @argoproj/argocd-approvers @argoproj/argocd-approvers-ci +/.github/** @argoproj/argocd-approvers @argoproj/argocd-approvers-ci From 6232e520c20d108681c9cd036cfba4344663fca4 Mon Sep 17 00:00:00 2001 From: "JM (Jason Meridth)" Date: Thu, 12 Dec 2024 15:23:57 -0600 Subject: [PATCH 15/22] chore: update README get involved links (#647) Change links: - [x] from kubernetes slack to cncf slack - [x] from k8s gitop channel to cncf argo-cd-contributors channel Signed-off-by: jmeridth Signed-off-by: Almo Elda --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 633515ded..de8d75995 100644 --- a/README.md +++ b/README.md @@ -31,10 +31,10 @@ The GitOps Engine follows the [CNCF Code of Conduct](https://github.com/cncf/fou If you are as excited about GitOps and one common engine for it as much as we are, please get in touch. If you want to write code that's great, if you want to share feedback, ideas and use-cases, that's great too. -Find us on the [#gitops channel][gitops-slack] on Kubernetes Slack (get an [invite here][kube-slack]). +Find us on the [#argo-cd-contributors][argo-cd-contributors-slack] on CNCF Slack (get an [invite here][cncf-slack]). -[gitops-slack]: https://kubernetes.slack.com/archives/CBT6N1ASG -[kube-slack]: https://slack.k8s.io/ +[argo-cd-contributors-slack]: https://cloud-native.slack.com/archives/C020XM04CUW +[cncf-slack]: https://slack.cncf.io/ ### Contributing to the effort From 08289bc3d843d4a568cb416df228da0c2e0dfde2 Mon Sep 17 00:00:00 2001 From: Mykola Pelekh Date: Mon, 16 Dec 2024 17:52:26 +0200 Subject: [PATCH 16/22] fix: avoid resources lock contention utilizing channel (#629) * fix: avoid resources lock contention utilizing channel Signed-off-by: Mykola Pelekh * feat: process events in batch when the mode is enabled (default is `false`) Signed-off-by: Mykola Pelekh * test: update unit tests to verify batch events processing flag Signed-off-by: Mykola Pelekh * feat: make eventProcessingInterval option configurable (default is 0.1s) Signed-off-by: Mykola Pelekh * fixup! feat: make eventProcessingInterval option configurable (default is 0.1s) Signed-off-by: Mykola Pelekh --------- Signed-off-by: Mykola Pelekh Signed-off-by: Almo Elda --- pkg/cache/cluster.go | 142 ++++++++++++++++++++++++++++++-- pkg/cache/cluster_test.go | 122 +++++++++++++++------------ pkg/cache/mocks/ClusterCache.go | 20 +++++ pkg/cache/settings.go | 14 ++++ pkg/cache/settings_test.go | 17 ++++ 5 files changed, 252 insertions(+), 63 deletions(-) diff --git a/pkg/cache/cluster.go b/pkg/cache/cluster.go index 3f662effc..7e2e621fc 100644 --- a/pkg/cache/cluster.go +++ b/pkg/cache/cluster.go @@ -57,6 +57,8 @@ const ( // Limit is required to avoid memory spikes during cache initialization. // The default limit of 50 is chosen based on experiments. defaultListSemaphoreWeight = 50 + // defaultEventProcessingInterval is the default interval for processing events + defaultEventProcessingInterval = 100 * time.Millisecond ) const ( @@ -75,6 +77,11 @@ type apiMeta struct { watchCancel context.CancelFunc } +type eventMeta struct { + event watch.EventType + un *unstructured.Unstructured +} + // ClusterInfo holds cluster cache stats type ClusterInfo struct { // Server holds cluster API server URL @@ -96,6 +103,9 @@ type ClusterInfo struct { // OnEventHandler is a function that handles Kubernetes event type OnEventHandler func(event watch.EventType, un *unstructured.Unstructured) +// OnProcessEventsHandler handles process events event +type OnProcessEventsHandler func(duration time.Duration, processedEventsNumber int) + // OnPopulateResourceInfoHandler returns additional resource metadata that should be stored in cache type OnPopulateResourceInfoHandler func(un *unstructured.Unstructured, isRoot bool) (info interface{}, cacheManifest bool) @@ -137,6 +147,8 @@ type ClusterCache interface { OnResourceUpdated(handler OnResourceUpdatedHandler) Unsubscribe // OnEvent register event handler that is executed every time when new K8S event received OnEvent(handler OnEventHandler) Unsubscribe + // OnProcessEventsHandler register event handler that is executed every time when events were processed + OnProcessEventsHandler(handler OnProcessEventsHandler) Unsubscribe } type WeightedSemaphore interface { @@ -153,6 +165,7 @@ func NewClusterCache(config *rest.Config, opts ...UpdateSettingsFunc) *clusterCa cache := &clusterCache{ settings: Settings{ResourceHealthOverride: &noopSettings{}, ResourcesFilter: &noopSettings{}}, apisMeta: make(map[schema.GroupKind]*apiMeta), + eventMetaCh: nil, listPageSize: defaultListPageSize, listPageBufferSize: defaultListPageBufferSize, listSemaphore: semaphore.NewWeighted(defaultListSemaphoreWeight), @@ -169,8 +182,10 @@ func NewClusterCache(config *rest.Config, opts ...UpdateSettingsFunc) *clusterCa }, watchResyncTimeout: defaultWatchResyncTimeout, clusterSyncRetryTimeout: ClusterRetryTimeout, + eventProcessingInterval: defaultEventProcessingInterval, resourceUpdatedHandlers: map[uint64]OnResourceUpdatedHandler{}, eventHandlers: map[uint64]OnEventHandler{}, + processEventsHandlers: map[uint64]OnProcessEventsHandler{}, log: log, listRetryLimit: 1, listRetryUseBackoff: false, @@ -185,9 +200,11 @@ func NewClusterCache(config *rest.Config, opts ...UpdateSettingsFunc) *clusterCa type clusterCache struct { syncStatus clusterCacheSync - apisMeta map[schema.GroupKind]*apiMeta - serverVersion string - apiResources []kube.APIResourceInfo + apisMeta map[schema.GroupKind]*apiMeta + batchEventsProcessing bool + eventMetaCh chan eventMeta + serverVersion string + apiResources []kube.APIResourceInfo // namespacedResources is a simple map which indicates a groupKind is namespaced namespacedResources map[schema.GroupKind]bool @@ -195,6 +212,8 @@ type clusterCache struct { watchResyncTimeout time.Duration // sync retry timeout for cluster when sync error happens clusterSyncRetryTimeout time.Duration + // ticker interval for events processing + eventProcessingInterval time.Duration // size of a page for list operations pager. listPageSize int64 @@ -224,6 +243,7 @@ type clusterCache struct { populateResourceInfoHandler OnPopulateResourceInfoHandler resourceUpdatedHandlers map[uint64]OnResourceUpdatedHandler eventHandlers map[uint64]OnEventHandler + processEventsHandlers map[uint64]OnProcessEventsHandler openAPISchema openapi.Resources gvkParser *managedfields.GvkParser @@ -299,6 +319,29 @@ func (c *clusterCache) getEventHandlers() []OnEventHandler { return handlers } +// OnProcessEventsHandler register event handler that is executed every time when events were processed +func (c *clusterCache) OnProcessEventsHandler(handler OnProcessEventsHandler) Unsubscribe { + c.handlersLock.Lock() + defer c.handlersLock.Unlock() + key := c.handlerKey + c.handlerKey++ + c.processEventsHandlers[key] = handler + return func() { + c.handlersLock.Lock() + defer c.handlersLock.Unlock() + delete(c.processEventsHandlers, key) + } +} +func (c *clusterCache) getProcessEventsHandlers() []OnProcessEventsHandler { + c.handlersLock.Lock() + defer c.handlersLock.Unlock() + handlers := make([]OnProcessEventsHandler, 0, len(c.processEventsHandlers)) + for _, h := range c.processEventsHandlers { + handlers = append(handlers, h) + } + return handlers +} + // GetServerVersion returns observed cluster version func (c *clusterCache) GetServerVersion() string { return c.serverVersion @@ -440,6 +483,10 @@ func (c *clusterCache) Invalidate(opts ...UpdateSettingsFunc) { for i := range opts { opts[i](c) } + + if c.batchEventsProcessing { + c.invalidateEventMeta() + } c.apisMeta = nil c.namespacedResources = nil c.log.Info("Invalidated cluster") @@ -669,7 +716,7 @@ func (c *clusterCache) watchEvents(ctx context.Context, api kube.APIResourceInfo return fmt.Errorf("Failed to convert to *unstructured.Unstructured: %v", event.Object) } - c.processEvent(event.Type, obj) + c.recordEvent(event.Type, obj) if kube.IsCRD(obj) { var resources []kube.APIResourceInfo crd := v1.CustomResourceDefinition{} @@ -823,6 +870,12 @@ func (c *clusterCache) sync() error { for i := range c.apisMeta { c.apisMeta[i].watchCancel() } + + if c.batchEventsProcessing { + c.invalidateEventMeta() + c.eventMetaCh = make(chan eventMeta) + } + c.apisMeta = make(map[schema.GroupKind]*apiMeta) c.resources = make(map[kube.ResourceKey]*Resource) c.namespacedResources = make(map[schema.GroupKind]bool) @@ -864,6 +917,10 @@ func (c *clusterCache) sync() error { return err } + if c.batchEventsProcessing { + go c.processEvents() + } + // Each API is processed in parallel, so we need to take out a lock when we update clusterCache fields. lock := sync.Mutex{} err = kube.RunAllAsync(len(apis), func(i int) error { @@ -926,6 +983,14 @@ func (c *clusterCache) sync() error { return nil } +// invalidateEventMeta closes the eventMeta channel if it is open +func (c *clusterCache) invalidateEventMeta() { + if c.eventMetaCh != nil { + close(c.eventMetaCh) + c.eventMetaCh = nil + } +} + // EnsureSynced checks cache state and synchronizes it if necessary func (c *clusterCache) EnsureSynced() error { syncStatus := &c.syncStatus @@ -1231,7 +1296,7 @@ func (c *clusterCache) GetManagedLiveObjs(targetObjs []*unstructured.Unstructure return managedObjs, nil } -func (c *clusterCache) processEvent(event watch.EventType, un *unstructured.Unstructured) { +func (c *clusterCache) recordEvent(event watch.EventType, un *unstructured.Unstructured) { for _, h := range c.getEventHandlers() { h(event, un) } @@ -1240,15 +1305,74 @@ func (c *clusterCache) processEvent(event watch.EventType, un *unstructured.Unst return } + if c.batchEventsProcessing { + c.eventMetaCh <- eventMeta{event, un} + } else { + c.lock.Lock() + defer c.lock.Unlock() + c.processEvent(key, eventMeta{event, un}) + } +} + +func (c *clusterCache) processEvents() { + log := c.log.WithValues("functionName", "processItems") + log.V(1).Info("Start processing events") + c.lock.Lock() - defer c.lock.Unlock() + ch := c.eventMetaCh + c.lock.Unlock() + + eventMetas := make([]eventMeta, 0) + ticker := time.NewTicker(c.eventProcessingInterval) + defer ticker.Stop() + + for { + select { + case evMeta, ok := <-ch: + if !ok { + log.V(2).Info("Event processing channel closed, finish processing") + return + } + eventMetas = append(eventMetas, evMeta) + case <-ticker.C: + if len(eventMetas) > 0 { + c.processEventsBatch(eventMetas) + eventMetas = eventMetas[:0] + } + } + } +} + +func (c *clusterCache) processEventsBatch(eventMetas []eventMeta) { + log := c.log.WithValues("functionName", "processEventsBatch") + start := time.Now() + c.lock.Lock() + log.V(1).Info("Lock acquired (ms)", "duration", time.Since(start).Milliseconds()) + defer func() { + c.lock.Unlock() + duration := time.Since(start) + // Update the metric with the duration of the events processing + for _, handler := range c.getProcessEventsHandlers() { + handler(duration, len(eventMetas)) + } + }() + + for _, evMeta := range eventMetas { + key := kube.GetResourceKey(evMeta.un) + c.processEvent(key, evMeta) + } + + log.V(1).Info("Processed events (ms)", "count", len(eventMetas), "duration", time.Since(start).Milliseconds()) +} + +func (c *clusterCache) processEvent(key kube.ResourceKey, evMeta eventMeta) { existingNode, exists := c.resources[key] - if event == watch.Deleted { + if evMeta.event == watch.Deleted { if exists { c.onNodeRemoved(key) } - } else if event != watch.Deleted { - c.onNodeUpdated(existingNode, c.newResource(un)) + } else { + c.onNodeUpdated(existingNode, c.newResource(evMeta.un)) } } diff --git a/pkg/cache/cluster_test.go b/pkg/cache/cluster_test.go index 26815c07e..8f3263da6 100644 --- a/pkg/cache/cluster_test.go +++ b/pkg/cache/cluster_test.go @@ -184,6 +184,12 @@ func TestEnsureSynced(t *testing.T) { } func TestStatefulSetOwnershipInferred(t *testing.T) { + var opts []UpdateSettingsFunc + opts = append(opts, func(c *clusterCache) { + c.batchEventsProcessing = true + c.eventProcessingInterval = 1 * time.Millisecond + }) + sts := &appsv1.StatefulSet{ TypeMeta: metav1.TypeMeta{APIVersion: "apps/v1", Kind: kube.StatefulSetKind}, ObjectMeta: metav1.ObjectMeta{UID: "123", Name: "web", Namespace: "default"}, @@ -196,63 +202,71 @@ func TestStatefulSetOwnershipInferred(t *testing.T) { }, } - t.Run("STSTemplateNameNotMatching", func(t *testing.T) { - cluster := newCluster(t, sts) - err := cluster.EnsureSynced() - require.NoError(t, err) - - pvc := mustToUnstructured(&v1.PersistentVolumeClaim{ - TypeMeta: metav1.TypeMeta{Kind: kube.PersistentVolumeClaimKind}, - ObjectMeta: metav1.ObjectMeta{Name: "www1-web-0", Namespace: "default"}, - }) - - cluster.processEvent(watch.Added, pvc) - - cluster.lock.Lock() - defer cluster.lock.Unlock() - - refs := cluster.resources[kube.GetResourceKey(pvc)].OwnerRefs - - assert.Len(t, refs, 0) - }) - - t.Run("STSTemplateNameNotMatching", func(t *testing.T) { - cluster := newCluster(t, sts) - err := cluster.EnsureSynced() - require.NoError(t, err) - - pvc := mustToUnstructured(&v1.PersistentVolumeClaim{ - TypeMeta: metav1.TypeMeta{Kind: kube.PersistentVolumeClaimKind}, - ObjectMeta: metav1.ObjectMeta{Name: "www1-web-0", Namespace: "default"}, - }) - cluster.processEvent(watch.Added, pvc) - - cluster.lock.Lock() - defer cluster.lock.Unlock() + tests := []struct { + name string + cluster *clusterCache + pvc *v1.PersistentVolumeClaim + expectedRefs []metav1.OwnerReference + expectNoOwner bool + }{ + { + name: "STSTemplateNameNotMatching", + cluster: newCluster(t, sts), + pvc: &v1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{Kind: kube.PersistentVolumeClaimKind}, + ObjectMeta: metav1.ObjectMeta{Name: "www1-web-0", Namespace: "default"}, + }, + expectNoOwner: true, + }, + { + name: "MatchingSTSExists", + cluster: newCluster(t, sts), + pvc: &v1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{Kind: kube.PersistentVolumeClaimKind}, + ObjectMeta: metav1.ObjectMeta{Name: "www-web-0", Namespace: "default"}, + }, + expectedRefs: []metav1.OwnerReference{{APIVersion: "apps/v1", Kind: kube.StatefulSetKind, Name: "web", UID: "123"}}, + }, + { + name: "STSTemplateNameNotMatchingWithBatchProcessing", + cluster: newClusterWithOptions(t, opts, sts), + pvc: &v1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{Kind: kube.PersistentVolumeClaimKind}, + ObjectMeta: metav1.ObjectMeta{Name: "www1-web-0", Namespace: "default"}, + }, + expectNoOwner: true, + }, + { + name: "MatchingSTSExistsWithBatchProcessing", + cluster: newClusterWithOptions(t, opts, sts), + pvc: &v1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{Kind: kube.PersistentVolumeClaimKind}, + ObjectMeta: metav1.ObjectMeta{Name: "www-web-0", Namespace: "default"}, + }, + expectedRefs: []metav1.OwnerReference{{APIVersion: "apps/v1", Kind: kube.StatefulSetKind, Name: "web", UID: "123"}}, + }, + } - refs := cluster.resources[kube.GetResourceKey(pvc)].OwnerRefs + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := tc.cluster.EnsureSynced() + require.NoError(t, err) - assert.Len(t, refs, 0) - }) + pvc := mustToUnstructured(tc.pvc) + tc.cluster.recordEvent(watch.Added, pvc) - t.Run("MatchingSTSExists", func(t *testing.T) { - cluster := newCluster(t, sts) - err := cluster.EnsureSynced() - require.NoError(t, err) + require.Eventually(t, func() bool { + tc.cluster.lock.Lock() + defer tc.cluster.lock.Unlock() - pvc := mustToUnstructured(&v1.PersistentVolumeClaim{ - TypeMeta: metav1.TypeMeta{Kind: kube.PersistentVolumeClaimKind}, - ObjectMeta: metav1.ObjectMeta{Name: "www-web-0", Namespace: "default"}, + refs := tc.cluster.resources[kube.GetResourceKey(pvc)].OwnerRefs + if tc.expectNoOwner { + return len(refs) == 0 + } + return assert.ElementsMatch(t, refs, tc.expectedRefs) + }, 5*time.Second, 10*time.Millisecond, "Expected PVC to have correct owner reference") }) - cluster.processEvent(watch.Added, pvc) - - cluster.lock.Lock() - defer cluster.lock.Unlock() - - refs := cluster.resources[kube.GetResourceKey(pvc)].OwnerRefs - - assert.ElementsMatch(t, refs, []metav1.OwnerReference{{APIVersion: "apps/v1", Kind: kube.StatefulSetKind, Name: "web", UID: "123"}}) - }) + } } func TestEnsureSyncedSingleNamespace(t *testing.T) { @@ -596,7 +610,7 @@ func TestChildDeletedEvent(t *testing.T) { err := cluster.EnsureSynced() require.NoError(t, err) - cluster.processEvent(watch.Deleted, mustToUnstructured(testPod1())) + cluster.recordEvent(watch.Deleted, mustToUnstructured(testPod1())) rsChildren := getChildren(cluster, mustToUnstructured(testRS())) assert.Equal(t, []*Resource{}, rsChildren) @@ -620,7 +634,7 @@ func TestProcessNewChildEvent(t *testing.T) { uid: "2" resourceVersion: "123"`) - cluster.processEvent(watch.Added, newPod) + cluster.recordEvent(watch.Added, newPod) rsChildren := getChildren(cluster, mustToUnstructured(testRS())) sort.Slice(rsChildren, func(i, j int) bool { diff --git a/pkg/cache/mocks/ClusterCache.go b/pkg/cache/mocks/ClusterCache.go index c9fbc8f9c..a4afa3119 100644 --- a/pkg/cache/mocks/ClusterCache.go +++ b/pkg/cache/mocks/ClusterCache.go @@ -262,6 +262,26 @@ func (_m *ClusterCache) OnEvent(handler cache.OnEventHandler) cache.Unsubscribe return r0 } +// OnProcessEventsHandler provides a mock function with given fields: handler +func (_m *ClusterCache) OnProcessEventsHandler(handler cache.OnProcessEventsHandler) cache.Unsubscribe { + ret := _m.Called(handler) + + if len(ret) == 0 { + panic("no return value specified for OnProcessEventsHandler") + } + + var r0 cache.Unsubscribe + if rf, ok := ret.Get(0).(func(cache.OnProcessEventsHandler) cache.Unsubscribe); ok { + r0 = rf(handler) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(cache.Unsubscribe) + } + } + + return r0 +} + // OnResourceUpdated provides a mock function with given fields: handler func (_m *ClusterCache) OnResourceUpdated(handler cache.OnResourceUpdatedHandler) cache.Unsubscribe { ret := _m.Called(handler) diff --git a/pkg/cache/settings.go b/pkg/cache/settings.go index a7194d0ca..241c75d3b 100644 --- a/pkg/cache/settings.go +++ b/pkg/cache/settings.go @@ -170,3 +170,17 @@ func SetRespectRBAC(respectRBAC int) UpdateSettingsFunc { } } } + +// SetBatchEventsProcessing allows to set whether to process events in batch +func SetBatchEventsProcessing(batchProcessing bool) UpdateSettingsFunc { + return func(cache *clusterCache) { + cache.batchEventsProcessing = batchProcessing + } +} + +// SetEventProcessingInterval allows to set the interval for processing events +func SetEventProcessingInterval(interval time.Duration) UpdateSettingsFunc { + return func(cache *clusterCache) { + cache.eventProcessingInterval = interval + } +} diff --git a/pkg/cache/settings_test.go b/pkg/cache/settings_test.go index 28ac963cb..fdc1a7412 100644 --- a/pkg/cache/settings_test.go +++ b/pkg/cache/settings_test.go @@ -55,3 +55,20 @@ func TestSetWatchResyncTimeout(t *testing.T) { cache = NewClusterCache(&rest.Config{}, SetWatchResyncTimeout(timeout)) assert.Equal(t, timeout, cache.watchResyncTimeout) } + +func TestSetBatchEventsProcessing(t *testing.T) { + cache := NewClusterCache(&rest.Config{}) + assert.False(t, cache.batchEventsProcessing) + + cache.Invalidate(SetBatchEventsProcessing(true)) + assert.True(t, cache.batchEventsProcessing) +} + +func TestSetEventsProcessingInterval(t *testing.T) { + cache := NewClusterCache(&rest.Config{}) + assert.Equal(t, defaultEventProcessingInterval, cache.eventProcessingInterval) + + interval := 1 * time.Second + cache.Invalidate(SetEventProcessingInterval(interval)) + assert.Equal(t, interval, cache.eventProcessingInterval) +} From d833440b8e3026e07f006b9a28afb64d8b4ef414 Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 13:39:28 +0200 Subject: [PATCH 17/22] add health check for crd Signed-off-by: Almo Elda --- pkg/health/health.go | 5 ++ pkg/health/health_customresourcedefinition.go | 83 +++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 pkg/health/health_customresourcedefinition.go diff --git a/pkg/health/health.go b/pkg/health/health.go index b93d8c967..8047ca263 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -116,6 +116,11 @@ func GetHealthCheckFunc(gvk schema.GroupVersionKind) func(obj *unstructured.Unst case kube.IngressKind: return getIngressHealth } + case "apiextensions": + switch gvk.Kind { + case kube.CustomResourceDefinitionKind: + return getCustomResourceDefinitionHealth + } case "argoproj.io": switch gvk.Kind { case "Workflow": diff --git a/pkg/health/health_customresourcedefinition.go b/pkg/health/health_customresourcedefinition.go new file mode 100644 index 000000000..76e53dbd8 --- /dev/null +++ b/pkg/health/health_customresourcedefinition.go @@ -0,0 +1,83 @@ +package health + +import ( + "fmt" + + "github.com/argoproj/gitops-engine/pkg/utils/kube" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" +) + +func getCustomResourceDefinitionHealth(obj *unstructured.Unstructured) (*HealthStatus, error) { + gvk := obj.GroupVersionKind() + switch gvk { + case apiextensionsv1.SchemeGroupVersion.WithKind(kube.CustomResourceDefinitionKind): + var crd apiextensionsv1.CustomResourceDefinition + err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &crd) + if err != nil { + return nil, fmt.Errorf("failed to convert unstructured CustomResourceDefinition to typed: %v", err) + } + return getApiExtenstionsV1CustomResourceDefinitionHealth(&crd) + default: + return nil, fmt.Errorf("unsupported CustomResourceDefinition GVK: %s", gvk) + } +} + +func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.CustomResourceDefinition) (*HealthStatus, error) { + if crd.ObjectMeta.DeletionTimestamp != nil { + return &HealthStatus{ + Status: HealthStatusProgressing, + Message: "CRD is being terminated", + }, nil + } + if crd.Status.Conditions == nil { + return &HealthStatus{ + Status: HealthStatusProgressing, + Message: "Status conditions not found", + }, nil + } + + var ( + isEstablished bool + hasViolations bool + violationMsg string + establishedMsg string + ) + + // Check conditions + for _, condition := range crd.Status.Conditions { + switch condition.Type { + case apiextensionsv1.Established: + if condition.Status == apiextensionsv1.ConditionTrue { + isEstablished = true + } else { + establishedMsg = condition.Message + } + case "NonStructuralSchema": + if condition.Status == apiextensionsv1.ConditionTrue { + hasViolations = true + violationMsg = condition.Message + } + } + } + + // Return appropriate health status + switch { + case !isEstablished: + return &HealthStatus{ + Status: HealthStatusDegraded, + Message: fmt.Sprintf("CRD is not established: %s", establishedMsg), + }, nil + case hasViolations: + return &HealthStatus{ + Status: HealthStatusDegraded, + Message: fmt.Sprintf("Schema violations found: %s", violationMsg), + }, nil + default: + return &HealthStatus{ + Status: HealthStatusHealthy, + Message: "CRD is healthy", + }, nil + } +} From 68157163505fb33abe1f43d3c27cdd0821edd074 Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 15:36:16 +0200 Subject: [PATCH 18/22] deleted handling of deletionTimestamp, added more conditions to handle and changed condition msg to a signle variable to be returned Signed-off-by: Almo Elda --- pkg/health/health.go | 4 +- pkg/health/health_customresourcedefinition.go | 45 ++++++++++++------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/pkg/health/health.go b/pkg/health/health.go index 8047ca263..f3a7c12c3 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -70,7 +70,6 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver Message: "Pending deletion", }, nil } - if healthOverride != nil { health, err := healthOverride.GetResourceHealth(obj) if err != nil { @@ -84,7 +83,6 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver return health, nil } } - if healthCheck := GetHealthCheckFunc(obj.GroupVersionKind()); healthCheck != nil { if health, err = healthCheck(obj); err != nil { health = &HealthStatus{ @@ -116,7 +114,7 @@ func GetHealthCheckFunc(gvk schema.GroupVersionKind) func(obj *unstructured.Unst case kube.IngressKind: return getIngressHealth } - case "apiextensions": + case "apiextensions.k8s.io": switch gvk.Kind { case kube.CustomResourceDefinitionKind: return getCustomResourceDefinitionHealth diff --git a/pkg/health/health_customresourcedefinition.go b/pkg/health/health_customresourcedefinition.go index 76e53dbd8..7a880a75d 100644 --- a/pkg/health/health_customresourcedefinition.go +++ b/pkg/health/health_customresourcedefinition.go @@ -25,12 +25,6 @@ func getCustomResourceDefinitionHealth(obj *unstructured.Unstructured) (*HealthS } func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.CustomResourceDefinition) (*HealthStatus, error) { - if crd.ObjectMeta.DeletionTimestamp != nil { - return &HealthStatus{ - Status: HealthStatusProgressing, - Message: "CRD is being terminated", - }, nil - } if crd.Status.Conditions == nil { return &HealthStatus{ Status: HealthStatusProgressing, @@ -39,40 +33,61 @@ func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.Cust } var ( - isEstablished bool - hasViolations bool - violationMsg string - establishedMsg string + isEstablished bool + isTerminating bool + namesNotAccepted bool + hasViolations bool + conditionMsg string ) // Check conditions for _, condition := range crd.Status.Conditions { switch condition.Type { + case apiextensionsv1.Terminating: + if condition.Status == apiextensionsv1.ConditionTrue { + isTerminating = true + conditionMsg = condition.Message + } + case apiextensionsv1.NamesAccepted: + if condition.Status == apiextensionsv1.ConditionTrue { + namesNotAccepted = true + conditionMsg = condition.Message + } case apiextensionsv1.Established: if condition.Status == apiextensionsv1.ConditionTrue { isEstablished = true } else { - establishedMsg = condition.Message + conditionMsg = condition.Message } - case "NonStructuralSchema": + case apiextensionsv1.NonStructuralSchema: if condition.Status == apiextensionsv1.ConditionTrue { hasViolations = true - violationMsg = condition.Message + conditionMsg = condition.Message } } } // Return appropriate health status switch { + case isTerminating: + return &HealthStatus{ + Status: HealthStatusProgressing, + Message: fmt.Sprintf("CRD is being terminated: %s", conditionMsg), + }, nil + case namesNotAccepted: + return &HealthStatus{ + Status: HealthStatusDegraded, + Message: fmt.Sprintf("CRD names have not been accepted: %s", conditionMsg), + }, nil case !isEstablished: return &HealthStatus{ Status: HealthStatusDegraded, - Message: fmt.Sprintf("CRD is not established: %s", establishedMsg), + Message: fmt.Sprintf("CRD is not established: %s", conditionMsg), }, nil case hasViolations: return &HealthStatus{ Status: HealthStatusDegraded, - Message: fmt.Sprintf("Schema violations found: %s", violationMsg), + Message: fmt.Sprintf("Schema violations found: %s", conditionMsg), }, nil default: return &HealthStatus{ From 14522517fd21edcfb73724c653cb06229f145570 Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 16:07:41 +0200 Subject: [PATCH 19/22] adding tests Signed-off-by: Almo Elda --- pkg/health/health_customresourcedefinition.go | 5 +- pkg/health/health_test.go | 8 +++ pkg/health/testdata/crd-v1-healthy.yaml | 56 +++++++++++++++++ .../crd-v1-no-conditions-progressing.yaml | 46 ++++++++++++++ .../crd-v1-non-structual-degraded.yaml | 61 +++++++++++++++++++ .../crd-v1-not-established-degraded.yaml | 56 +++++++++++++++++ .../crd-v1-terminating-progressing.yaml | 57 +++++++++++++++++ 7 files changed, 287 insertions(+), 2 deletions(-) create mode 100644 pkg/health/testdata/crd-v1-healthy.yaml create mode 100644 pkg/health/testdata/crd-v1-no-conditions-progressing.yaml create mode 100644 pkg/health/testdata/crd-v1-non-structual-degraded.yaml create mode 100644 pkg/health/testdata/crd-v1-not-established-degraded.yaml create mode 100644 pkg/health/testdata/crd-v1-terminating-progressing.yaml diff --git a/pkg/health/health_customresourcedefinition.go b/pkg/health/health_customresourcedefinition.go index 7a880a75d..0e3fa7b73 100644 --- a/pkg/health/health_customresourcedefinition.go +++ b/pkg/health/health_customresourcedefinition.go @@ -25,7 +25,8 @@ func getCustomResourceDefinitionHealth(obj *unstructured.Unstructured) (*HealthS } func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.CustomResourceDefinition) (*HealthStatus, error) { - if crd.Status.Conditions == nil { + + if crd.Status.Conditions == nil || crd.Status.Conditions != nil && len(crd.Status.Conditions) == 0 { return &HealthStatus{ Status: HealthStatusProgressing, Message: "Status conditions not found", @@ -49,7 +50,7 @@ func getApiExtenstionsV1CustomResourceDefinitionHealth(crd *apiextensionsv1.Cust conditionMsg = condition.Message } case apiextensionsv1.NamesAccepted: - if condition.Status == apiextensionsv1.ConditionTrue { + if condition.Status == apiextensionsv1.ConditionFalse { namesNotAccepted = true conditionMsg = condition.Message } diff --git a/pkg/health/health_test.go b/pkg/health/health_test.go index 45ff74941..deb55342e 100644 --- a/pkg/health/health_test.go +++ b/pkg/health/health_test.go @@ -118,6 +118,14 @@ func TestAPIService(t *testing.T) { assertAppHealth(t, "./testdata/apiservice-v1beta1-false.yaml", HealthStatusProgressing) } +func TestCustomResourceDefinitionHealth(t *testing.T) { + assertAppHealth(t, "./testdata/crd-v1-healthy.yaml", HealthStatusHealthy) + assertAppHealth(t, "./testdata/crd-v1-non-structual-degraded.yaml", HealthStatusDegraded) + assertAppHealth(t, "./testdata/crd-v1-not-established-degraded.yaml", HealthStatusDegraded) + assertAppHealth(t, "./testdata/crd-v1-terminating-progressing.yaml", HealthStatusProgressing) + assertAppHealth(t, "./testdata/crd-v1-no-conditions-progressing.yaml", HealthStatusProgressing) +} + func TestGetArgoWorkflowHealth(t *testing.T) { sampleWorkflow := unstructured.Unstructured{Object: map[string]interface{}{ "spec": map[string]interface{}{ diff --git a/pkg/health/testdata/crd-v1-healthy.yaml b/pkg/health/testdata/crd-v1-healthy.yaml new file mode 100644 index 000000000..09e00f70b --- /dev/null +++ b/pkg/health/testdata/crd-v1-healthy.yaml @@ -0,0 +1,56 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: no conflicts found + reason: NoConflicts + status: 'True' + type: NamesAccepted + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: the initial names have been accepted + reason: InitialNamesAccepted + status: 'True' + type: Established + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/pkg/health/testdata/crd-v1-no-conditions-progressing.yaml b/pkg/health/testdata/crd-v1-no-conditions-progressing.yaml new file mode 100644 index 000000000..f12ce016b --- /dev/null +++ b/pkg/health/testdata/crd-v1-no-conditions-progressing.yaml @@ -0,0 +1,46 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: [] + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/pkg/health/testdata/crd-v1-non-structual-degraded.yaml b/pkg/health/testdata/crd-v1-non-structual-degraded.yaml new file mode 100644 index 000000000..3c7a07f3e --- /dev/null +++ b/pkg/health/testdata/crd-v1-non-structual-degraded.yaml @@ -0,0 +1,61 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: no conflicts found + reason: NoConflicts + status: 'True' + type: NamesAccepted + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: the initial names have been accepted + reason: InitialNamesAccepted + status: 'True' + type: Established + - lastTransitionTime: '2024-10-26T19:44:57Z' + message: 'spec.preserveUnknownFields: Invalid value: true: must be false' + reason: Violations + status: 'True' + type: NonStructuralSchema + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/pkg/health/testdata/crd-v1-not-established-degraded.yaml b/pkg/health/testdata/crd-v1-not-established-degraded.yaml new file mode 100644 index 000000000..5982bfa53 --- /dev/null +++ b/pkg/health/testdata/crd-v1-not-established-degraded.yaml @@ -0,0 +1,56 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: no conflicts found + reason: NoConflicts + status: 'False' + type: NamesAccepted + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: the initial names have been accepted + reason: InitialNamesAccepted + status: 'False' + type: Established + storedVersions: + - v1alpha1 \ No newline at end of file diff --git a/pkg/health/testdata/crd-v1-terminating-progressing.yaml b/pkg/health/testdata/crd-v1-terminating-progressing.yaml new file mode 100644 index 000000000..73d19e483 --- /dev/null +++ b/pkg/health/testdata/crd-v1-terminating-progressing.yaml @@ -0,0 +1,57 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: examples.example.io + deletionTimestamp: '2024-12-28T13:51:19Z' +spec: + conversion: + strategy: None + group: example.io + names: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + preserveUnknownFields: true + scope: Namespaced + versions: + - additionalPrinterColumns: + - description: >- + CreationTimestamp is a timestamp representing the server time when + this object was created. It is not guaranteed to be set in + happens-before order across separate operations. Clients may not set + this value. It is represented in RFC3339 form and is in UTC. + + + Populated by the system. Read-only. Null for lists. More info: + https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata + jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + served: true + storage: true + subresources: {} +status: + acceptedNames: + kind: Example + listKind: ExampleList + plural: examples + shortNames: + - ex + singular: example + conditions: + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: no conflicts found + reason: NoConflicts + status: 'True' + type: NamesAccepted + - lastTransitionTime: '2024-05-19T23:35:28Z' + message: the initial names have been accepted + reason: InitialNamesAccepted + status: 'True' + type: Established + storedVersions: + - v1alpha1 \ No newline at end of file From 7403989798d8286609582b77cdb006f9b8509755 Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 16:10:38 +0200 Subject: [PATCH 20/22] reverting blank lines Signed-off-by: Almo Elda --- pkg/health/health.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/health/health.go b/pkg/health/health.go index f3a7c12c3..d5e2fcd65 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -70,6 +70,7 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver Message: "Pending deletion", }, nil } + if healthOverride != nil { health, err := healthOverride.GetResourceHealth(obj) if err != nil { @@ -83,6 +84,7 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver return health, nil } } + if healthCheck := GetHealthCheckFunc(obj.GroupVersionKind()); healthCheck != nil { if health, err = healthCheck(obj); err != nil { health = &HealthStatus{ From 8aee5794dd839e41ed9630501190ce342d741916 Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 15:36:16 +0200 Subject: [PATCH 21/22] deleted handling of deletionTimestamp, added more conditions to handle and changed condition msg to a signle variable to be returned Signed-off-by: Almo Elda --- pkg/health/health.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/health/health.go b/pkg/health/health.go index d5e2fcd65..f3a7c12c3 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -70,7 +70,6 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver Message: "Pending deletion", }, nil } - if healthOverride != nil { health, err := healthOverride.GetResourceHealth(obj) if err != nil { @@ -84,7 +83,6 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver return health, nil } } - if healthCheck := GetHealthCheckFunc(obj.GroupVersionKind()); healthCheck != nil { if health, err = healthCheck(obj); err != nil { health = &HealthStatus{ From 769a2cfee3d0b3d3f595437040f29c1684b1e250 Mon Sep 17 00:00:00 2001 From: Almo Elda Date: Sat, 28 Dec 2024 16:10:38 +0200 Subject: [PATCH 22/22] reverting blank lines Signed-off-by: Almo Elda --- pkg/health/health.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/health/health.go b/pkg/health/health.go index f3a7c12c3..d5e2fcd65 100644 --- a/pkg/health/health.go +++ b/pkg/health/health.go @@ -70,6 +70,7 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver Message: "Pending deletion", }, nil } + if healthOverride != nil { health, err := healthOverride.GetResourceHealth(obj) if err != nil { @@ -83,6 +84,7 @@ func GetResourceHealth(obj *unstructured.Unstructured, healthOverride HealthOver return health, nil } } + if healthCheck := GetHealthCheckFunc(obj.GroupVersionKind()); healthCheck != nil { if health, err = healthCheck(obj); err != nil { health = &HealthStatus{