Adding support for `Function version` #113

Vandita2020 · 2023-10-12T22:56:33Z

Summary:
Adding support to publish Version for a Function.

Description:
This PR adds implementation of AWS Lambda’s Function Version feature in the ACK. Function versions allow developers to publish new versions of their Lambda functions while maintaining older versions for backward compatibility or testing purposes. This will allow developers to publish new versions of their lambda functions and manage function updates seamlessly using ACK.

Version resource for a Function can be created using following manifest:

apiVersion: lambda.services.k8s.aws/v1alpha1
kind: Version
metadata:
  name: $VERSION_NAME
  annotations:
    services.k8s.aws/region: $AWS_REGION
spec:
  functionName: $FUNCTION_NAME
  codeSHA256: $HASH
  revisionID: $REVISION_ID
  description: version created by ACK lambda-controller e2e tests

The PR supports creation and deletion of Version resource. It also includes E2E tests to verify the functionality.

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

ack-prow · 2023-10-12T22:56:36Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

a-hilaly

Since we already have a LayerVersion resource, and to avoid UX confusions, should we name this resource FunctionVersion instead?

valerena · 2023-10-23T22:26:33Z

Since we already have a LayerVersion resource, and to avoid UX confusions, should we name this resource FunctionVersion instead?

It's a good call, but the resource is called just Version in other places, like the API is just called PublishVersion, and there's an AWS::Lambda::Version resource in CFN and a Version construct in CDK.

I don't know what would create more confusion really

valerena · 2023-10-24T17:54:31Z

generator.yaml

@@ -7,11 +7,28 @@ ignore:
    # FunctionUrlConfig
    # LayerVersion
 operations:
-  PublishLayerVersion:
+  PublishLayerVersion: #opID


Is this comment needed or relevant? (can we remove it?)

My mistake, have removed it now. Thanks!

valerena · 2023-10-24T17:56:39Z

templates/hooks/version/sdk_create_pre_build_request.go.tpl

+if err != nil {
+	return nil, err
+}
+bigList := list.Versions


Maybe bigList should be called something more specific, like versionsList or previousVersions (Or something that makes more sense, for when it's used in the sdk_create_pre_set_output.go.tpl file)

Makes sense, I went with versionList. We're getting list of Versions for LayerVersion resource and assigned it to variable named versionList only, made sense to use the same naming for this case too. Let me know if we should keep the naming different.

a-hilaly

Ty @Vandita2020 - left two comments inline.

a-hilaly · 2023-12-11T17:20:06Z

pkg/resource/version/sdk.go

+	res := &svcsdk.ListVersionsByFunctionInput{}
+	res.FunctionName = desired.ko.Spec.FunctionName
+	var list *svcsdk.ListVersionsByFunctionOutput
+	list, err = rm.sdkapi.ListVersionsByFunctionWithContext(ctx, res)
+	if err != nil {
+		return nil, err
+	}
+	versionList := list.Versions
+
+	for ok := list.NextMarker != nil; ok; ok = (list.NextMarker != nil) {
+		res.Marker = list.NextMarker
+		list, err = rm.sdkapi.ListVersionsByFunctionWithContext(ctx, res)
+		if err != nil {
+			return nil, err
+		}
+		versionList = append(versionList, list.Versions...)
+	}


Suggested change

res := &svcsdk.ListVersionsByFunctionInput{}

res.FunctionName = desired.ko.Spec.FunctionName

var list *svcsdk.ListVersionsByFunctionOutput

list, err = rm.sdkapi.ListVersionsByFunctionWithContext(ctx, res)

if err != nil {

return nil, err

}

versionList := list.Versions

for ok := list.NextMarker != nil; ok; ok = (list.NextMarker != nil) {

res.Marker = list.NextMarker

list, err = rm.sdkapi.ListVersionsByFunctionWithContext(ctx, res)

if err != nil {

return nil, err

}

versionList = append(versionList, list.Versions...)

}

marker := nil

versionsList := []svcsdk.Version

for {

listVersionsInput := &svcsdk.ListVersionsByFunctionInput{

FunctionName: desired.ko.Spec.FunctionName,

Marker: marker

}

listVersionResponse, err := rm.sdkapi.ListVersionsByFunctionWithContext(ctx, listVersionsInput)

if err != nil {

return nil, err

}

versionsList = append(versionLists, listVersionResponse.Versions...)

if listVersionResponse.NextMarker == nil {

break

}

marker = listVersionResponse.NextMarker

}

Made the suggested changes. Thanks!

a-hilaly · 2023-12-11T17:22:09Z

test/e2e/resources/version_with_check.yaml

+    services.k8s.aws/region: $AWS_REGION
+spec:
+  functionName: $FUNCTION_NAME
+  codeSHA256: $HASH


How is the $HASH set

codeSHA256 and RevisionId fields are optional and are used to check whether the Version is published only when there is some change in the code or its configuration. I have added new E2E test to verify the code by setting $HASH to publish the Version.

Vandita2020 · 2024-01-02T21:09:30Z

/test all

a-hilaly · 2024-01-10T02:34:05Z

test/e2e/resources/functionVersion.yaml

Can we name this file function_version.yaml , to keep the same style used for the files in the same directory?

This file is similar to version_with_check.yaml file, so I deleted it. Don't remember how it got here.

a-hilaly · 2024-01-10T02:35:34Z

test/e2e/tests/test_version.py

+        # Check function version doesn't exist
+        assert not lambda_validator.version_exists(lambda_function_name, version_number)
+
+    def test_version_with_check(self, lambda_client, lambda_function):


What does check here means?

To check that the version we created above got deleted. We call getFunction API and pass the Version number along with Function name as we are only deleting a version and not the whole function.

a-hilaly · 2024-01-10T02:41:47Z

apis/v1alpha1/version.go

+	Architectures []*string `json:"architectures,omitempty"`
+	// The size of the function's deployment package, in bytes.
+	// +kubebuilder:validation:Optional
+	CodeSize *int64 `json:"codeSize,omitempty"`
+	// The function's dead letter queue.
+	// +kubebuilder:validation:Optional
+	DeadLetterConfig *DeadLetterConfig `json:"deadLetterConfig,omitempty"`


Most of the fields in here are not really relevant to a function, should we get rid of them?

We're using PublishVersion API to set the status and its response has all these fields. So we cannot do anything with the extra fields.

a-hilaly · 2024-01-10T18:09:52Z

/test all

a-hilaly

Thank you very much for your contribution @Vandita2020 ! rock on!
/lgtm

ack-prow · 2024-01-10T21:11:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-hilaly, 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~~ [Vandita2020,a-hilaly]

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

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 Oct 12, 2023

ack-prow bot requested review from jljaco and RedbackThomson October 12, 2023 22:56

ack-prow bot added the approved label Oct 12, 2023

Vandita2020 force-pushed the functionVersion branch 2 times, most recently from 06873ce to e6c9b38 Compare October 19, 2023 23:36

Vandita2020 marked this pull request as ready for review October 23, 2023 20:26

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 Oct 23, 2023

ack-prow bot requested review from a-hilaly and vijtrip2 October 23, 2023 20:26

a-hilaly reviewed Oct 23, 2023

View reviewed changes

Vandita2020 force-pushed the functionVersion branch from e6c9b38 to bde333e Compare October 23, 2023 23:18

valerena reviewed Oct 24, 2023

View reviewed changes

a-hilaly mentioned this pull request Dec 11, 2023

Support passing Function reference in Version resource #114

Merged

a-hilaly reviewed Dec 11, 2023

View reviewed changes

Vandita2020 added 8 commits January 2, 2024 13:31

Adding Version resource

53ba75a

Create Version resource

82b04c2

Delete functionality

4686ccf

mend

11ca091

Check code changes before creating new version resource

f45e5dd

Adding E2E tests

b1e4a49

Review changes

02d14fe

Review changes: rewriting code for Create in neat way

38fb0c4

Vandita2020 force-pushed the functionVersion branch from 1c8f77d to 0be1990 Compare January 4, 2024 19:36

a-hilaly reviewed Jan 10, 2024

View reviewed changes

Adding additional E2E tests

c5682d9

Vandita2020 force-pushed the functionVersion branch from 0be1990 to c5682d9 Compare January 10, 2024 21:06

a-hilaly approved these changes Jan 10, 2024

View reviewed changes

ack-prow bot assigned a-hilaly Jan 10, 2024

ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jan 10, 2024

ack-prow bot merged commit 4a72b7b into aws-controllers-k8s:main Jan 10, 2024

Adding support for Function version #113

Adding support for Function version #113

Uh oh!

Conversation

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

Uh oh!

Uh oh!

ack-prow bot commented Oct 12, 2023

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

valerena commented Oct 23, 2023

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!

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!

Vandita2020 commented Jan 2, 2024

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!

a-hilaly commented Jan 10, 2024

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

ack-prow bot commented Jan 10, 2024

Uh oh!

Uh oh!

Adding support for `Function version` #113

Adding support for `Function version` #113

Vandita2020 commented Oct 12, 2023 •

edited

Loading