Add images verify cmd #18

0xFelix · 2022-03-29T09:22:14Z

What this PR does / why we need it:

This adds the cmd 'images verify', which verifies that previously built
and pushed containerdisks are bootable and that guests are working.

The example definitions of the docs are used to test containerdisks by
creating VMs with the respective containerdisk set as the boot source.
After waiting for a VM to become ready, a defined set of tests is run on
the corresponding VMI. If all tests succeed, the value of the containerdisk
annotation 'verified' is set to 'true' and the updated image manifest is
pushed to the registry.

Which issue(s) this PR fixes:

Fixes #2

Special notes for your reviewer:

Merge after #17

Release note:

Add cmd to verify freshly built containerdisks

0xFelix · 2022-03-29T09:22:32Z

/cc @akrejcir

go.mod

artifacts/centos/centos.go

go.mod

pkg/tests/ssh.go

0xFelix · 2022-03-31T14:37:11Z

@rmohr Can you please have a look again? I addressed your comments.

This prevents a segfault when an unexpected error occurs, which cannot be unwrapped (e.g. tls verification error on insecure registry). Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

Do not always log 'Pushing' and let the user know what is really happening instead. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

rmohr

Overall pretty nice, I have one conceptual question.

cmd/medius/images/verify.go

This moves the worker logic for image cmds into a separate file, so it can be used in multiple image cmds. The logic was also refactored, so it is easier to use, supports contexts and duplicated code during use is prevented. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

This moves the PushImage function into the repository package to make it available for use in other image cmds. Also use the uniform identifier 'imgRef' for images. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

The function tarball.LayerFromReader internally uses iotuil.ReadAll, which buffers all data in memory and can lead to problems with large files. Replace it with tarball.LayerFromOpener and implement a StreamLayerOpener, that does not return an error chan and can be opened multiple times. Thanks to akrejcir for the idea and proposal. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

The methods for creating example VM definitons are added to the artifact interface so correct definitions for each type of guest can be created. This also allows to test the example definitions in another command (e.g. images verify). Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

The removed code added the main version of the CentOS Stream artifact to AdditionalUniqueTags which is not an additional unique tag. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

rmohr

Pretty close. Not sure if you agree, but looks much simpler now than before to me. One recommendation regarding to the result handing which I would love to see.

cmd/medius/images/push.go

cmd/medius/images/verify.go

0xFelix · 2022-04-13T12:15:19Z

@rmohr I agree, it is simpler now. The commit changing to OCIv1 could be useful in the future perhaps. I addressed your change request. Does it look good now?

0xFelix · 2022-04-13T12:25:57Z

@rmohr I just noticed I could move the --workers and --results-file flags to the imagesCmd. Do you agree?

rmohr · 2022-04-13T12:30:18Z

@rmohr I just noticed I could move the --workers and --results-file flags to the imagesCmd. Do you agree?

sounds good. 👍

Flags that can be shared by image cmds are refactored into ImagesOptions and moved to imagesCmd. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

With this change the 'images push' command writes a results file after building and pushing containerdisks. The results file contains all built images, their sha256 sum and the pushed tags. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

This adds the cmd 'images verify', which verifies that previously built and pushed containerdisks are bootable and that guests are working. The example definitions of the docs are used to test containerdisks by creating VMs with the respective containerdisk set as the boot source. After waiting for a VM to become ready, a defined set of tests is run on the corresponding VMI. If all tests succeed, the verified value of the containerdisk is set to 'true' in the results file. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

The test checks if the agent is running on the guest by attempting to get the guest os info from the agent. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

The test checks if ssh is working on the guest by attempting to use the credentials generated by images verify to login into the guest and to run a simple command (e.g. 'echo hello'). Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

0xFelix · 2022-04-14T08:50:40Z

@rmohr Mind giving it a look again?

rmohr

/lgtm
/approve

I think this is good enough to take it in. If we want to improve stuff or detect issues we can follow up on it.

Great work!

rmohr · 2022-04-21T12:25:02Z

pkg/api/artifact.go

+	PrivateKey interface{}
+}
+
+type ArtifactResult struct {


Since we kind of build a pipeline with the new commands, we probably want to add an indicator if something should have been created/verified but failed. We can do that in a follow up.

kubevirt-bot · 2022-04-21T12:25:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rmohr

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rmohr]

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

kubevirt-bot added release-note dco-signoff: yes labels Mar 29, 2022

kubevirt-bot requested review from dominikholler and rmohr March 29, 2022 09:22

kubevirt-bot added the size/XXL label Mar 29, 2022

kubevirt-bot requested a review from akrejcir March 29, 2022 09:22

rmohr reviewed Mar 29, 2022

View reviewed changes

go.mod Outdated Show resolved Hide resolved

rmohr reviewed Mar 29, 2022

View reviewed changes

artifacts/centos/centos.go Outdated Show resolved Hide resolved

rmohr reviewed Mar 29, 2022

View reviewed changes

go.mod Outdated Show resolved Hide resolved

rmohr reviewed Mar 29, 2022

View reviewed changes

pkg/tests/ssh.go Outdated Show resolved Hide resolved

kubevirt-bot added dco-signoff: no and removed dco-signoff: yes labels Mar 30, 2022

0xFelix force-pushed the images-verify branch 3 times, most recently from 28911f1 to 86d7f7d Compare March 31, 2022 14:27

kubevirt-bot added dco-signoff: yes and removed dco-signoff: no labels Mar 31, 2022

0xFelix force-pushed the images-verify branch 2 times, most recently from 21097ee to caa7c30 Compare March 31, 2022 14:33

0xFelix requested a review from rmohr March 31, 2022 14:37

kubevirt-bot added the needs-rebase label Mar 31, 2022

0xFelix added 3 commits April 1, 2022 08:27

Prevent null pointer access when checking for unknown tag

c502efb

This prevents a segfault when an unexpected error occurs, which cannot be unwrapped (e.g. tls verification error on insecure registry). Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

Improve logging of dry-runs

5346d33

Do not always log 'Pushing' and let the user know what is really happening instead. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

Fix typos in log messages

43bd6b4

Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

rmohr reviewed Apr 11, 2022

View reviewed changes

cmd/medius/images/verify.go Outdated Show resolved Hide resolved

cmd/medius/images/verify.go Outdated Show resolved Hide resolved

0xFelix force-pushed the images-verify branch from ece6f5a to aaf177d Compare April 11, 2022 12:47

0xFelix added 6 commits April 12, 2022 13:06

Use const for shasum label

7551f70

Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

Move PushImage to repository package

8b2513e

This moves the PushImage function into the repository package to make it available for use in other image cmds. Also use the uniform identifier 'imgRef' for images. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

Do not add duplicate tag to AdditionalUniqueTags of CentOS Stream

2b158ba

The removed code added the main version of the CentOS Stream artifact to AdditionalUniqueTags which is not an additional unique tag. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

0xFelix force-pushed the images-verify branch from aaf177d to 25b4845 Compare April 13, 2022 08:54

rmohr requested changes Apr 13, 2022

View reviewed changes

cmd/medius/images/push.go Outdated Show resolved Hide resolved

cmd/medius/images/verify.go Outdated Show resolved Hide resolved

kubevirt-bot assigned rmohr Apr 13, 2022

0xFelix force-pushed the images-verify branch from 25b4845 to be7ba74 Compare April 13, 2022 12:09

0xFelix added 2 commits April 14, 2022 08:54

Refactor CLI flags

bc2262e

Flags that can be shared by image cmds are refactored into ImagesOptions and moved to imagesCmd. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

0xFelix force-pushed the images-verify branch from be7ba74 to 6878b3d Compare April 14, 2022 07:29

0xFelix added 3 commits April 14, 2022 09:39

Add test to check if qemu-guest-agent is running

f61dd6e

The test checks if the agent is running on the guest by attempting to get the guest os info from the agent. Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

Add test to check if ssh is working

80dfd9c

The test checks if ssh is working on the guest by attempting to use the credentials generated by images verify to login into the guest and to run a simple command (e.g. 'echo hello'). Signed-off-by: Felix Matouschek <fmatouschek@redhat.com>

0xFelix force-pushed the images-verify branch from 6878b3d to 80dfd9c Compare April 14, 2022 07:41

0xFelix requested review from rmohr and akrejcir April 20, 2022 07:50

rmohr approved these changes Apr 21, 2022

View reviewed changes

kubevirt-bot added the lgtm label Apr 21, 2022

kubevirt-bot added the approved label Apr 21, 2022

kubevirt-bot merged commit 2b2810e into kubevirt:main Apr 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add images verify cmd #18

Add images verify cmd #18

0xFelix commented Mar 29, 2022 •

edited

Loading

Uh oh!

0xFelix commented Mar 29, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xFelix commented Mar 31, 2022

Uh oh!

rmohr left a comment

Uh oh!

Uh oh!

Uh oh!

rmohr left a comment

Uh oh!

Uh oh!

Uh oh!

0xFelix commented Apr 13, 2022

Uh oh!

0xFelix commented Apr 13, 2022

Uh oh!

rmohr commented Apr 13, 2022

Uh oh!

0xFelix commented Apr 14, 2022

Uh oh!

rmohr left a comment

Uh oh!

rmohr Apr 21, 2022

Uh oh!

kubevirt-bot commented Apr 21, 2022

Uh oh!

Add images verify cmd #18

Add images verify cmd #18

Conversation

0xFelix commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xFelix commented Mar 29, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xFelix commented Mar 31, 2022

Uh oh!

rmohr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rmohr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

0xFelix commented Apr 13, 2022

Uh oh!

0xFelix commented Apr 13, 2022

Uh oh!

rmohr commented Apr 13, 2022

Uh oh!

0xFelix commented Apr 14, 2022

Uh oh!

rmohr left a comment

Choose a reason for hiding this comment

Uh oh!

rmohr Apr 21, 2022

Choose a reason for hiding this comment

Uh oh!

kubevirt-bot commented Apr 21, 2022

Uh oh!

0xFelix commented Mar 29, 2022 •

edited

Loading