From f99ef8ca3e2356b3499ca67338a760576779f075 Mon Sep 17 00:00:00 2001 From: Pudong Zheng Date: Thu, 13 Oct 2022 09:30:44 +0000 Subject: [PATCH 1/4] [ws-manager] Do not follow redirect when ready probe --- components/ws-manager/pkg/manager/probe.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/components/ws-manager/pkg/manager/probe.go b/components/ws-manager/pkg/manager/probe.go index 2ff0d44dc73f2f..d3839b753f05b5 100644 --- a/components/ws-manager/pkg/manager/probe.go +++ b/components/ws-manager/pkg/manager/probe.go @@ -70,6 +70,9 @@ func (p *WorkspaceReadyProbe) Run(ctx context.Context) WorkspaceProbeResult { Transport: &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, }, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, } for { From bf2ce4fc90f1d39de1324d944ff8ceadeb45d9d3 Mon Sep 17 00:00:00 2001 From: Pudong Zheng Date: Thu, 13 Oct 2022 09:31:19 +0000 Subject: [PATCH 2/4] [integration-test] use correct workspace id --- test/pkg/integration/workspace.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/pkg/integration/workspace.go b/test/pkg/integration/workspace.go index a4fe10f5697c49..2d244bc0eedb10 100644 --- a/test/pkg/integration/workspace.go +++ b/test/pkg/integration/workspace.go @@ -155,7 +155,7 @@ func LaunchWorkspaceDirectly(t *testing.T, ctx context.Context, api *ComponentAP req := &wsmanapi.StartWorkspaceRequest{ Id: instanceID.String(), - ServicePrefix: instanceID.String(), + ServicePrefix: workspaceID, Metadata: &wsmanapi.WorkspaceMetadata{ Owner: gitpodBuiltinUserID, MetaId: workspaceID, From 1688f919a9856b7ea0581be3ce01ade9205fff7a Mon Sep 17 00:00:00 2001 From: Pudong Zheng Date: Thu, 13 Oct 2022 11:03:08 +0000 Subject: [PATCH 3/4] [ws-manager] check ide-ready response --- components/ws-manager/pkg/manager/probe.go | 31 +++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/components/ws-manager/pkg/manager/probe.go b/components/ws-manager/pkg/manager/probe.go index d3839b753f05b5..12bf1ad22fbd85 100644 --- a/components/ws-manager/pkg/manager/probe.go +++ b/components/ws-manager/pkg/manager/probe.go @@ -7,12 +7,14 @@ package manager import ( "context" "crypto/tls" + "io/ioutil" "net/http" "net/url" "time" "github.com/gitpod-io/gitpod/common-go/log" "github.com/gitpod-io/gitpod/common-go/tracing" + "k8s.io/apimachinery/pkg/util/json" ) // WorkspaceProbeResult marks the result of a workspace probe @@ -38,7 +40,7 @@ type WorkspaceReadyProbe struct { // NewWorkspaceReadyProbe creates a new workspace probe func NewWorkspaceReadyProbe(workspaceID string, workspaceURL url.URL) WorkspaceReadyProbe { - workspaceURL.Path += "/_supervisor/v1/status/ide" + workspaceURL.Path += "/_supervisor/v1/status/ide/wait/true" readyURL := workspaceURL.String() return WorkspaceReadyProbe{ @@ -92,14 +94,31 @@ func (p *WorkspaceReadyProbe) Run(ctx context.Context) WorkspaceProbeResult { // we've timed out - do not log this as it would spam the logs for no good reason continue } - resp.Body.Close() - if resp.StatusCode == http.StatusOK { - break + if resp.StatusCode != http.StatusOK { + resp.Body.Close() + log.WithField("url", p.readyURL).WithField("status", resp.StatusCode).Debug("workspace did not respond to ready probe with OK status") + time.Sleep(p.RetryDelay) + continue } - log.WithField("url", p.readyURL).WithField("status", resp.StatusCode).Debug("workspace did not respond to ready probe with OK status") - time.Sleep(p.RetryDelay) + rawBody, err := ioutil.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + log.WithField("url", p.readyURL).WithField("status", resp.StatusCode).WithError(err).Debug("ready probe failed: cannot read body") + continue + } + var probeResult struct { + Ok bool `json:"ok"` + } + err = json.Unmarshal(rawBody, &probeResult) + if err != nil { + log.WithField("url", p.readyURL).WithField("status", resp.StatusCode).WithError(err).Debug("ready probe failed: unable to unmarshal json") + continue + } + if probeResult.Ok { + break + } } // workspace is actually ready From 15414c33a646f831a22f88b58531f21d38b0dfcf Mon Sep 17 00:00:00 2001 From: Pudong Zheng Date: Thu, 13 Oct 2022 14:46:22 +0000 Subject: [PATCH 4/4] [integration-test] make workspace integtation test pass --- .../components/ws-manager/prebuild_test.go | 78 +++++++++++-------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/test/tests/components/ws-manager/prebuild_test.go b/test/tests/components/ws-manager/prebuild_test.go index 5fd34a4dc91176..8b4ec52127e994 100644 --- a/test/tests/components/ws-manager/prebuild_test.go +++ b/test/tests/components/ws-manager/prebuild_test.go @@ -7,6 +7,7 @@ package wsmanager import ( "context" "encoding/json" + "errors" "fmt" "net/rpc" "path/filepath" @@ -48,7 +49,7 @@ func TestPrebuildWorkspaceTaskSuccess(t *testing.T) { CheckoutLocation: "empty", WorkspaceRoot: "/workspace/empty", Task: []gitpod.TasksItems{ - {Init: "echo \"some output\" > someFile; sleep 20; exit 0;"}, + {Init: "echo \"some output\" > someFile; sleep 10; exit 0;"}, }, }, { @@ -57,7 +58,7 @@ func TestPrebuildWorkspaceTaskSuccess(t *testing.T) { CheckoutLocation: "empty", WorkspaceRoot: "/workspace/empty", Task: []gitpod.TasksItems{ - {Init: "echo \"some output\" > someFile; sleep 20; exit 0;"}, + {Init: "echo \"some output\" > someFile; sleep 10; exit 0;"}, }, FF: []wsmanapi.WorkspaceFeatureFlag{wsmanapi.WorkspaceFeatureFlag_PERSISTENT_VOLUME_CLAIM}, }, @@ -74,7 +75,7 @@ func TestPrebuildWorkspaceTaskSuccess(t *testing.T) { // TODO: change to use server API to launch the workspace, so we could run the integration test as the user code flow // which is client -> server -> ws-manager rather than client -> ws-manager directly - _, stopWs, err := integration.LaunchWorkspaceDirectly(t, ctx, api, integration.WithRequestModifier(func(req *wsmanapi.StartWorkspaceRequest) error { + ws, stopWs, err := integration.LaunchWorkspaceDirectly(t, ctx, api, integration.WithRequestModifier(func(req *wsmanapi.StartWorkspaceRequest) error { req.Type = wsmanapi.WorkspaceType_PREBUILD tasks, err := json.Marshal(test.Task) @@ -103,17 +104,24 @@ func TestPrebuildWorkspaceTaskSuccess(t *testing.T) { t.Fatalf("cannot launch a workspace: %q", err) } defer func() { - sctx, scancel := context.WithTimeout(context.Background(), 5*time.Minute) - defer scancel() - - sapi := integration.NewComponentAPI(sctx, cfg.Namespace(), kubeconfig, cfg.Client()) - defer sapi.Done(t) - - _, err = stopWs(true, sapi) - if err != nil { + // stop workspace in defer function to prevent we forget to stop the workspace + if err := stopWorkspace(t, cfg, stopWs); err != nil { t.Errorf("cannot stop workspace: %q", err) } }() + _, err = integration.WaitForWorkspace(ctx, api, ws.Req.Id, func(status *wsmanapi.WorkspaceStatus) bool { + if status.Phase != wsmanapi.WorkspacePhase_STOPPED { + return false + } + if status.Conditions.HeadlessTaskFailed != "" { + t.Logf("Conditions: %v", status.Conditions) + t.Fatal("unexpected HeadlessTaskFailed condition") + } + return true + }) + if err != nil { + t.Fatalf("failed for wait workspace stop: %q", err) + } }) } return ctx @@ -124,8 +132,6 @@ func TestPrebuildWorkspaceTaskSuccess(t *testing.T) { } func TestPrebuildWorkspaceTaskFail(t *testing.T) { - t.Skip("status never returns HeadlessTaskFailed (exit 1)") - f := features.New("prebuild"). WithLabel("component", "server"). Assess("it should create a prebuild and fail after running the defined tasks", func(_ context.Context, t *testing.T, cfg *envconf.Config) context.Context { @@ -141,7 +147,7 @@ func TestPrebuildWorkspaceTaskFail(t *testing.T) { req.Type = wsmanapi.WorkspaceType_PREBUILD req.Spec.Envvars = append(req.Spec.Envvars, &wsmanapi.EnvironmentVariable{ Name: "GITPOD_TASKS", - Value: `[{ "init": "echo \"some output\" > someFile; sleep 20; exit 1;" }]`, + Value: `[{ "init": "echo \"some output\" > someFile; sleep 10; exit 1;" }]`, }) return nil })) @@ -150,15 +156,9 @@ func TestPrebuildWorkspaceTaskFail(t *testing.T) { } defer func() { - sctx, scancel := context.WithTimeout(context.Background(), 5*time.Minute) - defer scancel() - - sapi := integration.NewComponentAPI(sctx, cfg.Namespace(), kubeconfig, cfg.Client()) - defer sapi.Done(t) - - _, err = stopWs(true, sapi) - if err != nil { - t.Fatal(err) + // stop workspace in defer function to prevent we forget to stop the workspace + if err := stopWorkspace(t, cfg, stopWs); err != nil { + t.Errorf("cannot stop workspace: %q", err) } }() @@ -173,7 +173,7 @@ func TestPrebuildWorkspaceTaskFail(t *testing.T) { return true }) if err != nil { - t.Fatalf("cannot start workspace: %q", err) + t.Fatalf("failed for wait workspace stop: %q", err) } return ctx @@ -186,7 +186,7 @@ func TestPrebuildWorkspaceTaskFail(t *testing.T) { const ( prebuildLogPath string = "/workspace/.gitpod" prebuildLog string = "'🤙 This task ran as a workspace prebuild'" - initTask string = "echo \"some output\" > someFile; sleep 90;" + initTask string = "echo \"some output\" > someFile; sleep 10;" regularPrefix string = "ws-" ) @@ -256,7 +256,7 @@ func TestOpenWorkspaceFromPrebuild(t *testing.T) { // create a prebuild and stop workspace // TODO: change to use server API to launch the workspace, so we could run the integration test as the user code flow // which is client -> server -> ws-manager rather than client -> ws-manager directly - _, prebuildStopWs, err := integration.LaunchWorkspaceDirectly(t, ctx, api, integration.WithRequestModifier(func(req *wsmanapi.StartWorkspaceRequest) error { + ws, prebuildStopWs, err := integration.LaunchWorkspaceDirectly(t, ctx, api, integration.WithRequestModifier(func(req *wsmanapi.StartWorkspaceRequest) error { req.Type = wsmanapi.WorkspaceType_PREBUILD req.Spec.Envvars = append(req.Spec.Envvars, &wsmanapi.EnvironmentVariable{ Name: "GITPOD_TASKS", @@ -278,8 +278,13 @@ func TestOpenWorkspaceFromPrebuild(t *testing.T) { if err != nil { t.Fatalf("cannot launch a workspace: %q", err) } - - prebuildSnapshot, prebuildVSInfo, err := stopWorkspaceAndFindSnapshot(prebuildStopWs, api) + defer func() { + // stop workspace in defer function to prevent we forget to stop the workspace + if err := stopWorkspace(t, cfg, prebuildStopWs); err != nil { + t.Errorf("cannot stop workspace: %q", err) + } + }() + prebuildSnapshot, prebuildVSInfo, err := watchStopWorkspaceAndFindSnapshot(ctx, ws.Req.Id, api) if err != nil { t.Fatalf("stop workspace and find snapshot error: %v", err) } @@ -551,7 +556,7 @@ func TestPrebuildAndRegularWorkspaceDifferentWorkspaceClass(t *testing.T) { }) // create a prebuild and stop workspace - _, prebuildStopWs, err := integration.LaunchWorkspaceDirectly(t, ctx, api, integration.WithRequestModifier(func(req *wsmanapi.StartWorkspaceRequest) error { + ws, prebuildStopWs, err := integration.LaunchWorkspaceDirectly(t, ctx, api, integration.WithRequestModifier(func(req *wsmanapi.StartWorkspaceRequest) error { req.Type = wsmanapi.WorkspaceType_PREBUILD req.Spec.Class = test.PrebuildWorkspaceClass req.Spec.Envvars = append(req.Spec.Envvars, &wsmanapi.EnvironmentVariable{ @@ -574,8 +579,14 @@ func TestPrebuildAndRegularWorkspaceDifferentWorkspaceClass(t *testing.T) { if err != nil { t.Fatalf("cannot launch a workspace: %q", err) } + defer func() { + // stop workspace in defer function to prevent we forget to stop the workspace + if err := stopWorkspace(t, cfg, prebuildStopWs); err != nil { + t.Errorf("cannot stop workspace: %q", err) + } + }() - prebuildSnapshot, vsInfo, err := stopWorkspaceAndFindSnapshot(prebuildStopWs, api) + prebuildSnapshot, vsInfo, err := watchStopWorkspaceAndFindSnapshot(ctx, ws.Req.Id, api) if err != nil { t.Fatalf("stop workspace and find snapshot error: %v", err) } @@ -821,11 +832,14 @@ func findVolumeSnapshot(ctx context.Context, isPVCEnable bool, instanceID, names return vsInfo, nil } -func stopWorkspaceAndFindSnapshot(StopWorkspaceFunc integration.StopWorkspaceFunc, api *integration.ComponentAPI) (string, *wsmanapi.VolumeSnapshotInfo, error) { - lastStatus, err := StopWorkspaceFunc(true, api) +func watchStopWorkspaceAndFindSnapshot(ctx context.Context, instanceId string, api *integration.ComponentAPI) (string, *wsmanapi.VolumeSnapshotInfo, error) { + lastStatus, err := integration.WaitForWorkspaceStop(ctx, api, instanceId) if err != nil { return "", nil, err } + if lastStatus.Conditions.HeadlessTaskFailed != "" { + return "", nil, errors.New("unexpected HeadlessTaskFailed condition") + } if lastStatus == nil || lastStatus.Conditions == nil || lastStatus.Conditions.VolumeSnapshot == nil { return "", nil, nil }