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

mirror: Support mirroring to disk as a target #126

Merged

Conversation

smarterclayton
Copy link
Contributor

@smarterclayton smarterclayton commented Oct 12, 2019

Allows release mirroring to disk. oc adm release mirror is updated to use
this mechanism. The mirror command, when --to-dir is specified, prints out
the command that can be used to restore the content.

Mirroring a release to disk:

oc adm release mirror quay.io/openshift-release-dev/ocp-release:4.1.18 --to-dir=/tmp/release

Mirroring that back to another registry

oc image mirror --from-dir=/tmp/release file://openshift/release myregistry.com/my/repository

Now that mirror supports local file destinations, add a simple server command under oc image serve that allows someone to mirror a release to disk and then host it within a firewall.

I.e.:

oc adm release mirror ... --to-dir=/srv/repo

# Move inside the firewall

oc image serve --dir=/srv/repo

The server performs minimal HTTP header mangling to support modern container registry clients. Optional --tls-key and --tls-cert flags allow the server to listen on TLS.

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 12, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton

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-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 12, 2019
@smarterclayton
Copy link
Contributor Author

/assign @abhinavdahiya

Gradually improving the flexibility of the commands to allow offline content replication. Open to suggestions (I'm trying to improve the flexibility of the image commands while making release mirroring easier). Builds on feedback from folks trying to clone things offline.

I think this would be a candidate for back port to simplify 4.2 offline support.

@smarterclayton
Copy link
Contributor Author

/retest

@abhinavdahiya
Copy link
Contributor

I like this, will try to review by EOD.

@smarterclayton
Copy link
Contributor Author

smarterclayton commented Oct 14, 2019

Specifically this is different than skopeo because

a) it creates a disk layout that matches the HTTP layout of a registry server, like we already do for s3
b) the disk layout matches the human assumptions about the registry
c) it allows us to reuse blobs (although there is an optimization missing which would scan the directory for duplicate blobs and link them during mirror)
d) this will be useful in ci and testing flows to recreate specific repos
e) this is the most barebones possible registry approach and is fairly tractable to keep working
f) it reuses conventions for our tooling, has parallel download, and uses the same code we are already using to mirror

if len(o.ToDir) > 0 {
fmt.Fprintf(o.Out, "\nTo upload local images to a registry, run:\n\n oc image mirror --from-dir=%s file://%s* REGISTRY/REPOSITORY\n\n", o.ToDir, to)
} else if len(o.To) > 0 {
if o.PrintImageContentInstructions {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we move this to conditional block a8c5d8f#diff-6c5f7c1cbfa2abdaa2847f0982a31e5fR507 ??

@abhinavdahiya
Copy link
Contributor

a8c5d8f

The release/mirror.go changed look good.
I don't have complete grasp of cli/mirror changes, went through them as a single pass and had few questions.. will keep digging on those. But for sanity check we should have somebody from experienced with image-registry stufff take a peek.

@pearj
Copy link

pearj commented Oct 18, 2019

I just discovered if the pull secret file doesn't exist it segfaults:
ie running: ./oc adm -a pull_secret_doesnexist.json release mirror quay.io/openshift-release-dev/ocp-release:4.2.0 --to-dir=/tmp/ocp-release

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x60 pc=0x1fafc59]

goroutine 1 [running]:
github.com/openshift/oc/pkg/cli/image/manifest.(*SecurityOptions).Context(0xc00037f130, 0x7ffe89fd5816, 0x15, 0xc0011c09a0)
        /root/oc/pkg/cli/image/manifest/manifest.go:90 +0x59
github.com/openshift/oc/pkg/cli/image/extract.(*Options).Run(0xc00037f0e0, 0x0, 0x0)
        /root/oc/pkg/cli/image/extract/extract.go:306 +0x4a
github.com/openshift/oc/pkg/cli/admin/release.(*ExtractOptions).Run(0xc0001e1dc0, 0xc000518860, 0x2af1f05)
        /root/oc/pkg/cli/admin/release/extract.go:210 +0x3eb
github.com/openshift/oc/pkg/cli/admin/release.(*MirrorOptions).Run(0xc001259040, 0x0, 0x2cd1c30)
        /root/oc/pkg/cli/admin/release/mirror.go:276 +0x2aea
github.com/openshift/oc/pkg/cli/admin/release.NewMirror.func1(0xc00049d400, 0xc000e22c80, 0x1, 0x4)
        /root/oc/pkg/cli/admin/release/mirror.go:80 +0xa0
github.com/spf13/cobra.(*Command).execute(0xc00049d400, 0xc000e22c40, 0x4, 0x4, 0xc00049d400, 0xc000e22c40)
        /root/go/pkg/mod/github.com/spf13/cobra@v0.0.5/command.go:830 +0x2ae
github.com/spf13/cobra.(*Command).ExecuteC(0xc001078a00, 0xb, 0xc001078a00, 0xb)
        /root/go/pkg/mod/github.com/spf13/cobra@v0.0.5/command.go:914 +0x2fc
github.com/spf13/cobra.(*Command).Execute(...)
        /root/go/pkg/mod/github.com/spf13/cobra@v0.0.5/command.go:864
main.main()
        /root/oc/cmd/oc/oc.go:103 +0x815

@smarterclayton
Copy link
Contributor Author

Thanks, separate bug but will update it here

@smarterclayton
Copy link
Contributor Author

Comments addressed, did a bit more manual testing.

@smarterclayton
Copy link
Contributor Author

I want to get some soak time on this, and get e2es going. Going to tag today unless there are other comments.

@smarterclayton smarterclayton added the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2019
We should not access context unless there is no error.
Allows release mirroring to disk. `oc adm release mirror` is
updated to use this mechanism. The mirror command, when --to-dir
is specified, prints out the command that can be used to restore
the content.
Mark the command as no longer experimental as well.
Now that mirror supports local file destinations, add a simple server
command under `oc image serve` that allows someone to mirror a
release to disk and then host it within a firewall.

I.e.:

    oc adm release mirror ... --to-dir=/srv/repo

    # Move inside the firewall

    oc image serve --dir=/srv/repo

The server performs minimal HTTP header mangling to support modern
container registry clients. Optional --tls-key and --tls-cert
flags allow the server to listen on TLS.
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 31, 2019
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@smarterclayton smarterclayton added the lgtm Indicates that a PR is ready to be merged. label Nov 1, 2019
@smarterclayton
Copy link
Contributor Author

/retest

2 similar comments
@smarterclayton
Copy link
Contributor Author

/retest

@smarterclayton
Copy link
Contributor Author

/retest

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. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants