Add external connection IPs to connection secret if service type is set to LoadBalancer for PostgreSQL Managed by VSHN #46

wejdross · 2023-07-26T07:09:32Z

Summary

Added new field under spec.parameters.network called ServiceType, if it's set to LoadBalancer, while defaulting to ClusterIP, comp-function starts and add 2 ip addresses to connection secret allowing to access cluster externally without port-forwarding
tested with metallb and kind cluster
it must spawn in-cluster k8s client and check for services in specific namespace to grab IP addresses

Checklist

Categorize the PR by setting a good title and adding one of the labels:
bug, enhancement, documentation, change, breaking, dependency
as they show up in the changelog
[-] Update tests.
Link this PR to related issues.

Kidswiss

We should be really careful about using the kubeclient directly. Crossplane themselves advice us to not do that. See my comment for more details.

pkg/comp-functions/functions/vshn-postgres-func/loadBalancer.go

wejdross · 2023-07-28T08:45:39Z

@Kidswiss I converted it into draft, please take a look, I'll write some tests in meanwhile, so far I tested it manually and it works pretty well locally

Kidswiss

I've added a comment on how to properly instruct provider-kubernetes to observe an object.

I guess if we keep using this more and more we could add that as a function to the runtime library.

README.md

Kidswiss · 2023-07-28T11:48:41Z

pkg/comp-functions/functions/vshn-postgres-func/loadBalancer.go

+		},
+	}
+
+	// if there is nothin to do, return early


You could move this before line 28

pkg/comp-functions/functions/vshn-postgres-func/loadBalancer.go

Kidswiss · 2023-07-28T11:59:43Z

pkg/comp-functions/functions/vshn-postgres-func/loadBalancer.go

+				Type: pointer.String("LoadBalancer"),
+			},
+		}
+		err = iof.Desired.PutIntoObject(ctx, cluster, "cluster")


I would put the creation of the objects into their own functions. Makes the flow of the logic easier to follow.

For example updateClusterWithLoadbalancer or addServiceObserver. You can check out https://github.com/vshn/appcat/blob/master/pkg/comp-functions/functions/vshnredis/backup.go#L32 for a nice example.

Co-authored-by: Kidswiss <[email protected]>

pkg/comp-functions/runtime/desired.go

Co-authored-by: Kidswiss <[email protected]>

…/PSQLServiceType

Kidswiss · 2023-08-03T10:43:37Z

pkg/comp-functions/runtime/desired.go

@@ -69,8 +69,16 @@ func (d *DesiredResources) GetFromObject(ctx context.Context, o client.Object, k
 	return d.fromKubeObject(ctx, ko, o)
 }

-// PutIntoObject adds or updates the desired resource into its kube object


Please don't remove this comment.

Kidswiss · 2023-08-03T10:44:22Z

pkg/comp-functions/runtime/desired.go

 func (d *DesiredResources) PutIntoObject(ctx context.Context, o client.Object, kon string, refs ...xkube.Reference) error {
+	return d.putIntoObject(ctx, false, o, kon, refs...)
+}
+


This one should also have a comment.

Kidswiss · 2023-08-03T10:49:56Z

cmd/grpc.go

+	Network                   = "unix"
+	AddressFlag               = "@crossplane/fn/default.sock"
+	DevMode                   = false
+	ExternalConnectionEnabled = false


I wouldn't pass this as a flag.

We usually have these configurations in the composition itself in the config: https://github.com/vshn/component-appcat/blob/master/component/vshn_postgres.jsonnet#L849

They are dynamic and can even be set per composition, this makes them a lot more flexible. You can access the configs via iof.Config in the compisition function.

Kidswiss · 2023-08-03T10:51:00Z

cmd/grpc.go

@@ -137,6 +139,12 @@ func (s *server) RunFunction(ctx context.Context, in *pb.RunFunctionRequest) (*p
 	if err != nil {
 		return nil, status.Errorf(codes.Aborted, "cannot enable devMode: %s", err)
 	}
+	if ExternalConnectionEnabled {


Instead of conditionally adding this, just add it to the list like all other functions.

Then the first thing you do in the function is checking if the loadbalancer config is enabled or not and either just return or run the function.

wejdross · 2023-08-04T08:08:13Z

@Kidswiss All changes You requested pushed with latest commit, please take a look, I'll also update component-appcat to reflect new config parameter

Kidswiss · 2023-08-07T07:36:33Z

@wejdross please re-request reviews when I should have another look. Then it pops up in https://github.com/pulls/review-requested. Then I should hopefully not miss it :)

Having a look now.

Kidswiss

Some small things, the rest LGTM

README.md

pkg/comp-functions/runtime/desired.go

pkg/comp-functions/runtime/runtime.go

Co-authored-by: Kidswiss <[email protected]>

…/PSQLServiceType

pkg/comp-functions/runtime/runtime.go

…et to LoadBalancer for PostgreSQL Managed by VSHN (#46) * adding comp-function to manage external IPs in connection secret * adding comp-function to manage external IPs in connection secret * adding docs how to run gRPC locally * new logic for LoadBalancer comp-func * adding tests * Update README.md Co-authored-by: Kidswiss <[email protected]> * Update pkg/comp-functions/runtime/desired.go Co-authored-by: Kidswiss <[email protected]> * refactored code, working, tests passing * refactoring code * refactoring code * reusing config instead of cmdline params * fixing old tests, adding new test case * Update pkg/comp-functions/runtime/desired.go Co-authored-by: Kidswiss <[email protected]> * adding PR suggestions * adding PR suggestions * renaming function * renaming function --------- Co-authored-by: [email protected] <[email protected]> Co-authored-by: Kidswiss <[email protected]>

wejdross force-pushed the add/PSQLServiceType branch from 34d8bc4 to ec6b1a9 Compare July 26, 2023 07:13

wejdross requested review from Kidswiss and TheBigLee July 26, 2023 07:14

wejdross mentioned this pull request Jul 26, 2023

Adding necessary field names to handle external IPs in connection secret vshn/component-appcat#191

Merged

4 tasks

Kidswiss requested changes Jul 27, 2023

View reviewed changes

pkg/comp-functions/functions/vshn-postgres-func/loadBalancer.go Outdated Show resolved Hide resolved

pkg/comp-functions/functions/vshn-postgres-func/loadBalancer.go Outdated Show resolved Hide resolved

[email protected] added 2 commits July 27, 2023 10:08

adding comp-function to manage external IPs in connection secret

946f2e5

adding comp-function to manage external IPs in connection secret

8b39edd

wejdross force-pushed the add/PSQLServiceType branch from ec6b1a9 to 8b39edd Compare July 27, 2023 08:08

[email protected] added 2 commits July 27, 2023 11:27

adding docs how to run gRPC locally

af6cc48

new logic for LoadBalancer comp-func

bbfae1c

wejdross requested a review from Kidswiss July 28, 2023 08:41

wejdross marked this pull request as draft July 28, 2023 08:44

Kidswiss requested changes Jul 28, 2023

View reviewed changes

[email protected] and others added 2 commits July 28, 2023 16:13

adding tests

847f65e

Update README.md

9d9edae

Co-authored-by: Kidswiss <[email protected]>

Kidswiss reviewed Aug 2, 2023

View reviewed changes

pkg/comp-functions/runtime/desired.go Outdated Show resolved Hide resolved

wejdross added the enhancement New feature or request label Aug 2, 2023

wejdross and others added 5 commits August 2, 2023 13:27

Update pkg/comp-functions/runtime/desired.go

624ba03

Co-authored-by: Kidswiss <[email protected]>

refactored code, working, tests passing

c80329b

Merge branch 'add/PSQLServiceType' of github.com:vshn/appcat into add…

5b61114

…/PSQLServiceType

refactoring code

887cc20

refactoring code

77f23a1

wejdross requested a review from Kidswiss August 3, 2023 09:20

wejdross marked this pull request as ready for review August 3, 2023 09:21

Kidswiss reviewed Aug 3, 2023

View reviewed changes

reusing config instead of cmdline params

35c3725

fixing old tests, adding new test case

909613e

Kidswiss approved these changes Aug 7, 2023

View reviewed changes

README.md Show resolved Hide resolved

pkg/comp-functions/runtime/desired.go Show resolved Hide resolved

pkg/comp-functions/runtime/runtime.go Outdated Show resolved Hide resolved

wejdross and others added 4 commits August 7, 2023 10:44

Update pkg/comp-functions/runtime/desired.go

bdbc80b

Co-authored-by: Kidswiss <[email protected]>

adding PR suggestions

59ffec6

Merge branch 'add/PSQLServiceType' of github.com:vshn/appcat into add…

8c76255

…/PSQLServiceType

adding PR suggestions

4c1c4c4

Kidswiss reviewed Aug 8, 2023

View reviewed changes

pkg/comp-functions/runtime/runtime.go Outdated Show resolved Hide resolved

[email protected] added 2 commits August 8, 2023 10:25

renaming function

1f74105

renaming function

1d0f3b2

wejdross merged commit a551592 into master Aug 8, 2023
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add external connection IPs to connection secret if service type is set to LoadBalancer for PostgreSQL Managed by VSHN #46

Add external connection IPs to connection secret if service type is set to LoadBalancer for PostgreSQL Managed by VSHN #46

wejdross commented Jul 26, 2023

Kidswiss left a comment

wejdross commented Jul 28, 2023

Kidswiss left a comment

Kidswiss Jul 28, 2023

Kidswiss Jul 28, 2023

Kidswiss Aug 3, 2023

Kidswiss Aug 3, 2023

Kidswiss Aug 3, 2023

Kidswiss Aug 3, 2023

wejdross commented Aug 4, 2023

Kidswiss commented Aug 7, 2023

Kidswiss left a comment

Add external connection IPs to connection secret if service type is set to LoadBalancer for PostgreSQL Managed by VSHN #46

Add external connection IPs to connection secret if service type is set to LoadBalancer for PostgreSQL Managed by VSHN #46

Conversation

wejdross commented Jul 26, 2023

Summary

Checklist

Kidswiss left a comment

Choose a reason for hiding this comment

wejdross commented Jul 28, 2023

Kidswiss left a comment

Choose a reason for hiding this comment

Kidswiss Jul 28, 2023

Choose a reason for hiding this comment

Kidswiss Jul 28, 2023

Choose a reason for hiding this comment

Kidswiss Aug 3, 2023

Choose a reason for hiding this comment

Kidswiss Aug 3, 2023

Choose a reason for hiding this comment

Kidswiss Aug 3, 2023

Choose a reason for hiding this comment

Kidswiss Aug 3, 2023

Choose a reason for hiding this comment

wejdross commented Aug 4, 2023

Kidswiss commented Aug 7, 2023

Kidswiss left a comment

Choose a reason for hiding this comment