Merge pull request #787 from dymurray/statusupdate

pkg/ansible/controller,pkg/helm/controller; Enable status subresource by default

Author: hasbro17

CHANGELOG.md

@@ -25,6 +25,7 @@
 
 - The SDK now uses logr as the default logger to unify the logging output with the controller-runtime logs. Users can still use a logger of their own choice. See the [logging doc](https://github.com/operator-framework/operator-sdk/blob/master/doc/user/logging.md) on how the SDK initializes and uses logr.
 - Ansible Operator CR status better aligns with [conventions](https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md#typical-status-properties). ([#639](https://github.com/operator-framework/operator-sdk/pull/639))
+- The SDK now generates the CRD with the status subresource enabled by default. See the [client doc](https://github.com/operator-framework/operator-sdk/blob/masster/doc/user/client.md#updating-status-subresource) on how to update the status ssubresource. ([#787](https://github.com/operator-framework/operator-sdk/pull/787))
 
 ### Added
 

doc/ansible/dev/developer_guide.md

@@ -321,6 +321,84 @@ NAME                     DESIRED   CURRENT   UP-TO-DATE   AVAILABLE   AGE
 foo-operator       1         1         1            1           1m
 ```
 
+## Custom Resource Status Management
+The operator will automatically update the CR's `status` subresource with
+generic information about the previous Ansible run. This includes the number of
+successful and failed tasks and relevant error messages as seen below:
+
+```yaml
+status:
+  conditions:
+    - ansibleResult:
+      changed: 3
+      completion: 2018-12-03T13:45:57.13329
+      failures: 1
+      ok: 6
+      skipped: 0
+    lastTransitionTime: 2018-12-03T13:45:57Z
+    message: 'Status code was -1 and not [200]: Request failed: <urlopen error [Errno
+      113] No route to host>'
+    reason: Failed
+    status: "True"
+    type: Failure
+  - lastTransitionTime: 2018-12-03T13:46:13Z
+    message: Running reconciliation
+    reason: Running
+    status: "True"
+    type: Running
+```
+
+Ansible Operator also allows you as the developer to supply custom status
+values with the [k8s_status][k8s_status_module] Ansible Module. This allows the
+developer to update the `status` from within Ansible with any key/value pair as
+desired. By default, Ansible Operator will always include the generic Ansible
+run output as shown above. If you would prefer your application *not* update
+the status with Ansible output and would prefer to track the status manually
+from your application, then simply update the watches file with `manageStatus`:
+```yaml
+- version: v1
+  group: api.example.com
+  kind: Foo
+  role: /opt/ansible/roles/Foo
+  manageStatus: false
+```
+
+To update the `status` subresource with key `foo` and value `bar`, `k8s_status`
+can be used as shown:
+```yaml
+- k8s_status:
+    api_version: app.example.com/v1
+    kind: Foo
+    name: "{{ meta.name }}"
+    namespace: "{{ meta.namespace }}"
+    status:
+      foo: bar
+```
+
+### Using k8s_status Ansible module with `up local`
+This section covers the required steps to using the `k8s_status` Ansible module
+with `operator-sdk up local`. If you are unfamiliar with managing status from
+the Ansible Operator, see the [proposal for user-driven status
+management][manage_status_proposal].
+
+If your operator takes advantage of the `k8s_status` Ansible module and you are
+interested in testing the operator with `operator-sdk up local`, then it is
+imperative that the module is installed in a location that Ansible expects.
+This is done with the `library` configuration option for Ansible. For our
+example, we will assume the user is placing third-party Ansible modules in
+`/usr/share/ansible/library`.
+
+To install the `k8s_status` module, first set `ansible.cfg` to search in
+`/usr/share/ansible/library` for installed Ansible modules:
+```bash
+$ echo "library=/usr/share/ansible/library/" >> /etc/ansible/ansible.cfg
+```
+
+Add `k8s_status.py` to `/usr/share/ansible/library/`:
+```bash
+$ wget https://raw.githubusercontent.com/fabianvf/ansible-k8s-status-module/master/k8s_status.py -O /usr/share/ansible/library/k8s_status.py
+```
+
 ## Extra vars sent to Ansible
 The extra vars that are sent to Ansible are managed by the operator. The `spec`
 section will pass along the key-value pairs as extra vars.  This is equivalent
@@ -364,7 +442,9 @@ operator. The `meta` fields can be accesses via dot notation in Ansible as so:
 ```
 
 [k8s_ansible_module]:https://docs.ansible.com/ansible/2.6/modules/k8s_module.html
+[k8s_status_module]:https://github.com/fabianvf/ansible-k8s-status-module
 [openshift_restclient_python]:https://github.com/openshift/openshift-restclient-python
 [ansible_operator_user_guide]:../user-guide.md
+[manage_status_proposal]:../../proposals/ansible-operator-status.md
 [time_pkg]:https://golang.org/pkg/time/
 [time_parse_duration]:https://golang.org/pkg/time/#ParseDuration

doc/user-guide.md

@@ -116,7 +116,7 @@ For this example replace the generated Controller file `pkg/controller/memcached
 The example Controller executes the following reconciliation logic for each `Memcached` CR:
 - Create a memcached Deployment if it doesn't exist
 - Ensure that the Deployment size is the same as specified by the `Memcached` CR spec
-- Update the `Memcached` CR status with the names of the memcached pods
+- Update the `Memcached` CR status using the status writer with the names of the memcached pods
 
 The next two subsections explain how the Controller watches resources and how the reconcile loop is triggered. Skip to the [Build](#build-and-run-the-operator) section to see how to build and run the operator.
 

doc/user/client.md

@@ -217,7 +217,8 @@ func (r *ReconcileApp) Reconcile(request reconcile.Request) (reconcile.Result, e
 ```Go
 // Update updates the given obj in the Kubernetes cluster. obj must be a
 // struct pointer so that obj can be updated with the content returned
-// by the API server.
+// by the API server. Update does *not* update the resource's status
+// subresource
 func (c Client) Update(ctx context.Context, obj runtime.Object) error
 ```
 Example:
@@ -244,6 +245,44 @@ func (r *ReconcileApp) Reconcile(request reconcile.Request) (reconcile.Result, e
 }
 ```
 
+##### Updating Status Subresource
+
+When updating the [status subresource][cr-status-subresource] from the client,
+the StatusWriter must be used which can be gotten with `Status()`
+
+##### Status
+
+```Go
+// Status() returns a StatusWriter object that can be used to update the
+// object's status subresource
+func (c Client) Status() (client.StatusWriter, error)
+```
+
+Example:
+```Go
+import (
+	"context"
+	cachev1alpha1 "github.com/example-inc/memcached-operator/pkg/apis/cache/v1alpha1"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+)
+
+func (r *ReconcileApp) Reconcile(request reconcile.Request) (reconcile.Result, error) {
+	...
+	
+	mem := &cachev1alpha.Memcached{}
+	err := r.client.Get(context.TODO(), request.NamespacedName, mem)
+
+	...
+
+	ctx := context.TODO()
+	mem.Status.Nodes = []string{"pod1", "pod2"}
+	err := r.client.Status().Update(ctx, mem)
+
+	...
+}
+```
+
+
 #### Delete
 
 ```Go
@@ -385,7 +424,7 @@ func (r *ReconcileApp) Reconcile(request reconcile.Request) (reconcile.Result, e
 	podNames := getPodNames(podList.Items)
 	if !reflect.DeepEqual(podNames, app.Status.Nodes) {
 		app.Status.Nodes = podNames
-		if err := r.client.Update(context.TODO(), app); err != nil {
+		if err := r.client.Status().Update(context.TODO(), app); err != nil {
 			return reconcile.Result{}, err
 		}
 	}
@@ -453,3 +492,4 @@ func labelsForApp(name string) map[string]string {
 [doc-reconcile-reconciler]:https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg/reconcile#Reconciler
 [doc-osdk-handle]:https://github.com/operator-framework/operator-sdk/blob/master/doc/design/milestone-0.0.2/action-api.md#handler
 [doc-types-nsname]:https://godoc.org/k8s.io/apimachinery/pkg/types#NamespacedName
+[cr-status-subresource]:https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#status-subresource

example/memcached-operator/memcached_controller.go.tmpl

@@ -153,7 +153,7 @@ func (r *ReconcileMemcached) Reconcile(request reconcile.Request) (reconcile.Res
 	// Update status.Nodes if needed
 	if !reflect.DeepEqual(podNames, memcached.Status.Nodes) {
 		memcached.Status.Nodes = podNames
-		err := r.client.Update(context.TODO(), memcached)
+		err := r.client.Status().Update(context.TODO(), memcached)
 		if err != nil {
 			reqLogger.Error(err, "failed to update Memcached status")
 			return reconcile.Result{}, err

pkg/ansible/controller/reconcile.go

@@ -35,6 +35,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
@@ -115,7 +116,7 @@ func (r *AnsibleOperatorReconciler) Reconcile(request reconcile.Request) (reconc
 	}
 
 	if r.ManageStatus {
-		err = r.markRunning(u)
+		err = r.markRunning(u, request.NamespacedName)
 		if err != nil {
 			return reconcileResult, err
 		}
@@ -189,15 +190,20 @@ func (r *AnsibleOperatorReconciler) Reconcile(request reconcile.Request) (reconc
 		return reconcileResult, nil
 	}
 	if r.ManageStatus {
-		err = r.markDone(u, statusEvent, failureMessages)
+		err = r.markDone(u, request.NamespacedName, statusEvent, failureMessages)
 		if err != nil {
 			logger.Error(err, "failed to mark status done")
 		}
 	}
 	return reconcileResult, err
 }
 
-func (r *AnsibleOperatorReconciler) markRunning(u *unstructured.Unstructured) error {
+func (r *AnsibleOperatorReconciler) markRunning(u *unstructured.Unstructured, namespacedName types.NamespacedName) error {
+	// Get the latest resource to prevent updating a stale status
+	err := r.Client.Get(context.TODO(), namespacedName, u)
+	if err != nil {
+		return err
+	}
 	statusInterface := u.Object["status"]
 	statusMap, _ := statusInterface.(map[string]interface{})
 	crStatus := ansiblestatus.CreateFromMap(statusMap)
@@ -219,15 +225,25 @@ func (r *AnsibleOperatorReconciler) markRunning(u *unstructured.Unstructured) er
 		)
 		ansiblestatus.SetCondition(&crStatus, *c)
 		u.Object["status"] = crStatus.GetJSONMap()
-		err := r.Client.Update(context.TODO(), u)
+		err := r.Client.Status().Update(context.TODO(), u)
 		if err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func (r *AnsibleOperatorReconciler) markDone(u *unstructured.Unstructured, statusEvent eventapi.StatusJobEvent, failureMessages eventapi.FailureMessages) error {
+func (r *AnsibleOperatorReconciler) markDone(u *unstructured.Unstructured, namespacedName types.NamespacedName, statusEvent eventapi.StatusJobEvent, failureMessages eventapi.FailureMessages) error {
+	logger := logf.Log.WithName("markDone")
+	// Get the latest resource to prevent updating a stale status
+	err := r.Client.Get(context.TODO(), namespacedName, u)
+	if apierrors.IsNotFound(err) {
+		logger.Info("resource not found, assuming it was deleted", err)
+		return nil
+	}
+	if err != nil {
+		return err
+	}
 	statusInterface := u.Object["status"]
 	statusMap, _ := statusInterface.(map[string]interface{})
 	crStatus := ansiblestatus.CreateFromMap(statusMap)
@@ -262,7 +278,7 @@ func (r *AnsibleOperatorReconciler) markDone(u *unstructured.Unstructured, statu
 	// This needs the status subresource to be enabled by default.
 	u.Object["status"] = crStatus.GetJSONMap()
 
-	return r.Client.Update(context.TODO(), u)
+	return r.Client.Status().Update(context.TODO(), u)
 }
 
 func contains(l []string, s string) bool {

pkg/ansible/controller/status/types.go

@@ -149,14 +149,21 @@ func createConditionFromMap(cm map[string]interface{}) Condition {
 
 // Status - The status for custom resources managed by the operator-sdk.
 type Status struct {
-	Conditions []Condition `json:"conditions"`
+	Conditions   []Condition            `json:"conditions"`
+	CustomStatus map[string]interface{} `json:"-"`
 }
 
 // CreateFromMap - create a status from the map
 func CreateFromMap(statusMap map[string]interface{}) Status {
+	customStatus := make(map[string]interface{})
+	for key, value := range statusMap {
+		if key != "conditions" {
+			customStatus[key] = value
+		}
+	}
 	conditionsInterface, ok := statusMap["conditions"].([]interface{})
 	if !ok {
-		return Status{Conditions: []Condition{}}
+		return Status{Conditions: []Condition{}, CustomStatus: customStatus}
 	}
 	conditions := []Condition{}
 	for _, ci := range conditionsInterface {
@@ -167,7 +174,7 @@ func CreateFromMap(statusMap map[string]interface{}) Status {
 		}
 		conditions = append(conditions, createConditionFromMap(cm))
 	}
-	return Status{Conditions: conditions}
+	return Status{Conditions: conditions, CustomStatus: customStatus}
 }
 
 // GetJSONMap - gets the map value for the status object.
@@ -177,12 +184,11 @@ func CreateFromMap(statusMap map[string]interface{}) Status {
 // unstructured will fail and throw runtime exceptions.
 // Please note that this will return an empty map on error.
 func (status *Status) GetJSONMap() map[string]interface{} {
-	m := map[string]interface{}{}
 	b, err := json.Marshal(status)
 	if err != nil {
 		log.Error(err, "unable to marshal json")
-		return m
+		return status.CustomStatus
 	}
-	json.Unmarshal(b, &m)
-	return m
+	json.Unmarshal(b, &status.CustomStatus)
+	return status.CustomStatus
 }

pkg/helm/controller/reconcile.go

@@ -166,7 +166,7 @@ func (r HelmOperatorReconciler) Reconcile(request reconcile.Request) (reconcile.
 
 func (r HelmOperatorReconciler) updateResourceStatus(o *unstructured.Unstructured, status *types.HelmAppStatus) error {
 	o.Object["status"] = status
-	return r.Client.Update(context.TODO(), o)
+	return r.Client.Status().Update(context.TODO(), o)
 }
 
 func contains(l []string, s string) bool {

pkg/scaffold/crd.go

@@ -56,4 +56,6 @@ spec:
     singular: {{ .Resource.LowerKind }}
   scope: Namespaced
   version: {{ .Resource.Version }}
+  subresources:
+    status: {}
 `

pkg/scaffold/crd_test.go

@@ -50,4 +50,6 @@ spec:
     singular: appservice
   scope: Namespaced
   version: v1alpha1
+  subresources:
+    status: {}
 `