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

Switch to using OCI Artifacts via build-trusted-artifacts rather than Jib #1824

Merged
merged 14 commits into from
Jun 26, 2024

Conversation

rnc
Copy link
Collaborator

@rnc rnc commented Jun 13, 2024

As https://github.com/GoogleContainerTools/jib doesn't preserve symbolic links this switches to https://github.com/konflux-ci/build-trusted-artifacts/ which is based upon https://oras.land/

Requires #1842 merged first.

Copy link
Contributor

openshift-ci bot commented Jun 13, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link

codecov bot commented Jun 13, 2024

Codecov Report

Attention: Patch coverage is 77.98165% with 48 lines in your changes missing coverage. Please review.

Project coverage is 39.84%. Comparing base (c81fb44) to head (5974af2).
Report is 4 commits behind head on main.

Files Patch % Lines
pkg/reconciler/dependencybuild/buildrecipeyaml.go 84.61% 20 Missing and 8 partials ⚠️
...s/container/deploy/DeployPreBuildImageCommand.java 0.00% 10 Missing ⚠️
...edhat/hacbs/container/deploy/TagDeployCommand.java 36.36% 6 Missing and 1 partial ⚠️
...hat/hacbs/container/deploy/BuildVerifyCommand.java 0.00% 2 Missing ⚠️
...cbs/common/images/ociclient/OCIRegistryClient.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1824      +/-   ##
============================================
- Coverage     40.37%   39.84%   -0.54%     
+ Complexity      813      786      -27     
============================================
  Files           308      308              
  Lines         14225    14237      +12     
  Branches       1451     1445       -6     
============================================
- Hits           5744     5673      -71     
- Misses         7817     7918     +101     
+ Partials        664      646      -18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rnc rnc force-pushed the OCI branch 2 times, most recently from 8b9dd39 to a358181 Compare June 14, 2024 14:38
@rnc rnc requested a review from stuartwdouglas June 14, 2024 16:02
@rnc rnc changed the title Initial attempt to use OCI Artifacts Switch to using OCI Artifacts via build-trusted-artifacts rather than Jib Jun 14, 2024
@rnc rnc force-pushed the OCI branch 2 times, most recently from 43f3c94 to 6c44b93 Compare June 18, 2024 06:20
@@ -0,0 +1,3 @@
#This file is used to enable renovate to update the digests. It is updated by renovate then substituted into the golang code.

FROM quay.io/redhat-user-workloads/rhtap-build-tenant/trusted-artifacts/trusted-artifacts@sha256:9e2ffee0cb28f8a0ed7895a357c73f006005b26ef143f00df067f090282e8cbd
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should really be provided by config, rather than being baked into the binary. This is fine for now, but we should look at this later, as we will need a general solution for Konflux tasks we are referencing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure. I did consider adding to e.g. system-config.yaml. I'm not actually renovate will catch this (but equally I know we have to update the builder images manually which is not ideal). I don't know how Konflux keeps the references up to date.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can configure renovate to handle this. Konflux runs renovate manually to update references, its run by the build service as a scheduled task.

@rnc rnc force-pushed the OCI branch 5 times, most recently from 05c383f to fe857dd Compare June 19, 2024 16:01
@stuartwdouglas
Copy link
Collaborator

The minikube tests are failing with:

Creating pre-build-image archive
jq: error: Could not open file /root/.docker/config.json: No such file or directory
jq: error (at <stdin>:1): null (null) and null (null) cannot be multiplied

@rnc rnc force-pushed the OCI branch 7 times, most recently from 32ffbb9 to 86ba642 Compare June 21, 2024 10:00
@rnc rnc marked this pull request as ready for review June 21, 2024 10:00
import io.quarkus.logging.Log;
import picocli.CommandLine;

@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
@CommandLine.Command(name = "deploy")
public class DeployCommand implements Runnable {

Copy link
Collaborator Author

@rnc rnc Jun 21, 2024

Choose a reason for hiding this comment

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

I renamed MavenDeployCommand to TagDeployCommand and the original DeployCommand to BuildVerifyCommand.

@@ -338,22 +356,26 @@ func createPipelineSpec(log logr.Logger, tool string, commitTime int64, jbsConfi
{Name: PipelineResultImageDigest},
{Name: PipelineResultPassedVerification},
{Name: PipelineResultVerificationResult},
{Name: PipelineResultGitArchive},
// TODO: ### DeployPreBuildSource and Deploy push to git. Currently the former is used for GitArchive results.
// {Name: PipelineResultGitArchive},
Copy link
Collaborator Author

@rnc rnc Jun 21, 2024

Choose a reason for hiding this comment

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

Previously PipelineResultGitArchive was storing the result of Deploy's git push while DeployPreBuildSource git push was 'lost'. Currently PipelineResultGitArchive is now storing the result of DeployPreBuildSource. I haven't (yet) added another pipeline result for the Deploy pipeline to also store the result of its git archiving - do we want to add that (or perhaps in a subsequent PR) ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can come later. It is not 100% clear if we will be pushing to git at all, or if that will come from repour/PNC.

@rnc rnc force-pushed the OCI branch 2 times, most recently from 5fb7ec4 to 2ba5dab Compare June 21, 2024 15:15
@rnc
Copy link
Collaborator Author

rnc commented Jun 21, 2024

Current status - there is an issue with rebuilding contaminants ; @stuartwdouglas I suspect moving the Deploy around may have affected it. Perhaps we can sync in order check how its meant to work?

@rnc rnc force-pushed the OCI branch 3 times, most recently from f0aa740 to 6027568 Compare June 24, 2024 16:02
// HACK : OCIRepositoryClient assumes that the artifacts are in a directory and it then places them
// within 'artifacts/...'. This is due to build-trusted-artifact changes as its storage stores
// the contents of a directory not including the directory itself.
return List.of(layer1Path, layer2Path, layer3Path.resolve("com"));
Copy link
Collaborator Author

@rnc rnc Jun 24, 2024

Choose a reason for hiding this comment

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

Not overly fond of this, but the current test doesn't match how build-trusted-artifacts/oras stores (it stores the contents of a directory versus the directory itself) : https://github.com/konflux-ci/build-trusted-artifacts/blob/main/create-oci.sh#L84

// using AUTHFILE to override. Setting ORAS_OPTIONS to ensure the archive is compatible with jib (for OCIRepositoryClient).
preBuildImageArgs := fmt.Sprintf(`echo "Creating pre-build-image archive"
echo $REGISTRY_TOKEN > ~/config.json
export ORAS_OPTIONS="%s --image-spec=v1.0 --artifact-type application/vnd.oci.image.config.v1+json"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ensures that the OCIRegistry can read the archives.

@rnc rnc force-pushed the OCI branch 3 times, most recently from e27be9a to 5720c25 Compare June 25, 2024 09:35
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

2 participants