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

[image-builder] Export image config #5693

Merged
merged 6 commits into from
Sep 29, 2021

Conversation

csweichel
Copy link
Contributor

@csweichel csweichel commented Sep 14, 2021

Description

This PR makes image-builder properly export the image config when building the workspace image. buildkit does not do this by default and the exporter needs to be told which image-config to export.

This PR also corrects a failure mode introduced by the instability of the failed condition of workspace status updates produced by ws-manager. Because of this instability, image-builder would falsely report an image-build as done when it really was not. We solve this by re-resolving the supposedly built image ref and returning a failure condition if it wasn't built. There's also a unit test for this.

Related Issue(s)

fixes #5597
fixes #5497

How to test

TODO: unit tests are upcoming (hence this being a draft)

Release Notes

[image-builder] Include environment variables in the built workspace image

@csweichel
Copy link
Contributor Author

/assign @aledbf

@aledbf
Copy link
Member

aledbf commented Sep 14, 2021

@csweichel I still see the same behavio?

Opening this PR in staging I get the same behavior (not inheriting PATH from dev-environment image)

oci-tool fetch image reg.csweichel-image-builder-mk-does-5597.staging.gitpod-dev.com:30099/remote/e0b0b0e4-7075-48b6-8934-53635ad28047:latest
{
  "architecture": "amd64",
  "config": {
    "Env": [
      "PATH=/ide/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
      "EDITOR=/ide/bin/code",
      "VISUAL=/ide/bin/code",
      "GP_OPEN_EDITOR=/ide/bin/code",
      "GIT_EDITOR=/ide/bin/code --wait",
      "GP_PREVIEW_BROWSER=/ide/bin/code --preview",
      "GP_EXTERNAL_BROWSER=/ide/bin/code --openExternal"
    ],
    "WorkingDir": "/"
  },

@csweichel csweichel force-pushed the csweichel/image-builder-mk-does-5597 branch from 76bcdae to 3e0bf75 Compare September 14, 2021 15:10
@codecov
Copy link

codecov bot commented Sep 14, 2021

Codecov Report

Merging #5693 (48e5919) into main (60a93e7) will increase coverage by 18.83%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #5693       +/-   ##
===========================================
+ Coverage   19.04%   37.88%   +18.83%     
===========================================
  Files           2       16       +14     
  Lines         168     4136     +3968     
===========================================
+ Hits           32     1567     +1535     
- Misses        134     2443     +2309     
- Partials        2      126      +124     
Flag Coverage Δ
components-local-app-app-linux ?
components-local-app-app-windows ?
components-supervisor-app 37.88% <50.00%> (?)

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

Impacted Files Coverage Δ
components/supervisor/pkg/supervisor/tasks.go 58.97% <50.00%> (ø)
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go
components/supervisor/pkg/ports/served-ports.go 77.02% <0.00%> (ø)
components/supervisor/pkg/ports/ports.go 60.21% <0.00%> (ø)
components/supervisor/pkg/terminal/ring-buffer.go 45.65% <0.00%> (ø)
components/supervisor/pkg/supervisor/supervisor.go 6.35% <0.00%> (ø)
components/supervisor/pkg/ports/ports-config.go 79.85% <0.00%> (ø)
components/supervisor/pkg/ports/exposed-ports.go 0.00% <0.00%> (ø)
... and 9 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 60a93e7...48e5919. Read the comment docs.

@roboquat roboquat added size/XL and removed size/L labels Sep 14, 2021
@csweichel csweichel force-pushed the csweichel/image-builder-mk-does-5597 branch from 6e9a803 to cf1b4d4 Compare September 14, 2021 15:56
@roboquat roboquat added size/XXL and removed size/XL labels Sep 15, 2021
@csweichel csweichel force-pushed the csweichel/image-builder-mk-does-5597 branch 2 times, most recently from c638235 to afa9d40 Compare September 15, 2021 16:53
@csweichel
Copy link
Contributor Author

Things fail oddly here:

failed to solve: failed commit on ref \\\"manifest-sha256:1498980c832450ae54c3a055b828eb9afd8417aa31be10597701dca5d35e6c6f\\\": cannot reuse body, request must be retried\"

@csweichel
Copy link
Contributor Author

csweichel commented Sep 16, 2021

/werft run

👍 started the job as gitpod-build-csweichel-image-builder-mk-does-5597.11

@csweichel
Copy link
Contributor Author

Issue we're facing is containerd/containerd#5978

@csweichel csweichel force-pushed the csweichel/image-builder-mk-does-5597 branch 2 times, most recently from 9bbda75 to 2cce40e Compare September 24, 2021 16:06
@csweichel
Copy link
Contributor Author

csweichel commented Sep 24, 2021

/werft run

👍 started the job as gitpod-build-csweichel-image-builder-mk-does-5597.17

@aledbf
Copy link
Member

aledbf commented Sep 24, 2021

https://csweichel-image-builder-mk-does-5597.staging.gitpod-dev.com/#https://github.com/gitpod-io/gitpod/

Screenshot from 2021-09-24 14-44-35

{"host":"eu.gcr.io","level":"debug","message":"fetch response received","response.header.accept-ranges":"none","response.header.cache-control":"private","response.header.content-type":"application/json","response.header.date":"Fri, 24 Sep 2021 17:46:08 GMT","response.header.docker-distribution-api-version":"registry/2.0","response.header.server":"Docker Registry","response.header.vary":"Accept-Encoding","response.header.x-frame-options":"SAMEORIGIN","response.header.x-xss-protection":"0","response.status":"404 Not Found","severity":"DEBUG","time":"2021-09-24T17:46:08Z","url":"https://eu.gcr.io/v2/gitpod-core-dev/registry/workspace-images/manifests/5f4d43c7eed6f8277f2ae06148ff9b25b68a87989e6f503eb0ec2545ffc97dc0"}
{"host":"eu.gcr.io","level":"info","message":"trying next host - response was http.StatusNotFound","severity":"INFO","time":"2021-09-24T17:46:08Z"}

@csweichel
Copy link
Contributor Author

Unfortunately we don't have an updated preview environment yet :(
The build doesn't work because of some networking issue in the build cluster

@csweichel csweichel force-pushed the csweichel/image-builder-mk-does-5597 branch 2 times, most recently from 22cdf67 to 7eb9e03 Compare September 27, 2021 15:04
@csweichel csweichel force-pushed the csweichel/image-builder-mk-does-5597 branch from 7eb9e03 to 6a71487 Compare September 28, 2021 06:56
@csweichel
Copy link
Contributor Author

After switching to crane the workspace image push finally works as expected.
Once containerd/containerd#5978 is fixed, we should switch back to buildkit.

@csweichel csweichel marked this pull request as ready for review September 28, 2021 07:11
@csweichel csweichel force-pushed the csweichel/image-builder-mk-does-5597 branch from 6a71487 to 48e5919 Compare September 28, 2021 07:15
@csweichel csweichel requested review from aledbf and removed request for geropl September 28, 2021 07:15
@aledbf
Copy link
Member

aledbf commented Sep 28, 2021

/werft run

👍 started the job as gitpod-build-csweichel-image-builder-mk-does-5597.24

@aledbf
Copy link
Member

aledbf commented Sep 28, 2021

/approve
/lgtm

@roboquat roboquat added the lgtm label Sep 28, 2021
@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: d860ebe7424793dd471a476320c056cf211d66ac

@aledbf
Copy link
Member

aledbf commented Sep 28, 2021

/assign @corneliusludmann

@corneliusludmann
Copy link
Contributor

/approve

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, corneliusludmann

Associated issue: #5597

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

@roboquat roboquat merged commit 20ee598 into main Sep 29, 2021
@roboquat roboquat deleted the csweichel/image-builder-mk-does-5597 branch September 29, 2021 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants