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

WIP: [V2] CFE-971: create and mirror a graph image for ocp releases #721

Closed
wants to merge 12 commits into from

Conversation

sherine-k
Copy link
Contributor

@sherine-k sherine-k commented Oct 25, 2023

Description

When the user specifies graph: true in the imageSetConfig (mirror.platform.graph), oc-mirror generates a graph-image (following directives from this doc and pushes it to the specified disk or mirror.

In order to have retro-compatibility in v2, and in order to reduce the module dependencies of oc-mirror, the implementation suggested in this PR is based on containers/buildah, instead of the existing implementation with go-containerregistry and crane

Fixes CFE-971

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 imageSetConfig:

kind: ImageSetConfiguration
apiVersion: mirror.openshift.io/v1alpha2
mirror:
 platform:
   channels:
   - name: stable-4.13
     minVersion: 4.13.10
     maxVersion: 4.13.10
   graph: true

We can run oc-mirror for MirrorToDisk first: This will create a graph-image and save it to local storage

$ ./bin/oc-mirror --v2 -c cfe-969.yml -p 6000 file:///home/skhoury/release
... 
2023/10/25 10:56:51  [INFO]   : mode mirrorToDisk 
2023/10/25 10:56:51  [INFO]   : local storage registry will log to /home/skhoury/go/src/github.com/openshift/oc-mirror/logs/registry.log
2023/10/25 10:56:51  [INFO]   : starting local storage on :6000
d
...
2023/10/25 10:56:52  [INFO]   : creating graph data image
2023/10/25 10:56:52  [INFO]   : graph image localhost:6000/graph-image@sha256:41f0b09707f893900679adc712e78f7f73d7d8b245bf68c921726b43743373e0 created and pushed to cache.
2023/10/25 10:56:52  [INFO]   : total release images to copy 184
...

Next, we can perform a DiskToMirror, which would push the graph-image, along with release images, to the mirror

$ ./bin/oc-mirror --v2 -c cfe-969.yml -p 6000 --from file:///home/skhoury/release docker://localhost:5000/cfe971 --dest-tls-verify=false

Expected Outcome

After mirroring is complete, we need to check the graph-image:

  • The contents of /var/lib/cincinnati-graph-data need to belong to root.
  • The CMD of the image needs to be compliant with the documentation

This can be achieved like this for example:

╭─╼skhoury@fedora ~/.../openshift/oc-mirror 
╰─ (CFE-971)-$ podman pull localhost:5000/cfe971/graph-image:latest
Trying to pull localhost:5000/cfe971/graph-image:latest...
Getting image source signatures
Copying blob 3c5f5168e8c4 done   | 
Copying blob cc7c08d56aad skipped: already exists  
Copying config fd8881a489 done   | 
Writing manifest to image destination
fd8881a489058a887b1f82311762f1289f1434b67f83463ec78f596cbb9dd966
╭─╼skhoury@fedora ~/.../openshift/oc-mirror 
╰─ (CFE-971)-$ podman run -it "localhost:5000/cfe971/graph-image:latest" /bin/bash
[root@96b2ab6ddc60 /]# ls -l /var/lib/cincinnati-graph-data/
total 16
-rw-rw-r--. 1 root root 11357 Oct 25 07:26 LICENSE
drwxr-xr-x. 1 root root 31668 Oct 25 07:48 blocked-edges
drwxr-xr-x. 1 root root  1410 Oct 25 07:48 channels
drwxr-xr-x. 1 root root    26 Oct 25 07:48 raw
-rw-rw-r--. 1 root root     6 Oct 25 07:26 version
[root@96b2ab6ddc60 /]# exit
exit
╭─╼skhoury@fedora ~/.../openshift/oc-mirror 
╰─ (CFE-971)-$ podman inspect "localhost:5000/cfe971/graph-image:latest" | jq .[0].Config.Cmd
[
  "/bin/bash",
  "-c",
  "exec cp -rp /var/lib/cincinnati-graph-data//* /var/lib/cincinnati/graph-data"
]

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 25, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 25, 2023

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

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

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 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 Oct 25, 2023
@openshift-ci
Copy link

openshift-ci bot commented Oct 25, 2023

[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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 25, 2023
Copy link
Contributor

@lmzuccarelli lmzuccarelli left a comment

Choose a reason for hiding this comment

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

Great that you got it working with buildah, well done @sherine-k

v2/pkg/release/constants.go Show resolved Hide resolved
// oc-mirror runs in rootless mode. It is therefore necessary to
// ensure that oc-mirror is re-executed in a user namespace where
// it has root privileges.
if buildah.InitReexec() {
Copy link
Contributor

Choose a reason for hiding this comment

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

@sherine-k - should we not gate this ? i.e move it to the executor.go , and only set this if we have graph: true set

go.mod Show resolved Hide resolved
@lmzuccarelli
Copy link
Contributor

I see the e2e test failing here

reading subuid mappings for user \"1003670000\" and subgid mappings for group \"1003670000\": no subuid ranges found for user \"1003670000\" in /etc/subuid"

@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 25, 2023

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

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

Description

When the user specifies graph: true in the imageSetConfig (mirror.platform.graph), oc-mirror generates a graph-image (following directives from this doc and pushes it to the specified disk or mirror.

In order to have retro-compatibility in v2, and in order to reduce the module dependencies of oc-mirror, the implementation suggested in this PR is based on containers/buildah, instead of the existing implementation with go-containerregistry and crane

Fixes CFE-971

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 imageSetConfig:

kind: ImageSetConfiguration
apiVersion: mirror.openshift.io/v1alpha2
mirror:
platform:
  channels:
  - name: stable-4.13
    minVersion: 4.13.10
    maxVersion: 4.13.10
  graph: true

We can run oc-mirror for MirrorToDisk first: This will create a graph-image and save it to local storage

$ ./bin/oc-mirror --v2 -c cfe-969.yml -p 6000 file:///home/skhoury/release
... 
2023/10/25 10:56:51  [INFO]   : mode mirrorToDisk 
2023/10/25 10:56:51  [INFO]   : local storage registry will log to /home/skhoury/go/src/github.com/openshift/oc-mirror/logs/registry.log
2023/10/25 10:56:51  [INFO]   : starting local storage on :6000
d
...
2023/10/25 10:56:52  [INFO]   : creating graph data image
2023/10/25 10:56:52  [INFO]   : graph image localhost:6000/graph-image@sha256:41f0b09707f893900679adc712e78f7f73d7d8b245bf68c921726b43743373e0 created and pushed to cache.
2023/10/25 10:56:52  [INFO]   : total release images to copy 184
...

Next, we can perform a DiskToMirror, which would push the graph-image, along with release images, to the mirror

$ ./bin/oc-mirror --v2 -c cfe-969.yml -p 6000 --from file:///home/skhoury/release docker://localhost:5000/cfe971 --dest-tls-verify=false

Expected Outcome

After mirroring is complete, we need to check the graph-image:

  • The contents of /var/lib/cincinnati-graph-data need to belong to root.
  • The CMD of the image needs to be compliant with the documentation

This can be achieved like this for example:

╭─╼skhoury@fedora ~/.../openshift/oc-mirror 
╰─ ([CFE-971](https://issues.redhat.com//browse/CFE-971))-$ podman pull localhost:5000/cfe971/graph-image:latest
Trying to pull localhost:5000/cfe971/graph-image:latest...
Getting image source signatures
Copying blob 3c5f5168e8c4 done   | 
Copying blob cc7c08d56aad skipped: already exists  
Copying config fd8881a489 done   | 
Writing manifest to image destination
fd8881a489058a887b1f82311762f1289f1434b67f83463ec78f596cbb9dd966
╭─╼skhoury@fedora ~/.../openshift/oc-mirror 
╰─ ([CFE-971](https://issues.redhat.com//browse/CFE-971))-$ podman run -it "localhost:5000/cfe971/graph-image:latest" /bin/bash
[root@96b2ab6ddc60 /]# ls -l /var/lib/cincinnati-graph-data/
total 16
-rw-rw-r--. 1 root root 11357 Oct 25 07:26 LICENSE
drwxr-xr-x. 1 root root 31668 Oct 25 07:48 blocked-edges
drwxr-xr-x. 1 root root  1410 Oct 25 07:48 channels
drwxr-xr-x. 1 root root    26 Oct 25 07:48 raw
-rw-rw-r--. 1 root root     6 Oct 25 07:26 version
[root@96b2ab6ddc60 /]# exit
exit
╭─╼skhoury@fedora ~/.../openshift/oc-mirror 
╰─ ([CFE-971](https://issues.redhat.com//browse/CFE-971))-$ podman inspect "localhost:5000/cfe971/graph-image:latest" | jq .[0].Config.Cmd
[
 "/bin/bash",
 "-c",
 "exec cp -rp /var/lib/cincinnati-graph-data//* /var/lib/cincinnati/graph-data"
]

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.

@sherine-k
Copy link
Contributor Author

I see the e2e test failing here

reading subuid mappings for user \"1003670000\" and subgid mappings for group \"1003670000\": no subuid ranges found for user \"1003670000\" in /etc/subuid"

E2E is failing because these tests would be running in a container on a k8s cluster.
Something like the following would allow it to run, but that would require changes in the ci-configs...

securityContext:
       capabilities:
         add:
           - CAP_SETGID
           - CAP_SETUID

I'll privilege first a way to use unshare only in v2, so that e2e aren't affected for the moment.

@aguidirh
Copy link
Contributor

/test images

@sherine-k sherine-k changed the title WIP: [V2] CFE-971: create and mirror a graph image for ocp releases [V2] CFE-971: create and mirror a graph image for ocp releases Oct 27, 2023
@openshift-ci 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 Oct 27, 2023
@sherine-k sherine-k changed the title [V2] CFE-971: create and mirror a graph image for ocp releases WIP: [V2] CFE-971: create and mirror a graph image for ocp releases Oct 27, 2023
@openshift-ci 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 Oct 27, 2023
CONTRIBUTING.md Outdated Show resolved Hide resolved
@sherine-k
Copy link
Contributor Author

Team decision is to use a different implementation (with go-containerregistry):
This containers/buildah implementation has heavy consequences on debugging and running e2e tests in CI

@sherine-k sherine-k closed this Oct 27, 2023
@openshift-ci
Copy link

openshift-ci bot commented Oct 27, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants