CFE-977: Implementation of a blob gatherer for images in the local cache #732

sherine-k · 2023-11-17T09:20:16Z

Description

This PR is part of a larger user story CFE-977 - Generate smaller archives

In this PR, we focus on identifying those blobs (both layers, config and manifests of various formats (docker-v2 or oci), both single arch and multi arch) that form a container image that is saved to oc-mirror's local cache.

The implementation relies on interacting directly with the local cache registry using containers/image/v5 api.

This PR replaces #730

For the moment, the use of BlobGatherer is not fully integrated with the MirrorToDisk workflow. Following PRs will be in charge of that.

Fixes # CFE-979

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Expected Outcome

Please describe the outcome expected from the tests

openshift-ci-robot · 2023-11-17T09:20:20Z

@sherine-k: This pull request references CFE-977 which is a valid jira issue.

In response to this:

Description

This PR is part of a larger user story CFE-977 - Generate smaller archives

In this PR, we focus on identifying those blobs (both layers, config and manifests of various formats (docker-v2 or oci), both single arch and multi arch) that form a container image that is saved to oc-mirror's local cache.

The implementation relies on interacting directly with the local cache registry using containers/image/v5 api.

This PR replaces #730

For the moment, the use of BlobGatherer is not fully integrated with the MirrorToDisk workflow. Following PRs will be in charge of that.

Fixes # CFE-979

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)

New feature (non-breaking change which adds functionality)

Breaking change (fix or feature that would cause existing functionality to not work as expected)

This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Expected Outcome

Please describe the outcome expected from the tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci · 2023-11-17T09:21:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sherine-k

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [sherine-k]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

lmzuccarelli

@sherine-k - some minor changes, other than that great work, lets take the WIP off once you have updated and then we can merge

lmzuccarelli · 2023-11-20T09:05:37Z

v2/pkg/archive/image-blob-gatherer.go

+		return blobs, err
+	}
+
+	// policyContext, err := o.opts.Global.GetPolicyContext()


Are we going to ignore signature verification ?

I'm not sure it is important to verify signatures when collecting the blobs for images that we are mirroring.
That said, I think you bring up a good point for the signatures: do signatures need to be transported in the archives? I have no idea.
WDYT?

lmzuccarelli · 2023-11-20T09:08:36Z

v2/pkg/image/image.go

+	if strings.Contains(imgRef, "://") {
+		imgSpec.ReferenceWithTransport = imgRef
+		imgSplit := strings.Split(imgRef, "://")
+		imgSpec.Transport = imgSplit[0] + "://"


Should we do a len check here - just to avoid ugly panics ?

This is fenced behind the strings.Contains. Do you think we should add another if ?

lmzuccarelli · 2023-11-20T09:09:01Z

v2/pkg/image/image.go

+	}
+	if strings.Contains(imgSpec.Name, "@") {
+		imgSplit := strings.Split(imgSpec.Name, "@")
+		if len(imgSplit) > 1 {


lmzuccarelli · 2023-11-20T09:09:40Z

v2/pkg/image/image.go

-		hash = strings.Split(imgSplit[1], ":")[1]
+	if imgSpec.Transport == "docker://" {
+		imageNameComponents := strings.Split(imgSpec.Name, "/")
+		if len(imageNameComponents) > 2 {


lmzuccarelli · 2023-11-20T09:11:35Z

v2/pkg/operator/local_stored_collector.go

-				dest = strings.Join([]string{o.Opts.Destination, pathWithoutDNS}, "/")
+				src = img.Image
+				transportAndPath := strings.Split(img.Image, "://")
+				dest = dockerProtocol + strings.Join([]string{o.Opts.Destination, transportAndPath[1]}, "/")


Are we sure this is going to to have index 1 , ie should we check len

aguidirh · 2023-11-20T11:30:35Z

Hi @sherine-k,

Everything is working fine. I compared the output of the main branch with the one from this PR and both are identical. My image set config was the one below:

kind: ImageSetConfiguration
apiVersion: mirror.openshift.io/v1alpha2
mirror:
  platform:
    channels:
    - name: stable-4.13
      minVersion: 4.13.10
      maxVersion: 4.13.10
    graph: true
  operators:
    - catalog: registry.redhat.io/redhat/redhat-operator-index:v4.12
      packages:
       - name: aws-load-balancer-operator
  additionalImages: 
   - name: registry.redhat.io/ubi8/ubi:latest
   - name: registry.redhat.io/ubi9/ubi@sha256:20f695d2a91352d4eaa25107535126727b5945bff38ed36a3e59590f495046f0

After doing mirrorToDisk and diskToMirror I got the following output in my destination registry:

[aguidi@fedora oc-mirror]$ curl http://localhost:7000/v2/_catalog | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   329  100   329    0     0  84035      0 --:--:-- --:--:-- --:--:--  107k
{
  "repositories": [
    "albo/aws-load-balancer-controller-rhel8",
    "albo/aws-load-balancer-operator-bundle",
    "albo/aws-load-balancer-rhel8-operator",
    "openshift/graph-image",
    "openshift-release-dev/ocp-release",
    "openshift-release-dev/ocp-v4.0-art-dev",
    "openshift4/ose-kube-rbac-proxy",
    "redhat/redhat-operator-index",
    "ubi8/ubi",
    "ubi9/ubi"
  ]
}

[aguidi@fedora oc-mirror]$ curl http://localhost:7000/v2/albo/aws-load-balancer-controller-rhel8/tags/list | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   120  100   120    0     0  39267      0 --:--:-- --:--:-- --:--:-- 40000
{
  "name": "albo/aws-load-balancer-controller-rhel8",
  "tags": [
    "d7bc36451217",
    "cfd210a2cb70",
    "cad8f6380b4d",
    "537fc6c0219e"
  ]
}

[aguidi@fedora oc-mirror]$ curl http://localhost:7000/v2/albo/aws-load-balancer-operator-bundle/tags/list | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   119  100   119    0     0  41564      0 --:--:-- --:--:-- --:--:-- 59500
{
  "name": "albo/aws-load-balancer-operator-bundle",
  "tags": [
    "50b9402635dd",
    "3939323bc858",
    "d4d65d0d7c24",
    "e4799ae4d44a"
  ]
}

[aguidi@fedora oc-mirror]$ curl http://localhost:7000/v2/albo/aws-load-balancer-rhel8-operator/tags/list | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   118  100   118    0     0  20622      0 --:--:-- --:--:-- --:--:-- 23600
{
  "name": "albo/aws-load-balancer-rhel8-operator",
  "tags": [
    "95c45fae0ca9",
    "ab38b37c14f7",
    "cbb31de2108b",
    "b13db5fb28d1"
  ]
}

[aguidi@fedora oc-mirror]$ curl http://localhost:7000/v2/openshift/graph-image/tags/list | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    51  100    51    0     0  13253      0 --:--:-- --:--:-- --:--:-- 17000
{
  "name": "openshift/graph-image",
  "tags": [
    "latest"
  ]
}

[aguidi@fedora oc-mirror]$ curl http://localhost:7000/v2/openshift-release-dev/ocp-release/tags/list | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    71  100    71    0     0  12491      0 --:--:-- --:--:-- --:--:-- 14200
{
  "name": "openshift-release-dev/ocp-release",
  "tags": [
    "4.13.10-x86_64"
  ]
}

[aguidi@fedora oc-mirror]$ curl http://localhost:7000/v2/openshift4/ose-kube-rbac-proxy/tags/list | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   111  100   111    0     0  53468      0 --:--:-- --:--:-- --:--:-- 55500
{
  "name": "openshift4/ose-kube-rbac-proxy",
  "tags": [
    "3658954f1990",
    "e3dad360d035",
    "422e4fbe1ed8",
    "1dddb0988d16"
  ]
}

[aguidi@fedora oc-mirror]$ curl http://localhost:7000/v2/redhat/redhat-operator-index/tags/list | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    57  100    57    0     0  25076      0 --:--:-- --:--:-- --:--:-- 28500
{
  "name": "redhat/redhat-operator-index",
  "tags": [
    "v4.12"
  ]
}

[aguidi@fedora oc-mirror]$ curl http://localhost:7000/v2/ubi8/ubi/tags/list | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    38  100    38    0     0  13581      0 --:--:-- --:--:-- --:--:-- 19000
{
  "name": "ubi8/ubi",
  "tags": [
    "latest"
  ]
}

[aguidi@fedora oc-mirror]$ curl http://localhost:7000/v2/ubi9/ubi/tags/list | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    44  100    44    0     0  15094      0 --:--:-- --:--:-- --:--:-- 22000
{
  "name": "ubi9/ubi",
  "tags": [
    "20f695d2a913"
  ]
}

openshift-ci · 2023-11-20T13:46:17Z

@sherine-k: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

aguidirh · 2023-11-20T14:06:54Z

/label acknowledge-critical-fixes-only

lmzuccarelli · 2023-11-20T14:09:35Z

/lgtm

openshift-bot · 2023-11-20T17:41:37Z

[ART PR BUILD NOTIFIER]

This PR has been included in build oc-mirror-plugin-container-v4.15.0-202311201709.p0.gd1fa811.assembly.stream for distgit oc-mirror-plugin.
All builds following this will include this PR.

sherine-k added 4 commits November 17, 2023 09:33

Fix bug setting insecure on graph image builder

86154f3

Remove HelloFromV2 function

3016132

Change graph_test.go to use SecurePolicy instead of InsecurePolicy

22981de

Create method ParseReference and type ImageSpec in package v2/image

88c3dd6

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 17, 2023

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 17, 2023

openshift-ci bot requested review from aguidirh and lmzuccarelli November 17, 2023 09:21

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 17, 2023

sherine-k force-pushed the CFE-979c branch from 4f5eb5e to bbe549b Compare November 17, 2023 15:11

lmzuccarelli reviewed Nov 20, 2023

View reviewed changes

sherine-k force-pushed the CFE-979c branch from bbe549b to 25cace3 Compare November 20, 2023 10:15

sherine-k mentioned this pull request Nov 20, 2023

DO_NOT_MERGE - CFE-977: Implementation of a blob gatherer for images in the local cache #730

Closed

4 tasks

Add BlobGatherer implementation using containers/image module

ccfbbb4

sherine-k force-pushed the CFE-979c branch from 25cace3 to ccfbbb4 Compare November 20, 2023 13:38

openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Nov 20, 2023

sherine-k changed the title ~~WIP: CFE-977: Implementation of a blob gatherer for images in the local cache~~ CFE-977: Implementation of a blob gatherer for images in the local cache Nov 20, 2023

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 20, 2023

openshift-ci bot assigned lmzuccarelli Nov 20, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2023

openshift-merge-bot bot merged commit d1fa811 into openshift:main Nov 20, 2023
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CFE-977: Implementation of a blob gatherer for images in the local cache #732

CFE-977: Implementation of a blob gatherer for images in the local cache #732

sherine-k commented Nov 17, 2023

openshift-ci-robot commented Nov 17, 2023 •

edited by openshift-ci bot

Description

Type of change

How Has This Been Tested?

Expected Outcome

openshift-ci bot commented Nov 17, 2023

lmzuccarelli left a comment

lmzuccarelli Nov 20, 2023

sherine-k Nov 20, 2023

lmzuccarelli Nov 20, 2023

sherine-k Nov 20, 2023

lmzuccarelli Nov 20, 2023

lmzuccarelli Nov 20, 2023

lmzuccarelli Nov 20, 2023

aguidirh commented Nov 20, 2023 •

edited

openshift-ci bot commented Nov 20, 2023

aguidirh commented Nov 20, 2023

lmzuccarelli commented Nov 20, 2023

openshift-bot commented Nov 20, 2023

CFE-977: Implementation of a blob gatherer for images in the local cache #732

CFE-977: Implementation of a blob gatherer for images in the local cache #732

Conversation

sherine-k commented Nov 17, 2023

Description

Type of change

How Has This Been Tested?

Expected Outcome

openshift-ci-robot commented Nov 17, 2023 • edited by openshift-ci bot

Description

Type of change

How Has This Been Tested?

Expected Outcome

openshift-ci bot commented Nov 17, 2023

lmzuccarelli left a comment

Choose a reason for hiding this comment

lmzuccarelli Nov 20, 2023

Choose a reason for hiding this comment

sherine-k Nov 20, 2023

Choose a reason for hiding this comment

lmzuccarelli Nov 20, 2023

Choose a reason for hiding this comment

sherine-k Nov 20, 2023

Choose a reason for hiding this comment

lmzuccarelli Nov 20, 2023

Choose a reason for hiding this comment

lmzuccarelli Nov 20, 2023

Choose a reason for hiding this comment

lmzuccarelli Nov 20, 2023

Choose a reason for hiding this comment

aguidirh commented Nov 20, 2023 • edited

openshift-ci bot commented Nov 20, 2023

aguidirh commented Nov 20, 2023

lmzuccarelli commented Nov 20, 2023

openshift-bot commented Nov 20, 2023

openshift-ci-robot commented Nov 17, 2023 •

edited by openshift-ci bot

aguidirh commented Nov 20, 2023 •

edited