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.

Sign up for GitHub

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

Jump to bottom

add docker image for gpctl and kubecdl #8555

Merged
merged 2 commits into from
Mar 4, 2022
Merged

add docker image for gpctl and kubecdl #8555

merged 2 commits into from
Mar 4, 2022

Conversation

sagor999
Copy link
Contributor

@sagor999 sagor999 commented Mar 2, 2022 •
edited
Loading

Description

add docker image for gpctl and kubecdl so that we don't need to build it in other werft jobs

Related Issue(s)

Fixes https://github.com/gitpod-io/ops/issues/733

How to test

curl -L https://github.com/csweichel/oci-tool/releases/download/v0.2.0/oci-tool_0.2.0_linux_amd64.tar.gz | tar xz
chmod +x oci-tool

./oci-tool fetch file eu.gcr.io/gitpod-core-dev/dev/dev-utils:commit-d66b864f634482937265b3b7bfb1ed09df731bd7 app/gpctl
chmod +x gpctl

./gpctl --help

Release Notes

Add docker images for gpctl and kubecdl

Documentation

@codecov
Copy link

codecov bot commented Mar 3, 2022 •
edited
Loading

Codecov Report

Merging #8555 (8c11792) into main (217ac59) will decrease coverage by 1.13%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8555      +/-   ##
==========================================
- Coverage   12.31%   11.17%   -1.14%     
==========================================
  Files          20       18       -2     
  Lines        1161      993     -168     
==========================================
- Hits          143      111      -32     
+ Misses       1014      880     -134     
+ Partials        4        2       -2
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)
components-ide-code-codehelper-app ?
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?

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

Impacted Files Coverage Δ
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go

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 217ac59...8c11792. Read the comment docs.

@sagor999 sagor999 force-pushed the pavel/gpctl branch 3 times, most recently from f00e3bd to 4a021a3 Compare March 3, 2022 00:52
@sagor999 sagor999 changed the title add docker image for gpctl add docker image for gpctl and kubecdl Mar 3, 2022
@sagor999 sagor999 marked this pull request as ready for review March 3, 2022 00:57
@sagor999 sagor999 requested a review from a team March 3, 2022 00:57
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label Mar 3, 2022
@princerachit
Copy link
Contributor

I like this, but on the same time I can recall some old discussion on why we don't add gpctl as build target (my memory is not serving me well). I will wait for @csweichel approval before merging this PR.

/hold

princerachit
princerachit previously approved these changes Mar 3, 2022
View reviewed changes
@csweichel
Copy link
Contributor

I reckon we could put those tools into a single Docker image (utils or dev-utils or something like that). If we made that image FROM scratch or ensured it's a single COPY we would enable downloading those tools without the need for a Docker daemon, e.g. using oci-tool.

@csweichel
Copy link
Contributor

I've just added a new command to the oci-tool (v0.2.0 release) which lets us download files from an image layer, e.g. the installer:

oci-tool fetch file eu.gcr.io/gitpod-core-dev/build/installer:main.2583 app/installer

The same would work for gpctl or kubecdl.

@roboquat roboquat added size/S and removed size/M labels Mar 3, 2022
@sagor999
Copy link
Contributor Author

sagor999 commented Mar 3, 2022

@csweichel @princerachit PTAL

@utam0k
Copy link
Contributor

utam0k commented Mar 4, 2022 •
edited by werft-gitpod-dev-com bot
Loading

@sagor999 I couldn't see the previous commit 😭 Maybe do we have to update the description of PR?

@sagor999
Copy link
Contributor Author

sagor999 commented Mar 4, 2022

@utam0k thank you, you are right. I updated how to test section.

utam0k
utam0k previously approved these changes Mar 4, 2022
View reviewed changes
Copy link
Contributor

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

The code itself worked fine.

@utam0k
Copy link
Contributor

utam0k commented Mar 4, 2022

/hold

@utam0k utam0k requested a review from csweichel March 4, 2022 00:26
csweichel
csweichel previously approved these changes Mar 4, 2022
View reviewed changes
dev/BUILD.yaml Outdated Show resolved Hide resolved
@csweichel
Copy link
Contributor

csweichel commented Mar 4, 2022 •
edited
Loading

/hold

just holding due to the nit above. Please feel free to ignore and merge - having that entry won't have any adverse side-effects.

@sagor999 sagor999 dismissed stale reviews from csweichel and utam0k via 8c11792 March 4, 2022 15:36
@sagor999 sagor999 requested a review from csweichel March 4, 2022 15:37
kylos101
kylos101 approved these changes Mar 4, 2022
View reviewed changes
Copy link
Contributor

@kylos101 kylos101 left a comment

Choose a reason for hiding this comment

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

Works well! Thanks @sagor999 and @csweichel for the oci-tool feature add. Also, good to know about scratch.

I downloaded like so ./oci-tool fetch file eu.gcr.io/gitpod-core-dev/dev/dev-utils:pavel-gpctl.10 app/gpctl.

@kylos101
Copy link
Contributor

kylos101 commented Mar 4, 2022

/unhold

Feedback from @csweichel has been addressed, hence removing hold.

@roboquat roboquat merged commit 3b07473 into main Mar 4, 2022
@roboquat roboquat deleted the pavel/gpctl branch March 4, 2022 15:59
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kylos101 kylos101 kylos101 approved these changes

@utam0k utam0k utam0k left review comments

@princerachit princerachit princerachit left review comments

@csweichel csweichel Awaiting requested review from csweichel

Assignees
No one assigned
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note size/S team: workspace Issue belongs to the Workspace team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sagor999 @princerachit @csweichel @utam0k @kylos101 @roboquat