CLID-265,CLID-259: generates folders based on the filtered catalog digest #945

aguidirh · 2024-11-05T19:11:06Z

Description

This PR generates the folders based on the digest of the operator catalog filtered. Using the digest of the filtered catalog allows oc-mirror to identify if the operator catalog was already rebuilt previously, avoiding in this way rebuilding again unnecessarily.

With this approach will be also possible to filter a catalog based on the original catalog saved on the local cache.

The operator collector currently is a little complex and the refactorings to make it easier to understand and maintain will be handled in a separated PR.

Fixes CLID-265

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?

With the following ImageSetConfiguration:

kind: ImageSetConfiguration
apiVersion: mirror.openshift.io/v2alpha1
mirror:
  operators:
    - catalog: registry.redhat.io/redhat/redhat-operator-index:v4.15 
      packages:
       - name: aws-load-balancer-operator
       - name: 3scale-operator
       - name: node-observability-operator

I ran mirrorToDisk:

./bin/oc-mirror -c /home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/alex-isc/isc.yaml file:///home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/clid-265 --alpha-ctlg-filter=true --v2

And diskToMirror

./bin/oc-mirror -c /home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/alex-isc/isc.yaml --from file:///home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/clid-265 docker://localhost:6000 --alpha-ctlg-filter=true --v2 --dest-tls-verify=false

And MirrorToMirror

./bin/oc-mirror -c /home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/alex-isc/isc.yaml --workspace file:///home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/clid-265 docker://localhost:6000 --alpha-ctlg-filter=true --v2 --dest-tls-verify=false

Expected Outcome

All the flows above should pass successfully.

NOTE: Delete, targetTag and targetCatalog were not tested yet.

openshift-ci-robot · 2024-11-05T19:11:10Z

@aguidirh: This pull request references CLID-265 which is a valid jira issue.

In response to this:

Description

This PR generates the folders based on the digest of the operator catalog filtered. Using the digest of the filtered catalog allows oc-mirror to identify if the operator catalog was already rebuilt previously, avoiding in this way rebuilding again unnecessarily.

With this approach will be also possible to filter a catalog based on the original catalog saved on the local cache.

The operator collector currently is a little complex and the refactorings to make it easier to understand and maintain will be handled in a separated PR.

Fixes CLID-265

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?

With the following ImageSetConfiguration:
kind: ImageSetConfiguration
apiVersion: mirror.openshift.io/v2alpha1
mirror:
 operators:
   - catalog: registry.redhat.io/redhat/redhat-operator-index:v4.15 
     packages:
      - name: aws-load-balancer-operator
      - name: 3scale-operator
      - name: node-observability-operator
I ran mirrorToDisk:
./bin/oc-mirror -c /home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/alex-isc/isc.yaml file:///home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/clid-265 --alpha-ctlg-filter=true --v2
And diskToMirror
./bin/oc-mirror -c /home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/alex-isc/isc.yaml --from file:///home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/clid-265 docker://localhost:6000 --alpha-ctlg-filter=true --v2 --dest-tls-verify=false
And MirrorToMirror
./bin/oc-mirror -c /home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/alex-isc/isc.yaml --workspace file:///home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/clid-265 docker://localhost:6000 --alpha-ctlg-filter=true --v2 --dest-tls-verify=false
Expected Outcome

All the flows above should pass successfully.

NOTE: Delete, targetTag and targetCatalog were not tested yet.

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2024-11-05T19:11:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aguidirh

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~~ [aguidirh]

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

openshift-ci-robot · 2024-11-05T19:34:51Z

@aguidirh: This pull request references CLID-265 which is a valid jira issue.

This pull request references CLID-259 which is a valid jira issue.

In response to this:

Description

This PR generates the folders based on the digest of the operator catalog filtered. Using the digest of the filtered catalog allows oc-mirror to identify if the operator catalog was already rebuilt previously, avoiding in this way rebuilding again unnecessarily.

With this approach will be also possible to filter a catalog based on the original catalog saved on the local cache.

The operator collector currently is a little complex and the refactorings to make it easier to understand and maintain will be handled in a separated PR.

Fixes CLID-265

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?

With the following ImageSetConfiguration:
kind: ImageSetConfiguration
apiVersion: mirror.openshift.io/v2alpha1
mirror:
 operators:
   - catalog: registry.redhat.io/redhat/redhat-operator-index:v4.15 
     packages:
      - name: aws-load-balancer-operator
      - name: 3scale-operator
      - name: node-observability-operator
I ran mirrorToDisk:
./bin/oc-mirror -c /home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/alex-isc/isc.yaml file:///home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/clid-265 --alpha-ctlg-filter=true --v2
And diskToMirror
./bin/oc-mirror -c /home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/alex-isc/isc.yaml --from file:///home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/clid-265 docker://localhost:6000 --alpha-ctlg-filter=true --v2 --dest-tls-verify=false
And MirrorToMirror
./bin/oc-mirror -c /home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/alex-isc/isc.yaml --workspace file:///home/aguidi/go/src/github.com/aguidirh/oc-mirror/alex-tests/clid-265 docker://localhost:6000 --alpha-ctlg-filter=true --v2 --dest-tls-verify=false
Expected Outcome

All the flows above should pass successfully.

NOTE: Delete, targetTag and targetCatalog were not tested yet.

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2024-11-05T19:35:53Z

@aguidirh: 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-sigs/prow repository. I understand the commands that are listed here.

sherine-k

Hi @aguidirh
A few comments and nits...
As you say in the description, the complexity is becoming high, but we can solve this gradually.
I think that the rebuild is happening twice, because prepareM2D is saving both to the cache and to the destination registry.
we need to discuss what to do in order not to rebuild twice, and not to push multiple times...
right now, I still cant test catalog rebuild properly, because all rebuilds hit the error: OCPBUGS-44281:
"[linux/ppc64le]: creating build container: copying system image from manifest list: Source image rejected: A signature was required, but no signature exists"

sherine-k · 2024-11-06T13:58:05Z

v2/internal/pkg/imagebuilder/rebuild_catalog.go

-			localDest = dockerProtocol + strings.Join([]string{o.LocalFQDN, imgSpec.PathComponent}, "/") + ":" + imgSpec.Tag
-			remoteDest = strings.Join([]string{o.Destination, imgSpec.PathComponent}, "/") + ":" + imgSpec.Tag
-		}
+		srcCache := dockerProtocol + strings.Join([]string{o.LocalFQDN, imgSpec.PathComponent}, "/") + ":" + filepath.Base(filteredDir)


Hi Alex,
I thought about this too, and I solved this a little differently in #947
Actually, if you take the copyImageSchema that comes from the items in catalogSchema.AllImages, the src ref, dest ref and origin ref are already calculated by the collector, and can simply be reused.

sherine-k · 2024-11-06T14:00:37Z

v2/internal/pkg/imagebuilder/rebuild_catalog.go

-			localDest = dockerProtocol + strings.Join([]string{o.LocalFQDN, imgSpec.PathComponent}, "/") + ":" + imgSpec.Tag
-			remoteDest = strings.Join([]string{o.Destination, imgSpec.PathComponent}, "/") + ":" + imgSpec.Tag
-		}
+		srcCache := dockerProtocol + strings.Join([]string{o.LocalFQDN, imgSpec.PathComponent}, "/") + ":" + filepath.Base(filteredDir)


Suggested change

srcCache := dockerProtocol + strings.Join([]string{o.LocalFQDN, imgSpec.PathComponent}, "/") + ":" + filepath.Base(filteredDir)

updatedDest := strings.Join([]string{o.LocalFQDN, imgSpec.PathComponent}, "/") + ":" + filepath.Base(filteredDir)

srcCache := dockerProtocol + updatedDest

sherine-k · 2024-11-06T14:04:19Z

v2/internal/pkg/imagebuilder/rebuild_catalog.go

+		digestOnly := digest.String()
+		if strings.Contains(digestOnly, ":") {
+			digestOnly = strings.Split(digest.String(), ":")[1]
+		}


Suggested change

digestOnly := digest.String()

if strings.Contains(digestOnly, ":") {

digestOnly = strings.Split(digest.String(), ":")[1]

}

digestOnly := digest.Encoded()

clid-265: generates folders based on the filtered catalog digest

3eed6e6

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

openshift-ci bot requested review from lmzuccarelli and sherine-k November 5, 2024 19:11

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

aguidirh changed the title ~~CLID-265: generates folders based on the filtered catalog digest~~ CLID-265,CLID-259: generates folders based on the filtered catalog digest Nov 5, 2024

sherine-k mentioned this pull request Nov 6, 2024

CLID-262:WIP: Create a separate interface for rebuild catalogs #947

Open

4 tasks

sherine-k reviewed Nov 6, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLID-265,CLID-259: generates folders based on the filtered catalog digest #945

CLID-265,CLID-259: generates folders based on the filtered catalog digest #945

aguidirh commented Nov 5, 2024

openshift-ci-robot commented Nov 5, 2024 •

edited by openshift-ci bot

Loading

Description

Type of change

How Has This Been Tested?

Expected Outcome

openshift-ci bot commented Nov 5, 2024

openshift-ci-robot commented Nov 5, 2024 •

edited by openshift-ci bot

Loading

Description

Type of change

How Has This Been Tested?

Expected Outcome

openshift-ci bot commented Nov 5, 2024

sherine-k left a comment •

edited

Loading

sherine-k Nov 6, 2024

sherine-k Nov 6, 2024

sherine-k Nov 6, 2024

	srcCache := dockerProtocol + strings.Join([]string{o.LocalFQDN, imgSpec.PathComponent}, "/") + ":" + filepath.Base(filteredDir)
	updatedDest := strings.Join([]string{o.LocalFQDN, imgSpec.PathComponent}, "/") + ":" + filepath.Base(filteredDir)
	srcCache := dockerProtocol + updatedDest

CLID-265,CLID-259: generates folders based on the filtered catalog digest #945

Are you sure you want to change the base?

CLID-265,CLID-259: generates folders based on the filtered catalog digest #945

Conversation

aguidirh commented Nov 5, 2024

Description

Type of change

How Has This Been Tested?

Expected Outcome

openshift-ci-robot commented Nov 5, 2024 • edited by openshift-ci bot Loading

Description

Type of change

How Has This Been Tested?

Expected Outcome

openshift-ci bot commented Nov 5, 2024

openshift-ci-robot commented Nov 5, 2024 • edited by openshift-ci bot Loading

Description

Type of change

How Has This Been Tested?

Expected Outcome

openshift-ci bot commented Nov 5, 2024

sherine-k left a comment • edited Loading

Choose a reason for hiding this comment

sherine-k Nov 6, 2024

Choose a reason for hiding this comment

sherine-k Nov 6, 2024

Choose a reason for hiding this comment

sherine-k Nov 6, 2024

Choose a reason for hiding this comment

openshift-ci-robot commented Nov 5, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Nov 5, 2024 •

edited by openshift-ci bot

Loading

sherine-k left a comment •

edited

Loading