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.

Sign up for GitHub

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

Provide layers referenced by an image stream as layers subresource #19969

Merged
merged 4 commits into from
Jul 13, 2018
Merged

Provide layers referenced by an image stream as layers subresource #19969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b3d5dc
bump(*): Pick up new API changes
smarterclayton Jul 10, 2018
16d3f5d
Allow the -delete-namespace flag to work on origin tests
smarterclayton Jun 25, 2018
81e6e1c
Add internal types for ImageStreamLayers
smarterclayton Jun 12, 2018
aac71d5
Provide layers referenced by an image stream as layers subresource
smarterclayton Jun 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions api/docs/apis-image.openshift.io/v1.ImageStream.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,60 @@ The server guarantees that the objects returned when using continue will be iden
* application/vnd.kubernetes.protobuf


[[Get-apis-image.openshift.io-v1-namespaces-namespace-imagestreams-name-layers]]
=== Get layers of a ImageStream in a namespace
Read layers of the specified ImageStream

==== HTTP request
----
GET /apis/image.openshift.io/v1/namespaces/$NAMESPACE/imagestreams/$NAME/layers HTTP/1.1
Authorization: Bearer $TOKEN
Accept: application/json
Connection: close
----

==== Curl request
----
$ curl -k \
-H "Authorization: Bearer $TOKEN" \
-H 'Accept: application/json' \
https://$ENDPOINT/apis/image.openshift.io/v1/namespaces/$NAMESPACE/imagestreams/$NAME/layers
----

==== Path parameters
[cols="1,5", options="header"]
|===
|Parameter|Description
|name|name of the ImageStreamLayers
|namespace|object name and auth scope, such as for teams and projects
|===

==== Query parameters
[cols="1,5", options="header"]
|===
|Parameter|Description
|pretty|If 'true', then the output is pretty printed.
|===

==== Responses
[cols="1,5", options="header"]
|===
|HTTP Code|Schema
|200 OK|v1.ImageStreamLayers
|401 Unauthorized|
|===

==== Consumes

* \*/*

==== Produces

* application/json
* application/yaml
* application/vnd.kubernetes.protobuf


[[Get-apis-image.openshift.io-v1-namespaces-namespace-imagestreams-name-secrets]]
=== Get secrets of a ImageStream in a namespace
Read secrets of the specified ImageStream
Expand Down
38 changes: 38 additions & 0 deletions api/protobuf-spec/github_com_openshift_api_image_v1.proto
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

138 changes: 138 additions & 0 deletions api/swagger-spec/openshift-openapi-spec.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions glide.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/cmd/server/origin/legacy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ func LegacyStorage(storage map[schema.GroupVersion]map[string]rest.Storage) map[

case *imagestreametcd.REST:
legacyStorage[resource] = &imagestreametcd.LegacyREST{REST: storage}
case *imagestreametcd.LayersREST:
delete(legacyStorage, resource)
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be necessary. Don't add yourself to the whitelist above and you should be good.

Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be necessary. Don't add yourself to the whitelist above and you should be good.

this is required for some reason?

Copy link
Contributor

Choose a reason for hiding this comment

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

comment bump


case *routeetcd.REST:
store := *storage.Store
Expand Down
1 change: 1 addition & 0 deletions pkg/image/apis/image/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func addKnownTypes(scheme *runtime.Scheme) error {
&ImageStreamTag{},
&ImageStreamTagList{},
&ImageStreamImage{},
&ImageStreamLayers{},
&ImageStreamImport{},
&kapi.SecretList{},
)
Expand Down
37 changes: 37 additions & 0 deletions pkg/image/apis/image/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,43 @@ type ImageStreamImage struct {
// DockerImageReference points to a Docker image.
type DockerImageReference = reference.DockerImageReference

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// ImageStreamLayers describes information about the layers referenced by images in this
// image stream.
type ImageStreamLayers struct {
metav1.TypeMeta
// Standard object's metadata.
metav1.ObjectMeta
// blobs is a map of blob name to metadata about the blob.
Blobs map[string]ImageLayerData
// images is a map between an image name and the names of the blobs and manifests that
// comprise the image.
Images map[string]ImageBlobReferences
}

// ImageBlobReferences describes the blob references within an image.
type ImageBlobReferences struct {
// layers is the list of blobs that compose this image, from base layer to top layer.
// All layers referenced by this array will be defined in the blobs map. Some images
// may have zero layers.
// +optional
Layers []string
// manifest, if set, is the blob that contains the image manifest. Some images do
// not have separate manifest blobs and this field will be set to nil if so.
// +optional
Manifest *string
}

// ImageLayerData contains metadata about an image layer.
type ImageLayerData struct {
// Size of the layer in bytes as defined by the underlying store. This field is
// optional if the necessary information about size is not available.
LayerSize *int64
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have an immediate use-case for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed for the caches used inside of the registry that answer hits about blobs. If we don't have this info, we'd have to make an other O(N) calls to get it. Same for media type.

Copy link
Contributor

Choose a reason for hiding this comment

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

On registry side this information will be cached once again. Maybe it's better to make a watcher on registry side and cache it just once per registry ?

Copy link
Contributor

Choose a reason for hiding this comment

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

caching on the apiserver side has the benefit of helping anyone who invokes the api, not just the registry. (e.g. maybe pruning uses it in the future?)

Copy link
Contributor

Choose a reason for hiding this comment

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

@bparees My question was because of the worry of spending the master's memory on another large cache.

Copy link
Contributor

Choose a reason for hiding this comment

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

I figured, I was just expressing a reason it might be worth paying that cost. (Ultimately I think @smarterclayton convinced himself that this cache won't be that big)

// MediaType of the referenced object.
MediaType string
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see this being consumed either.

Copy link
Contributor

Choose a reason for hiding this comment

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

comment bump

}

// +genclient
// +genclient:onlyVerbs=create
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down
Loading