From 3b62dd4458e0fd005402b7207f4f2cec9ba5fd70 Mon Sep 17 00:00:00 2001 From: raul Date: Tue, 18 Apr 2023 10:02:34 +0200 Subject: [PATCH] Add ignore conditions We can't control 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. Conditions inside ignore conditions field will be ignored when checking the Bundle state Signed-off-by: raul --- charts/fleet-crd/templates/crds.yaml | 48 ++++++++++++ modules/agent/pkg/deployer/monitor.go | 61 ++++++++++++++- modules/agent/pkg/deployer/monitor_test.go | 78 +++++++++++++++++++ pkg/apis/fleet.cattle.io/v1alpha1/bundle.go | 8 ++ .../v1alpha1/zz_generated_deepcopy.go | 30 +++++++ 5 files changed, 222 insertions(+), 3 deletions(-) create mode 100644 modules/agent/pkg/deployer/monitor_test.go diff --git a/charts/fleet-crd/templates/crds.yaml b/charts/fleet-crd/templates/crds.yaml index b009ad072f..9dc87b0d26 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 0fd2dd5c03..520be8a101 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,13 @@ func nonReady(plan apply.Plan) (result []fleet.NonReadyStatus) { return } if u, ok := obj.(*unstructured.Unstructured); ok { + if ignoreOptions.Conditions != nil { + err := excludeIgnoredConditions(u, ignoreOptions) + if err != nil { + logrus.Errorf("failed to ignore conditions: %v", err) + } + } + summary := summary.Summarize(u) if !summary.IsReady() { result = append(result, fleet.NonReadyStatus{ @@ -288,5 +297,51 @@ func nonReady(plan apply.Plan) (result []fleet.NonReadyStatus) { } } - return + return result +} + +// excludeIgnoredConditions remove 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/modules/agent/pkg/deployer/monitor_test.go b/modules/agent/pkg/deployer/monitor_test.go new file mode 100644 index 0000000000..3e353acec0 --- /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 podInitializedAndNotReady 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) + } + }) + } +} diff --git a/pkg/apis/fleet.cattle.io/v1alpha1/bundle.go b/pkg/apis/fleet.cattle.io/v1alpha1/bundle.go index 4ec0f0fec9..4130a80783 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 7777b7f203..985895510b 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