Repository: operator-framework/operator-sdk (go)

Issue:
reconcile-period and default value
<!-- Please submit only documentation-related issues with this form, or follow the
Contribute to Operator SDK guidelines (https://sdk.operatorframework.io/docs/contribution-guidelines/reporting-issues/) to submit a PR. Use this template while reporting a bug and provide as much info as possible. Not doing so may result in your bug not being addressed in a timely manner. Thanks!
-->

### What is the URL of the document?

(https://sdk.operatorframework.io/docs/building-operators/helm/)

### Which section(s) is the issue in?

Reference for Helm-based Operators

### What needs fixing?

For the  Helm-based Operators you can pass the parameter --reconcile-period but what's the default value ?

Is there a way to set it per CR in the watches.yaml like Ansible ?

#### Additional context




Pull request:
Closes #5565: Helm reconcile period from CR annotations or watches.yaml
<!--

Welcome to the Operator SDK! Before contributing, make sure to:

- Read the contributing guidelines https://github.com/operator-framework/operator-sdk/blob/master/CONTRIBUTING.MD
- Rebase your branch on the latest upstream master
- Link any relevant issues, PR's, or documentation
- Check that the commit message is concice and helpful:
    - When fixing an issue, add "Closes #<ISSUE_NUMBER>"
    - Sign your commit https://github.com/apps/dco
- Follow the below checklist if making a user-facing change 

-->

**Description of the change:**
1. Added the parsing of the `helm.sdk.operatorframework.io/reconcile-period` key in the custom resource's annotations. 
2. Added the option to specify the reconcile-period through the `watches.yaml` file.

The value obtained from either of these two is set to the `RequeueAfter` field of the reconciler for Helm. If both are specified, then the CR annotations takes precedence.

**Motivation for the change:**
Close #5565 

**Checklist**
- [x] Add a new changelog fragment in `changelog/fragments`
- [x] Add or update relevant sections of the docs website in [`website/content/en/docs`](https://github.com/operator-framework/operator-sdk/tree/master/website/content/en/docs)


**Note:**
As mentioned in my [comment](https://github.com/operator-framework/operator-sdk/issues/5565#issuecomment-1049421940) on the issue: this functionality already exists via a command line flag. This PR is to provide the user with the option of doing the same in the CR annotations or `watches.yaml`, which there seems to be a demand for. This feature already exists for the Ansible-based operator, so I thought it would be a good idea to do the same for the Helm-based operator. 

The current order of precedence is: flag < watches < CR annotations.

Patch:
diff --git a/changelog/fragments/helm-annotations-reconcile-period.yaml b/changelog/fragments/helm-annotations-reconcile-period.yaml
new file mode 100644
index 00000000..cf581abe
--- /dev/null
+++ b/changelog/fragments/helm-annotations-reconcile-period.yaml
@@ -0,0 +1,11 @@
+entries:
+  - description: >
+      For the helm/v1 plugin, parsed the "helm.sdk.operatorframework.io/reconcile-period" 
+      value from the custom resource annotations for helm operators. This value is then 
+      set to the 'ReconcilePeriod' field of the reconciler to reconcile the cluster
+      in the specified intervals of time.
+
+    kind: "addition"
+
+    # Is this a breaking change?
+    breaking: false
diff --git a/internal/cmd/helm-operator/run/cmd.go b/internal/cmd/helm-operator/run/cmd.go
index e909d27c..d6deb6d6 100644
--- a/internal/cmd/helm-operator/run/cmd.go
+++ b/internal/cmd/helm-operator/run/cmd.go
@@ -21,6 +21,7 @@ import (
 	"os"
 	"runtime"
 	"strings"
+	"time"
 
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -194,11 +195,16 @@ func run(cmd *cobra.Command, f *flags.Flags) {
 	}
 	for _, w := range ws {
 		// Register the controller with the factory.
+		reconcilePeriod := f.ReconcilePeriod
+		if w.ReconcilePeriod.Duration != time.Duration(0) {
+			reconcilePeriod = w.ReconcilePeriod.Duration
+		}
+
 		err := controller.Add(mgr, controller.WatchOptions{
 			Namespace:               namespace,
 			GVK:                     w.GroupVersionKind,
 			ManagerFactory:          release.NewManagerFactory(mgr, w.ChartDir),
-			ReconcilePeriod:         f.ReconcilePeriod,
+			ReconcilePeriod:         reconcilePeriod,
 			WatchDependentResources: *w.WatchDependentResources,
 			OverrideValues:          w.OverrideValues,
 			MaxConcurrentReconciles: f.MaxConcurrentReconciles,
diff --git a/internal/helm/controller/reconcile.go b/internal/helm/controller/reconcile.go
index 74f7e2a7..e005a552 100644
--- a/internal/helm/controller/reconcile.go
+++ b/internal/helm/controller/reconcile.go
@@ -61,8 +61,9 @@ const (
 	// Deprecated: use uninstallFinalizer. This will be removed in operator-sdk v2.0.0.
 	uninstallFinalizerLegacy = "uninstall-helm-release"
 
-	helmUpgradeForceAnnotation  = "helm.sdk.operatorframework.io/upgrade-force"
-	helmUninstallWaitAnnotation = "helm.sdk.operatorframework.io/uninstall-wait"
+	helmUpgradeForceAnnotation    = "helm.sdk.operatorframework.io/upgrade-force"
+	helmUninstallWaitAnnotation   = "helm.sdk.operatorframework.io/uninstall-wait"
+	helmReconcilePeriodAnnotation = "helm.sdk.operatorframework.io/reconcile-period"
 )
 
 // Reconcile reconciles the requested resource by installing, updating, or
@@ -387,10 +388,39 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
 		Name:     expectedRelease.Name,
 		Manifest: expectedRelease.Manifest,
 	}
+
+	// Determine the correct reconcile period based on the existing value in the reconciler and the
+	// annotations in the custom resource. If a reconcile period is specified in the custom resource
+	// annotations, this value will take precedence over the the existing reconcile period value
+	// (which came from either the command-line flag or the watches.yaml file).
+	finalReconcilePeriod, err := determineReconcilePeriod(r.ReconcilePeriod, o)
+	if err != nil {
+		log.Error(err, "Error: unable to parse reconcile period from the custom resource's annotations")
+		return reconcile.Result{}, err
+	}
+	r.ReconcilePeriod = finalReconcilePeriod
+
 	err = r.updateResourceStatus(ctx, o, status)
 	return reconcile.Result{RequeueAfter: r.ReconcilePeriod}, err
 }
 
+// returns the reconcile period that will be set to the RequeueAfter field in the reconciler. If any period
+// is specified in the custom resource's annotations, this will be returned. If not, the existing reconcile period
+// will be returned. An error will be thrown if the custom resource time period is not in proper format.
+func determineReconcilePeriod(currentPeriod time.Duration, o *unstructured.Unstructured) (time.Duration, error) {
+	// If custom resource annotations are present, they will take precedence over the command-line flag
+	if annot, exists := o.UnstructuredContent()["metadata"].(map[string]interface{})["annotations"]; exists {
+		if timeDuration, present := annot.(map[string]interface{})[helmReconcilePeriodAnnotation]; present {
+			annotationsPeriod, err := time.ParseDuration(timeDuration.(string))
+			if err != nil {
+				return currentPeriod, err // First return value does not matter, since err != nil
+			}
+			return annotationsPeriod, nil
+		}
+	}
+	return currentPeriod, nil
+}
+
 // returns the boolean representation of the annotation string
 // will return false if annotation is not set
 func hasAnnotation(anno string, o *unstructured.Unstructured) bool {
diff --git a/internal/helm/watches/watches.go b/internal/helm/watches/watches.go
index e2f43a1d..49f2109f 100644
--- a/internal/helm/watches/watches.go
+++ b/internal/helm/watches/watches.go
@@ -40,6 +40,7 @@ type Watch struct {
 	WatchDependentResources *bool                `json:"watchDependentResources,omitempty"`
 	OverrideValues          map[string]string    `json:"overrideValues,omitempty"`
 	Selector                metav1.LabelSelector `json:"selector"`
+	ReconcilePeriod         metav1.Duration      `json:"reconcilePeriod,omitempty"`
 }
 
 // UnmarshalYAML unmarshals an individual watch from the Helm watches.yaml file
diff --git a/website/content/en/docs/building-operators/helm/reference/advanced_features/reconcile-period.md b/website/content/en/docs/building-operators/helm/reference/advanced_features/reconcile-period.md
new file mode 100644
index 00000000..9bc4569a
--- /dev/null
+++ b/website/content/en/docs/building-operators/helm/reference/advanced_features/reconcile-period.md
@@ -0,0 +1,26 @@
+---
+title: Reconcile Period from Custom Resource Annotations
+linkTitle: Custom Resource Annotations Reconcile Period
+weight: 200
+description: Allow a user to set the desired reconcile period from the custom resource's annotations
+---
+
+While running a Helm-based operator, the reconcile-period can be specified through the custom resource's annotations under the `helm.sdk.operatorframework.io/reconcile-period` key. 
+This feature guarantees that an operator will get reconciled, at minimum, in the specified interval of time. In other words, it ensures that the cluster will not go longer
+than the specified reconcile-period without being reconciled. However, the cluster may be reconciled at any moment if there are changes detected in the desired state.
+
+The reconcile period can be specified in the custom resource's annotations in the following manner: 
+
+```sh
+...
+metadata:
+  name: nginx-sample
+  annotations:
+    helm.sdk.operatorframework.io/reconcile-period: 5s
+...
+```
+
+The value that is present under this key must be in the h/m/s format. For example, 1h2m4s, 3m0s, 4s are all valid values, but 1x3m9s is invalid. 
+
+**NOTE**: This is just one way of specifying the reconcile period for Helm-based operators. There are two other ways: using the `--reconcile-period` command-line flag and under the 'reconcilePeriod'
+key in the watches.yaml file. If these three methods are used simultaneously to specify reconcile period (which they should not be), the order of precedence is as follows. Custom Resource Annotations > watches.yaml > command-line flag.


Test patch:
diff --git a/internal/helm/controller/reconcile_test.go b/internal/helm/controller/reconcile_test.go
index 188e936f..c26c3b23 100644
--- a/internal/helm/controller/reconcile_test.go
+++ b/internal/helm/controller/reconcile_test.go
@@ -16,11 +16,61 @@ package controller
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
+func TestDetermineReconcilePeriod(t *testing.T) {
+	testPeriod1, _ := time.ParseDuration("10s")
+	obj1 := &unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"annotations": map[string]interface{}{
+					"name":                        "test-obj-1",
+					helmReconcilePeriodAnnotation: "3s",
+				},
+			},
+		},
+	}
+	expected1, _ := time.ParseDuration("3s")
+	finalPeriod1, err := determineReconcilePeriod(testPeriod1, obj1)
+	assert.Equal(t, nil, err, "Verify that no error is returned on parsing the time period")
+	assert.Equal(t, expected1, finalPeriod1, "Verify that the annotations period takes precedence")
+
+	testPeriod2, _ := time.ParseDuration("1h3m4s")
+	obj2 := &unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"annotations": map[string]interface{}{
+					"name": "test-obj-2",
+				},
+			},
+		},
+	}
+	expected2, _ := time.ParseDuration("1h3m4s")
+	finalPeriod2, err := determineReconcilePeriod(testPeriod2, obj2)
+	assert.Equal(t, nil, err, "Verify that no error is returned on parsing the time period")
+	assert.Equal(t, expected2, finalPeriod2, "Verify that when no time period is present under the CR's annotations, the original time period value gets used")
+
+	testPeriod3, _ := time.ParseDuration("5m15s")
+	obj3 := &unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"annotations": map[string]interface{}{
+					"name":                        "test-obj-3",
+					helmReconcilePeriodAnnotation: "4x",
+				},
+			},
+		},
+	}
+	finalPeriod3, err := determineReconcilePeriod(testPeriod3, obj3)
+	expected3, _ := time.ParseDuration("5m15s")
+	assert.NotEqual(t, nil, err, "Verify that error is thrown when invalid time period is passed in the CR annotations")
+	assert.Equal(t, expected3, finalPeriod3, "Verify that when a faulty time period is present under the CR's annotations, the original time period value gets used")
+}
+
 func TestHasAnnotation(t *testing.T) {
 	upgradeForceTests := []struct {
 		input       map[string]interface{}