Skip to content
This repository was archived by the owner on Oct 10, 2020. It is now read-only.

[merged] System containers: basic support for verify#506

Closed
giuseppe wants to merge 5 commits intoprojectatomic:masterfrom
giuseppe:system-containers-basic-verify
Closed

[merged] System containers: basic support for verify#506
giuseppe wants to merge 5 commits intoprojectatomic:masterfrom
giuseppe:system-containers-basic-verify

Conversation

@giuseppe
Copy link
Collaborator

@giuseppe giuseppe commented Aug 1, 2016

No description provided.

tag = ":".join(imagebranch.replace("ociimage/", "").rsplit('-', 1))
timestamp = OSTree.commit_get_timestamp(commit)

return {'Id' : commit_rev, 'RepoTags' : [tag], 'Names' : [], 'Created': timestamp,
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this ID actually be the original docker image digest? And we insert a new OstreeChecksum field or something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the problem is that the commit having the manifest metadata cannot be mapped to a Docker layer, so I preferred to use the OSTree commit for both layers and the ID, so for now I am using the same ID used for images list.

With @yuqi-zhang we are currently discussing about the ID to use in the images list output as well (which currently is the commit_rev too), as we are thinking about changing there as well, so that it uses the Docker layer ID, but we didn't yet figure out what ID should be used for the metadata commit, which gives the name to the image.

@cgwalters
Copy link
Member

No tests had to change for this? Is there no coverage?

@giuseppe
Copy link
Collaborator Author

giuseppe commented Aug 1, 2016

there is only a basic test for verify which is not really exaustive. More tests must be added for the system containers part as well.

@giuseppe giuseppe force-pushed the system-containers-basic-verify branch from 0d9f277 to d674162 Compare August 3, 2016 08:07
@giuseppe
Copy link
Collaborator Author

giuseppe commented Aug 3, 2016

I added a fixup patch that adds a test for verify system images.

I made the change for the image ID we use in a separate patch, so now we use:

  1. The Digest from the manifest if the commit has the Docker manifest in the metadata
  2. The $SHA part from ociimage/$SHA if it is a checksum
  3. The OSTree commit checksum

This should enable us now to fully support images --all for system containers @yuqi-zhang

@giuseppe giuseppe force-pushed the system-containers-basic-verify branch from d674162 to 32b59ed Compare August 3, 2016 08:18
@rhatdan
Copy link
Member

rhatdan commented Aug 3, 2016

LGTM

@rhatdan
Copy link
Member

rhatdan commented Aug 3, 2016

@cgwalters PTAL

@giuseppe giuseppe force-pushed the system-containers-basic-verify branch 2 times, most recently from f13bc75 to ca8918f Compare August 3, 2016 15:37
@giuseppe
Copy link
Collaborator Author

giuseppe commented Aug 3, 2016

rebased ⬆️

It returns the information in a similar way to the Docker inspect
format.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
To find the ID for an image, try in order:

1) Digest from the docker.manifest metadata
2) $SHA if $SHA in ociimage/$SHA is a valid checksum
3) OSTree commit

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe giuseppe force-pushed the system-containers-basic-verify branch from ca8918f to 151e787 Compare August 3, 2016 20:52
@rhatdan
Copy link
Member

rhatdan commented Aug 4, 2016

@rh-atomic-bot r+

@rh-atomic-bot
Copy link

📌 Commit 151e787 has been approved by rhatdan

@rh-atomic-bot
Copy link

⌛ Testing commit 151e787 with merge c367f7d...

rh-atomic-bot pushed a commit that referenced this pull request Aug 4, 2016
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #506
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Aug 4, 2016
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #506
Approved by: rhatdan
rh-atomic-bot pushed a commit that referenced this pull request Aug 4, 2016
To find the ID for an image, try in order:

1) Digest from the docker.manifest metadata
2) $SHA if $SHA in ociimage/$SHA is a valid checksum
3) OSTree commit

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Closes: #506
Approved by: rhatdan
@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: rhatdan
Pushing c367f7d to master...

@rh-atomic-bot rh-atomic-bot changed the title System containers: basic support for verify [merged] System containers: basic support for verify Aug 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants