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 hv:kubevirt platform:generic arch:amd64 to builds. #4364

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

andrewd-zededa
Copy link
Contributor

@andrewd-zededa andrewd-zededa commented Oct 16, 2024

Multus, cdi, longhorn support arm64 but k8s kubevirt does not have release builds yet.

Just start building for now to help minimize regressions, publishing will start later when the bulk of PRs get merged.

@OhmSpectator
Copy link
Member

@andrewd-zededa, argh, our workflows skip the build step if the changes touch workflow only. You can try introducing a minor change in some code to trigger the build. Will it work, @yash-zededa?

@andrewd-zededa
Copy link
Contributor Author

ah ok, @OhmSpectator I appreciate the help.

@andrewd-zededa andrewd-zededa force-pushed the hvkubevirt-pr-builds branch 6 times, most recently from a9130e4 to 59e2add Compare October 16, 2024 20:46
@andrewd-zededa
Copy link
Contributor Author

After a few experiments I switched to this commit which removed all other hv types in the action matrix except kubevirt and it still runs the kvm and Xen builds. The actions must run the copy in master I think, unless i've missed something.

I'll switch over to a commit which I think should work, rename the PR and then we can review.

@andrewd-zededa
Copy link
Contributor Author

andrewd-zededa commented Oct 16, 2024

@OhmSpectator I switched back to what I think would add a kubevirt amd64 generic build (if merged).

Multus, cdi, longhorn support arm64 but k8s kubevirt does not have
release builds yet.

Signed-off-by: Andrew Durbin <[email protected]>
@eriknordmark
Copy link
Contributor

If you want to publish the images for releases you also need to update .github/workflows/publish.yml
There are others such as buildondemand.yml and assets.yml

@uncleDecart
Copy link
Member

You can try introducing a minor change in some code to trigger the build. Will it work, @yash-zededa?

Yes, this will work @OhmSpectator

If you want to publish the images for releases you also need to update .github/workflows/publish.yml
There are others such as buildondemand.yml and assets.yml

If you're not publishing you're just checking that specific configuration is built

@yash-zededa
Copy link
Collaborator

You can try introducing a minor change in some code to trigger the build. Will it work, @yash-zededa?

Yes, this will work @OhmSpectator

If you want to publish the images for releases you also need to update .github/workflows/publish.yml
There are others such as buildondemand.yml and assets.yml

If you're not publishing you're just checking that specific configuration is built

I don't think it will work, but we can give it a try.

The publish.yml file is configured to trigger only for branches, not pull requests. GitHub Actions would require the changes to already be in the branch before it can apply the new configuration.

@andrewd-zededa
Copy link
Contributor Author

If you want to publish the images for releases you also need to update .github/workflows/publish.yml There are others such as buildondemand.yml and assets.yml

@eriknordmark I wasn't sure if we were ready to start publishing the builds, many PRs coming and could introduce odd upgrade issues.

@OhmSpectator
Copy link
Member

So, are we okay with merging the current version to test the build without publishing the results currently?

@andrewd-zededa andrewd-zededa changed the title (TESTING) Add hv:kubevirt platform:generic arch:amd64 to builds. Add hv:kubevirt platform:generic arch:amd64 to builds. Oct 17, 2024
@andrewd-zededa
Copy link
Contributor Author

@OhmSpectator @uncleDecart @eriknordmark I cleaned up the PR title and description to ready for merging.

@OhmSpectator
Copy link
Member

@OhmSpectator @uncleDecart @eriknordmark I cleaned up the PR title and description to ready for merging.

ok, let's test it!

@OhmSpectator OhmSpectator merged commit 47936cf into lf-edge:master Oct 17, 2024
11 checks passed
@andrewd-zededa
Copy link
Contributor Author

Thanks @OhmSpectator I see kubevirt in the current list of PR build for #4304, looks good I think.

@milan-zededa
Copy link
Contributor

Thanks @OhmSpectator I see kubevirt in the current list of PR build for #4304, looks good I think.

One thing though is that pillar for HV=kubevirt (pkg/pillar-kube) is not built inside packages workflow, but rather it is done in the eve workflow right now:
https://github.com/lf-edge/eve/actions/runs/11390610907/job/31692818110?pr=4304#step:10:1070

This is OK for now, but if we add kubevirt build for arm64, the same pillar package will be built twice.

@deitch
Copy link
Contributor

deitch commented Oct 18, 2024

Wasn't that eventually supposed to merge into just one pkg/pillar?

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.

7 participants