[Gitpod CLI] `gp rebuild` #15638

andreafalzetti · 2023-01-09T13:41:10Z

Description

We're introducing a new command to gp cli, to facilitate users in testing the the build and run of custom Docker workspace images.

Note: if the user closes the terminal via the IDE once they are inside the debug container, the event won't be sent.

Related Issue(s)

Relates to #7671

How to test

Start a workspace https://ide-gp-build.preview.gitpod-dev.com/#https://github.com/gitpod-io/empty
Run gp rebuild and observe error message
Specify a custom dockerfile (both using image: xxx or image.file
If you specify a non-existing file, and re-run, observe the error
Check in Segment Staging Trusted that gp_command is being sent with the following fields:
- errorCode
Configure a real image and re-run, exit the container
Check in Segment Staging Trusted that gp_command is being sent with the following fields:
- durationMs
- dockerBuildDurationSeconds

Release Notes

new command: `gp rebuild`

Documentation

https://github.com/gitpod-io/website/pull/3226

Werft options:

/werft with-local-preview
If enabled this will build install/preview
/werft with-preview
/werft with-large-vm
/werft with-integration-tests=all
Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh
/werft analytics=segment

components/gitpod-cli/cmd/rebuild.go

components/gitpod-cli/pkg/utils/trackEvent.go

components/gitpod-cli/cmd/rebuild.go

akosyakov · 2023-01-12T15:37:10Z

components/gitpod-cli/cmd/rebuild.go

+				event.Set("ErrorCode", utils.RebuildErrorCode_DockerfileNotFound).Send(ctx)
+				return
+			}
+			dockerfile, err := os.ReadFile(dockerfilePath)


err is swallowed, we should give a root cause to the user

akosyakov · 2023-01-12T15:37:33Z

components/gitpod-cli/cmd/rebuild.go

+			return
+		}
+
+		tmpDir, err := os.MkdirTemp("", "gp-rebuild-*")


err is swallowed, we should give a root cause to the user

akosyakov · 2023-01-12T15:43:22Z

components/gitpod-cli/cmd/rebuild.go

+			}
+			dockerfile, err := os.ReadFile(dockerfilePath)
+			if err != nil {
+				log.Fatal("Could not read the Dockerfile")


@andreafalzetti I don't think the flow here can work. Maybe I'm missing something. Fatal calls os.Exit and it interrupts the program immediately, defer are not called and events are not sent.

I'm asking can we restructure top level routine to something like (pseudo code):

ctx := context.Background() event := utils.TrackEvent(ctx, ...) err := doRebuild(ctx, event) if err != nil && event.ErrorCode == "" { err.ErrorCode = "system" } event.Send() if err != nil { utils.LogError(..) // log to a user and GCP os.Exit(1) }

It looks also we swallow errors here and there, we should always propagate given errors to users.

akosyakov · 2023-01-13T06:09:25Z

components/gitpod-cli/pkg/utils/trackEvent.go

+	WorkspaceId                string  `json:"workspaceId,omitempty"`
+	InstanceId                 string  `json:"instanceId,omitempty"`
+	Timestamp                  int64   `json:"timestamp,omitempty"`
+	DockerBuildDurationSeconds float64 `json:"dockerBuildDurationSeconds,omitempty"`


Let’s call it imageBuildDurationSeconds. We are likely go away from docker next week to build the image.

akosyakov · 2023-01-13T11:49:22Z

Is there a reason why don’t we measure everything in milliseconds and drop suffix for unit, ie just duration and imageBuildDuration. We do it the same for other events?

andreafalzetti · 2023-01-13T12:05:50Z

Is there a reason why don’t we measure everything in milliseconds and drop suffix for unit, ie just duration and imageBuildDuration. We do it the same for other events?

The thought process is that image build will most likely take seconds so it would be more convinent to have it already in seconds.

About the suffix, I think it's a nice to have, in general, because it easily indicates the unit from the Segmenet/Mixpanel without having to access the code or remember what unit it was.

Happy to make it all ms and drop the suffix, no hard opinions

Co-authored-by: Victor Nogueira <[email protected]>

andreafalzetti · 2023-01-13T13:10:48Z

I've updated the PR with the suggestions from @akosyakov

I've tested almost all scenarios, but feel free to test again @felladrin 🙏

akosyakov

Code looks good, I have not tried, rely on Andrea and Victor for it

/hold

pease unhold when comfortable

felladrin · 2023-01-13T13:26:52Z

I'm running a quick test on it now.

akosyakov · 2023-01-13T13:35:25Z

@andreafalzetti follow up are to update docs and then contribute how to troubleshoot link to the image build screen? Maybe makes sense to talk to @gtsiolis where to add it, we will speak about it again next design meeting

felladrin

Tested! ✅

andreafalzetti · 2023-01-13T13:47:22Z

/unhold

gtsiolis · 2023-01-13T14:21:49Z

Exciting changes! ➰

@akosyakov I've added your comment above in #15638 (comment) in the next 🍳 IDE / Design Check-in on next Wed (Jan 18), see relevant discussion point (internal).

akosyakov · 2023-01-13T21:38:13Z

components/gitpod-cli/cmd/rebuild.go

+		return err
+	}
+
+	err = os.WriteFile(filepath.Join(tmpDir, "Dockerfile"), []byte(baseimage), 0644)


@andreafalzetti @felladrin is not it bogus? 🤔

We should build in the context of /workspace because Dockerfile can make usage of COPY statements?

I think we can use the current working direction to run docker command but then pass -f which pints to the temp dockerfile?

akosyakov · 2023-01-13T22:41:57Z

components/gitpod-cli/cmd/rebuild.go

+			fmt.Println("")
+			fmt.Println("Once you configure your Dockerfile, re-run this command to validate your changes")
+			event.Set("ErrorCode", utils.RebuildErrorCode_DockerfileEmpty)
+			return err


@andreafalzetti @felladrin for me this branch is hanging 🤔

I think the problem is that it returns nil here. It should return some real error. We need to double check all places, where we return without having err object in advance.

Update hanging actually because of my branch, but I think we should return an error here to exit with 1 properly or another value like ok if we don't want to print an error

i.e. something like:

ok, err = runRebuild(ctx, supervisorClient, event) if event.Data.ErrorCode == "" { if err != nil { event.Set("ErrorCode", utils.SystemErrorCode) } else if !ok { event.Set("ErrorCode", utils.UserErrorCode) } } event.Send(ctx) if err != nil { utils.LogError(ctx, err, "Failed to rebuild", supervisorClient) } var exitCode int if err != nil || !ok { exitCode = 1 } os.Exit(exitCode)

oops, I wonder how I missed this :(

will send a follow-up PR!

Update: ok, it's not hanging then.. I will look into this anyway

there is also the default branch of the switch having the same issue, I will check them all

andreafalzetti · 2023-01-13T22:57:36Z

@akosyakov I will double check that branch. About Dockerfile location it's a good pont, maybe the tmp file should be located in the root of the project 🤔

akosyakov · 2023-01-13T23:04:14Z

@andreafalzetti I think it does not matter, you can do docker build . -f /tmp/gp-rebuild/Dockerfile and don't set dockerCmd.Dir -> it will build then in the context of a current directory but with a temp Dockerfile

andreafalzetti · 2023-01-13T23:38:30Z

@akosyakov there are 2 other things I've noticed now:

if bash is not installed in the image, it fails (this is also true atm when we suggest launching bash from the docs, but it's a user action so it's revertable) in rebuild maybe we should consider using sh as there are higher changes it will work 🤔 WDYT?
if the user generates a non-zero exit code from within the container, it gets propagated outside (e.g. type exit 1)

andreafalzetti · 2023-01-14T00:31:39Z

I've opened a follow-up PR: #15740

Let's discuss improvements over there 🙏

andreafalzetti added component: gp cli team: IDE labels Jan 9, 2023

roboquat added release-note-none do-not-merge/work-in-progress labels Jan 9, 2023

This comment was marked as resolved.

Sign in to view

roboquat added the size/XL label Jan 9, 2023

This comment was marked as resolved.

Sign in to view

andreafalzetti force-pushed the ide/gp-build branch from 843e63b to 7009402 Compare January 9, 2023 13:45

This comment was marked as resolved.

Sign in to view

loujaybee assigned andreafalzetti Jan 9, 2023

andreafalzetti assigned felladrin Jan 9, 2023

andreafalzetti force-pushed the ide/gp-build branch 2 times, most recently from d2127d0 to 8b9db0c Compare January 9, 2023 19:29

This comment was marked as resolved.

Sign in to view

andreafalzetti force-pushed the ide/gp-build branch 2 times, most recently from 96499a9 to 755b956 Compare January 10, 2023 16:25

This comment was marked as resolved.

Sign in to view

akosyakov reviewed Jan 10, 2023

View reviewed changes

components/gitpod-cli/cmd/rebuild.go Outdated Show resolved Hide resolved

akosyakov reviewed Jan 10, 2023

View reviewed changes

components/gitpod-cli/cmd/rebuild.go Outdated Show resolved Hide resolved

akosyakov reviewed Jan 10, 2023

View reviewed changes

components/gitpod-cli/pkg/utils/trackEvent.go Outdated Show resolved Hide resolved

akosyakov reviewed Jan 10, 2023

View reviewed changes

components/gitpod-cli/pkg/utils/trackEvent.go Show resolved Hide resolved

andreafalzetti force-pushed the ide/gp-build branch 5 times, most recently from 227f8b6 to 70ed53b Compare January 12, 2023 12:19

roboquat added release-note and removed release-note-none labels Jan 12, 2023

akosyakov reviewed Jan 12, 2023

View reviewed changes

akosyakov reviewed Jan 13, 2023

View reviewed changes

andreafalzetti force-pushed the ide/gp-build branch from 3ebedc9 to c61d095 Compare January 13, 2023 09:32

andreafalzetti marked this pull request as draft January 13, 2023 12:07

roboquat added the do-not-merge/work-in-progress label Jan 13, 2023

gitpod-cli: add gp rebuild cmd

8f6e79e

Co-authored-by: Victor Nogueira <[email protected]>

andreafalzetti force-pushed the ide/gp-build branch from 6a27862 to 8f6e79e Compare January 13, 2023 13:09

andreafalzetti marked this pull request as ready for review January 13, 2023 13:10

roboquat removed the do-not-merge/work-in-progress label Jan 13, 2023

akosyakov approved these changes Jan 13, 2023

View reviewed changes

felladrin approved these changes Jan 13, 2023

View reviewed changes

roboquat removed the do-not-merge/hold label Jan 13, 2023

roboquat merged commit 1e24370 into main Jan 13, 2023

roboquat deleted the ide/gp-build branch January 13, 2023 13:48

akosyakov reviewed Jan 13, 2023

View reviewed changes

andreafalzetti mentioned this pull request Jan 14, 2023

[Gitpod CLI] gp rebuild improvements #15740

Merged

5 tasks

roboquat added deployed: IDE IDE change is running in production deployed Change is completely running in production labels Jan 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Gitpod CLI] `gp rebuild` #15638

[Gitpod CLI] `gp rebuild` #15638

andreafalzetti commented Jan 9, 2023 •

edited by filiptronicek

Loading

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

akosyakov Jan 12, 2023

akosyakov Jan 12, 2023

akosyakov Jan 12, 2023 •

edited

Loading

akosyakov Jan 13, 2023

akosyakov commented Jan 13, 2023 •

edited by werft-gitpod-dev-com bot

Loading

andreafalzetti commented Jan 13, 2023 •

edited

Loading

andreafalzetti commented Jan 13, 2023 •

edited by werft-gitpod-dev-com bot

Loading

akosyakov left a comment

felladrin commented Jan 13, 2023

akosyakov commented Jan 13, 2023

felladrin left a comment

andreafalzetti commented Jan 13, 2023

gtsiolis commented Jan 13, 2023 •

edited by werft-gitpod-dev-com bot

Loading

akosyakov Jan 13, 2023 •

edited

Loading

akosyakov Jan 13, 2023

akosyakov Jan 13, 2023 •

edited

Loading

akosyakov Jan 13, 2023

andreafalzetti Jan 13, 2023 •

edited

Loading

andreafalzetti Jan 13, 2023

andreafalzetti commented Jan 13, 2023 •

edited

Loading

akosyakov commented Jan 13, 2023

andreafalzetti commented Jan 13, 2023 •

edited

Loading

andreafalzetti commented Jan 14, 2023 •

edited by werft-gitpod-dev-com bot

Loading

[Gitpod CLI] gp rebuild #15638

[Gitpod CLI] gp rebuild #15638

Conversation

andreafalzetti commented Jan 9, 2023 • edited by filiptronicek Loading

Description

Related Issue(s)

How to test

Release Notes

Documentation

Werft options:

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

akosyakov Jan 12, 2023

Choose a reason for hiding this comment

akosyakov Jan 12, 2023

Choose a reason for hiding this comment

akosyakov Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

akosyakov Jan 13, 2023

Choose a reason for hiding this comment

akosyakov commented Jan 13, 2023 • edited by werft-gitpod-dev-com bot Loading

andreafalzetti commented Jan 13, 2023 • edited Loading

andreafalzetti commented Jan 13, 2023 • edited by werft-gitpod-dev-com bot Loading

akosyakov left a comment

Choose a reason for hiding this comment

felladrin commented Jan 13, 2023

akosyakov commented Jan 13, 2023

felladrin left a comment

Choose a reason for hiding this comment

andreafalzetti commented Jan 13, 2023

gtsiolis commented Jan 13, 2023 • edited by werft-gitpod-dev-com bot Loading

akosyakov Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

akosyakov Jan 13, 2023

Choose a reason for hiding this comment

akosyakov Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

akosyakov Jan 13, 2023

Choose a reason for hiding this comment

andreafalzetti Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

andreafalzetti Jan 13, 2023

Choose a reason for hiding this comment

andreafalzetti commented Jan 13, 2023 • edited Loading

akosyakov commented Jan 13, 2023

andreafalzetti commented Jan 13, 2023 • edited Loading

andreafalzetti commented Jan 14, 2023 • edited by werft-gitpod-dev-com bot Loading

[Gitpod CLI] `gp rebuild` #15638

[Gitpod CLI] `gp rebuild` #15638

andreafalzetti commented Jan 9, 2023 •

edited by filiptronicek

Loading

akosyakov Jan 12, 2023 •

edited

Loading

akosyakov commented Jan 13, 2023 •

edited by werft-gitpod-dev-com bot

Loading

andreafalzetti commented Jan 13, 2023 •

edited

Loading

andreafalzetti commented Jan 13, 2023 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Jan 13, 2023 •

edited by werft-gitpod-dev-com bot

Loading

akosyakov Jan 13, 2023 •

edited

Loading

akosyakov Jan 13, 2023 •

edited

Loading

andreafalzetti Jan 13, 2023 •

edited

Loading

andreafalzetti commented Jan 13, 2023 •

edited

Loading

andreafalzetti commented Jan 13, 2023 •

edited

Loading

andreafalzetti commented Jan 14, 2023 •

edited by werft-gitpod-dev-com bot

Loading