Removed discussion of metrics.

Added implementation details regarding controller predicates.

Rephrased and wordsmithed content.

Signed-off-by: Travis Mattera <travis@mattera.io>

rfcs/0006-alternative-suspend-control/README.md

@@ -4,7 +4,7 @@
 
 **Creation date:** 2023-09-20
 
-**Last update:** 2023-10-10
+**Last update:** 2023-10-18
 
 
 ## Summary
@@ -13,9 +13,9 @@ This RFC proposes an alternative method to indicate the suspended state of
 suspendable resources to flux controllers through object metadata. It presents
 an annotation key that can be used to suspend a resource from reconciliation as
 an alternative to the `.spec.suspend` field.  It does not address the
-deprecation of the the `.spec.suspend` field from the resource apis.  This
+deprecation of this field from the resource apis.  This annotation can
-annotation can optionally act as a vehicle for communicating contextual
+optionally act as a vehicle for communicating contextual information about the
-information about the suspended resource to users.
+suspended resource to users.
 
 
 ## Motivation
@@ -30,11 +30,12 @@ the reason for the suspension, in the object itself.
 ### Goals
 
 The flux reconciliation loop will support recognizing a resource's suspend
-status based on either the `.spec.suspend` field or a specific metadata
+status from either the api field or the designated metadata annotation key.
-annotation key.  The flux cli will similarly recognize this state with `get`
+The flux cli will similarly recognize this state with `get` commands and but
-commands and but will only manipulate the annotation key under `suspend` and
+will alter only the metadata under the `suspend` command.  The `resume` command
-`resume` commands.  The flux cli will support optionally setting the suspend
+will still alter the api field but additionally the metadata.  The
-metadata annotation value with a user supplied string for a contextual message.
+flux cli will support optionally setting the suspend metadata annotation value
+with a user supplied string for a contextual message.
 
 ### Non-Goals
 
@@ -46,9 +47,9 @@ RFC.
 
 Register a flux resource metadata key `reconcile.fluxcd.io/suspended` with a
 suspend semantic to be interpreted by controllers and manipulated by the cli.
-The presence of the annotation key is an alternative to the `.spec.suspend:
+The presence of the annotation key is an alternative to the `.spec.suspend` api
-true` api field setting when considering if a resource is suspended or not.
+field setting when considering if a resource is suspended or not. The
-The annotation key is set by a `flux suspend` command and removed by a `flux
+annotation key is set by a `flux suspend` command and removed by a `flux
 resume` command.  The annotation key value is open for communicating a message
 or reason for the object's suspension.  The value can be set using a
 `--message` flag to the `suspend` command.
@@ -58,15 +59,17 @@ or reason for the object's suspension.  The value can be set using a
 #### Suspend/Resume without Generation Roll
 
 Currently when a resource is set to suspended or resumed the `.spec.suspend`
-field is mutated which results in a roll of the generation number in the
+field is mutated which increments the `.metadata.generation` field and after
-`.metadata.generation` and fields `.status.observedGeneration` number.  The
+successful reconciliation the `.status.observedGeneration` number.  The
 community believes that the generation change for this reason is not in
-alignment with gitops principles.
+alignment with gitops principles.  In more detail, upon suspension the
+generation increments but the observed generation lags since reconciliation is
+not completed successfully.
 
 The flux controllers should recognize that a resource is suspended or
 unsuspended from the presence of a special metadata key -- this key can be
-added, removed or changed without patching the object and rolling the
+added, removed or changed without patching the object in such a way that the
-generation number.
+generation number increments.
 
 #### Seeing Suspend State
 
@@ -79,11 +82,11 @@ functionality that should be preserved.
 
 Often there is a purpose behind suspending a resource with the flux cli,
 whether it be during incident response, source manifest cutovers, or various
-other scenarios. The `flux diff` command provides a great UX for determining
+other scenarios. The `flux diff` command provides an illustrative UX for
-what will change if a suspended resource is resumed, but it doesn't help
+determining what will change if a suspended resource is resumed, but neither it
-explain _why_ something is paused or when it would be ok to resume
+nor `flux get` help explain _why_ something is paused or when it would be ok to
-reconciliation. On distributed teams this can become a point of friction as it
+resume reconciliation. On distributed teams this can become a point of friction
-needs to be communicated among group stakeholders.
+as it needs to be communicated among group stakeholders.
 
 Flux users should have a way to succinctly signal to other users why a resource
 is suspended on the resource itself.
@@ -107,129 +110,30 @@ not violate the current apis.
 ### Common
 
 The `reconcile.fluxcd.io/suspended` annotation key string and a getter function
-would be made avaiable for controllers the cli to recognize and manipulate the
+would be made avaiable for controllers and the cli to recognize and manipulate the
 suspend object metadata.
 
-``` # github.com/fluxcd/pkg/apis/meta
-
-
-// SuspendAnnotation is the annotation used to indicate an object is suspended
-and optionally to store metadata about why a resource // was set to suspended.
-const SuspendedAnnotation string = "reconcile.fluxcd.io/suspended"
-
-// SuspendAnnotationValue returns a value for the suspended annotation, which
-can be used to detect // changes; and, a boolean indicating whether the
-annotation was set. func SuspendedAnnotationValue(annotations
-map[string]string) (string, bool) { suspend, ok :=
-annotations[SuspendedAnnotation] return suspend, ok }
-
-// SetSuspendedAnnotation sets a suspend key with a supplied string value in an
-object's metadata annotations func SetSuspendedAnnotation(annotations
-*map[string]string, token string) { if annotations == nil { annotations =
-&map[string]string{} } if token != "" { (*annotations)[SuspendedAnnotation] =
-token } else { (*annotations)[SuspendedAnnotation] = "true" } }
-
-// UnsetSuspendedAnnotation removes a suspend key from an object's metadata
-annotations func UnsetSuspendedAnnotation(annotations *map[string]string) {
-delete(*annotations, SuspendedAnnotation) } ```
-
-An event predicate would skip attempting to reconcile a resource with the
-suspend annotation set.
-
-``` # github.com/fluxcd/pkg/apis/predicates
-
-# suspended.go
-
-// Update implements the default UpdateEvent filter for filtering by
-meta.SuspendedAnnotation existence func (SuspendedPredicate) Update(e
-event.UpdateEvent) bool { if e.ObjectOld == nil || e.ObjectNew == nil { return
-false }
-
-    if _, ok := metav1.SuspendedAnnotationValue(e.ObjectNew.GetAnnotations());
-    !ok { return true } return false } ```
-
 ### Controllers
 
 Flux controllers would skip reconciling a resource based on an `OR` of (1) the
 api `.spec.suspend` and (2) the existence of the suspend metadata annotation
-key.
+key.  This would be implemented in the controller predicates to completely skip
-
+any reconciliation cycle of suspended objects.
-``` # github.com/fluxcd/kustomize-controller/api/v1 # kustomization_types.go
-
-// IsSuspended returns the effective suspend state of the object. func (in
-Kustomization) IsSuspended() bool { if in.Spec.Suspend { return true } if _, ok
-:= meta.SuspendedAnnotationValue(in.Annotations); ok { return true } return
-false }
-
-# github.com/fluxcd/kustomize-controller/controller #
-kustomization_controller.go
-
-// Skip reconciliation if the object is suspended. // if obj.Spec.Suspend {  //
-no longer using `.spec.suspend` directly if obj.Status.IsSuspended {
-log.Info("Reconciliation is suspended for this object") return ctrl.Result{},
-nil } ```
 
 ### cli
 
-The flux cli would recognize the suspend state from the suspend object status.
+The `get` command would recognize the suspend state from the union of the
-
+`.spec.suspend` and the presence of the suspended annotation.
-``` # github.com/fluxcd/flux2/main
-
-# get_source_git.go
-
-func (a *gitRepositoryListAdapter) summariseItem(i int, includeNamespace bool,
-includeKind bool) []string { ... return append(nameColumns(&item,
-includeNamespace, includeKind), //    revision,
-strings.Title(strconv.FormatBool(item.Spec.Suspend)), status, msg) revision,
-strings.Title(strconv.FormatBool(item.IsSuspended)), status, msg) } ```
-
-The flux cli would manipulate the suspend metadata but forgo toggling the
-`.spec.suspend` setting.  An optional `--message|-m` flag would support setting
-the suspended annotation value to a user-specified string.
-
-``` # github.com/fluxcd/flux2/main
-
-# suspend.go
-
-type SuspendFlags struct { ... message string }
-
-func init() { ... suspendCmd.PersistentFlags().StringVarP(&suspendArgs.message,
-"message", "m", "", "set a message about why the resource is suspended, the
-message will show up under the reconcile.fluxcd.io/suspended annotation") ... }
-
-type suspendable interface { ... // setSuspended() setSuspended(message string)
-}
-
-type suspendCommand struct { ... // obj.setSuspended()
-obj.setSuspended(suspendArgs.message) }
-
-# suspend_helmrelease.go
-
-func (obj helmReleaseAdapter) isSuspended() bool { return
-obj.HelmRelease.IsSuspended()                               // use the resource
-api spec method }
-
-func (obj helmReleaseAdapter) setSuspended(message string) {
-meta.SetSuspendedAnnotation(&obj.HelmRelease.Annotations, message) // use the
-common annotation setter }
-
-# resume_helmrelease.go
-
-func (obj helmReleaseAdapter) setUnsuspended() {
-meta.UnsetSuspendedAnnotation(&obj.HelmRelease.Annotations)        // use the
-common annotation unsetter } ```
 
-### Metrics
+The `suspend` command would add the suspend annotation but forgo modifying the
+`.spec.suspend` field.
 
-Custom metrics can be [configured under
+The `resume` command would remove the suspend annotation and modify the
-kube-state-metrics](https://fluxcd.io/flux/monitoring/custom-metrics/#adding-custom-metrics)
+`.spec.suspend` field to `false`.
-using the object metadata annotation path.
 
-```
+The suspend annotation would by default be set to a generic value.  An optional
-- name: "resource_info" help: "The current state of a GitOps Toolkit resource."
+cli flag (eg `--message`) would support setting the suspended annotation value
-  each: type: Info info: labelsFromPath: name: [metadata, name] labelsFromPath:
+to a user-specified string.
-  ... suspendedAnnotation: [ metadata, annotations,
-  reconcile.fluxcd.io/suspended ] ```
 
 ## Implementation History