Repository: rancher/fleet
Pull request #1482 (2023-04-20 11:00:58)

Add ignore conditions
We can't control the status in Custom Resources that don't belong to Fleet or Rancher. This is causing Fleet to consider bundles in error state when they shouldn't. See what happens with [keda](https://github.com/rancher/fleet/issues/1386#issue-1615322308).

This PR adds a new field to set conditions to be ignored as suggested [here](https://github.com/rancher/fleet/issues/1386#issuecomment-1465929992). This allows users to ignore conditions that [wrangler Summarize](https://github.com/rancher/wrangler/blob/master/pkg/summary/summary.go#L88) thinks are wrong, but are in an ok status. Ignore Conditions can be specified in the `Bundle` or in the `fleet.yaml`

refers to https://github.com/rancher/fleet/issues/1386


Patch:
diff --git a/charts/fleet-crd/templates/crds.yaml b/charts/fleet-crd/templates/crds.yaml
index b009ad07..9dc87b0d 100644
--- a/charts/fleet-crd/templates/crds.yaml
+++ b/charts/fleet-crd/templates/crds.yaml
@@ -183,6 +183,18 @@ spec:
                   waitForJobs:
                     type: boolean
                 type: object
+              ignore:
+                properties:
+                  conditions:
+                    items:
+                      additionalProperties:
+                        nullable: true
+                        type: string
+                      nullable: true
+                      type: object
+                    nullable: true
+                    type: array
+                type: object
               keepResources:
                 type: boolean
               kustomize:
@@ -558,6 +570,18 @@ spec:
                         waitForJobs:
                           type: boolean
                       type: object
+                    ignore:
+                      properties:
+                        conditions:
+                          items:
+                            additionalProperties:
+                              nullable: true
+                              type: string
+                            nullable: true
+                            type: object
+                          nullable: true
+                          type: array
+                      type: object
                     keepResources:
                       type: boolean
                     kustomize:
@@ -1075,6 +1099,18 @@ spec:
                       waitForJobs:
                         type: boolean
                     type: object
+                  ignore:
+                    properties:
+                      conditions:
+                        items:
+                          additionalProperties:
+                            nullable: true
+                            type: string
+                          nullable: true
+                          type: object
+                        nullable: true
+                        type: array
+                    type: object
                   keepResources:
                     type: boolean
                   kustomize:
@@ -1226,6 +1262,18 @@ spec:
                       waitForJobs:
                         type: boolean
                     type: object
+                  ignore:
+                    properties:
+                      conditions:
+                        items:
+                          additionalProperties:
+                            nullable: true
+                            type: string
+                          nullable: true
+                          type: object
+                        nullable: true
+                        type: array
+                    type: object
                   keepResources:
                     type: boolean
                   kustomize:
diff --git a/modules/agent/pkg/deployer/monitor.go b/modules/agent/pkg/deployer/monitor.go
index 0fd2dd5c..68ebc299 100644
--- a/modules/agent/pkg/deployer/monitor.go
+++ b/modules/agent/pkg/deployer/monitor.go
@@ -2,6 +2,7 @@ package deployer
 
 import (
 	"encoding/json"
+	"fmt"
 	"sort"
 
 	jsonpatch "github.com/evanphx/json-patch"
@@ -16,6 +17,7 @@ import (
 	"github.com/rancher/wrangler/pkg/merr"
 	"github.com/rancher/wrangler/pkg/objectset"
 	"github.com/rancher/wrangler/pkg/summary"
+	"github.com/sirupsen/logrus"
 
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -166,7 +168,7 @@ func (m *Manager) MonitorBundle(bd *fleet.BundleDeployment) (DeploymentStatus, e
 		return status, err
 	}
 
-	status.NonReadyStatus = nonReady(plan)
+	status.NonReadyStatus = nonReady(plan, bd.Spec.Options.IgnoreOptions)
 	status.ModifiedStatus = modified(plan, resourcesPreviuosRelease)
 	status.Ready = false
 	status.NonModified = false
@@ -262,7 +264,7 @@ func isResourceInPreviousRelease(key objectset.ObjectKey, kind string, objsPrevi
 	return false
 }
 
-func nonReady(plan apply.Plan) (result []fleet.NonReadyStatus) {
+func nonReady(plan apply.Plan, ignoreOptions fleet.IgnoreOptions) (result []fleet.NonReadyStatus) {
 	defer func() {
 		sort.Slice(result, func(i, j int) bool {
 			return result[i].UID < result[j].UID
@@ -274,6 +276,12 @@ func nonReady(plan apply.Plan) (result []fleet.NonReadyStatus) {
 			return
 		}
 		if u, ok := obj.(*unstructured.Unstructured); ok {
+			if ignoreOptions.Conditions != nil {
+				if err := excludeIgnoredConditions(u, ignoreOptions); err != nil {
+					logrus.Errorf("failed to ignore conditions: %v", err)
+				}
+			}
+
 			summary := summary.Summarize(u)
 			if !summary.IsReady() {
 				result = append(result, fleet.NonReadyStatus{
@@ -288,5 +296,53 @@ func nonReady(plan apply.Plan) (result []fleet.NonReadyStatus) {
 		}
 	}
 
-	return
+	return result
+}
+
+// excludeIgnoredConditions removes the conditions that are included in ignoreOptions from the object passed as a parameter
+func excludeIgnoredConditions(obj *unstructured.Unstructured, ignoreOptions fleet.IgnoreOptions) error {
+	conditions, _, err := unstructured.NestedSlice(obj.Object, "status", "conditions")
+	if err != nil {
+		return err
+	}
+	conditionsWithoutIgnored := make([]interface{}, 0)
+
+	for _, condition := range conditions {
+		condition, ok := condition.(map[string]interface{})
+		if !ok {
+			return fmt.Errorf("condition: %#v can't be converted to map[string]interface{}", condition)
+		}
+		excludeCondition := false
+		for _, ignoredCondition := range ignoreOptions.Conditions {
+			if shouldExcludeCondition(condition, ignoredCondition) {
+				excludeCondition = true
+				break
+			}
+		}
+		if !excludeCondition {
+			conditionsWithoutIgnored = append(conditionsWithoutIgnored, condition)
+		}
+	}
+
+	err = unstructured.SetNestedSlice(obj.Object, conditionsWithoutIgnored, "status", "conditions")
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// shouldExcludeCondition returns true if all the elements of ignoredConditions are inside conditions
+func shouldExcludeCondition(conditions map[string]interface{}, ignoredConditions map[string]string) bool {
+	if len(ignoredConditions) > len(conditions) {
+		return false
+	}
+
+	for k, v := range ignoredConditions {
+		if vc, found := conditions[k]; !found || vc != v {
+			return false
+		}
+	}
+
+	return true
 }
diff --git a/pkg/apis/fleet.cattle.io/v1alpha1/bundle.go b/pkg/apis/fleet.cattle.io/v1alpha1/bundle.go
index 4ec0f0fe..4130a807 100644
--- a/pkg/apis/fleet.cattle.io/v1alpha1/bundle.go
+++ b/pkg/apis/fleet.cattle.io/v1alpha1/bundle.go
@@ -233,6 +233,9 @@ type BundleDeploymentOptions struct {
 
 	// KeepResources can be used to keep the deployed resources when removing the bundle
 	KeepResources bool `json:"keepResources,omitempty"`
+
+	//IgnoreOptions can be used to ignore fields when monitoring the bundle.
+	IgnoreOptions `json:"ignore,omitempty"`
 }
 
 type DiffOptions struct {
@@ -312,6 +315,11 @@ type HelmOptions struct {
 	DisablePreProcess bool `json:"disablePreProcess,omitempty"`
 }
 
+// IgnoreOptions defines conditions to be ignored when monitoring the Bundle.
+type IgnoreOptions struct {
+	Conditions []map[string]string `json:"conditions,omitempty"`
+}
+
 // Define helm values that can come from configmap, secret or external. Credit: https://github.com/fluxcd/helm-operator/blob/0cfea875b5d44bea995abe7324819432070dfbdc/pkg/apis/helm.fluxcd.io/v1/types_helmrelease.go#L439
 type ValuesFrom struct {
 	// The reference to a config map with release values.
diff --git a/pkg/apis/fleet.cattle.io/v1alpha1/zz_generated_deepcopy.go b/pkg/apis/fleet.cattle.io/v1alpha1/zz_generated_deepcopy.go
index 7777b7f2..98589551 100644
--- a/pkg/apis/fleet.cattle.io/v1alpha1/zz_generated_deepcopy.go
+++ b/pkg/apis/fleet.cattle.io/v1alpha1/zz_generated_deepcopy.go
@@ -200,6 +200,7 @@ func (in *BundleDeploymentOptions) DeepCopyInto(out *BundleDeploymentOptions) {
 		*out = new(DiffOptions)
 		(*in).DeepCopyInto(*out)
 	}
+	in.IgnoreOptions.DeepCopyInto(&out.IgnoreOptions)
 	return
 }
 
@@ -1561,6 +1562,35 @@ func (in *HelmOptions) DeepCopy() *HelmOptions {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *IgnoreOptions) DeepCopyInto(out *IgnoreOptions) {
+	*out = *in
+	if in.Conditions != nil {
+		in, out := &in.Conditions, &out.Conditions
+		*out = make([]map[string]string, len(*in))
+		for i := range *in {
+			if (*in)[i] != nil {
+				in, out := &(*in)[i], &(*out)[i]
+				*out = make(map[string]string, len(*in))
+				for key, val := range *in {
+					(*out)[key] = val
+				}
+			}
+		}
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IgnoreOptions.
+func (in *IgnoreOptions) DeepCopy() *IgnoreOptions {
+	if in == nil {
+		return nil
+	}
+	out := new(IgnoreOptions)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ImagePolicyChoice) DeepCopyInto(out *ImagePolicyChoice) {
 	*out = *in


Test patch:
diff --git a/modules/agent/pkg/deployer/monitor_test.go b/modules/agent/pkg/deployer/monitor_test.go
new file mode 100644
index 00000000..45f373db
--- /dev/null
+++ b/modules/agent/pkg/deployer/monitor_test.go
@@ -0,0 +1,78 @@
+package deployer
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+func TestExcludeIgnoredConditions(t *testing.T) {
+	podInitializedAndNotReady := v1.Pod{Status: v1.PodStatus{
+		Conditions: []v1.PodCondition{{Type: v1.PodReady, Status: v1.ConditionFalse}, {Type: v1.PodInitialized, Status: v1.ConditionTrue}},
+	}}
+	uPodInitializedAndNotReady, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&podInitializedAndNotReady)
+	if err != nil {
+		t.Errorf("can't convert podInitializedAndNotReady to unstructured: %v", err)
+	}
+	podInitialized := v1.Pod{Status: v1.PodStatus{
+		Conditions: []v1.PodCondition{{Type: v1.PodInitialized, Status: v1.ConditionTrue}},
+	}}
+	uPodInitialized, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&podInitialized)
+	if err != nil {
+		t.Errorf("can't convert podInitialized to unstructured: %v", err)
+	}
+	tests := map[string]struct {
+		obj           *unstructured.Unstructured
+		ignoreOptions fleet.IgnoreOptions
+		expectedObj   *unstructured.Unstructured
+		expectedErr   error
+	}{
+		"nothing is changed without IgnoreOptions": {
+			obj:           &unstructured.Unstructured{Object: uPodInitializedAndNotReady},
+			ignoreOptions: fleet.IgnoreOptions{},
+			expectedObj:   &unstructured.Unstructured{Object: uPodInitializedAndNotReady},
+			expectedErr:   nil,
+		},
+		"nothing is changed when IgnoreOptions don't match any condition": {
+			obj:           &unstructured.Unstructured{Object: uPodInitializedAndNotReady},
+			ignoreOptions: fleet.IgnoreOptions{Conditions: []map[string]string{{"Not": "Found"}}},
+			expectedObj:   &unstructured.Unstructured{Object: uPodInitializedAndNotReady},
+			expectedErr:   nil,
+		},
+		"'Type: Ready' condition is excluded when IgnoreOptions contains 'Type: Ready' condition": {
+			obj:           &unstructured.Unstructured{Object: uPodInitializedAndNotReady},
+			ignoreOptions: fleet.IgnoreOptions{Conditions: []map[string]string{{"type": "Ready"}}},
+			expectedObj:   &unstructured.Unstructured{Object: uPodInitialized},
+			expectedErr:   nil,
+		},
+		"'Type: Ready' condition is excluded when IgnoreOptions contains 'Type: Ready, status: False' condition": {
+			obj:           &unstructured.Unstructured{Object: uPodInitializedAndNotReady},
+			ignoreOptions: fleet.IgnoreOptions{Conditions: []map[string]string{{"type": "Ready", "status": "False"}}},
+			expectedObj:   &unstructured.Unstructured{Object: uPodInitialized},
+			expectedErr:   nil,
+		},
+		"nothing is changed when IgnoreOptions contains 'type: Ready, status: True' condition": {
+			obj:           &unstructured.Unstructured{Object: uPodInitializedAndNotReady},
+			ignoreOptions: fleet.IgnoreOptions{Conditions: []map[string]string{{"type": "Ready", "status": "True"}}},
+			expectedObj:   &unstructured.Unstructured{Object: uPodInitializedAndNotReady},
+			expectedErr:   nil,
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			obj := test.obj
+			err := excludeIgnoredConditions(obj, test.ignoreOptions)
+			if err != test.expectedErr {
+				t.Errorf("expected error doesn't match: expected %v, got %v", test.expectedErr, err)
+			}
+			if !cmp.Equal(obj, test.expectedObj) {
+				t.Errorf("objects don't match: expected %v, got %v", test.expectedObj, obj)
+			}
+		})
+	}
+}