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

fix: multi-stage image build #831

Merged
merged 1 commit into from
Mar 7, 2022
Merged

fix: multi-stage image build #831

merged 1 commit into from
Mar 7, 2022

Conversation

VanillaSpoon
Copy link
Contributor

@VanillaSpoon VanillaSpoon commented Mar 4, 2022

Use a multi-stage image build for kas-fleet-manager rather than building the binary on the host

Description

Configured a multi-stage image build rather than building the binary on the host. This reduces broken images from incompatible dependencies.

Issue: https://issues.redhat.com/secure/RapidBoard.jspa?rapidView=8806&view=detail&selectedIssue=MGDSTRM-5007&quickFilter=48314

Checklist (Definition of Done)

  • All acceptance criteria specified in JIRA have been completed
  • Unit and integration tests added that prove the fix is effective or the feature works (tested against emulated and non-emulated OCM environment)
  • Documentation added for the feature
  • CI and all relevant tests are passing
  • Code Review completed
  • Verified independently by reviewer
  • Required metrics/dashboards/alerts have been added (or PR created).
  • Required Standard Operating Procedure (SOP) is added.
  • JIRA has created for changes required on the client side

Dockerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@machi1990 machi1990 left a comment

Choose a reason for hiding this comment

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

Left a suggestion about copying only the binary and nothing else.
Otherwise the image size will be bloated for things that we are not interested with.

@VanillaSpoon
Copy link
Contributor Author

Thanks @machi1990 I'll change that now

Copy link
Contributor

@machi1990 machi1990 left a comment

Choose a reason for hiding this comment

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

@VanillaSpoon thanks for the PR.

Code change looks good to me.

What steps (commands) would you advise someone take to quickly verify the change locally?

@machi1990 machi1990 requested a review from JameelB March 4, 2022 13:45
@machi1990
Copy link
Contributor

machi1990 commented Mar 4, 2022

I'd like this to have another eye, so I've asked a review from @JameelB.

@JameelB would you have some time to look at the PR? it should be quick to review and verify 🤞 Thanks

@VanillaSpoon
Copy link
Contributor Author

To test this change locally:

  1. Make sure that the kas-fleet-manager binary is not in the project root
  2. make image/build to build the docker image.
  3. docker run <image> to run the image.

(When docker run is executed, there might be errors due to missing secrets. That's normal.)

Copy link
Contributor

@JameelB JameelB 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, thanks for this @VanillaSpoon! just one question there :)

Dockerfile Show resolved Hide resolved
Copy link
Contributor

@machi1990 machi1990 left a comment

Choose a reason for hiding this comment

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

lgtm

let's wait for @JameelB thumbs up on the question before merging.

Thanks a lot for the great shift on this as well as going the extra step in verifying it. @VanillaSpoon

@machi1990 machi1990 merged commit 6fdce96 into bf2fc6cc711aee1a0c2a:main Mar 7, 2022
@machi1990 machi1990 deleted the MultiStageImageBuild branch March 7, 2022 10:04
miguelsorianod pushed a commit to miguelsorianod/ffm-fleet-manager-go-template that referenced this pull request Jan 16, 2024
Also upgrade the required go version to 1.17 to fix reported CVEs on 1.16

This applys the
bf2fc6cc711aee1a0c2a/kas-fleet-manager#873 and
bf2fc6cc711aee1a0c2a/kas-fleet-manager#831 PRs
which were done in kas-fleet-manager
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants