Skip to content
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.

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

[dev] Move BPF VM creation into image build #4959

Merged
merged 3 commits into from
Jul 28, 2021

Conversation

geropl
Copy link
Member

@geropl geropl commented Jul 26, 2021

Fixes #4945.

Also, makes the img even smaller by changing the format from raw to qcow2.
image

Test

  • start a workspace on this branch
  • see that the VM is still started up on workspace start, but without the init phase/log

Future work:

  • build time is still quite high if we need to rebuild the dev image. Converging all in-VM steps into a single script instead of starting/stopping it for every step should help.

@github-actions
Copy link
Contributor

⚠️ Hew reviewer! BE CAREFUL ⚠️
Review the code before opening in your Gitpod. .gitpod.yaml was changed and it might be harfmul.

@geropl geropl force-pushed the geropl/move-qemu-image-build-to-4945 branch from 87fcc4c to 38393f5 Compare July 27, 2021 08:53
@github-actions
Copy link
Contributor

⚠️ Hew reviewer! BE CAREFUL ⚠️
Review the code before opening in your Gitpod. .gitpod.yaml was changed and it might be harfmul.

@geropl geropl force-pushed the geropl/move-qemu-image-build-to-4945 branch from 38393f5 to 1809fac Compare July 27, 2021 08:55
@github-actions
Copy link
Contributor

⚠️ Hew reviewer! BE CAREFUL ⚠️
Review the code before opening in your Gitpod. .gitpod.yaml was changed and it might be harfmul.

@geropl geropl force-pushed the geropl/move-qemu-image-build-to-4945 branch from 1809fac to 29c6d37 Compare July 27, 2021 09:02
@github-actions
Copy link
Contributor

⚠️ Hew reviewer! BE CAREFUL ⚠️
Review the code before opening in your Gitpod. .gitpod.yaml was changed and it might be harfmul.

@github-actions
Copy link
Contributor

⚠️ Hew reviewer! BE CAREFUL ⚠️
Review the code before opening in your Gitpod. .gitpod.yaml was changed and it might be harfmul.

@codecov
Copy link

codecov bot commented Jul 27, 2021

Codecov Report

Merging #4959 (0941a08) into main (89db73e) will increase coverage by 31.47%.
The diff coverage is n/a.

❗ Current head 0941a08 differs from pull request most recent head 1a21d80. Consider uploading reports for the commit 1a21d80 to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##           main    #4959       +/-   ##
=========================================
+ Coverage      0   31.47%   +31.47%     
=========================================
  Files         0      123      +123     
  Lines         0    21424    +21424     
=========================================
+ Hits          0     6743     +6743     
- Misses        0    14090    +14090     
- Partials      0      591      +591     
Flag Coverage Δ
components-blobserve-app 29.88% <ø> (?)
components-common-go-lib 32.41% <ø> (?)
components-content-service-api-go-lib ∅ <ø> (?)
components-content-service-app 14.46% <ø> (?)
components-content-service-lib 14.46% <ø> (?)
components-ee-agent-smith-app 26.92% <ø> (?)
components-ee-kedge-app 45.48% <ø> (?)
components-ee-ws-scheduler-app 62.84% <ø> (?)
components-gitpod-cli-app 9.74% <ø> (?)
components-gitpod-protocol-go-lib ∅ <ø> (?)
components-image-builder-api-go-lib ∅ <ø> (?)
components-image-builder-app 34.44% <ø> (?)
components-image-builder-bob-app ∅ <ø> (?)
components-image-builder-mk3-app 6.53% <ø> (?)
components-licensor-lib 79.52% <ø> (?)
components-local-app-api-go-lib ∅ <ø> (?)
components-local-app-app-darwin ∅ <ø> (?)
components-local-app-app-linux ∅ <ø> (?)
components-local-app-app-windows ∅ <ø> (?)
components-registry-facade-api-go-lib ∅ <ø> (?)
components-registry-facade-app 11.57% <ø> (?)
components-registry-facade-lib 11.57% <ø> (?)
components-service-waiter-app ∅ <ø> (?)
components-supervisor-api-go-lib ∅ <ø> (?)
components-supervisor-app 36.75% <ø> (?)
components-workspacekit-app 7.69% <ø> (?)
components-ws-daemon-api-go-lib ∅ <ø> (?)
components-ws-daemon-app 22.30% <ø> (?)
components-ws-daemon-nsinsider-app ∅ <ø> (?)
components-ws-manager-api-go-lib ∅ <ø> (?)
components-ws-manager-app 36.52% <ø> (?)
components-ws-manager-bridge-api-go-lib ∅ <ø> (?)
components-ws-proxy-app 67.35% <ø> (?)
dev-blowtorch-app ∅ <ø> (?)
dev-loadgen-app ∅ <ø> (?)
dev-poolkeeper-app ∅ <ø> (?)
dev-sweeper-app ∅ <ø> (?)
dev-version-manifest-app 83.56% <ø> (?)

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

Impacted Files Coverage Δ
components/ws-daemon/pkg/content/config.go 62.50% <0.00%> (ø)
components/supervisor/pkg/ports/tunnel.go 63.63% <0.00%> (ø)
components/common-go/namegen/workspaceid.go 47.82% <0.00%> (ø)
components/supervisor/pkg/ports/ports.go 59.33% <0.00%> (ø)
components/supervisor/pkg/supervisor/config.go 4.51% <0.00%> (ø)
components/supervisor/pkg/terminal/ring-buffer.go 45.65% <0.00%> (ø)
components/ee/kedge/pkg/kedge/collector.go 25.58% <0.00%> (ø)
components/ws-proxy/pkg/proxy/auth.go 100.00% <0.00%> (ø)
components/content-service/pkg/layer/provider.go 36.76% <0.00%> (ø)
components/ee/ws-scheduler/pkg/scheduler/config.go 62.50% <0.00%> (ø)
... and 113 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89db73e...1a21d80. Read the comment docs.

@geropl geropl marked this pull request as ready for review July 27, 2021 10:04
@geropl
Copy link
Member Author

geropl commented Jul 27, 2021

@aledbf Would be nice if you found time for a review today or tomorrow 🙏

@aledbf
Copy link
Member

aledbf commented Jul 27, 2021

/werft run

👍 started the job as gitpod-build-geropl-move-qemu-image-build-to-4945.5

@@ -12,15 +12,14 @@ fi

set -euo pipefail

script_dirname="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
outdir="${script_dirname}/_output"
outdir="/root/_output"


rm -Rf ~/.ssh
cp -r "${outdir}/.ssh" ~/.ssh
Copy link
Member

Choose a reason for hiding this comment

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

I get this error when I start a workspace:

gitpod /workspace/gitpod $ leeway run components/ee/agent-smith:qemu
cp: cannot stat '/root/_output/.ssh': Permission denied

@@ -12,15 +12,14 @@ fi

set -euo pipefail

script_dirname="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
outdir="${script_dirname}/_output"
outdir="/root/_output"


rm -Rf ~/.ssh
cp -r "${outdir}/.ssh" ~/.ssh
Copy link
Member

Choose a reason for hiding this comment

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

sudo solves the issue

@github-actions
Copy link
Contributor

⚠️ Hew reviewer! BE CAREFUL ⚠️
Review the code before opening in your Gitpod. .gitpod.yaml was changed and it might be harfmul.

@aledbf
Copy link
Member

aledbf commented Jul 27, 2021

/lgtm

@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 1819057cb06f306f2fb98a436965d5337bf1b744

@aledbf
Copy link
Member

aledbf commented Jul 27, 2021

/hold

(not sure if someone else wants to review the PR)

@csweichel
Copy link
Contributor

csweichel commented Jul 27, 2021

The VM still works as indented 🚀

image

I also see the .ssh permission issue though

@aledbf
Copy link
Member

aledbf commented Jul 27, 2021

I also see the .ssh permission issue though

Right, I fixed the copy but not the permissions

@aledbf aledbf force-pushed the geropl/move-qemu-image-build-to-4945 branch from 3da9dbe to 1a21d80 Compare July 27, 2021 18:23
@roboquat roboquat removed the lgtm label Jul 27, 2021
@github-actions
Copy link
Contributor

⚠️ Hew reviewer! BE CAREFUL ⚠️
Review the code before opening in your Gitpod. .gitpod.yaml was changed and it might be harfmul.

@aledbf
Copy link
Member

aledbf commented Jul 27, 2021

@csweichel please try again

@csweichel
Copy link
Contributor

/lgtm

@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: d04f80ec4ef1c0ed7fa0e6e637ef8feac2e49034

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, csweichel, geropl

Associated issue: #4945

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@csweichel
Copy link
Contributor

/hold cancel

@roboquat roboquat merged commit 529ca01 into main Jul 28, 2021
@roboquat roboquat deleted the geropl/move-qemu-image-build-to-4945 branch July 28, 2021 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move qemu image build to Docker image rather than the init tasks
4 participants