[installer] make sure dashboard is deployed after server and papi-server #19042

mustard-mh · 2023-11-09T08:01:35Z

Description

Summary generated by Copilot

`🤖 Generated by Copilot at 8857387`

This pull request adds a feature flag mechanism to the service-waiter component using the ConfigCat SDK, and uses it to control the waiting for the public-api-server and the server components in the dashboard deployment. It also adds the component name as a custom attribute for experiments. The changes affect the components/common-go/experiments, components/service-waiter/cmd, and install/installer/pkg/components/dashboard packages.

Related Issue(s)

Fixes EXP-822

How to test

Open PR with Gitpod
Change anything in server i.e. add a console.log
Exec leeway dev:preview pods should deployed successfully
Change anything in public-api-server i.e. fmt.Println
Exec leeway dev:preview pods should deployed successfully

Control with Feature Flag

Change Feature Flag service_waiter_skip_components value to skip true
It should skip all components' service-waiter
With attribute component set with server or public-api-server or server,public-api-server, it should skip target service waiter

Documentation

Preview status

Gitpod was successfully deployed to your preview environment.

🏷️ Name - hw-exp-822
🔗 URL - hw-exp-822.preview.gitpod-dev.com/workspaces.
📚 Documentation - See our internal documentation for information on how to interact with your preview environment.
📦 Version - hw-EXP-822-gha.19713
🗒️ Logs - GCP Logs Explorer

Build Options

Build

/werft with-werft
Run the build with werft instead of GHA
leeway-no-cache
/werft no-test
Run Leeway with --dont-test

Publish

/werft publish-to-npm
/werft publish-to-jb-marketplace

Installer

analytics=segment
with-dedicated-emulation
workspace-feature-flags
Add desired feature flags to the end of the line above, space separated

Preview Environment / Integration Tests

/werft with-local-preview
If enabled this will build install/preview
/werft with-preview
/werft with-large-vm
/werft with-gce-vm
If enabled this will create the environment on GCE infra
with-integration-tests=all
Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh. If enabled, with-preview and with-large-vm will be enabled.
with-monitoring

/hold

components/service-waiter/cmd/component.go

akosyakov · 2023-11-10T08:26:54Z

components/service-waiter/cmd/component.go

@@ -46,6 +48,11 @@ var componentCmd = &cobra.Command{
 		ctx, cancel := context.WithTimeout(cmd.Context(), timeout)
 		defer cancel()

+		if shouldSkipComponentWaiter(ctx) {


Should not we call it each time inside the waitPodsImage loop?

They behavior the same

mustard-mh · 2023-11-10T11:45:53Z

components/service-waiter/cmd/component.go

+func startWaitFeatureFlag(ctx context.Context, timeout time.Duration) {
+	featureFlagCtx, cancel := context.WithTimeout(ctx, timeout)
+	defer cancel()
+	client := experiments.NewClient(experiments.WithDefaultClient(nil))


We need default client to nil, since it will never be nil without it -> it will return a client NewAlwaysReturningDefaultValueClient. Then self-hosted Gitpod Instance may need to keep wait for it

components/service-waiter/cmd/component.go

akosyakov

We discussed that to land it the metric should be added. We deploy it with false state, then we enable it on dogfood, then on Cloud, and afterwards for other cells.

pre-approved since we talked about it, but i did not try and metric should be added

/hold

mustard-mh · 2023-11-13T15:08:01Z

@akosyakov Force push changes into 198d71d

Could you take a look? It may happen in preview env that pods ready too fast so there's no related metrics for them

mustard-mh · 2023-11-13T17:04:27Z

Checked with preview env, that metric works. It took me a lot of time to debug with service-waiter. Configcat config fetch seems will failed for the start.

🥲 And I just recognized that we forget the very first idea to wait: if not skip, we need to keep fetch feature flag. In case its value is changed.

Addressed with 198d71d , metric test result see below

$ kubectl exec -it <ide_metrics_pod_name> sh
/ # apk add curl
/ # curl http://localhost:9500/metrics | grep service_waiter
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  3690    0  3690    0     0  3673k      0 --:--:-- --:--:-- --:--:-- 3603k
# HELP service_waiter_skip_components_result_total Total number of wait result of service_waiter/component service_waiter_skip_components flag
# TYPE service_waiter_skip_components_result_total counter
service_waiter_skip_components_result_total{ok="true",value="false"} 3
/ #

mustard-mh · 2023-11-13T17:05:33Z

Tested again with deploy server / public-api with leeway run dev:preview we are good to go if metric 198d71d is alright @akosyakov

akosyakov · 2023-11-14T09:47:42Z

@geropl Could you help reviewing it?

akosyakov · 2023-11-14T09:49:25Z

Could you take a look? It may happen in preview env that pods ready too fast so there's no related metrics for them

Do you mean first deployment or each time?

geropl · 2023-11-14T09:55:36Z

Will test now, too!

mustard-mh · 2023-11-14T10:01:54Z

Could you take a look? It may happen in preview env that pods ready too fast so there's no related metrics for them

Do you mean first deployment or each time?

@akosyakov It'll not affect component deployment, but we don't have metric in this case, since feature flag's value is not got yet.

geropl · 2023-11-14T10:14:45Z

components/service-waiter/cmd/component.go

+			return defaultValue, false, fetchTimes
+		default:
+			stringValue := client.GetStringValue(ctx, experiments.ServiceWaiterSkipComponentsFlag, "NONE", experiments.Attributes{
+				GitpodHost: componentCmdOpt.gitpodHost,


geropl · 2023-11-14T10:24:43Z

components/service-waiter/cmd/component_test.go

+		fetchTimes    int
+	}{
+		{
+			name: "happy path",


geropl · 2023-11-14T10:41:32Z

components/service-waiter/cmd/component.go

+	getShouldSkipComponentWaiter := func() {
+		startTime := time.Now()
+		value, isActualValue, fetchTimes := ActualWaitFeatureFlag(featureFlagCtx, client, defaultSkip)
+		avgTime := time.Since(startTime) / time.Duration(fetchTimes)


@mustard-mh When waiting for longer than timeout, I got a integer divide by zero here (I scaled server down to 0 replicas to test):

panic: runtime error: integer divide by zero goroutine 18 [running]: github.com/gitpod-io/gitpod/service-waiter/cmd.startWaitFeatureFlag.func1() github.com/gitpod-io/gitpod/service-waiter/cmd/component.go:168 +0x4e5 github.com/gitpod-io/gitpod/service-waiter/cmd.startWaitFeatureFlag({0x1a40810?, 0xc000434c40?}, 0x0?) github.com/gitpod-io/gitpod/service-waiter/cmd/component.go:174 +0x1be created by github.com/gitpod-io/gitpod/service-waiter/cmd.glob..func2 in goroutine 1 github.com/gitpod-io/gitpod/service-waiter/cmd/component.go:56 +0x45b

geropl · 2023-11-14T10:47:55Z

Finished testing:

happy path: ✔️
skip with feature flag: ✔️
skip after timeout: got an error (see comment)

I double checked the code and tests, looks really good! 🙏 Once the last thing is fixed we should 🏁 and merge. 🚀

mustard-mh · 2023-11-14T13:17:51Z

Addressed, thank you 🚢

mustard-mh · 2023-11-14T13:18:38Z

/unhold

[installer] make sure dashboard is deployed after server and papi-server

8857387

mustard-mh requested review from a team as code owners November 9, 2023 08:01

roboquat added the do-not-merge/hold label Nov 9, 2023

mustard-mh marked this pull request as draft November 9, 2023 08:01

roboquat added do-not-merge/work-in-progress size/M labels Nov 9, 2023

github-actions bot added team: staff-engineers team: team-experience labels Nov 9, 2023

mustard-mh added 2 commits November 9, 2023 08:33

fix build

71d7918

Add unit tests

d299722

roboquat added size/L and removed size/M labels Nov 9, 2023

mustard-mh marked this pull request as ready for review November 9, 2023 10:10

roboquat removed the do-not-merge/work-in-progress label Nov 9, 2023

akosyakov reviewed Nov 9, 2023

View reviewed changes

components/service-waiter/cmd/component.go Outdated Show resolved Hide resolved

akosyakov reviewed Nov 9, 2023

View reviewed changes

components/service-waiter/cmd/component.go Outdated Show resolved Hide resolved

geropl reviewed Nov 9, 2023

View reviewed changes

components/service-waiter/cmd/component.go Show resolved Hide resolved

address feedback

efc8c90

akosyakov reviewed Nov 10, 2023

View reviewed changes

wait feature flag until get actual value of timed out

a5b3cce

roboquat added size/XL and removed size/L labels Nov 10, 2023

default config cat client nil

cc4cca0

mustard-mh commented Nov 10, 2023

View reviewed changes

akosyakov reviewed Nov 10, 2023

View reviewed changes

components/service-waiter/cmd/component.go Show resolved Hide resolved

mustard-mh added 3 commits November 10, 2023 12:10

log avg fetch time

5960757

1

2518798

mock feature flag hang

e1dcb63

akosyakov approved these changes Nov 13, 2023

View reviewed changes

akosyakov mentioned this pull request Nov 13, 2023

[public-api] aligns with latest guidelines #19038

Merged

14 tasks

mustard-mh force-pushed the hw/EXP-822 branch from c4ea20d to 470749f Compare November 13, 2023 15:06

Add metric

198d71d

mustard-mh force-pushed the hw/EXP-822 branch from 72b4afd to 198d71d Compare November 13, 2023 17:04

geropl reviewed Nov 14, 2023

View reviewed changes

components/service-waiter/cmd/component_test.go

fetchTimes int

}{

{

name: "happy path",

Copy link

Member

geropl Nov 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧡

geropl reviewed Nov 14, 2023

View reviewed changes

fixup

c6aa11c

roboquat removed the do-not-merge/hold label Nov 14, 2023

roboquat merged commit a5021da into main Nov 14, 2023

roboquat deleted the hw/EXP-822 branch November 14, 2023 13:20

[installer] make sure dashboard is deployed after server and papi-server #19042

[installer] make sure dashboard is deployed after server and papi-server #19042

Uh oh!

Conversation

mustard-mh commented Nov 9, 2023 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

🤖 Generated by Copilot at 8857387

Related Issue(s)

How to test

Documentation

Preview status

Build Options

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akosyakov Nov 10, 2023

Choose a reason for hiding this comment

Uh oh!

mustard-mh Nov 10, 2023

Choose a reason for hiding this comment

Uh oh!

mustard-mh Nov 10, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

akosyakov left a comment

Choose a reason for hiding this comment

Uh oh!

mustard-mh commented Nov 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mustard-mh commented Nov 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mustard-mh commented Nov 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akosyakov commented Nov 14, 2023

Uh oh!

akosyakov commented Nov 14, 2023

Uh oh!

geropl commented Nov 14, 2023

Uh oh!

mustard-mh commented Nov 14, 2023

Uh oh!

geropl Nov 14, 2023

Choose a reason for hiding this comment

Uh oh!

geropl Nov 14, 2023

Choose a reason for hiding this comment

Uh oh!

geropl Nov 14, 2023

Choose a reason for hiding this comment

Uh oh!

geropl commented Nov 14, 2023

Uh oh!

mustard-mh commented Nov 14, 2023

Uh oh!

mustard-mh commented Nov 14, 2023

Uh oh!

Uh oh!

mustard-mh commented Nov 9, 2023 •

edited by github-actions bot

Loading

`🤖 Generated by Copilot at 8857387`

mustard-mh commented Nov 13, 2023 •

edited

Loading

mustard-mh commented Nov 13, 2023 •

edited

Loading

mustard-mh commented Nov 13, 2023 •

edited

Loading