Enable multiple update operations support on single resource (aws-con…

…trollers-k8s#245)

Issue aws-controllers-k8s#210

Description of changes:
The changes in this PR are to enable custom resource update scenario where Update operation on given resource does not support fields that are supported by its Create method, and there exist specialized methods to update such fields. These specialized method response provides the Resource object indicating the Update on the resource.

For example: create-replication-group API arguments include:

num-node-groups
replicas-per-node-group

however, modify-replication-group API arguments does not, instead modify-replication-group-shard-configuration and increase-replica-count, decrease-replica-count APIs corresponds to the related Update.

The approach taken in this PR is to add the details of such fields and corresponding custom update methods in the generator.yaml. For example:

resources:
  ReplicationGroup:
    custom_update_operations:
      UpdateShardConfiguration:
        fields:
          - NumNodeGroups
      UpdateReplicaCount:
        fields:
          - ReplicasPerNodeGroup

And custom update methods' (UpdateShardConfiguration and UpdateReplicaCount) implementation is provided in a separate code file: aws-controllers-k8s/services/elasticache/pkg/resource/replication_group/custom_update_api.go

These methods are supplied with DiffReporter which helps the code in this file determine the specific Update API to invoke based on the difference in desired, current resource details.

Co-authored-by: Kumar Gaurav Sharma <kumargsh@amazon.com>

pkg/compare/reporter.go

@@ -0,0 +1,45 @@
+package compare
+
+import (
+	"fmt"
+	"github.com/google/go-cmp/cmp"
+	"strings"
+)
+
+type DiffItem struct {
+	Path string
+	ValueA string
+	ValueB string
+}
+
+func (diff *DiffItem) String() string {
+	return fmt.Sprintf("%#v:\n\t-: %+v\n\t+: %+v\n", diff.Path, diff.ValueA, diff.ValueB)
+}
+
+type Reporter struct {
+	path        cmp.Path
+	Differences []DiffItem
+}
+
+func (reporter *Reporter) PushStep(ps cmp.PathStep) {
+	reporter.path = append(reporter.path, ps)
+}
+
+func (reporter *Reporter) PopStep() {
+	reporter.path = reporter.path[:len(reporter.path)-1]
+}
+
+func (reporter *Reporter) Report(result cmp.Result) {
+	if !result.Equal() {
+		a, b := reporter.path.Last().Values()
+		reporter.Differences = append(reporter.Differences, DiffItem{reporter.path.String(), fmt.Sprintf("%v",a), fmt.Sprintf("%v",b)});
+	}
+}
+
+func (reporter *Reporter) String() string {
+	var diffs []string
+	for _, diff := range reporter.Differences {
+		diffs = append(diffs, diff.String())
+	}
+	return strings.Join(diffs, "\n")
+}

pkg/generate/config/config.go

@@ -72,6 +72,16 @@ type ResourceConfig struct {
 	// Found and other common error types for primary resources, and thus we
 	// need these instructions.
 	Exceptions *ExceptionsConfig `json:"exceptions,omitempty"`
+
+	// CustomUpdateOperations provides custom operations that depend upon the
+	// changes to the fields on custom resource.
+	// For APIs where resource Update method does not have all fields that
+	// are supported by resource Create method and remaining fields have
+	// specialized API methods.
+	// Maps custom operation name (key) to list of Resource fields paths (values)
+	// so that custom operation is invoked if field value changes.
+	CustomUpdateOperations map[string]CustomUpdateOperationConfig `json:"custom_update_operations,omitempty"`
+
 	// Renames identifies fields in Operations that should be renamed.
 	Renames *RenamesConfig `json:"renames,omitempty"`
 	// ListOperation contains instructions for the code generator to generate
@@ -86,6 +96,14 @@ type ResourceConfig struct {
 	ListOperation *ListOperationConfig `json:"list_operation,omitempty"`
 }
 
+// Custom resource fields paths list that map to custom operation
+type CustomUpdateOperationConfig struct {
+	// string paths to fields on the custom resource
+	// Example:
+	//  For field ReplicationGroup.Spec.ReplicasPerNodeGroup, the string value would be: Spec.ReplicasPerNodeGroup
+	DiffPaths []string `json:"diff_paths"`
+}
+
 // UnpackAttributesMapConfig informs the code generator that the API follows a
 // pattern or using an "Attributes" `map[string]*string` that contains real,
 // schema'd fields of the primary resource, and that those fields should be

pkg/model/crd.go

@@ -304,6 +304,36 @@ func (r *CRD) IsPrimaryARNField(fieldName string) bool {
 		strings.EqualFold(fieldName, r.Names.Original+"arn")
 }
 
+// HasCustomUpdateOperations returns true if the resource has custom update operations
+// specified in generator config
+func (r *CRD) HasCustomUpdateOperations() bool {
+	if r.genCfg != nil {
+		resGenConfig, found := r.genCfg.Resources[r.Names.Original]
+		if found && resGenConfig.CustomUpdateOperations != nil {
+			return true
+		}
+	}
+	return false
+}
+
+// GetCustomUpdateOperations returns map of diff path (as key) and custom operation (as value) on custom resource,
+// as specified in generator config
+func (r *CRD) GetCustomUpdateOperations() map[string]string {
+	var diffPathToCustomOperationMap map[string]string
+	if r.genCfg != nil {
+		diffPathToCustomOperationMap = make(map[string]string)
+		resGenConfig, found := r.genCfg.Resources[r.Names.Original]
+		if found && resGenConfig.CustomUpdateOperations != nil {
+			for customOperation, fields := range resGenConfig.CustomUpdateOperations {
+				for _, diffPath := range fields.DiffPaths {
+					diffPathToCustomOperationMap[diffPath] = customOperation
+				}
+			}
+		}
+	}
+	return diffPathToCustomOperationMap
+}
+
 // ExceptionCode returns the name of the resource's Exception code for the
 // Exception having the exception code. If the generator config has
 // instructions for overriding the name of an exception code for a resource for

pkg/runtime/reconciler.go

@@ -163,14 +163,14 @@ func (r *reconciler) sync(
 		if r.rd.Equal(desired, latest) {
 			return nil
 		}
-		diff := r.rd.Diff(desired, latest)
+		diffReporter := r.rd.Diff(desired, latest)
 		r.log.V(1).Info(
 			"desired resource state has changed",
-			"diff", diff,
+			"diff", diffReporter.String(),
 			"arn", latest.Identifiers().ARN(),
 			"is_adopted", isAdopted,
 		)
-		latest, err = rm.Update(ctx, desired)
+		latest, err = rm.Update(ctx, desired, diffReporter)
 		if err != nil {
 			return err
 		}

pkg/types/aws_resource_descriptor.go

@@ -14,6 +14,7 @@
 package types
 
 import (
+	ackcompare "github.com/aws/aws-controllers-k8s/pkg/compare"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8srt "k8s.io/apimachinery/pkg/runtime"
 )
@@ -37,11 +38,11 @@ type AWSResourceDescriptor interface {
 	// the same. In other words, the Equal() method should be called with the
 	// same concrete implementing AWSResource type
 	Equal(AWSResource, AWSResource) bool
-	// Diff returns a string representing the difference between two supplied
-	// AWSResources/ The underlying types of the two supplied AWSResources
-	// should be the same. In other words, the Diff() method should be called
-	// with the same concrete implementing AWSResource type
-	Diff(AWSResource, AWSResource) string
+	// Diff returns a Reporter which provides the difference between two supplied
+	// AWSResources. The underlying types of the two supplied AWSResources should
+	// be the same. In other words, the Diff() method should be called with the
+	// same concrete implementing AWSResource type
+	Diff(AWSResource, AWSResource) *ackcompare.Reporter
 	// UpdateCRStatus accepts an AWSResource object and changes the Status
 	// sub-object of the AWSResource's Kubernetes custom resource (CR) and
 	// returns whether any changes were made

pkg/types/aws_resource_manager.go

@@ -15,7 +15,7 @@ package types
 
 import (
 	"context"
-
+	ackcompare "github.com/aws/aws-controllers-k8s/pkg/compare"
 	ackv1alpha1 "github.com/aws/aws-controllers-k8s/apis/core/v1alpha1"
 )
 
@@ -44,7 +44,8 @@ type AWSResourceManager interface {
 	// higher-level reonciler determines whether or not the desired differs
 	// from the latest observed and decides whether to call the resource
 	// manager's Update method
-	Update(context.Context, AWSResource) (AWSResource, error)
+	Update(context.Context, AWSResource, *ackcompare.Reporter) (AWSResource, error)
+
 	// Delete attempts to destroy the supplied AWSResource in the backend AWS
 	// service API.
 	Delete(context.Context, AWSResource) error

services/bookstore/pkg/resource/book/descriptor.go

@@ -14,6 +14,7 @@
 package book
 
 import (
+	ackcompare "github.com/aws/aws-controllers-k8s/pkg/compare"
 	acktypes "github.com/aws/aws-controllers-k8s/pkg/types"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -76,18 +77,19 @@ func (d *resourceDescriptor) Equal(
 	return cmp.Equal(ac.ko, bc.ko, opts)
 }
 
-// Diff returns a string representing the difference between two supplied
+// Diff returns a Reporter which provides the difference between two supplied
 // AWSResources. The underlying types of the two supplied AWSResources should
 // be the same. In other words, the Diff() method should be called with the
 // same concrete implementing AWSResource type
 func (d *resourceDescriptor) Diff(
 	a acktypes.AWSResource,
 	b acktypes.AWSResource,
-) string {
+) *ackcompare.Reporter {
 	ac := a.(*resource)
 	bc := b.(*resource)
-	opts := cmpopts.EquateEmpty()
-	return cmp.Diff(ac.ko, bc.ko, opts)
+	var diffReporter ackcompare.Reporter
+	cmp.Equal(ac.ko, bc.ko, cmp.Reporter(&diffReporter), cmp.AllowUnexported(svcapitypes.Book{}))
+	return &diffReporter
 }
 
 // UpdateCRStatus accepts an AWSResource object and changes the Status

services/bookstore/pkg/resource/book/manager.go

@@ -17,6 +17,7 @@ import (
 	"context"
 	"fmt"
 
+	ackcompare "github.com/aws/aws-controllers-k8s/pkg/compare"
 	ackv1alpha1 "github.com/aws/aws-controllers-k8s/apis/core/v1alpha1"
 	ackrt "github.com/aws/aws-controllers-k8s/pkg/runtime"
 	acktypes "github.com/aws/aws-controllers-k8s/pkg/types"
@@ -103,6 +104,7 @@ func (rm *resourceManager) Create(
 func (rm *resourceManager) Update(
 	ctx context.Context,
 	res acktypes.AWSResource,
+	diffReporter *ackcompare.Reporter,
 ) (acktypes.AWSResource, error) {
 	r := rm.concreteResource(res)
 	if r.ko == nil {

services/ecr/config/controller/deployment.yaml

@@ -27,11 +27,11 @@ spec:
         - ./bin/controller
         args:
         - --aws-account-id
-        - "${AWS_ACCOUNT_ID}"
+        - "$(AWS_ACCOUNT_ID)"
         - --aws-region
-        - "${AWS_REGION}"
+        - "$(AWS_REGION)"
         - --enable-development-logging
-        - "${ACK_ENABLE_DEVELOPMENT_LOGGING}"
+        - "$(ACK_ENABLE_DEVELOPMENT_LOGGING)"
         image: controller:latest
         name: controller
         resources:

services/ecr/config/crd/bases/ecr.services.k8s.aws_repositories.yaml

@@ -15,6 +15,8 @@ spec:
     plural: repositories
     singular: repository
   scope: Namespaced
+  subresources:
+    status: {}
   validation:
     openAPIV3Schema:
       description: Repository is the Schema for the Repositories API
@@ -69,20 +71,20 @@ spec:
                 constructed ARN for the resource
               properties:
                 arn:
-                  description: ARN is the Amazon Resource Name for the resource. This
-                    is a globally-unique identifier and is set only by the ACK service
-                    controller once the controller has orchestrated the creation of
-                    the resource OR when it has verified that an "adopted" resource
+                  description: 'ARN is the Amazon Resource Name for the resource.
+                    This is a globally-unique identifier and is set only by the ACK
+                    service controller once the controller has orchestrated the creation
+                    of the resource OR when it has verified that an "adopted" resource
                     (a resource where the ARN annotation was set by the Kubernetes
-                    user on the CR) exists and matches the supplied CR's Spec field
-                    values.
+                    user on the CR) exists and matches the supplied CR''s Spec field
+                    values. TODO(vijat@): Find a better strategy for resources that
+                    do not have ARN in CreateOutputResponse https://github.com/aws/aws-controllers-k8s/issues/270'
                   type: string
                 ownerAccountID:
                   description: OwnerAccountID is the AWS Account ID of the account
                     that owns the backend AWS service API resource.
                   type: string
               required:
-              - arn
               - ownerAccountID
               type: object
             conditions: