Skip to content

chore(image-builder): Add README, update config JSON schema #14205

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

Merged
merged 1 commit into from
Oct 28, 2022

Conversation

WVerlaek
Copy link
Member

Description

  • Adds a README for image-builder-mk3
  • Small updates to the README for image-builder-bob
  • Regenerate image-builder-mk3 config JSON schema and update example config (was still using image-builder-mk2 config)
  • Remove telepresence.sh: this script hadn't been updated to work with image-builder-mk3, and I couldn't get it working either within reasonable time (believe it requires the telepresence install to be upgraded). It looks like this script isn't being used cause it's been outdated for a while, and as there's also the debug.sh script to iterate on local changes I thought it would be best to remove it. Can always add it back if/when we need to?

Related Issue(s)

How to test

  • Follow the updated READMEs
  • For the updated schema, this was generated by running image-builder generate config

Release Notes

NONE

Documentation

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

@WVerlaek WVerlaek requested a review from a team October 26, 2022 17:33
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label Oct 26, 2022
Comment on lines 3 to 10
{ "path": "../image-builder-api" },
{ "path": "../image-builder-bob" },
{ "path": "../image-builder-mk3" },
{ "path": "../ws-manager" },
{ "path": "../server" },
{ "path": "../../test" },
{ "path": "../../dev/gpctl" }
{ "path": "../../dev/gpctl" },
{ "path": "../../install/installer" }
Copy link
Member Author

Choose a reason for hiding this comment

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

found api and installer useful to be included as well in this workspace

@WVerlaek WVerlaek self-assigned this Oct 26, 2022
@WVerlaek WVerlaek changed the title chore(image-builder): Add README, update schema, remove unused code chore(image-builder): Add README, update config JSON schema Oct 26, 2022
Comment on lines +5 to +12
Bob is a CLI responsible for building and pushing workspace images during workspace startup.

For each image build, a headless workspace gets created in the meta cluster by `image-builder-mk3` ([#7845](https://github.com/gitpod-io/gitpod/issues/7845) will move the headless workspace and image-builder to the workspace cluster), in this headless workspace runs:
- `bob proxy`, which gets started by workspacekit in ring1, and receives credentials for pushing images to a docker registry. It proxies and authenticates the image pushes from `bob build`.
- `bob build` as a workspace task, which builds the
- **base layer**, if a custom Dockerfile is specified in `.gitpod.yaml`. If this base image has already been built for the workspace, this step is skipped, and the reference of the previously built image is used instead to build the workspace image next.
- **workspace image**, which builds an image from the base layer, where the base layer is either a previously built custom Dockerfile or a public image.
These images get pushed over `localhost` to `bob proxy`, as `bob build` does not receive the credentials to push to private registries.
Copy link
Contributor

Choose a reason for hiding this comment

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

💯 It helps me to understand image-builder-bob. I wanted this README when I first developed image-builder.

What about putting a statement that this image does not include the supervisor, etc.? I think this is often misunderstood as the role of the registry-facade.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @utam0k great suggestion, updated and will also put it in the architecture docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

💯 It helps me to understand image-builder-bob. I wanted this README when I first developed image-builder.

+1

@WVerlaek WVerlaek requested a review from utam0k October 27, 2022 09:32
@utam0k
Copy link
Contributor

utam0k commented Oct 28, 2022

Thanks, @WVerlaek

@iQQBot
Copy link
Contributor

iQQBot commented Oct 28, 2022

this PR stuck our tide merge pool, could you rebase to main and squash these commits? @WVerlaek

@@ -1,11 +1,13 @@
{
"folders": [
{ "path": "../image-builder-api" },
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{ "path": "../image-builder-api" },
{ "path": "../image-builder-api/go" },

The biggest part of the purpose of writing this workspace is because of the golang code hint problem, so it would be good using golang package path

Comment on lines 4 to 11
"wsman": {
"address": "ws-manager:8080",
"tls": {
"ca": "/wsman-certs/ca.crt",
"crt": "/wsman-certs/tls.crt",
"key": "/wsman-certs/tls.key"
}
},
Copy link
Contributor

@iQQBot iQQBot Oct 28, 2022

Choose a reason for hiding this comment

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

The example configuration file does not work, it is missing files (tls) and it not able to connect with ws-manager

Copy link
Member Author

Choose a reason for hiding this comment

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

Is the intention of an example-config.json to be run locally? I took this example config from the image-builder-mk3-config ConfigMap in a cluster, but let me try to see if I can change it to work locally

Copy link
Member Author

Choose a reason for hiding this comment

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

Done - updated the README with instructions on running image-builder-mk3 locally. Also added a section on using grpcurl to call the RPC methods on the locally running API

@WVerlaek WVerlaek force-pushed the wv/image-builder-mk3-readme branch from b54935a to dbd87b2 Compare October 28, 2022 07:08
@WVerlaek
Copy link
Member Author

this PR stuck our tide merge pool, could you rebase to main and squash these commits? @WVerlaek

Done

@roboquat roboquat added size/XL and removed size/L labels Oct 28, 2022
@iQQBot
Copy link
Contributor

iQQBot commented Oct 28, 2022

this PR stuck our tide merge pool, could you rebase to main and squash these commits? @WVerlaek

Done

image

It would help if you used rebase and fixup commit 3 to commit 1

@WVerlaek
Copy link
Member Author

@iQQBot sure let me do that - thought it would be easier to review to see the separate changes made after the last review. What happens when we merge, do the commits get squashed, or will all commits in the PR end up on main?

@WVerlaek WVerlaek force-pushed the wv/image-builder-mk3-readme branch from d3c6698 to 27a462d Compare October 28, 2022 08:23
@iQQBot
Copy link
Contributor

iQQBot commented Oct 28, 2022

@iQQBot sure let me do that - thought it would be easier to review to see the separate changes made after the last review. What happens when we merge, do the commits get squashed, or will all commits in the PR end up on main?

unfortunately, we don't enable squash, so all commits in the PR end up on main, we need to squash it manual

image

Github provide compare button, so while review we can use it

@roboquat roboquat merged commit b29560b into main Oct 28, 2022
@roboquat roboquat deleted the wv/image-builder-mk3-readme branch October 28, 2022 08:27
@WVerlaek
Copy link
Member Author

@iQQBot thanks! good to know

@roboquat roboquat added the deployed: workspace Workspace team change is running in production label Nov 3, 2022
@roboquat roboquat added the deployed Change is completely running in production label Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note-none size/XL team: workspace Issue belongs to the Workspace team
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants