-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nixos/docker-containers.nix: enhance functionality #78366
Conversation
See also: #64580 |
ea9b5f9
to
93d281c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've addressed my own change requests. Looks good to me now.
Thanks for this. That's exactly what i was looking for. The only thing that bugs me a bit about all the current |
Not really. Docker wants the image to have a name. This is orthogonal from the whole loading process. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you enrich the test by adding a testcase for these features?
Another way to manage the image would be to use Skopeo since it is able to pull image from a registry as well as from local file. But we would have to change the interface of this module, and i'm not sure this is what we want ;)
For instance, instead of image
, we could define these two options:
imageUrl = "docker://alpine"
imageName = "alpine"
or
imageUrl = "docker-archive://${dockerTools.buildImage {name...}}"
imageName = "myImage"
These attribute could then be used to build a Skopeo command:
skopeo copy ${imageUrl} docker-deamon://${imageName}
which loads a image to the local Docker deamon.
I definitely tried to keep the module interface stable. There are other concerns that I would probably like to address if we were to break the interface, but I don't have the time to rewrite the module right now. |
I've modified the tests to use the new |
@GrahamcOfBorg test docker-containers |
@GrahamcOfBorg test docker-containers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contributions ;)
I'm just waiting for the ofborg test to merge it!
I finally ran the test on my laptop and it was green. |
Motivation for this change
Things done