Initial changes for late initializing resource fields. #37
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vijtrip2
added
the
do-not-merge/hold
jaypipes
requested changes
Jul 26, 2021
|
Good to review but Please merge only after aws-controllers-k8s/code-generator#138 is merged. |
a-hilaly
reviewed
Jul 27, 2021
jaypipes
requested changes
Jul 27, 2021
jaypipes
requested changes
Aug 2, 2021
There was a problem hiding this comment.
@vijtrip2 I think I'd almost prefer to separate out the code that adds the retry functionality from the main PR that adds the LateInitialize() interface method... Any chance you can split those into separate PRs? It would be ideal to merge functionality for immediate server-side defaulting and then later work on the whole retry/delayed defaults stuff.
a-hilaly
approved these changes
Aug 3, 2021
jaypipes
requested changes
Aug 11, 2021
vijtrip2
removed
the
do-not-merge/hold
jaypipes
approved these changes
Aug 12, 2021
Comment on lines
+375
to
+378
| // Throw the patching error if reconciler is unable to patch the resource with late initializations | ||
| if patchErr != nil { | ||
| err = patchErr | ||
| } |
There was a problem hiding this comment.
This is fine. We'll be shadowing any non-nil err that may have been returned on line 367, but that's OK I guess.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: A-Hilaly, jaypipes, vijtrip2 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ack-bot
pushed a commit
to aws-controllers-k8s/code-generator
that referenced
this pull request
Aug 16, 2021
…#138) Issue #, if available: aws-controllers-k8s/community#812 Description of changes: * Initial implementation of `AWSResourceManager.LateInitialize()` method code generator * Adds two new hooks `late_initialize_pre_read_one` & `late_initialize_post_read_one` * Introduces new LateInitializeConfig with DelaySeconds as single parameter * Corresponding runtime PR : aws-controllers-k8s/runtime#37 ------------------------------------ Testing * Unit Tests Successful ``` ➜ code-generator git:(li-mlp) ✗ make test go test -tags codegen ./... ? github.com/aws-controllers-k8s/code-generator/cmd/ack-generate [no test files] ? github.com/aws-controllers-k8s/code-generator/cmd/ack-generate/command [no test files] ok github.com/aws-controllers-k8s/code-generator/pkg/generate/ack 2.346s ok github.com/aws-controllers-k8s/code-generator/pkg/generate/code 17.798s ? github.com/aws-controllers-k8s/code-generator/pkg/generate/config [no test files] ? github.com/aws-controllers-k8s/code-generator/pkg/generate/crossplane [no test files] ? github.com/aws-controllers-k8s/code-generator/pkg/generate/olm [no test files] ? github.com/aws-controllers-k8s/code-generator/pkg/generate/templateset [no test files] ? github.com/aws-controllers-k8s/code-generator/pkg/metadata [no test files] ok github.com/aws-controllers-k8s/code-generator/pkg/model 14.671s ok github.com/aws-controllers-k8s/code-generator/pkg/model/multiversion 5.152s ok github.com/aws-controllers-k8s/code-generator/pkg/names (cached) ? github.com/aws-controllers-k8s/code-generator/pkg/testutil [no test files] ? github.com/aws-controllers-k8s/code-generator/pkg/util [no test files] ? github.com/aws-controllers-k8s/code-generator/pkg/version [no test files] ``` * Controller generation successful ``` ➜ code-generator git:(li-mlp) ✗ make build-controller building ack-generate ... ok. installing controller-gen v0.6.1 ... ok. Copying common custom resource definitions into ecr Building Kubernetes API objects for ecr Generating deepcopy code for ecr Generating custom resource definitions for ecr Building service controller for ecr Generating RBAC manifests for ecr Running gofmt against generated code for ecr ``` * Sample generated code for ecr repository ```go // LateInitialize returns an acktypes.AWSResource after setting the late initialized // fields from the readOne call. This method will initialize the optional fields // which were not provided by the k8s user but were defaulted by the AWS service. // If there are no such fields to be initialized, the returned object is similar to // object passed in the parameter. func (rm *resourceManager) LateInitialize( ctx context.Context, res acktypes.AWSResource, ) (acktypes.AWSResource, error) { rlog := ackrtlog.FromContext(ctx) // If there are no fields to late initialize, do nothing if len(lateInitializeFieldNames) == 0 { rlog.Debug("no late initialization required.") return res, nil } r := rm.concreteResource(res) if r.ko == nil { // Should never happen... if it does, it's buggy code. panic("resource manager's LateInitialize() method received resource with nil CR object") } koWithDefaults := r.ko.DeepCopy() latestWithDefaults := &resource{koWithDefaults} lateInitConditionReason := "" lateInitConditionMessage := "" observed, err := rm.sdkFind(ctx, latestWithDefaults) if err != nil { lateInitConditionMessage = "Unable to complete Read operation required for late initialization" lateInitConditionReason = "Late Initialization Failure" ackcondition.SetLateInitialized(latestWithDefaults, corev1.ConditionFalse, &lateInitConditionMessage, &lateInitConditionReason) return latestWithDefaults, ackrequeue.NeededAfter(err, time.Duration(0)*time.Second) } observedKo := observed.ko if observedKo.Spec.Name != nil && koWithDefaults.Spec.Name == nil { koWithDefaults.Spec.Name = observedKo.Spec.Name } incompleteInitialization := rm.incompleteLateInitialization(latestWithDefaults) if incompleteInitialization { // Add the condition with LateInitialized=False lateInitConditionMessage = "Late initialization did not complete, requeuing with delay of 5 seconds" lateInitConditionReason = "Delayed Late Initialization" ackcondition.SetLateInitialized(latestWithDefaults, corev1.ConditionFalse, &lateInitConditionMessage, &lateInitConditionReason) return latestWithDefaults, ackrequeue.NeededAfter(nil, time.Duration(5)*time.Second) } // Set LateIntialized condition to True lateInitConditionMessage = "Late initialization successful" lateInitConditionReason = "Late initialization successful" ackcondition.SetLateInitialized(latestWithDefaults, corev1.ConditionTrue, &lateInitConditionMessage, &lateInitConditionReason) return latestWithDefaults, nil } // incompleteLateInitialization return true if there are fields which were supposed to be // late initialized but are not. If all the fields are late initialized, false is returned func (rm *resourceManager) incompleteLateInitialization( latestWithDefaults *resource, ) bool { ko := latestWithDefaults.ko if ko.Spec.Name == nil { return true } return false } ``` * local-kind-e2e-test logs ``` │ │ 2021-07-26T19:38:08.701Z DEBUG ackrt > r.Sync {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "kind": "Repository" │ │ 2021-07-26T19:38:08.701Z DEBUG ackrt >> rm.ReadOne {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": f │ │ 2021-07-26T19:38:08.701Z DEBUG ackrt >>> rm.sdkFind {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": │ │ 2021-07-26T19:38:09.641Z DEBUG ackrt <<< rm.sdkFind {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": │ │ 2021-07-26T19:38:09.641Z DEBUG ackrt << rm.ReadOne {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": f │ │ 2021-07-26T19:38:09.641Z DEBUG ackrt >> r.setResourceManaged {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_a │ │ 2021-07-26T19:38:09.641Z DEBUG ackrt >>> kc.Patch (metadata + spec) {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2" │ │ 2021-07-26T19:38:09.694Z DEBUG ackrt <<< kc.Patch (metadata + spec) {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2" │ │ 2021-07-26T19:38:09.694Z DEBUG ackrt marked resource as managed {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "i │ │ 2021-07-26T19:38:09.694Z DEBUG ackrt << r.setResourceManaged {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_a │ │ 2021-07-26T19:38:09.694Z DEBUG ackrt >> rm.Create {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": fa │ │ 2021-07-26T19:38:09.694Z DEBUG ackrt >>> rm.sdkCreate {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted" │ │ 2021-07-26T19:38:09.725Z DEBUG ackrt <<< rm.sdkCreate {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted" │ │ 2021-07-26T19:38:09.725Z DEBUG ackrt << rm.Create {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": fa │ │ 2021-07-26T19:38:09.725Z INFO ackrt created new resource {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopt │ │ 2021-07-26T19:38:09.725Z DEBUG ackrt >> rm.LateInitialize {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adop │ │ 2021-07-26T19:38:09.725Z INFO ackrt calculated late initialization delay is 0 seconds {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "re │ │ 2021-07-26T19:38:09.725Z DEBUG ackrt >>> rm.sdkFind {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": │ │ 2021-07-26T19:38:09.748Z DEBUG ackrt <<< rm.sdkFind {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": │ │ 2021-07-26T19:38:09.748Z DEBUG ackrt << rm.LateInitialize {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adop │ │ 2021-07-26T19:38:09.748Z DEBUG ackrt >> r.patchResourceMetadataAndSpec {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west │ │ 2021-07-26T19:38:09.748Z DEBUG ackrt >>> kc.Patch (metadata + spec) {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2" │ │ 2021-07-26T19:38:09.758Z DEBUG ackrt <<< kc.Patch (metadata + spec) {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2" │ │ 2021-07-26T19:38:09.759Z DEBUG ackrt patched resource metadata and spec {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-wes │ │ 2021-07-26T19:38:09.759Z DEBUG ackrt << r.patchResourceMetadataAndSpec {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west │ │ 2021-07-26T19:38:09.759Z DEBUG ackrt >> r.patchResourceStatus {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_ │ │ 2021-07-26T19:38:09.759Z DEBUG ackrt >>> kc.Patch (status) {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_ado │ │ 2021-07-26T19:38:09.794Z DEBUG ackrt <<< kc.Patch (status) {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_ado │ │ 2021-07-26T19:38:09.795Z DEBUG ackrt patched resource status {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_a │ │ 2021-07-26T19:38:09.795Z DEBUG ackrt << r.patchResourceStatus {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_ │ │ 2021-07-26T19:38:09.795Z DEBUG ackrt < r.Sync {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": false, │ │ 2021-07-26T19:38:09.795Z DEBUG controller-runtime.controller Successfully Reconciled {"controller": "repository", "request": "default/ecr-repository-wy6n6wjm8"} ``` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue: aws-controllers-k8s/community#812
Changes:
AWSResourceManager.LateInitialize()methodCorresponding Code generator PR: aws-controllers-k8s/code-generator#138
Please do not merge until code generator PR is merged
Testing