Image pull implementation by CertainLach · Pull Request #26 · containers/containrs

CertainLach · 2020-09-08T18:30:46Z

I like the idea of Rust CRI implementation, and want to help in it :D

Is there any implementation plan/architecture description?

If no - i suggest using sled for internal storage, this is fast embedded kv database, which is pretty stable

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind dependency-change
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #16

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

openshift-ci-robot · 2020-09-08T18:30:49Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: CertainLach
To complete the pull request process, please assign harche
You can assign the PR to them by writing /assign @harche in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

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

Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

src/runtime.rs

saschagrunert · 2020-09-08T18:53:33Z

I like the idea of Rust CRI implementation, and want to help in it :D

Nice, that's wonderful to hear! ❤️

Is there any implementation plan/architecture description?

Not yet, but are you interested in contributing to some? :) We could start with a fresh documentation page inside the repository. WDYT?

src/runtime.rs

Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

…rvice Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

…rvice Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

CertainLach · 2020-09-10T21:01:02Z

I reduced scope of this PR, because currently i stuck at some bits of multiple transports support, feel free to merge this as partially finished work, i will commit more transports soon :D

saschagrunert · 2020-09-11T08:40:55Z

Can you please rebase and resolve the conflict? 😇 Fixing the CI would be necessary now too.

openshift-ci-robot · 2020-09-12T14:49:05Z

@CertainLach: PR needs rebase.

Details

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.

harche

Awesome work @CertainLach :) Thanks for your PR. I just had some minor comments, but they are just nits.

harche · 2020-09-13T08:12:14Z

src/image_service.rs

+        Ok(MyImage {
+            database: sled::open(db_path)?,
+            layers,
+            registries: vec![DockerRegistryClientV2::new(


Maybe instead of hard coding the registries here maybe in future we can read from the config file, just like how we do it currently in CRI-O.

harche · 2020-09-13T08:17:07Z

src/image_service.rs

+    pub async fn pull_image(&self, spec: &ImageSpec) -> Result<Option<Manifest>> {
+        // TODO: handle registry name in image name (I.e docker.io/redis:latest)
+        let parts = spec.image.split(':').collect::<Vec<&str>>();
+        assert_eq!(parts.len(), 2, "bad name format");


In case image tag is absent, should we just default to latest instead ?

harche · 2020-09-13T08:27:02Z

src/image_service.rs

+            };
+            let mut found_manifest = None;
+            for manifest in manifests.manifests {
+                // TODO: check platform with passed annotations


I guess you meant in case of index manifest, we should check against architecture and os fields to see if they match.

saschagrunert · 2020-10-15T18:30:13Z

@CertainLach do you mind giving this a rebase?

CertainLach added 2 commits September 8, 2020 23:24

Implement image storage db

03caafa

Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

Implement list images request

e69f952

Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

openshift-ci-robot requested review from rhatdan and umohnani8 September 8, 2020 18:30

Image list test

4b0b935

Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

CertainLach force-pushed the image-service branch from e76cea3 to 4b0b935 Compare September 8, 2020 18:31

openshift-ci-robot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Sep 8, 2020

haircommander reviewed Sep 8, 2020

View reviewed changes

src/runtime.rs Outdated Show resolved Hide resolved

saschagrunert reviewed Sep 8, 2020

View reviewed changes

src/runtime.rs Outdated Show resolved Hide resolved

CertainLach added 2 commits September 9, 2020 01:38

Generalize database field name

9ec0fa8

Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

Simplify database corruption error message

6a8e27c

Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

CertainLach force-pushed the image-service branch from 5fe7fa3 to 6a8e27c Compare September 8, 2020 20:40

openshift-ci-robot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Sep 8, 2020

Merge commit '416f7863bdc20ef030c0b5e04bae4f4549b1863f' into image-se…

635ae13

…rvice Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

openshift-ci-robot added release-note-none and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 8, 2020

Implement image pull

f6677cf

Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

CertainLach mentioned this pull request Sep 8, 2020

Implement OCI image spec #28

Closed

Layer locks, remove temp layers on failure

8be0d6d

Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 9, 2020

Merge commit '14c2660951ecf74fee1d00ba83a9bf97f9579f7a' into image-se…

30a8986

…rvice Signed-off-by: Yaroslav Bolyukin <iam@lach.pw>

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 9, 2020

CertainLach changed the title ~~Image service implementation~~ Image pull implementation Sep 10, 2020

CertainLach marked this pull request as ready for review September 10, 2020 20:59

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 10, 2020

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 12, 2020

harche reviewed Sep 13, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Image pull implementation#26

Image pull implementation#26
CertainLach wants to merge 9 commits intocontainers:masterfrom
CertainLach:image-service

CertainLach commented Sep 8, 2020 •

edited

Loading

Uh oh!

openshift-ci-robot commented Sep 8, 2020

Uh oh!

Uh oh!

saschagrunert commented Sep 8, 2020

Uh oh!

Uh oh!

CertainLach commented Sep 10, 2020

Uh oh!

saschagrunert commented Sep 11, 2020

Uh oh!

openshift-ci-robot commented Sep 12, 2020

Uh oh!

harche left a comment

Uh oh!

harche Sep 13, 2020

Uh oh!

harche Sep 13, 2020 •

edited

Loading

Uh oh!

harche Sep 13, 2020 •

edited

Loading

Uh oh!

saschagrunert commented Oct 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

CertainLach commented Sep 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

openshift-ci-robot commented Sep 8, 2020

Uh oh!

Uh oh!

saschagrunert commented Sep 8, 2020

Uh oh!

Uh oh!

CertainLach commented Sep 10, 2020

Uh oh!

saschagrunert commented Sep 11, 2020

Uh oh!

openshift-ci-robot commented Sep 12, 2020

Uh oh!

harche left a comment

Choose a reason for hiding this comment

Uh oh!

harche Sep 13, 2020

Choose a reason for hiding this comment

Uh oh!

harche Sep 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harche Sep 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saschagrunert commented Oct 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CertainLach commented Sep 8, 2020 •

edited

Loading

harche Sep 13, 2020 •

edited

Loading

harche Sep 13, 2020 •

edited

Loading