images, containers: converge metadata API conventions #1151
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1151 +/- ##
==========================================
- Coverage 31.88% 31.78% -0.11%
==========================================
Files 27 27
Lines 2161 2168 +7
==========================================
Hits 689 689
- Misses 1324 1331 +7
Partials 148 148
Continue to review full report at Codecov.
|
api/services/images/v1/images.proto
Outdated
| @@ -26,6 +28,11 @@ service Images { | |||
| // List returns a list of all images known to containerd. | |||
| rpc List(ListImagesRequest) returns (ListImagesResponse); | |||
|
|
|||
| // Create an image reocrd in the metadata store. | |||
cmd/dist/images.go
Outdated
| } | ||
| sort.Strings(labelPairs) | ||
| labels := "-" | ||
| if len(labelPairs) > 0 { |
There was a problem hiding this comment.
the loop around image.Labels to fill up labelPairs and the sort on the result can be moved under a if len(image.Labels) > 0
cmd/dist/images.go
Outdated
| fieldpaths = append(fieldpaths, strings.Join([]string{"labels", k}, ".")) | ||
| } | ||
|
|
||
| fmt.Println("send labels", labels) |
filters/parser.go
Outdated
| // ParseAll parses each filter in ss and returns a filter that will return true | ||
| // if any filter matches the expression. | ||
| // | ||
| // If not fitlers are provided, the filter will match anything. |
There was a problem hiding this comment.
s/not fitlers/ no filters/
| @@ -13,14 +14,21 @@ import ( | |||
|
|
|||
| // Image provides the model for how containerd views container images. | |||
| type Image struct { | |||
There was a problem hiding this comment.
can't we use the api type here?
There was a problem hiding this comment.
I'm going to do that in a separate change. That is my current TODO item.
The primary feature we get with this PR is support for filters and labels on the image metadata store. In the process of doing this, the conventions for the API have been converged between containers and images, providing a model for other services. With images, `Put` (renamed to `Update` briefly) has been split into a `Create` and `Update`, allowing one to control the behavior around these operations. `Update` now includes support for masking fields at the datastore-level across both the containers and image service. Filters are now just string values to interpreted directly within the data store. This should allow for some interesting future use cases in which the datastore might use the syntax for more efficient query paths. The containers service has been updated to follow these conventions as closely as possible. Signed-off-by: Stephen J Day <stephen.day@docker.com>
stevvooe
force-pushed
the
image-metadata-service
branch
from
10d045e
to
7f4c4ae
Compare
|
@mlaventure Thanks for the review! |
|
LGTM |
| } | ||
| } | ||
| } else { | ||
| updated = image |
There was a problem hiding this comment.
Should we really just completely override here or should we be stricter about immutable data. The only field which would apply here is created at. Allowing that field to be updated I think will lead to more accidental override rather than filling in use cases where it is intentionally overridden.
| updated = image | ||
| } | ||
|
|
||
| // TODO(stevvooe): Should only mark as updated if we have actual changes. |
There was a problem hiding this comment.
My thought, no we should update the timestamp anytime this function is called. Similar to touching a file, it should be possible to update a timestamp without updating the values.
|
LGTM Going to merge this one now, I would still like to discuss |
…bility Initial support for traffic shaping
The primary feature we get with this PR is support for filters and
labels on the image metadata store. In the process of doing this, the
conventions for the API have been converged between containers and
images, providing a model for other services.
With images,
Put(renamed toUpdatebriefly) has been split into aCreateandUpdate, allowing one to control the behavior around theseoperations.
Updatenow includes support for masking fields at thedatastore-level across both the containers and image service. Filters
are now just string values to interpreted directly within the data
store. This should allow for some interesting future use cases in which
the datastore might use the syntax for more efficient query paths.
The containers service has been updated to follow these conventions as
closely as possible.
Signed-off-by: Stephen J Day stephen.day@docker.com