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

Add Replicated/KOTS dependencies to image #8538

Merged
merged 1 commit into from
Mar 4, 2022
Merged

Add Replicated/KOTS dependencies to image #8538

merged 1 commit into from
Mar 4, 2022

Conversation

mrsimonemms
Copy link
Contributor

Description

Add Replicated and KOTS dependencies to the main image. This allows us to do the development required in /install/kots

How to test

Release Notes

NONE

Documentation

@mrsimonemms
Copy link
Contributor Author

/hold

@codecov
Copy link

codecov bot commented Mar 2, 2022

Codecov Report

Merging #8538 (f4d55df) into main (baee93a) will decrease coverage by 1.13%.
The diff coverage is n/a.

❗ Current head f4d55df differs from pull request most recent head 1ce88bd. Consider uploading reports for the commit 1ce88bd to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8538      +/-   ##
==========================================
- 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-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 baee93a...1ce88bd. Read the comment docs.

@roboquat roboquat added size/S and removed size/XS labels Mar 2, 2022
@mrsimonemms mrsimonemms marked this pull request as ready for review March 2, 2022 14:32
@mrsimonemms mrsimonemms requested a review from a team March 2, 2022 14:32
Comment on lines 235 to 236
RUN brew install replicatedhq/replicated/cli kubectl helm && \
curl https://kots.io/install | bash
Copy link
Contributor

Choose a reason for hiding this comment

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

My 2 cents on this:

  • kubectl is already part of the image, we don't need to install it here again.
  • I would prefer to avoid using brew and use the Linux install step instead:
    $ curl https://raw.githubusercontent.com/replicatedhq/replicated/master/install.sh | bash
    
  • Both, the Replicated CLI install bash script and the KOTS install bash script simply download the latest release from the GitHub release page. We could also download the binaries directly (and check the sha hash etc.). However, then we need to take care that we update the version regularly.
  • We used to have helm in the image. You could use the commit 88ff2c2 as a reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sorry my bad. I copy/pasta from the old PR and forgot to remove the relevant stuff. Will update

Copy link
Member

Choose a reason for hiding this comment

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

I support your points, Cornelius.

then we need to take care that we update the version regularly.

+1 for version locking. Upgrading the versions from time to time manually is usually less painful than stuff breaking when you have the least time for and waste hours until you realize that an automated version update happened.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made the fixes and added version locks.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2022

⚠️ Hey reviewer! BE CAREFUL ⚠️
Review the code before opening in your Gitpod. .gitpod.yml was changed and it might be harmful.

@mrsimonemms mrsimonemms marked this pull request as ready for review March 2, 2022 15:51
@mrsimonemms mrsimonemms requested a review from meysholdt March 2, 2022 15:51
Copy link
Contributor

@corneliusludmann corneliusludmann left a comment

Choose a reason for hiding this comment

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

looks good to me

@mrsimonemms can the hold label be removed?

@mrsimonemms
Copy link
Contributor Author

/unhold

@roboquat roboquat merged commit c6ab2d3 into main Mar 4, 2022
@roboquat roboquat deleted the sje/add-kots branch March 4, 2022 10:17
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.

5 participants