Merge pull request #59 from echen-98/test-failures

fixes to log delivery modification

Author: kumargauravsharma

apis/v1alpha1/ack-generate-metadata.yaml

@@ -1,13 +1,13 @@
 ack_generate_info:
-  build_date: "2021-10-12T16:29:01Z"
-  build_hash: 4b30ff5578e2f570d1c5b1741f3098be0d78e246
-  go_version: go1.16.5
+  build_date: "2021-10-14T18:44:40Z"
+  build_hash: 385779a205bea50e8762b76bc75cab957cf723b9
+  go_version: go1.15.2
   version: v0.15.1
-api_directory_checksum: 69c917d1e1ee2b196c23799a02417b920297bc74
+api_directory_checksum: 8e4808b17b3a814f0a624eee8d68a0b45ba5e2c4
 api_version: v1alpha1
 aws_sdk_go_version: v1.38.52
 generator_config_info:
-  file_checksum: 261e03a6b9fb25805b045293947f13e5674d2924
+  file_checksum: 28e3cd3119df8028a65eb0729a462b4354d27fa9
   original_file_name: generator.yaml
 last_modification:
   reason: API generation

apis/v1alpha1/annotations.go

@@ -0,0 +1,20 @@
+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//     http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package v1alpha1
+
+const (
+	// AnnotationPrefix is the prefix for all annotations specifically for
+	// the elasticache service.
+	AnnotationPrefix = "elasticache.services.k8s.aws/"
+)

apis/v1alpha1/generator.yaml

@@ -56,10 +56,12 @@ resources:
       AuthToken:
         is_secret: true
       LogDeliveryConfigurations:
-        is_read_only: true
+        is_read_only: true # creates an additional status field of the same name
         from:
           operation: CreateReplicationGroup
           path: ReplicationGroup.LogDeliveryConfigurations
+        compare: # removes the spec field from automatic delta comparison
+          is_ignored: true
     hooks:
       sdk_read_many_post_set_output:
         template_path: hooks/replication_group/sdk_read_many_post_set_output.go.tpl

generator.yaml

@@ -56,10 +56,12 @@ resources:
       AuthToken:
         is_secret: true
       LogDeliveryConfigurations:
-        is_read_only: true
+        is_read_only: true # creates an additional status field of the same name
         from:
           operation: CreateReplicationGroup
           path: ReplicationGroup.LogDeliveryConfigurations
+        compare: # removes the spec field from automatic delta comparison
+          is_ignored: true
     hooks:
       sdk_read_many_post_set_output:
         template_path: hooks/replication_group/sdk_read_many_post_set_output.go.tpl

helm/templates/deployment.yaml

@@ -61,6 +61,8 @@ spec:
           valueFrom:
             fieldRef:
               fieldPath: metadata.namespace
+        - name: AWS_ACCOUNT_ID
+          value: {{ .Values.aws.account_id | quote }}
         - name: AWS_REGION
           value: {{ .Values.aws.region }}
         - name: AWS_ENDPOINT_URL

helm/values.yaml

@@ -38,6 +38,7 @@ resources:
 aws:
   # If specified, use the AWS region for AWS API calls
   region: ""
+  account_id: ""
   endpoint_url: ""
 
 # log level for the controller

pkg/resource/replication_group/annotations.go

@@ -0,0 +1,25 @@
+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//     http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package replication_group
+
+import (
+	svcapitypes "github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1"
+)
+
+const (
+	// AnnotationLastRequestedLDCs is an annotation whose value is the marshaled list of pointers to
+	// LogDeliveryConfigurationRequest structs passed in as input to either the create or modify API called most
+	// recently
+	AnnotationLastRequestedLDCs = svcapitypes.AnnotationPrefix + "last-requested-log-delivery-configurations"
+)

pkg/resource/replication_group/custom_set_output.go

@@ -15,6 +15,8 @@ package replication_group
 
 import (
 	"context"
+	"encoding/json"
+
 	svcapitypes "github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1"
 	ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1"
 	"github.com/aws/aws-sdk-go/service/elasticache"
@@ -54,6 +56,7 @@ func (rm *resourceManager) CustomCreateReplicationGroupSetOutput(
 	ko *svcapitypes.ReplicationGroup,
 ) (*svcapitypes.ReplicationGroup, error) {
 	rm.customSetOutput(resp.ReplicationGroup, ko)
+	rm.setAnnotationsFields(r, ko)
 	return ko, nil
 }
 
@@ -64,6 +67,16 @@ func (rm *resourceManager) CustomModifyReplicationGroupSetOutput(
 	ko *svcapitypes.ReplicationGroup,
 ) (*svcapitypes.ReplicationGroup, error) {
 	rm.customSetOutput(resp.ReplicationGroup, ko)
+
+	// reset latest.spec.LDC to original value in desired to prevent stale data
+	// from the modify API being merged back into desired upon spec patching
+	var logDeliveryConfig []*svcapitypes.LogDeliveryConfigurationRequest
+	for _, ldc := range r.ko.Spec.LogDeliveryConfigurations {
+		logDeliveryConfig = append(logDeliveryConfig, ldc.DeepCopy())
+	}
+	ko.Spec.LogDeliveryConfigurations = logDeliveryConfig
+
+	rm.setAnnotationsFields(r, ko)
 	return ko, nil
 }
 
@@ -142,31 +155,8 @@ func (rm *resourceManager) customSetOutput(
 		ko.Status.AllowedScaleDownModifications = nil
 		ko.Status.AllowedScaleUpModifications = nil
 	}
-}
-
-// newListAllowedNodeTypeModificationsPayLoad returns an SDK-specific struct for the HTTP request
-// payload of the ListAllowedNodeTypeModifications API call.
-func (rm *resourceManager) newListAllowedNodeTypeModificationsPayLoad(respRG *elasticache.ReplicationGroup) (
-	*svcsdk.ListAllowedNodeTypeModificationsInput, error) {
-	res := &svcsdk.ListAllowedNodeTypeModificationsInput{}
-
-	if respRG.ReplicationGroupId != nil {
-		res.SetReplicationGroupId(*respRG.ReplicationGroupId)
-	}
-
-	return res, nil
-}
 
-func (rm *resourceManager) customSetOutputSupplementAPIs(
-	ctx context.Context,
-	r *resource,
-	respRG *elasticache.ReplicationGroup,
-	ko *svcapitypes.ReplicationGroup,
-) error {
-	events, err := rm.provideEvents(ctx, r.ko.Spec.ReplicationGroupID, 20)
-	if err != nil {
-		return err
-	}
+	// populate status logDeliveryConfigurations struct
 	if respRG.LogDeliveryConfigurations != nil {
 		var f11 []*svcapitypes.LogDeliveryConfiguration
 		for _, f11iter := range respRG.LogDeliveryConfigurations {
@@ -210,6 +200,31 @@ func (rm *resourceManager) customSetOutputSupplementAPIs(
 	} else {
 		ko.Status.LogDeliveryConfigurations = nil
 	}
+}
+
+// newListAllowedNodeTypeModificationsPayLoad returns an SDK-specific struct for the HTTP request
+// payload of the ListAllowedNodeTypeModifications API call.
+func (rm *resourceManager) newListAllowedNodeTypeModificationsPayLoad(respRG *elasticache.ReplicationGroup) (
+	*svcsdk.ListAllowedNodeTypeModificationsInput, error) {
+	res := &svcsdk.ListAllowedNodeTypeModificationsInput{}
+
+	if respRG.ReplicationGroupId != nil {
+		res.SetReplicationGroupId(*respRG.ReplicationGroupId)
+	}
+
+	return res, nil
+}
+
+func (rm *resourceManager) customSetOutputSupplementAPIs(
+	ctx context.Context,
+	r *resource,
+	respRG *elasticache.ReplicationGroup,
+	ko *svcapitypes.ReplicationGroup,
+) error {
+	events, err := rm.provideEvents(ctx, r.ko.Spec.ReplicationGroupID, 20)
+	if err != nil {
+		return err
+	}
 	ko.Status.Events = events
 	return nil
 }
@@ -249,3 +264,36 @@ func (rm *resourceManager) provideEvents(
 	}
 	return events, nil
 }
+
+// setAnnotationsFields copies the desired object's annotations, populates any
+// relevant fields, and sets the latest object's annotations to this newly populated map.
+// This should only be called upon a successful create or modify call.
+func (rm *resourceManager) setAnnotationsFields(
+	r *resource,
+	ko *svcapitypes.ReplicationGroup,
+) {
+	desiredAnnotations := r.ko.ObjectMeta.GetAnnotations()
+	annotations := make(map[string]string)
+	for k, v := range desiredAnnotations {
+		annotations[k] = v
+	}
+
+	rm.setLastRequestedLogDeliveryConfigurations(r, annotations)
+
+	ko.ObjectMeta.Annotations = annotations
+}
+
+// setLastRequestedLogDeliveryConfigurations copies desired.Spec.LogDeliveryConfigurations
+// into the annotations of the object.
+// r is the desired resource, and annotations is the annotations map modified by this method
+func (rm *resourceManager) setLastRequestedLogDeliveryConfigurations(
+	r *resource,
+	annotations map[string]string,
+) {
+	lastRequestedConfigs, err := json.Marshal(r.ko.Spec.LogDeliveryConfigurations)
+	if err != nil {
+		annotations[AnnotationLastRequestedLDCs] = "null"
+	} else {
+		annotations[AnnotationLastRequestedLDCs] = string(lastRequestedConfigs)
+	}
+}

pkg/resource/replication_group/delta.go

@@ -14,15 +14,18 @@
 package replication_group
 
 import (
+	"encoding/json"
+	"reflect"
 	"regexp"
 	"strings"
 
 	ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
 
+	svcapitypes "github.com/aws-controllers-k8s/elasticache-controller/apis/v1alpha1"
 	"github.com/aws-controllers-k8s/elasticache-controller/pkg/common"
 )
 
-// remove non-meaningful differences from delta
+// filterDelta removes non-meaningful differences from the delta and adds additional differences if necessary
 func filterDelta(
 	delta *ackcompare.Delta,
 	desired *resource,
@@ -37,6 +40,20 @@ func filterDelta(
 		}
 		// TODO: handle the case of a nil difference (especially when desired EV is nil)
 	}
+
+	// if server has given PreferredMaintenanceWindow a default value, no action needs to be taken
+	if delta.DifferentAt("Spec.PreferredMaintenanceWindow") {
+		if desired.ko.Spec.PreferredMaintenanceWindow == nil && latest.ko.Spec.PreferredMaintenanceWindow != nil {
+			common.RemoveFromDelta(delta, "Spec.PreferredMaintenanceWindow")
+		}
+	}
+
+	// note that the comparison is actually done between desired.Spec.LogDeliveryConfigurations and
+	// the last requested configurations saved in annotations (as opposed to latest.Spec.LogDeliveryConfigurations)
+	if logDeliveryRequiresUpdate(desired) {
+		delta.Add("Spec.LogDeliveryConfigurations", desired.ko.Spec.LogDeliveryConfigurations,
+			unmarshalLastRequestedLDCs(desired))
+	}
 }
 
 // returns true if desired and latest engine versions match and false otherwise
@@ -62,3 +79,24 @@ func engineVersionsMatch(
 
 	return false
 }
+
+// logDeliveryRequiresUpdate retrieves the last requested configurations saved in annotations and compares them
+// to the current desired configurations
+func logDeliveryRequiresUpdate(desired *resource) bool {
+	desiredConfigs := desired.ko.Spec.LogDeliveryConfigurations
+	lastRequestedConfigs := unmarshalLastRequestedLDCs(desired)
+	return !reflect.DeepEqual(desiredConfigs, lastRequestedConfigs)
+}
+
+// unmarshal the value found in annotations for the LogDeliveryConfigurations field requested in the last
+// successful create or modify call
+func unmarshalLastRequestedLDCs(desired *resource) []*svcapitypes.LogDeliveryConfigurationRequest {
+	var lastRequestedConfigs []*svcapitypes.LogDeliveryConfigurationRequest
+
+	annotations := desired.ko.ObjectMeta.GetAnnotations()
+	if val, ok := annotations[AnnotationLastRequestedLDCs]; ok {
+		_ = json.Unmarshal([]byte(val), &lastRequestedConfigs)
+	}
+
+	return lastRequestedConfigs
+}