Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Cleanup: Fix in-cluster-build sample #102

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

tcrawley-xilinx
Copy link
Contributor

I found two issues with the existing samples for in-cluster-builds:

  1. If the onload-clusterlocal didn't exist with the correct RoleBindings, then the KMM operator would fail to check whether the image existed (which prevented a build happening).
  2. Incorrect ImageStream specification (we were including an unnecessary /onload).

Both should be fixed with this patch.

The duplication of imagestream.yaml isn't ideal, but in-cluster-build-ocp and in-cluster-build-ocp-clusterlocal already have identical files (patch-onload.yaml) so I don't think it is the end of the world.

Testing done

On master: oc create -k config/samples/onload/overlays/in-cluster-build-ocp creates and Onload CR, but the KMM fails to create a build pod for the module.
With this patch it can create the build pod which runs correctly.

I found two issues with the existing samples for in-cluster-builds:
1. If the `onload-clusterlocal` didn't exist with the correct
   RoleBindings, then the KMM operator would fail to check whether the
   image existed (which prevented a build happening).
2. Incorrect ImageStream specification (we were including an unnecessary
   `/onload`).
Both should be fixed with this patch.
@tcrawley-xilinx tcrawley-xilinx requested a review from a team as a code owner December 11, 2023 16:05
@pcolledg-amd pcolledg-amd merged commit b1be743 into master Dec 14, 2023
10 checks passed
@pcolledg-amd pcolledg-amd deleted the reviews/tcrawley/cleanup-incluster branch December 14, 2023 16:20
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants