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

fix: adds a docker buildx step before release-snapshot #741

Merged
merged 4 commits into from
Jun 21, 2024
Merged

fix: adds a docker buildx step before release-snapshot #741

merged 4 commits into from
Jun 21, 2024

Conversation

faiq
Copy link
Contributor

@faiq faiq commented Jun 21, 2024 •
edited by dlipovetsky
Loading

What problem does this PR solve?:

This should fix the local development flow for make release-snapshot.
Tested on amd64 and arm64.

We also remove unnecessary use of third-party github actions for docker

We now have a make target that creates a dockerx builder container
with multiarch support. Both the release and release-snapshot make
targets depend on this make target.

The two github actions are no longer necessary, and are removed.

Which issue(s) this PR fixes:
Fixes #

How Has This Been Tested?:

Special notes for your reviewer:

@github-actions github-actions bot added the fix label Jun 21, 2024
@dlipovetsky dlipovetsky requested a review from supershal June 21, 2024 17:59
@dlipovetsky
Copy link
Contributor

Because it also builds multi-arch images, I think we want the release target to also depend on docker-buildx.

@faiq @dlipovetsky
fix: adds comment explaining target
3e500b3 
Co-authored-by: Daniel Lipovetsky <[email protected]>
supershal
supershal approved these changes Jun 21, 2024
View reviewed changes
Copy link
Contributor

@supershal supershal left a comment

Choose a reason for hiding this comment

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

I tested on mac + colima(x86) and mac + colima(arm64) successfully.

@dlipovetsky dlipovetsky enabled auto-merge (squash) June 21, 2024 18:25
@dlipovetsky dlipovetsky disabled auto-merge June 21, 2024 18:32
@dlipovetsky dlipovetsky enabled auto-merge (squash) June 21, 2024 18:36
@jimmidyson
Copy link
Member

I think an alternative way to fix this without having to create a new buildx builder is to change the Dockerfile to be multistage, the first stage to run the mindthegap create bundle with the native build host arch, the second stage just to copy the created bundle to the arch specific mindthegap image. This wouldn't require running the mindthegap binary with a different arch to the host.

jimmidyson
jimmidyson reviewed Jun 21, 2024
View reviewed changes
make/goreleaser.mk Outdated Show resolved Hide resolved
@dlipovetsky dlipovetsky mentioned this pull request Jun 21, 2024
Closed
@faiq faiq force-pushed the faiq/docker-buildx branch from 112d7f6 to 9ed761e Compare June 21, 2024 20:30
@github-actions github-actions bot added fix and removed fix labels Jun 21, 2024
@faiq faiq force-pushed the faiq/docker-buildx branch from 9ed761e to 07f8ea7 Compare June 21, 2024 20:32
@faiq @dlipovetsky
fix: specify builder
2d77b27 
Co-authored-by: dlipovetsky <[email protected]>
@faiq faiq force-pushed the faiq/docker-buildx branch from 07f8ea7 to 2d77b27 Compare June 21, 2024 20:34
@dlipovetsky dlipovetsky disabled auto-merge June 21, 2024 20:39
@dlipovetsky dlipovetsky enabled auto-merge (squash) June 21, 2024 20:41
@dlipovetsky dlipovetsky merged commit 4078346 into main Jun 21, 2024
17 checks passed
@dlipovetsky dlipovetsky deleted the faiq/docker-buildx branch June 21, 2024 20:57
@github-actions github-actions bot mentioned this pull request Jun 21, 2024
Merged
faiq pushed a commit that referenced this pull request Jun 24, 2024
@jimmidyson @github-actions
release(main): v0.10.0 (#685)
2d2a3f7 
🤖 I have created a release *beep* *boop*
---


## 0.10.0 (2024-06-24)

<!-- Release notes generated using configuration in .github/release.yaml
at main -->

## What's Changed
### Exciting New Features 🎉
* feat: Upgrade to Cilium v1.15.5 by @jimmidyson in
#689
* feat: Upgrade to Calico v3.28.0 by @jimmidyson in
#688
* feat: bumps caaph to v0.2.3 by @faiq in
#691
* feat: Add local-path-provisioner CSI by @jimmidyson in
#693
* feat: cluster-api v1.7.3 by @jimmidyson in
#714
* feat: bumps caaph to 0.2.4 by @faiq in
#718
* feat: Controller that copies ClusterClasses to namespaces by
@dlipovetsky in
#715
* feat: adds a mindthegap container and deployment by @faiq in
#637
* feat: implements BeforeClusterUpgrade hook by @faiq in
#682
### Fixes 🔧
* fix: use external Nutanix API types directly by @dkoshkin in
#698
* fix: Post-process clusterconfig CRDs for supported CSI providers by
@jimmidyson in
#695
* fix: nutanix credentials Secrets owner refs by @dkoshkin in
#711
* fix: credential provider response secret ownership by @dkoshkin in
#709
* fix: static credentials Secret generation by @dkoshkin in
#717
* fix: set ownerReference on imageRegistry and globalMirror Secrets by
@dkoshkin in
#720
* fix: Allow Nutanix CSI snapshot controller & webhook to run on CP
nodes by @dlipovetsky in
#723
* refactor: Use maps for CSI providers and storage classes by
@jimmidyson in
#696
* fix: CredentialProviderConfig matchImages to support registries with
port by @dkoshkin in
#724
* fix: Allow Node Feature Discovery garbage collector to run on
control-plane nodes by @dlipovetsky in
#722
* fix: RBAC role for namespace-sync controller to watch,list namespaces
by @dkoshkin in
#738
* fix: image registries not handling CA certificates by @dkoshkin in
#729
* fix: adds a docker buildx step before release-snapshot by @faiq in
#741
### Other Changes
* docs: Add released version to helm and clusterctl install by
@jimmidyson in
#683
* revert: Temporary lint config fix until next golangci-lint release
(#629) by @jimmidyson in
#686
* refactor: Delete unused code by @jimmidyson in
#687
* refactor: Reduce log verbosity for skipped handlers by @jimmidyson in
#692
* build: update Go to 1.22.4 by @dkoshkin in
#700
* build(deps): Upgrade CAPX version to v1.4.0 by @thunderboltsid in
#707
* build: Move CSI supported provider logic to script by @jimmidyson in
#703
* build: Add testifylint linter by @jimmidyson in
#706
* build: Update all tools by @jimmidyson in
#704
* refactor: rename credential provider response secret by @dkoshkin in
#710
* refactor: Simplify code by using slices.Clone by @jimmidyson in
#712
* refactor: consistently use the same SetOwnerReference function by
@dkoshkin in
#713
* refactor: kube-vip commands by @dkoshkin in
#699
* build: Fix an incorrect make variable passed to goreleaser by
@dlipovetsky in
#716
* build: Add 'chart-docs' make target by @dlipovetsky in
#727
* build: Make CAREN mindthegap reg multiarch by @jimmidyson in
#730
* Add helm values schema plugin by @dlipovetsky in
#728
* test(e2e): Use mesosphere fork with CRSBinding fix by @jimmidyson in
#736

## New Contributors
* @thunderboltsid made their first contribution in
#707

**Full Changelog**:
v0.9.0...v0.10.0

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimmidyson jimmidyson jimmidyson left review comments

@supershal supershal supershal approved these changes

@dlipovetsky dlipovetsky dlipovetsky approved these changes

Assignees
No one assigned
Labels
fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@faiq @dlipovetsky @jimmidyson @supershal