remove status.EndpointConfig* from endpoint CRD (#92)

Description of changes:
1. Remove `status.latestEndpointConfgName` from CRD
  - This field was added for customers to have a reliable way to wait for a successful update because the endpoint goes back to `InService` even after a failed update and the `ResourceSynced` condition was set to `True`. 
  - A customer could not rely on `ResourceSynced` or `endpointStatus` to get the accurate picture and they needed do multiple checks the update is successful.
    -  e.g. `endpointStatus == InService && ACK.ResourceSynced == true && ACK.Terminal == False && failureReason = nil`.
    - To simplify this, latestEndpointConfig was added to the status so Cx can wait on just `spec.EndpointConfigName == status.latestEndpointConfigName`
  - With this change, the `ResourceSynced` condition will not go to `True` on failed update since the desired state cannot be reached. Customers can just wait and timeout on:
    - `endpointStatus == InService && ACK.ResourceSynced == true` and check for failure reason in case of timeout. The latestEndpointConfig from service will be in the Terminal condition message for convinence
2. Move `status.lastEndpointConfigForUpdate` to metadata
  - This is purely a metadata field. It was only required by the controller to stop retrying an update and hence has been moved to annotation.
  - Moving this to annotations gives us a two-way door. In case we find a better heuristic in future to differentiate between update related error and related to service modification, this can be removed easily with having any backward breaking changes to the CRD

Testing:
```
pytest tests/test_endpoint.py
```
PR build will pass after #91  is merged since there is a dependency on the error message

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: surajkota

apis/v1alpha1/ack-generate-metadata.yaml

@@ -1,14 +1,14 @@
 ack_generate_info:
-  build_date: "2021-09-01T00:07:38Z"
-  build_hash: 6f22b7b568e25b4ee007bb1ab5f9338c16daf172
+  build_date: "2021-09-01T01:05:16Z"
+  build_hash: 07d7ea755625b2cac93bbf293bf5a8cab24ecb68
   go_version: go1.16.4 linux/amd64
   version: v0.13.0
-api_directory_checksum: 13c67d2ac904c5b3c1ddea34aba38d90f06f3adc
+api_directory_checksum: a0f6457435f0a16addd9cf7d0acef2830722bd78
 api_version: v1alpha1
 aws_sdk_go_version: v1.38.11
 generator_config_info:
-  file_checksum: 95bf43b6a68c009a14db1ba7bd91293ca03aee15
+  file_checksum: 5431bfc09117e092ee89a98edcbad6934363dea2
   original_file_name: generator.yaml
 last_modification:
   reason: API generation
-  timestamp: 2021-09-01 00:07:46.08290738 +0000 UTC
+  timestamp: 2021-09-01 01:05:19.682637714 +0000 UTC

apis/v1alpha1/endpoint.go

@@ -73,7 +73,7 @@ resources:
         - EndpointUpdateError
     hooks:
       sdk_read_one_post_set_output:
-        code: rm.customDescribeEndpointSetOutput(resp, ko)
+        code: rm.customDescribeEndpointSetOutput(ko)
       sdk_update_pre_build_request:
         template_path: endpoint/sdk_update_pre_build_request.go.tpl
       sdk_update_post_set_output:
@@ -96,16 +96,6 @@ resources:
         from:
           operation: DescribeEndpoint
           path: FailureReason
-      LatestEndpointConfigName:
-        is_read_only: true
-        from:
-          operation: DescribeEndpoint
-          path: EndpointConfigName
-      LastEndpointConfigNameForUpdate:
-        is_read_only: true
-        from:
-          operation: DescribeEndpointConfig
-          path: EndpointConfigName
       CreationTime:
         is_read_only: true
         from:

apis/v1alpha1/generator.yaml

@@ -162,17 +162,10 @@ spec:
                 description: If the status of the endpoint is Failed, the reason why
                   it failed.
                 type: string
-              lastEndpointConfigNameForUpdate:
-                description: Name of the Amazon SageMaker endpoint configuration.
-                type: string
               lastModifiedTime:
                 description: A timestamp that shows when the endpoint was last modified.
                 format: date-time
                 type: string
-              latestEndpointConfigName:
-                description: The name of the endpoint configuration associated with
-                  this endpoint.
-                type: string
               productionVariants:
                 description: An array of ProductionVariantSummary objects, one for
                   each model hosted behind this endpoint.

apis/v1alpha1/zz_generated.deepcopy.go

@@ -73,7 +73,7 @@ resources:
         - EndpointUpdateError
     hooks:
       sdk_read_one_post_set_output:
-        code: rm.customDescribeEndpointSetOutput(resp, ko)
+        code: rm.customDescribeEndpointSetOutput(ko)
       sdk_update_pre_build_request:
         template_path: endpoint/sdk_update_pre_build_request.go.tpl
       sdk_update_post_set_output:
@@ -96,16 +96,6 @@ resources:
         from:
           operation: DescribeEndpoint
           path: FailureReason
-      LatestEndpointConfigName:
-        is_read_only: true
-        from:
-          operation: DescribeEndpoint
-          path: EndpointConfigName
-      LastEndpointConfigNameForUpdate:
-        is_read_only: true
-        from:
-          operation: DescribeEndpointConfig
-          path: EndpointConfigName
       CreationTime:
         is_read_only: true
         from:

config/crd/bases/sagemaker.services.k8s.aws_endpoints.yaml

@@ -162,17 +162,10 @@ spec:
                 description: If the status of the endpoint is Failed, the reason why
                   it failed.
                 type: string
-              lastEndpointConfigNameForUpdate:
-                description: Name of the Amazon SageMaker endpoint configuration.
-                type: string
               lastModifiedTime:
                 description: A timestamp that shows when the endpoint was last modified.
                 format: date-time
                 type: string
-              latestEndpointConfigName:
-                description: The name of the endpoint configuration associated with
-                  this endpoint.
-                type: string
               productionVariants:
                 description: An array of ProductionVariantSummary objects, one for
                   each model hosted behind this endpoint.

generator.yaml

@@ -17,9 +17,15 @@
 package endpoint
 
 import (
+	"github.com/aws/aws-sdk-go/aws"
+
+	ackcondition "github.com/aws-controllers-k8s/runtime/pkg/condition"
+	ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
 	svcapitypes "github.com/aws-controllers-k8s/sagemaker-controller/apis/v1alpha1"
 	svccommon "github.com/aws-controllers-k8s/sagemaker-controller/pkg/common"
 	svcsdk "github.com/aws/aws-sdk-go/service/sagemaker"
+	
 )
 
 // CustomUpdateConditions sets conditions (terminal) on supplied endpoint.
@@ -36,5 +42,21 @@ func (rm *resourceManager) CustomUpdateConditions(
 	resourceName := resourceGK.Kind
 	// If the latestStatus == terminalStatus we will set
 	// the terminal condition and terminal message.
-	return svccommon.SetTerminalState(conditionManager, latestStatus, &resourceName, terminalStatus)
+	updated := svccommon.SetTerminalState(conditionManager, latestStatus, &resourceName, terminalStatus)
+
+	// Continue setting ResourceSynced condition to false in case of failed update
+	// since desired and latest will be different until the issue is fixed.
+	// Customer can use this condition state and FailureReason to determine
+	// the correct course of action in case the update to Endpoint fails
+	// Customer will also have additional information like latest endpointConfig
+	// in condition message and last endpointconfig used for update in annotations
+	if err != nil {
+		awsErr, ok := ackerr.AWSError(err)
+		if ok && awsErr.Code() == "EndpointUpdateError" {
+			ackcondition.SetSynced(conditionManager, corev1.ConditionFalse, aws.String(awsErr.Error()), nil)
+			return true
+		}
+	}
+
+	return updated
 }

helm/crds/sagemaker.services.k8s.aws_endpoints.yaml

@@ -11,13 +11,11 @@
 // express or implied. See the License for the specific language governing
 // permissions and limitations under the License.
 
-// Use this file if the Status/Spec of the CR needs to be modified after
-// create/describe/update operation
-
 package endpoint
 
 import (
 	"context"
+	"fmt"
 	"strings"
 
 	ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
@@ -39,39 +37,29 @@ var (
 
 	resourceName = resourceGK.Kind
 
-	FailUpdateError = awserr.New("EndpointUpdateError", "unable to update endpoint. check FailureReason", nil)
+	lastEndpointConfigForUpdateAnnotation = fmt.Sprintf("%s/last-endpoint-config-for-update", resourceGK.Group)
 
 	FailureReasonInternalServiceErrorPrefix = "Request to service failed"
 )
 
 // customDescribeEndpointSetOutput sets the resource ResourceSynced condition to False if endpoint is
 // being modified by AWS
-func (rm *resourceManager) customDescribeEndpointSetOutput(
-	resp *svcsdk.DescribeEndpointOutput,
-	ko *svcapitypes.Endpoint,
-) {
-	// Workaround: Field config for LatestEndpointConfigName of generator config
-	// does not code generate this correctly since this field is part of Spec
-	// SageMaker users will need following information:
-	// 	 - latestEndpointConfig
-	// 	 - desiredEndpointConfig
-	// 	 - LastEndpointConfigNameForUpdate
-	// 	 - FailureReason
-	// to determine the correct course of action in case of update to Endpoint fails
-	if resp.EndpointConfigName != nil {
-		ko.Status.LatestEndpointConfigName = resp.EndpointConfigName
-	} else {
-		ko.Status.LatestEndpointConfigName = nil
-	}
-
-	svccommon.SetSyncedCondition(&resource{ko}, resp.EndpointStatus, &resourceName, &modifyingStatuses)
+func (rm *resourceManager) customDescribeEndpointSetOutput(ko *svcapitypes.Endpoint) {
+	svccommon.SetSyncedCondition(&resource{ko}, ko.Status.EndpointStatus, &resourceName, &modifyingStatuses)
 }
 
 // customUpdateEndpointSetOutput sets the resource ResourceSynced condition to False if endpoint is
 // being updated. At this stage we know call to updateEndpoint was successful.
 func (rm *resourceManager) customUpdateEndpointSetOutput(ko *svcapitypes.Endpoint) {
-	// no nil check present here since Spec.EndpointConfigName is a required field
-	ko.Status.LastEndpointConfigNameForUpdate = ko.Spec.EndpointConfigName
+	// set last endpoint config name used for udapte in annotations
+	// no nil check present since Spec.EndpointConfigName is a required field
+	annotations := ko.ObjectMeta.GetAnnotations()
+	if annotations == nil {
+		annotations = make(map[string]string)
+	}
+	annotations[lastEndpointConfigForUpdateAnnotation] = *ko.Spec.EndpointConfigName
+	ko.ObjectMeta.SetAnnotations(annotations)
+
 	// injecting Updating status to keep the Sync condition message and status.endpointStatus in sync
 	ko.Status.EndpointStatus = aws.String(svcsdk.EndpointStatusUpdating)
 
@@ -98,7 +86,15 @@ func (rm *resourceManager) customUpdateEndpointPreChecks(
 
 	failureReason := latest.ko.Status.FailureReason
 	desiredEndpointConfig := desired.ko.Spec.EndpointConfigName
-	lastEndpointConfigForUpdate := desired.ko.Status.LastEndpointConfigNameForUpdate
+
+	var lastEndpointConfigForUpdate *string = nil
+	// get last endpoint config name used for update from annotations
+	annotations := desired.ko.ObjectMeta.GetAnnotations()
+	for k, v := range annotations {
+		if k == lastEndpointConfigForUpdateAnnotation {
+			lastEndpointConfigForUpdate = &v
+		}
+	}
 
 	// Case 2 - EndpointStatus == Failed
 	if *latestStatus == svcsdk.EndpointStatusFailed ||
@@ -118,7 +114,7 @@ func (rm *resourceManager) customUpdateEndpointPreChecks(
 		// 1 & 2 does not guarantee an update Failed. Hence we need to look at `*latestEndpointConfigName` to determine if the update was unsuccessful
 		// `*desiredEndpointConfig != *latestEndpointConfig` + `*desiredEndpointConfig == *lastEndpointConfigForUpdate`+ `FailureReason != nil` indicate that an update is needed,
 		// has already been tried and failed.
-		return FailUpdateError
+		return awserr.New("EndpointUpdateError", fmt.Sprintf("unable to update endpoint. check FailureReason. latest EndpointConfigName is %s", *latest.ko.Spec.EndpointConfigName), nil)
 	}
 
 	return nil