Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Add sandbox-image VM flag #598

Merged
merged 4 commits into from
May 18, 2020
Merged

Conversation

darkowlzz
Copy link
Contributor

This adds a new flag --sandbox-image for setting the VM sandbox image.
The VM spec contains a "sandbox" field, similar to "image" and "kernel"
fields.

Adds a new constant DEFAULT_SANDBOX_IMAGE_NAME.

Adds unit test for sandbox-image flag.

Autogen updates the deps. Deps update are in a separate commit.

darkowlzz added 2 commits May 17, 2020 00:41
This adds a new flag `--sandbox-image` for setting the VM sandbox image.
The VM spec contains a "sandbox" field, similar to "image" and "kernel"
fields.

Adds a new constant `DEFAULT_SANDBOX_IMAGE_NAME`.

Adds unit test for sandbox-image flag.
@darkowlzz darkowlzz requested a review from stealthybox as a code owner May 16, 2020 19:17
@darkowlzz
Copy link
Contributor Author

darkowlzz commented May 16, 2020

The CI fails because version.ImageTag() is not able to return a proper version for the sandbox image tag, resulting in error when running ignite due to missing image tag.

panic: interface conversion: interface is nil, not reference.NamedTagged [recovered]

	panic: interface conversion: interface is nil, not reference.NamedTagged

This maybe due to the shallow clone of the repo in the CIs.

Building manually with full git history works properly and the sandbox image tag is set as expected:

$ sudo bin/ignite create -h
...
Flags:
...
      --sandbox-image oci-image   Specify an OCI image for the VM sandbox (default weaveworks/ignite:v0.6.0-262-fe4b2aa7cf4c9f)
...

weaveworks/ignite:v0.6.0-262-fe4b2aa7cf4c9f in the above output.

We may need to improve how version.ImageTag() works.

Make kernel image name and tag directly configurable via ldflags.
Make sandbox image name and tag directly configurable via ldflags.
Remove gitVersion tag calculation for sandbox image.
Make sandbox image default properly in constants.
Refactor image string formatting before parsing OCI Refs in API defaulting logic.
Allow repo/name@sha256:<hash> image formats for sandbox and kernel via ldflags overrides. (May not yet be fully supported by all image and sandbox runtimes)
Respect `make DOCKER_USER="${USER}"` in ldflags.

Remove gitVersion workaround from .travisci e2e tests
@stealthybox
Copy link
Contributor

stealthybox commented May 18, 2020

The CI fails because version.ImageTag() is not able to return a proper version .
for the sandbox image tag, resulting in error when running ignite due to missing image tag.

This ended up not being the specific reason the tests were failing.

In the first commit I added, we patch the image defaulting logic to work without the help of ldflags and gitVersion.

With this patch, the tests are still failing on my local git copy.

The failures were due to the lack of missing fields on synthetic VM objects created within the tests.
These objects are not created using the defaulting logic from api-machinery, so their OCI fields need to manually be set.

I did not fix the lack of using defaulting functions in the tests, but I was able to manually add static values for the missing fields in the second commit, and the tests are now passing.

@stealthybox stealthybox merged commit 8187785 into weaveworks:master May 18, 2020
@stealthybox
Copy link
Contributor

I was able to test this field also properly defaults and can be overridden using sudo bin/ignite run --config ../ignite-scratch/anon-vm.json

@darkowlzz
Copy link
Contributor Author

This is great. A lot of improvements 🎉 Thanks a lot.

@luxas luxas added this to the v0.7.0 milestone Jun 2, 2020
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.

3 participants