Skip to content

Commit 0292eaa

Browse files
authored
Endpoint custom code using hooks and unit tests (#84)
### Description of changes: - Update custom code in endpoint to use hooks and common pkg - This was also needed to get the updated observed state in unit tests like `lastTransitionTime`, consistent messaging in conditions etc. - Unit tests for all CRUD operation - Ordered by conditions in code (check for required input/pre-checks, API returns error, set custom output post successful API call) - Overall coverage is at 75.2% but sdk.go is at 92.2% and custom code is 88.9% covered - only lines not covered in custom code are few nil checks ### Testing ``` make test pytest tests/test_endpoint.py ``` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 229074c commit 0292eaa

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+1103
-202
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,6 @@
55
/docs/site
66
bin
77
build
8+
9+
*.out
10+
*.html

Makefile

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ BUILDDATE=$(shell date -u +'%Y-%m-%dT%H:%M:%SZ')
1515
GO_LDFLAGS=-ldflags "-X main.version=$(VERSION) \
1616
-X main.buildHash=$(GITCOMMIT) \
1717
-X main.buildDate=$(BUILDDATE)"
18+
GOCOVER=go tool cover
1819

1920
.PHONY: all test clean-mocks mocks
2021

@@ -24,8 +25,9 @@ test: ## Run code tests
2425
go test -v ./...
2526

2627
test-cover: | mocks ## Run code tests with resources coverage
27-
go test -coverpkg=./pkg/resource/... -covermode=count -coverprofile=coverage.out ./...
28-
go tool cover -func=coverage.out
28+
go test -coverpkg=./pkg/resource/endpoint/... -covermode=count -coverprofile=coverage.out ./...
29+
$(GOCOVER) -func=coverage.out
30+
$(GOCOVER) -html=coverage.out -o coverage.html
2931

3032
clean-mocks: ## Remove mocks directory
3133
rm -r mocks
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
ack_generate_info:
2-
build_date: "2021-08-12T23:35:21Z"
2+
build_date: "2021-08-25T04:12:22Z"
33
build_hash: c77aa9c75d944952dee198029ba9822691cd82b0
44
go_version: go1.16.4 linux/amd64
55
version: v0.6.0
66
api_directory_checksum: f7275158658c496010e6e3d2d24b3aba37e2e1f4
77
api_version: v1alpha1
88
aws_sdk_go_version: v1.38.11
99
generator_config_info:
10-
file_checksum: 4acc645991dfd96fd543c3c34df274a8e4f4787e
10+
file_checksum: 39823b712f9bd1035dc1c5ff1323a91930ce3db0
1111
original_file_name: generator.yaml
1212
last_modification:
1313
reason: API generation
14-
timestamp: 2021-08-12 23:35:25.778426245 +0000 UTC
14+
timestamp: 2021-08-25 04:12:25.649196147 +0000 UTC

apis/v1alpha1/generator.yaml

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,11 @@ operations:
88
StopTransformJob:
99
operation_type: Delete
1010
resource_name: TransformJob
11-
DescribeEndpoint:
12-
set_output_custom_method_name: customDescribeEndpointSetOutput
1311
UpdateEndpoint:
14-
custom_implementation: customUpdateEndpoint
15-
set_output_custom_method_name: customUpdateEndpointSetOutput
1612
override_values:
1713
RetainAllVariantProperties: true
1814
DescribeModelPackage:
1915
custom_check_required_fields_missing_method: customCheckRequiredFieldsMissingMethod
20-
DeleteEndpoint:
21-
custom_implementation: customDeleteEndpoint
2216
StopHyperParameterTuningJob:
2317
operation_type: Delete
2418
resource_name: HyperParameterTuningJob
@@ -100,8 +94,14 @@ resources:
10094
# Custom error
10195
- EndpointUpdateError
10296
hooks:
103-
sdk_create_post_set_output:
104-
code: rm.customSetOutput(desired, aws.String(svcsdk.EndpointStatusCreating), ko)
97+
sdk_read_one_post_set_output:
98+
code: rm.customDescribeEndpointSetOutput(resp, ko)
99+
sdk_update_pre_build_request:
100+
template_path: endpoint/sdk_update_pre_build_request.go.tpl
101+
sdk_update_post_set_output:
102+
code: rm.customUpdateEndpointSetOutput(ko)
103+
sdk_delete_pre_build_request:
104+
template_path: common/sdk_delete_pre_build_request.go.tpl
105105
fields:
106106
EndpointStatus:
107107
is_read_only: true

generator.yaml

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,11 @@ operations:
88
StopTransformJob:
99
operation_type: Delete
1010
resource_name: TransformJob
11-
DescribeEndpoint:
12-
set_output_custom_method_name: customDescribeEndpointSetOutput
1311
UpdateEndpoint:
14-
custom_implementation: customUpdateEndpoint
15-
set_output_custom_method_name: customUpdateEndpointSetOutput
1612
override_values:
1713
RetainAllVariantProperties: true
1814
DescribeModelPackage:
1915
custom_check_required_fields_missing_method: customCheckRequiredFieldsMissingMethod
20-
DeleteEndpoint:
21-
custom_implementation: customDeleteEndpoint
2216
StopHyperParameterTuningJob:
2317
operation_type: Delete
2418
resource_name: HyperParameterTuningJob
@@ -100,8 +94,14 @@ resources:
10094
# Custom error
10195
- EndpointUpdateError
10296
hooks:
103-
sdk_create_post_set_output:
104-
code: rm.customSetOutput(desired, aws.String(svcsdk.EndpointStatusCreating), ko)
97+
sdk_read_one_post_set_output:
98+
code: rm.customDescribeEndpointSetOutput(resp, ko)
99+
sdk_update_pre_build_request:
100+
template_path: endpoint/sdk_update_pre_build_request.go.tpl
101+
sdk_update_post_set_output:
102+
code: rm.customUpdateEndpointSetOutput(ko)
103+
sdk_delete_pre_build_request:
104+
template_path: common/sdk_delete_pre_build_request.go.tpl
105105
fields:
106106
EndpointStatus:
107107
is_read_only: true

pkg/resource/endpoint/custom_set_output.go

Lines changed: 0 additions & 105 deletions
This file was deleted.

pkg/resource/endpoint/custom_update_api.go renamed to pkg/resource/endpoint/hooks.go

Lines changed: 62 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11,56 +11,92 @@
1111
// express or implied. See the License for the specific language governing
1212
// permissions and limitations under the License.
1313

14-
// Use this file to add custom implementation for any operation of intercept
15-
// the autogenerated code that trigger an update on an endpoint
14+
// Use this file if the Status/Spec of the CR needs to be modified after
15+
// create/describe/update operation
1616

1717
package endpoint
1818

1919
import (
2020
"context"
21-
"errors"
2221
"strings"
2322

2423
ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
25-
"github.com/aws-controllers-k8s/runtime/pkg/requeue"
24+
svcapitypes "github.com/aws-controllers-k8s/sagemaker-controller/apis/v1alpha1"
25+
svccommon "github.com/aws-controllers-k8s/sagemaker-controller/pkg/common"
26+
"github.com/aws/aws-sdk-go/aws"
2627
"github.com/aws/aws-sdk-go/aws/awserr"
2728
svcsdk "github.com/aws/aws-sdk-go/service/sagemaker"
2829
)
2930

3031
var (
32+
modifyingStatuses = []string{
33+
svcsdk.EndpointStatusCreating,
34+
svcsdk.EndpointStatusUpdating,
35+
svcsdk.EndpointStatusSystemUpdating,
36+
svcsdk.EndpointStatusRollingBack,
37+
svcsdk.EndpointStatusDeleting,
38+
}
39+
40+
resourceName = resourceGK.Kind
41+
3142
FailUpdateError = awserr.New("EndpointUpdateError", "unable to update endpoint. check FailureReason", nil)
3243

3344
FailureReasonInternalServiceErrorPrefix = "Request to service failed"
3445
)
3546

36-
// customUpdateEndpoint adds specialized logic to check if controller should
47+
// customDescribeEndpointSetOutput sets the resource ResourceSynced condition to False if endpoint is
48+
// being modified by AWS
49+
func (rm *resourceManager) customDescribeEndpointSetOutput(
50+
resp *svcsdk.DescribeEndpointOutput,
51+
ko *svcapitypes.Endpoint,
52+
) {
53+
// Workaround: Field config for LatestEndpointConfigName of generator config
54+
// does not code generate this correctly since this field is part of Spec
55+
// SageMaker users will need following information:
56+
// - latestEndpointConfig
57+
// - desiredEndpointConfig
58+
// - LastEndpointConfigNameForUpdate
59+
// - FailureReason
60+
// to determine the correct course of action in case of update to Endpoint fails
61+
if resp.EndpointConfigName != nil {
62+
ko.Status.LatestEndpointConfigName = resp.EndpointConfigName
63+
} else {
64+
ko.Status.LatestEndpointConfigName = nil
65+
}
66+
67+
svccommon.SetSyncedCondition(&resource{ko}, resp.EndpointStatus, &resourceName, &modifyingStatuses)
68+
}
69+
70+
// customUpdateEndpointSetOutput sets the resource ResourceSynced condition to False if endpoint is
71+
// being updated. At this stage we know call to updateEndpoint was successful.
72+
func (rm *resourceManager) customUpdateEndpointSetOutput(ko *svcapitypes.Endpoint) {
73+
// no nil check present here since Spec.EndpointConfigName is a required field
74+
ko.Status.LastEndpointConfigNameForUpdate = ko.Spec.EndpointConfigName
75+
// injecting Updating status to keep the Sync condition message and status.endpointStatus in sync
76+
ko.Status.EndpointStatus = aws.String(svcsdk.EndpointStatusUpdating)
77+
78+
svccommon.SetSyncedCondition(&resource{ko}, ko.Status.EndpointStatus, &resourceName, &modifyingStatuses)
79+
}
80+
81+
// customUpdateEndpointPreChecks adds specialized logic to check if controller should
3782
// proceeed with updateEndpoint call.
3883
// Update is blocked in the following cases:
39-
// 1. while EndpointStatus != InService
84+
// 1. while EndpointStatus != InService (handled by requeueUntilCanModify method)
4085
// 2. EndpointStatus == Failed
4186
// 3. A previous update to the Endpoint with same endpointConfigName failed
4287
// Method returns nil if endpoint can be updated, otherwise error depending on above cases
43-
func (rm *resourceManager) customUpdateEndpoint(
88+
func (rm *resourceManager) customUpdateEndpointPreChecks(
4489
ctx context.Context,
4590
desired *resource,
4691
latest *resource,
4792
delta *ackcompare.Delta,
48-
) (*resource, error) {
93+
) error {
4994
latestStatus := latest.ko.Status.EndpointStatus
5095
if latestStatus == nil {
51-
return nil, nil
52-
}
53-
54-
if *latestStatus != svcsdk.EndpointStatusFailed {
55-
// Case 1 - requeueAfter until endpoint is in InService state
56-
err := rm.endpointStatusAllowUpdates(ctx, latest)
57-
if err != nil {
58-
return nil, err
59-
}
96+
return nil
6097
}
6198

6299
failureReason := latest.ko.Status.FailureReason
63-
latestEndpointConfig := latest.ko.Spec.EndpointConfigName
64100
desiredEndpointConfig := desired.ko.Spec.EndpointConfigName
65101
lastEndpointConfigForUpdate := desired.ko.Status.LastEndpointConfigNameForUpdate
66102

@@ -72,7 +108,7 @@ func (rm *resourceManager) customUpdateEndpoint(
72108
// "Request to service failed" means update failed because of ISE and can be retried
73109
(failureReason != nil && lastEndpointConfigForUpdate != nil &&
74110
!strings.HasPrefix(*failureReason, FailureReasonInternalServiceErrorPrefix) &&
75-
*desiredEndpointConfig != *latestEndpointConfig &&
111+
delta.DifferentAt("Spec.EndpointConfigName") &&
76112
*desiredEndpointConfig == *lastEndpointConfigForUpdate) {
77113
// 1. FailureReason alone does mean an update failed it can appear because of other reasons(patching/scaling failed)
78114
// 2. *desiredEndpointConfig == *lastEndpointConfigForUpdate only tells us an update was tried with lastEndpointConfigForUpdate
@@ -82,36 +118,19 @@ func (rm *resourceManager) customUpdateEndpoint(
82118
// 1 & 2 does not guarantee an update Failed. Hence we need to look at `*latestEndpointConfigName` to determine if the update was unsuccessful
83119
// `*desiredEndpointConfig != *latestEndpointConfig` + `*desiredEndpointConfig == *lastEndpointConfigForUpdate`+ `FailureReason != nil` indicate that an update is needed,
84120
// has already been tried and failed.
85-
return nil, FailUpdateError
121+
return FailUpdateError
86122
}
87123

88-
return nil, nil
89-
}
90-
91-
// customDeleteEndpoint adds specialized logic to requeueAfter until endpoint is in
92-
// InService or Failed state before a deleteEndpoint can be called
93-
func (rm *resourceManager) customDeleteEndpoint(
94-
ctx context.Context,
95-
latest *resource,
96-
) error {
97-
latestStatus := latest.ko.Status.EndpointStatus
98-
if latestStatus != nil && *latestStatus == svcsdk.EndpointStatusFailed {
99-
return nil
100-
}
101-
return rm.endpointStatusAllowUpdates(ctx, latest)
124+
return nil
102125
}
103126

104-
// endpointStatusAllowUpdates is a helper method to determine if endpoint allows modification
105-
func (rm *resourceManager) endpointStatusAllowUpdates(
127+
// requeueUntilCanModify creates and returns an ackrequeue error
128+
// if a resource's latest status matches any of the defined modifying statuses.
129+
// This is so the controller requeues until the resource can be modifed
130+
func (rm *resourceManager) requeueUntilCanModify(
106131
ctx context.Context,
107132
r *resource,
108133
) error {
109134
latestStatus := r.ko.Status.EndpointStatus
110-
if latestStatus != nil && *latestStatus != svcsdk.EndpointStatusInService {
111-
return requeue.NeededAfter(
112-
errors.New("endpoint status does not allow modification, it is not in 'InService' state"),
113-
requeue.DefaultRequeueAfterDuration)
114-
}
115-
116-
return nil
135+
return svccommon.RequeueIfModifying(latestStatus, &resourceName, &modifyingStatuses)
117136
}

0 commit comments

Comments
 (0)