-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Allow images to be created on demand via the imagestreamtag api #14470
Conversation
Still WIP, tentatively targeted for 3.7 |
380dfbd
to
4321cf4
Compare
Registry seems to have huge fan out, this is a local test env but I would have expected ACL caching here @mfojtik @legionus |
4321cf4
to
5e95046
Compare
@bparees see the |
5e95046
to
00f5e8d
Compare
@smarterclayton is this also implementing this card? https://trello.com/c/dUYS0Hb1/540-5-support-importing-local-docker-images-using-oc-import-image-evg |
@soltysh FYI |
Origin Action Required: Pull request cannot be automatically merged, please rebase your branch from latest HEAD and push again |
pkg/cmd/cli/cmd/push/binary.go
Outdated
if err != nil { | ||
return fmt.Errorf("--from requires a valid image stream tag or image reference: %v", err) | ||
} | ||
if len(ref.Registry) > 0 || (len(ref.ID) == 0 && len(ref.Tag) == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like parsedockerimagereference will leave the tag empty (vs defaulting it to latest) if unspecified. are you intending to force people to specify :latest?
Yes forcing. http.Get does automatic proxy var support, doesn't it?
…On Fri, Jun 16, 2017 at 2:56 PM, Ben Parees ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pkg/cmd/cli/cmd/push/binary.go
<#14470 (comment)>:
> + }
+
+ to, err := imageapi.ParseDockerImageReference(o.To)
+ if err != nil {
+ return fmt.Errorf("--to is not a valid reference: %v", err)
+ }
+ if len(to.Registry) > 0 || len(to.ID) > 0 || len(to.Tag) == 0 {
+ return fmt.Errorf("--to must be NAMESPACE/NAME:TAG or NAME:TAG")
+ }
+
+ if len(o.BaseImage) > 0 {
+ ref, err := imageapi.ParseDockerImageReference(o.BaseImage)
+ if err != nil {
+ return fmt.Errorf("--from requires a valid image stream tag or image reference: %v", err)
+ }
+ if len(ref.Registry) > 0 || (len(ref.ID) == 0 && len(ref.Tag) == 0) {
looks like parsedockerimagereference will leave the tag empty (vs
defaulting it to latest) if unspecified. are you intending to force people
to specify :latest?
------------------------------
In pkg/cmd/cli/cmd/push/binary.go
<#14470 (comment)>:
> + if err != nil {
+ return err
+ }
+ return o.PrintObject([]*resource.Info{info})
+}
+
+func contentsForPathOrURL(s string, in io.Reader, subpaths ...string) (string, io.Reader, int64, error) {
+ switch {
+ case s == "-":
+ return "", in, -1, nil
+ case strings.Index(s, "http://") == 0 || strings.Index(s, "https://") == 0:
+ _, err := url.Parse(s)
+ if err != nil {
+ return "", nil, 0, fmt.Errorf("the URL passed to filename %q is not valid: %v", s, err)
+ }
+ res, err := http.Get(s)
proxy env var support?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#14470 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_pza8kFRX4bN3AYy9r6rZBeiKTQKoks5sEs_wgaJpZM4Nvjte>
.
|
sorry I guess it does, last time i looked into this i thought i found otherwise.
|
feb0d39
to
5a3a76d
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: smarterclayton No associated issue. Update pull-request body to add a reference to an issue, or get approval with The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/test all |
pkg/cmd/cli/cmd/push/binary.go
Outdated
This command assists users in creating new images that can be used as part of builds | ||
or as deployed applications. It accepts a zip or tar.gz file that will become a new | ||
image in an image stream. You may also specify image metadata like the entrypoint, | ||
environment variables, or labels to create a runnable image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's an implicit assumption here that the archive is going to be extracted when it is used to create the image layer. might want to make that explicit. (you get an image containing the contents of the zip, not an image containing the zip)
still seems like a lot of code for a marginal net gain over:
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: smarterclayton No associated issue. Update pull-request body to add a reference to an issue, or get approval with The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: smarterclayton No associated issue. Update pull-request body to add a reference to an issue, or get approval with The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
b7fc0b3
to
5877e77
Compare
5877e77
to
bdd364c
Compare
Also register the new clone API
bdd364c
to
ed60a6b
Compare
@smarterclayton: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
@smarterclayton PR needs rebase |
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
Still debating what I want to do with this. I considered moving it to its
own image command `oc image layer` and having it depend on layer reuse
working correctly, which should always be true for a given cluster.
…On Sun, Feb 25, 2018 at 6:29 PM, OpenShift Bot ***@***.***> wrote:
Issues go stale after 90d of inactivity.
Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually
close.
Exclude this issue from closing by commenting /lifecycle frozen.
If this issue is safe to close now please do so with /close.
/lifecycle stale
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#14470 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p7Eadkl2vL4cb8E4rQIR-c8aS48Fks5tYezmgaJpZM4Nvjte>
.
|
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
Rotten issues close after 30d of inactivity. Reopen the issue by commenting /close |
A new subresource
imagestreamtags/copy
is added which accepts an ImageStreamTagCopy. This describes a request to create a new image from the provided docker image metadata (command, labels, env, ports, etc) and provide a single layer via direct submission, dramatically simplifying the act of creating an image. The caller can create a new from scratch image or add a new layer on top of an existing image. The endpoint then talks to the registry and abstracts the necessary details like creating a manifest, uploading the blob contents, and returning the new image.The API is intended for use with binary uploads and other ad-hoc image composition.