[installer] set the ServiceType as LB for ws-proxy #8759

nandajavarma · 2022-03-11T14:26:23Z

Description

Currently, the type of the service created for ws-proxy is ClusterIP by default. When setting up in cloud in just the Workspace installation, this creates a requirement for having to post process the YAML to change the type to LoadBalancer. With this PR, the default value of ws-proxy service type is set as LoadBalancer when the installation is of the kind Workspace

Related Issue(s)

Fixes #7734

How to test

Set the value of kind in config file Workspace. Run the render command, installer render --config gitpod.config.yaml > gitpod.yaml. This will render Service definition of the app ws-proxy as type LoadBalancer

Release Notes

NONE

Documentation

No

codecov · 2022-03-11T14:31:08Z

Codecov Report

Merging #8759 (6e5f2a7) into main (bc1e9fa) will decrease coverage by 1.13%.
The diff coverage is n/a.

❗ Current head 6e5f2a7 differs from pull request most recent head 3c93bec. Consider uploading reports for the commit 3c93bec to get more accurate results

@@            Coverage Diff             @@
##             main    #8759      +/-   ##
==========================================
- Coverage   12.31%   11.17%   -1.14%     
==========================================
  Files          20       18       -2     
  Lines        1161      993     -168     
==========================================
- Hits          143      111      -32     
+ Misses       1014      880     -134     
+ Partials        4        2       -2

Flag	Coverage Δ
components-gitpod-cli-app	`11.17% <ø> (ø)`
components-local-app-app-darwin-amd64	`?`
components-local-app-app-darwin-arm64	`?`
components-local-app-app-linux-amd64	`?`
components-local-app-app-linux-arm64	`?`
components-local-app-app-windows-386	`?`
components-local-app-app-windows-amd64	`?`
components-local-app-app-windows-arm64	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

princerachit · 2022-03-14T09:44:47Z

install/installer/pkg/components/ws-proxy/objects.go

+			// Hence we use LoadBalancer type for the service
+			if cfg.Config.Kind == config.InstallationWorkspace {
+				service.Spec.Type = corev1.ServiceTypeLoadBalancer
+				service.Annotations["cloud.google.com/neg"] = `{"exposed_ports": {"80":{},"443": {}}}`


Do we need to hardcode google specific annotations here?

These annotations have been hardcoded for some time on the basis that a) they're ignored if not used and b) we don't really have an effective (and non-verbose) way of adding custom annotations to services. Elsewhere, there's also an annotation for external-dns with the same thinking

mrsimonemms

Looks good to me, save for the nit.

mrsimonemms · 2022-03-14T09:52:05Z

install/installer/README.md

@@ -163,12 +163,21 @@ yq eval-all --inplace \
  gitpod.yaml
 ```

+Similarly, if you are doing a `Workspace` only install (specifying `Workspace`
+as the kind in config) you might want to change the service type of `ws-proxy` to `ClusterIP`
+instead of the default `LoadBalancer`. You can post-process the yaml to change that.


nit: YAML, not yaml

princerachit

I did not test the changes but code lgtm

corneliusludmann · 2022-03-14T13:54:12Z

install/installer/README.md

+yq eval-all --inplace \
+  '(select(.kind == "Service" and .metadata.name == "ws-proxy") | .spec.type) |= "ClusterIP"' \
+  gitpod.yaml
+```


Off-topic: Do we have already an issue to get rid of this post-processing step and having a config value for the service type?

@corneliusludmann there is no such issue open specifically for making the service type configurable. I guess the question is do we plan on making this configurable, since our thought process was to not make it configurable.

I'm glad you brought that up. 🚀 Indeed, we decided like this in the past. However, 2 things have changed since then:

We introduced the experimental section. Everything that we won't support for some reason but need internally for SaaS should go there. We shouldn't need to rely on post-processing.

Since our official supported installation path will be the KOTS installation, we are a little more relaxed about the config surface.

What do you think, @mrsimonemms?

In principle, I'm happy for this to go into the experimental section. I think that should be a separate change though, as it will require discussion/agreement on the config surface change

Sorry for the inactivity on this issue. I have created this issue to track this requirement. Shall we merge this PR and continue the work there?

princerachit

code changes lgtm. did not test this though

Furisto · 2022-03-28T21:44:22Z

Hey @mrsimonemms, if I understood correctly you agree with the changes in this PR. If so, can you please approve it?

mrsimonemms

LGTM

nandajavarma requested review from a team March 11, 2022 14:26

roboquat added release-note-none size/M labels Mar 11, 2022

github-actions bot added team: delivery Issue belongs to the self-hosted team team: workspace Issue belongs to the Workspace team labels Mar 11, 2022

nandajavarma force-pushed the nvn/ws-proxy-lb branch from 28e4ea2 to cde6e8c Compare March 11, 2022 16:22

princerachit reviewed Mar 14, 2022

View reviewed changes

mrsimonemms reviewed Mar 14, 2022

View reviewed changes

princerachit previously approved these changes Mar 14, 2022

View reviewed changes

[installer] set the ServiceType as LB for ws-proxy

3c93bec

nandajavarma dismissed princerachit’s stale review via 3c93bec March 14, 2022 10:44

nandajavarma force-pushed the nvn/ws-proxy-lb branch from 6e5f2a7 to 3c93bec Compare March 14, 2022 10:44

corneliusludmann reviewed Mar 14, 2022

View reviewed changes

nandajavarma mentioned this pull request Mar 16, 2022

[installer] Make type and annotations of the services configurable #8838

Closed

corneliusludmann requested review from princerachit and mrsimonemms March 17, 2022 13:07

princerachit approved these changes Mar 22, 2022

View reviewed changes

mrsimonemms approved these changes Mar 29, 2022

View reviewed changes

roboquat merged commit 4b35a6f into main Mar 29, 2022

roboquat deleted the nvn/ws-proxy-lb branch March 29, 2022 07:56

roboquat added the deployed: workspace Workspace team change is running in production label Mar 31, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[installer] set the ServiceType as LB for ws-proxy #8759

[installer] set the ServiceType as LB for ws-proxy #8759

nandajavarma commented Mar 11, 2022

codecov bot commented Mar 11, 2022 •

edited

Loading

princerachit Mar 14, 2022

mrsimonemms Mar 14, 2022

mrsimonemms left a comment

mrsimonemms Mar 14, 2022

nandajavarma Mar 14, 2022

princerachit left a comment •

edited

Loading

corneliusludmann Mar 14, 2022

nandajavarma Mar 14, 2022

corneliusludmann Mar 14, 2022

mrsimonemms Mar 14, 2022

nandajavarma Mar 16, 2022

princerachit left a comment

Furisto commented Mar 28, 2022

mrsimonemms left a comment

[installer] set the ServiceType as LB for ws-proxy #8759

[installer] set the ServiceType as LB for ws-proxy #8759

Conversation

nandajavarma commented Mar 11, 2022

Description

Related Issue(s)

How to test

Release Notes

Documentation

codecov bot commented Mar 11, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrsimonemms left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

princerachit left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

princerachit left a comment

Choose a reason for hiding this comment

Furisto commented Mar 28, 2022

mrsimonemms left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 11, 2022 •

edited

Loading

princerachit left a comment •

edited

Loading