-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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-bob] Introduce URL processing for non docker api urls #10266
Conversation
424a311
to
c19db5d
Compare
d698cec
to
8811d94
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good I think, but would love to see unit tests.
Process docker api urls starting with /v2/ differently from non-docker api url whose path does not start with /v2/.
8811d94
to
1f9c1b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for _, test := range tests { | ||
t.Run(test.Name, func(t *testing.T) { | ||
rewriteDockerAPIURL(&test.in.u, test.in.fromRepo, test.in.toRepo, test.in.host, test.in.tag) | ||
if test.in.u.Path != test.u.Path { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @geropl Can we unhold here? This might be important for a self-hosted install. |
@csweichel I think so? The pin was removed here. Am I missing anything, @geropl ? Tomorrow (Wednesday), I'd like to rebase, unhold, and merge these PRs, so they can be tested in staging, in advance of Thursday's deploy. I'm just not sure how to initiate a deploy to staging, or, if that is automatic on merge to main.
We also need to revert this revert in a new PR (which I also plan to do tomorrow):
If a self-hosted install is in a pinch, rather than wait for this PR to get merged, I'd recommend creating a patch branch from the latest self-hosted release, and cherrypick #10266. I say that because it'd allow both work streams to continue independently, w/o creating dependencies on this PR being merged right away. |
💯 👍 @kylos101 @csweichel |
mark here |
/hold cancel |
Description
Introduce a new function
rewriteNonDockerAPIURL
which can be used to rewrite URLs in bob proxy when the URL endpoint is NOT a docker api endpoint. See all docker registry endpoints here, it must start with/v2/
.As a side effect of this change gitpod will support Google Artifact Registry as a container registry.
Related Issue(s)
Fixes #10241
How to test
Pre requisite
You need a self hosted version of gitpod to test out GAR and Azure. You will need to setup container registry during installation.
GAR
For GAR you will need to create a GAR in a google project e.g. playground and then create a service account which has write permission to that registry. After that create a key for the service account and use it to login locally in your workspace.
Example below. You can read more here
gitpod-config.yml
from instance to your local workspace. You need to update the container registry config in this file before rendering usingreapply.sh
. See example gitpod-config.yaml change here.kubeconfig
to point to this cluster. Better remove any other entry from your kubeconfig.reapply.sh
proxy-patch.json
trim.py
. This will help in faster iteration of gitpod installation update'./reapply.sh render
. Make Sure to update the reapply.sh file secret creation step first../reapply.sh deploy
. Wait for a minute or two for the pods to come up.I have tested with following registries:
Logs
Sample log from Image Build pod:
Release Notes
Documentation
https://github.com/gitpod-io/website/issues/2128