Skip to content

[ws-manager] Do not follow redirect when ready probe #13828

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 13, 2022
Merged

[ws-manager] Do not follow redirect when ready probe #13828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f99ef8c
[ws-manager] Do not follow redirect when ready probe
iQQBot Oct 13, 2022
bf2ce4f
[integration-test] use correct workspace id
iQQBot Oct 13, 2022
1688f91
[ws-manager] check ide-ready response
iQQBot Oct 13, 2022
15414c3
[integration-test] make workspace integtation test pass
iQQBot Oct 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions components/ws-manager/pkg/manager/probe.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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{
Expand Down Expand Up @@ -70,6 +72,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 {
Expand All @@ -89,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
Expand Down
2 changes: 1 addition & 1 deletion test/pkg/integration/workspace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
78 changes: 46 additions & 32 deletions test/tests/components/ws-manager/prebuild_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package wsmanager
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/rpc"
"path/filepath"
Expand Down Expand Up @@ -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;"},
},
},
{
Expand All @@ -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},
},
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
}))
Expand All @@ -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)
}
}()

Expand All @@ -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
Expand All @@ -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-"
)

Expand Down Expand Up @@ -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",
Expand All @@ -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)
}
Expand Down Expand Up @@ -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{
Expand All @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand Down