Adding `EventInvokeConfig` support for `Function` #92

Vandita2020 · 2023-05-17T21:23:13Z

Issue #, if available:

Summary
Adding support to configure EventInvokeConfig for a function in ACK.

Description
This PR adds support for EventInvokeConfig permitted by Lambda API operations in ACK. To implement this functionality I added EventInvokeConfig as an inline property to a function. Thus the user can directly set configurations for async invocation (EventInvokeConfig) while creating a function. The user can edit the following code to create eventInvokeConfig along with function.

apiVersion: lambda.services.k8s.aws/v1alpha1
 kind: Function
 metadata:
   name: $FUNCTION_NAME
   annotations:
     services.k8s.aws/region: $AWS_REGION
 spec:
   name: $FUNCTION_NAME
   code:
     s3Bucket: $BUCKET_NAME
     s3Key: $LAMBDA_FILE_NAME
   functionEventInvokeConfig:
     destinationConfig:
       onSuccess:
         destination: $ON_SUCCESS_DESTINATION
       onFailure:
         destination: $ON_FAILURE_DESTINATION
     maximumEventAgeInSeconds: $MAXIMUM_EVENT_AGE_IN_SECONDS
     maximumRetryAttempts: $MAXIMUM_RETRY_ATTEMPTS
   role: $LAMBDA_ROLE
   runtime: python3.9
   handler: main
   description: function created by ACK lambda-controller e2e tests

This PR includes both implementation code and e2e tests.

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

a-hilaly · 2023-05-18T20:44:42Z

/retest

a-hilaly · 2023-05-18T20:45:17Z

/test all

a-hilaly · 2023-05-24T16:58:01Z

test/e2e/tests/helper.py

+        try:
+            resp = self.lambda_client.get_function_event_invoke_config(
+                FunctionName = function_name,
+                # Qualifier = qualifier
+            )
+            return resp
+        except Exception as e:
+            logging.debug(e)
+            return None
+
+    def function_event_invoke_config_exists(self, function_name: str) -> bool:
+        return self.get_function_event_invoke_config(function_name) is not None


@Vandita2020 I suggest to test get_function_event_invoke_config and see how it works - i don't see any specific errors returned by the controller.
Also another route, would be to test the controller locally and see what are the function status.Conditions when you try to set an EventInvokeConfig

a-hilaly

Nice work on this @Vandita2020! Left few comments bellow

a-hilaly · 2023-05-25T17:24:19Z

test/e2e/service_bootstrap.py

        ),
+        EICQueueOnSuccess=Queue(
+            "ack-lambda-controller-queue" 


Should we change this to ack-lambda-controller-function-queue-eic-onsuccess? and apply same logic for the on failure?

a-hilaly · 2023-05-25T17:25:07Z

test/e2e/tests/test_function.py

+        k8s.patch_custom_resource(ref, cr)
+        time.sleep(UPDATE_WAIT_AFTER_SECONDS)
+
+        #Check function_snapstart update fields


Suggested change

#Check function_snapstart update fields

#Check function_event_invoke_config

Oops, corrected it.

a-hilaly · 2023-05-25T17:26:00Z

test/e2e/resources/function_event_invoke_config.yaml

+  reservedConcurrentExecutions: $RESERVED_CONCURRENT_EXECUTIONS
+  codeSigningConfigARN: "$CODE_SIGNING_CONFIG_ARN"


If you're not using this fields you can just remove them and remove the replacements in the pytest as well

a-hilaly · 2023-05-25T17:26:47Z

pkg/resource/function/hooks.go

@@ -547,6 +601,43 @@ func (rm *resourceManager) setResourceAdditionalFields(
 	}
 	ko.Spec.ReservedConcurrentExecutions = getFunctionConcurrencyOutput.ReservedConcurrentExecutions

+	var getFunctionEventInvokeConfigOutput *svcsdk.GetFunctionEventInvokeConfigOutput


nit: in the future we can split this function into 3 different functions...

a-hilaly · 2023-05-25T17:28:39Z

apis/v1alpha1/ack-generate-metadata.yaml

  go_version: go1.19
-  version: v0.26.1
-api_directory_checksum: a9fcef68210dd72b4b2e37052f2c1a9e971326c6
+  version: v0.25.0-9-g811e30b-dirty


Can you regenerate this using code-generator@v0.26.1

Also we might want to bring your changes to code-generator before merging this

Opened a PR for code-generator

a-hilaly · 2023-05-29T21:16:03Z

We need the code-generator PR to be merged first
/hold

ack-prow · 2023-05-30T00:26:29Z

@Vandita2020: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
lambda-codespell	`f0de64c`	link	true	`/test lambda-codespell`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Issue #, if available: This PR is created to support the code-generation for `FunctionEventInvokeConfig` feature in lambda-controller. [PR#92](aws-controllers-k8s/lambda-controller#92) Description of changes: This PR adds functionality to assign whole object to a field, instead of assigning a particular parameter of it. To clarify it further, in the below code, we created a new field `LayerStatuses` which stores the value of `Configuration.Layers` parameter taken from `GetFunction` operation. ``` LayerStatuses: from: operation: GetFunction path: Configuration.Layers ``` In our case we needed to add `FunctionEventInvokeConfig` as an inline property to `Function` resource, for which we needed to assign the whole object returned by `PutFunctionEventInvokeConfig` operation to field `FunctionEventInvokeConfig`. To make this work we added a new condition for it by passing `path` value as `.` , shown below: ``` FunctionEventInvokeConfig: from: operation: PutFunctionEventInvokeConfig path: . ``` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

RedbackThomson

/lgtm

a-hilaly

Nice, thank you @Vandita2020
/lgtm

ack-prow · 2023-06-07T21:29:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, RedbackThomson, Vandita2020

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:

~~OWNERS~~ [A-Hilaly,RedbackThomson]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

a-hilaly · 2023-06-07T21:55:06Z

/unhold

…s#92) Issue #, if available: **Summary** Adding support to configure `EventInvokeConfig` for a `function` in ACK. **Description** This PR adds support for `EventInvokeConfig` permitted by Lambda API operations in ACK. To implement this functionality I added `EventInvokeConfig` as an inline property to a `function`. Thus the user can directly set configurations for async invocation (EventInvokeConfig) while creating a function. The user can edit the following code to create `eventInvokeConfig` along with `function`. ``` apiVersion: lambda.services.k8s.aws/v1alpha1 kind: Function metadata: name: $FUNCTION_NAME annotations: services.k8s.aws/region: $AWS_REGION spec: name: $FUNCTION_NAME code: s3Bucket: $BUCKET_NAME s3Key: $LAMBDA_FILE_NAME functionEventInvokeConfig: destinationConfig: onSuccess: destination: $ON_SUCCESS_DESTINATION onFailure: destination: $ON_FAILURE_DESTINATION maximumEventAgeInSeconds: $MAXIMUM_EVENT_AGE_IN_SECONDS maximumRetryAttempts: $MAXIMUM_RETRY_ATTEMPTS role: $LAMBDA_ROLE runtime: python3.9 handler: main description: function created by ACK lambda-controller e2e tests ``` This PR includes both implementation code and e2e tests. **Acknowledgement** By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Issue #, if available: Related PR: #92 #93 **Summary** Fixing deletion part for `EventInvokeConfig` field. **Description:** This PR is related to bug fixes in [previous PR](#97) related to `EventInvokeConfig` implementation. When the user deleted the set configurations for Function/Alias resource, the configurations were not deleted from the Console side, instead it used to throw an error. Fixed the bug by introducing a new condition to check if the desired configurations are nil or not, if yes then call the delete API. Example of changes for `EventInvokeConfig` in Alias resource is given below: ``` if r.ko.Spec.FunctionEventInvokeConfig == nil { input_delete := &svcsdk.DeleteFunctionEventInvokeConfigInput{ FunctionName: aws.String(*r.ko.Spec.FunctionName), Qualifier: aws.String(*r.ko.Spec.Name), } _, err = rm.sdkapi.DeleteFunctionEventInvokeConfigWithContext(ctx, input_delete) rm.metrics.RecordAPICall("DELETE", "DeleteFunctionEventInvokeConfig", err) if err != nil { return nil, err } return r, nil } ``` This PR contains implementation code and E2E test to validate the changes. **Acknowledgment** By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Vandita2020 force-pushed the event_invoke_config branch 18 times, most recently from 92e8ba0 to 7bd9c3c Compare May 24, 2023 16:10

a-hilaly reviewed May 24, 2023

View reviewed changes

Vandita2020 force-pushed the event_invoke_config branch 5 times, most recently from a1c0864 to 758d323 Compare May 25, 2023 16:48

a-hilaly reviewed May 25, 2023

View reviewed changes

Vandita2020 mentioned this pull request May 26, 2023

Adding functionality to assign whole object to a field aws-controllers-k8s/code-generator#451

Merged

ack-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 29, 2023

Vandita2020 force-pushed the event_invoke_config branch 2 times, most recently from 6aa894b to b3403ba Compare May 30, 2023 00:09

Vandita2020 force-pushed the event_invoke_config branch from aef1d8c to 7a3d4e4 Compare May 30, 2023 00:31

RedbackThomson approved these changes Jun 1, 2023

View reviewed changes

ack-prow bot assigned RedbackThomson Jun 1, 2023

ack-prow bot added lgtm Indicates that a PR is ready to be merged. approved labels Jun 1, 2023

Vandita2020 changed the title ~~Adding support for EventInvokeConfig~~ Adding EventInvokeConfig support for Function Jun 2, 2023

ack-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 2, 2023

a-hilaly marked this pull request as ready for review June 7, 2023 18:15

ack-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 7, 2023

ack-prow bot requested review from RedbackThomson and vijtrip2 June 7, 2023 18:15

Adding functionality for EventInvokeConfig

2f55003

Vandita2020 force-pushed the event_invoke_config branch 2 times, most recently from aba6123 to c29968b Compare June 7, 2023 18:20

ack-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 7, 2023

Vandita2020 force-pushed the event_invoke_config branch from c29968b to 2f55003 Compare June 7, 2023 18:23

a-hilaly approved these changes Jun 7, 2023

View reviewed changes

ack-prow bot assigned a-hilaly Jun 7, 2023

ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jun 7, 2023

ack-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 7, 2023

ack-prow bot merged commit d5ed5df into aws-controllers-k8s:main Jun 7, 2023

Vandita2020 mentioned this pull request Aug 2, 2023

Bug fix for EventInvokeConfig #107

Merged

	#Check function_snapstart update fields
	#Check function_event_invoke_config

		reservedConcurrentExecutions: $RESERVED_CONCURRENT_EXECUTIONS
		codeSigningConfigARN: "$CODE_SIGNING_CONFIG_ARN"

Adding EventInvokeConfig support for Function #92

Adding EventInvokeConfig support for Function #92

Uh oh!

Conversation

Vandita2020 commented May 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

a-hilaly commented May 18, 2023

Uh oh!

a-hilaly commented May 18, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vandita2020 May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a-hilaly commented May 29, 2023

Uh oh!

ack-prow bot commented May 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RedbackThomson left a comment

Choose a reason for hiding this comment

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

ack-prow bot commented Jun 7, 2023

Uh oh!

a-hilaly commented Jun 7, 2023

Uh oh!

Uh oh!

Adding `EventInvokeConfig` support for `Function` #92

Adding `EventInvokeConfig` support for `Function` #92

Vandita2020 commented May 17, 2023 •

edited

Loading

Vandita2020 May 29, 2023 •

edited

Loading

ack-prow bot commented May 30, 2023 •

edited

Loading