-
Notifications
You must be signed in to change notification settings - Fork 31
Use standard buildah-oci-ta #2156
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
Conversation
| KonfluxGitDefinition = "https://raw.githubusercontent.com/konflux-ci/build-definitions/refs/heads/main/task/git-clone/0.1/git-clone.yaml" | ||
| KonfluxPreBuildDefinitions = "https://raw.githubusercontent.com/rnc/jvm-build-service/main/deploy/tasks/pre-build.yaml" | ||
| KonfluxBuildDefinitions = "https://raw.githubusercontent.com/konflux-ci/build-definitions/refs/heads/main/task/buildah-oci-ta/0.2/buildah-oci-ta.yaml" | ||
| KonfluxMavenDeployDefinitions = "https://raw.githubusercontent.com/rnc/jvm-build-service/main/deploy/tasks/maven-deployment.yaml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tecarter94 I realise this conflicts somewhat with your #2153. The problem is Konflux are constantly committing to the buildah-oci-ta task and it has diverged since I forked it. This PR removes the final modifications I made to the fork isolating them into the JBS code which then means we can use the latest version.
I think what is best after this, is to refork / update the buildah task again (and update the above KonfluxBuildDefinitions) so that we're running with a recent copy so you can make modifications again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cheers. It might be a while till I proceed with #2153, so I'll revisit this at that point.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2156 +/- ##
============================================
- Coverage 42.15% 41.97% -0.18%
Complexity 793 793
============================================
Files 286 286
Lines 13134 13162 +28
Branches 1388 1388
============================================
- Hits 5536 5525 -11
- Misses 6970 7006 +36
- Partials 628 631 +3 ☔ View full report in Codecov by Sentry. |
deploy/tasks/buildah-oci-ta.yaml
Outdated
| BUILDAH_ARGS+=("--build-arg=CACHE_URL=$CACHE_URL") | ||
| fi | ||
| unshare -Uf $UNSHARE_ARGS --keep-caps -r --map-users 1,1,65536 --map-groups 1,1,65536 -w ${SOURCE_CODE_DIR}/$CONTEXT -- buildah build \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I've removed my original changes, this file is now out of date and either needs deleting or updating again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've now removed it in this PR to make things clearer.
| }, | ||
| }, | ||
| } | ||
| // TODO: ### Enclose this within an annotation to denote test CI system in use? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really we only need to limit the task resources on CI due to the fact that minikube and the e2e can't allocate the CPU/memory that the buildah-oci-ta task specifies by default. I think I could create another annotation to represent the CI tests being used in order to wrap this block...
( I guess I could do it in this PR ... ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Latest commit has wrapped them in a new annotation.
| jobs: | ||
| wait-for-images: | ||
| runs-on: ubuntu-latest | ||
| runs-on: ubuntu-22.04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solves the problems with minikube / oc due to https://www.github.com/actions/runner-images/issues/10636
|
Discussed with @matejonnet offline - merging this to move us forward. |
No description provided.