Skip to content
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

Create containers remotely when currentSystem != Linux #412

Closed
wants to merge 1 commit into from

Conversation

@brocking
Copy link

@brocking brocking commented Mar 12, 2016

Resolves #398

@gilligan
Copy link
Contributor

@gilligan gilligan commented Jul 22, 2016

I just ran into the same problem - @domenkozar could this be merged ?

@domenkozar
Copy link
Member

@domenkozar domenkozar commented Dec 12, 2016

Someone has to review the code and test it. I currently don't have time for it, sorry :/

@3noch
Copy link

@3noch 3noch commented Jan 27, 2017

Any progress on this?

@johbo
Copy link
Contributor

@johbo johbo commented Mar 9, 2017

Just came across this issue. Just started to try this PR on my OSX box, so far it's working fine.

@yurrriq
Copy link
Member

@yurrriq yurrriq commented Mar 13, 2017

This didn't work for me when trying to deploy Digital Ocean or VirtualBox.

@johbo
Copy link
Contributor

@johbo johbo commented Mar 14, 2017

@yurrriq mind to share a few more details about your setup?

On my end I think that things just worked because I did already have remote building set up, so the code which tries to auto-configure the remote machines was not hit in my case.

@yurrriq
Copy link
Member

@yurrriq yurrriq commented Mar 14, 2017

I've got remote building set up too via nix-docker and I use nix-darwin to manage my config.

@brocking
Copy link
Author

@brocking brocking commented Mar 15, 2017

@yurrriq If the NIX_REMOTE_SYSTEMS variable is already set nixops won't amend it with the machines that it knows about. 1 You will need to yourself add the details of the machines you want used for remote building to the file which that variable points to.

On a more general note, I think the discussion in #260 has better solutions than what I attempted to resolve in this PR, because it relates to a slightly more general issue around nixops remote deployment. But when I opened this PR I didn't know about any of that discussion, and since it didn't attract much attention I thought I may as well leave it open. So if #260 is resolved this could be closed, but until then it may be helpful for it stay open for others to patch onto their nixops builds.

@betaboon
Copy link

@betaboon betaboon commented Nov 15, 2017

I just stumbled upon this problem when deploying containers into a virtualbox from macos.

I just tested this PR:

  • With a already working remote-build setup (configured using the nix-daemon plist) this solves the problem of not being able to deploy containers into a virtualbox from macos.
  • With the remote-build setup disabled (so no remote-build settings in nix-daemon plist) this still fails.

As #260 hasn't progressed in over a year, maybe we can progress on this PR?

@johbo
Copy link
Contributor

@johbo johbo commented Dec 27, 2017

Maybe we can split out a minimal change, in my case the change to container.nix does seem to do the trick already.

johbo added a commit to johbo/nixops that referenced this pull request Dec 27, 2017
Taken from NixOS#412 to generate a minimal change
as an improvement of the current situation regarding remote building.

Adjusted to use "mkOverride 900" to make it consistent with all other backends.

If building a container on a non-linux system and "nixpkgs.system" is not set,
then we default now to "x86_64-linux".
@johbo
Copy link
Contributor

@johbo johbo commented Dec 27, 2017

The changes in johbo@64d1636 did work fairly well in my case from darwin.

This would allow us to merge the good parts and leave the question regarding the moved chunk of python code open for the future. On my end I am not even so sure if we really should aim to automagically remote-build on the target machine. My bet is that we can live with a requirement that people on non-linux will have to set up remote building for nix in order to be able to use nixops on top of it.

As far as I understand the change, it does the following two bits:

  1. Apply a default for nixpkgs.system also for the container backend.

  2. In container.py there is a special twist: In create we first create a blank container and this one also needs to inherit the nixpkgs.system value. The whole magic around eval_args seems to address this problem.

Hope that's an alternative which makes it easier to merge in the needed parts. :)

betaboon added a commit to MapCaseGmbH/nixops that referenced this pull request Apr 25, 2018
Taken from NixOS#412 to generate a minimal change
as an improvement of the current situation regarding remote building.

Adjusted to use "mkOverride 900" to make it consistent with all other backends.

If building a container on a non-linux system and "nixpkgs.system" is not set,
then we default now to "x86_64-linux".
@dhess
Copy link
Contributor

@dhess dhess commented Jul 17, 2018

Unless I've missed something, it appears that @johbo's nice patch (johbo@64d1636) is no longer sufficient to build containers on macOS when using a recent nixpkgs. I've applied that patch to my own NixOps fork and when I try to build a container, I get a failure here:

https://github.com/NixOS/nixpkgs/blob/6ea4c3d335c9c7cf121125f41820bb00aa3043b2/pkgs/top-level/stage.nix#L154

So for some reason, NixOps is trying to evaluate pkgsi686Linux and I assume that, because the host platform is not Linux, this assertion is failing. I'm guessing this has something to do with the cross-compiling support not working properly with containers and remote builds from a Mac. (It's strange that a container built for an x86_64-linux would try to evaluate pkgsi686Linux, though, no?)

betaboon added a commit to MapCaseGmbH/nixops that referenced this pull request Jul 24, 2018
Taken from NixOS#412 to generate a minimal change
as an improvement of the current situation regarding remote building.

Adjusted to use "mkOverride 900" to make it consistent with all other backends.

If building a container on a non-linux system and "nixpkgs.system" is not set,
then we default now to "x86_64-linux".
betaboon added a commit to MapCaseGmbH/nixops that referenced this pull request Sep 24, 2018
Taken from NixOS#412 to generate a minimal change
as an improvement of the current situation regarding remote building.

Adjusted to use "mkOverride 900" to make it consistent with all other backends.

If building a container on a non-linux system and "nixpkgs.system" is not set,
then we default now to "x86_64-linux".
@grahamc
Copy link
Member

@grahamc grahamc commented Mar 26, 2020

Hello!

Thank you for this PR.

In the past several months, some major changes have taken place in
NixOps:

  1. Backends have been removed, preferring a plugin-based architecture.
    Here are some of them:

  2. NixOps Core has been updated to be Python 3 only, and at the
    same time, MyPy type hints have been added and are now strictly
    required during CI.

This is all accumulating in to what I hope will be a NixOps 2.0
release
. There is a tracking issue for that:
#1242 . It is possible that
more core changes will be made to NixOps for this release, with a
focus on simplifying NixOps core and making it easier to use and work
on.

My hope is that by adding types and more thorough automated testing,
it will be easier for contributors to make improvements, and for
contributions like this one to merge in the future.

However, because of the major changes, it has become likely that this
PR cannot merge right now as it is. The backlog of now-unmergable PRs
makes it hard to see which ones are being kept up to date.

If you would like to see this merge, please bring it up to date with
master and reopen it
. If the or mypy type checking fails, please
correct any issues and then reopen it. I will be looking primarily at
open PRs whose tests are all green.

Thank you again for the work you've done here, I am sorry to be
closing it now.

Graham

@grahamc grahamc closed this Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

9 participants
You can’t perform that action at this time.