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

Support multiple versions of nixpkgs in one network #665

Closed
wants to merge 1 commit into from

Conversation

@grahamc
Copy link
Member

@grahamc grahamc commented May 13, 2017

Having a machine named foo, described as:

foo = { # ...snipped...
  deployment.nix_path.nixpkgs = (builtins.filterSource
    (path: type: type != "directory" || baseNameOf path != ".git")
    ./../nixpkgs);
});

will have the custom nixpkgs set in the NIX_PATH as
nixpkgs=path-to-custom-nixpkgs.

Note this does not work with foo = { config, ... }: {... machines, but
having a second nix file in the network would work, and also:

let
  canary = machine: {
    deployment.nix_path.nixpkgs = (builtins.filterSource
      (path: type: type != "directory" || baseNameOf path != ".git")
      ./../nixpkgs);
    imports = [machine];
  };

  machine = { ... }: {
    # your machine config
  };
in {
  machineA = machine;
  machineB = canary machine;
}

Note that because this uses scopedImport, the nixops network and
machines may use import <nixpkgs> and have a consistent view of
nixpkgs.

Closes #649 (comment)

Having a machine named foo, described as:

    foo = { # ...snipped...
      deployment.nix_path.nixpkgs = (builtins.filterSource
        (path: type: type != "directory" || baseNameOf path != ".git")
        ./../nixpkgs);
    });

will have the custom nixpkgs set in the `NIX_PATH` as
`nixpkgs=path-to-custom-nixpkgs`.

Note this does not work with foo = { config, ... }: {... machines, but
having a second nix file in the network would work, and also:

    let
      canary = machine: {
        deployment.nix_path.nixpkgs = (builtins.filterSource
          (path: type: type != "directory" || baseNameOf path != ".git")
          ./../nixpkgs);
        imports = [machine];
      };

      machine = { ... }: {
        # your machine config
      };
    in {
      machineA = machine;
      machineB = canary machine;
    }

Note that because this uses scopedImport, the nixops network and
machines may use `import <nixpkgs>` and have a consistent view of
nixpkgs.
@grahamc
Copy link
Member Author

@grahamc grahamc commented May 13, 2017

Thank you, @cleverca22, for the excellent suggestions & help with scopedImport.

@grahamc
Copy link
Member Author

@grahamc grahamc commented May 13, 2017

I'd love a review by @edolstra and @rbvermaa.

@domenkozar domenkozar added the feature label May 14, 2017
@ip1981
Copy link
Contributor

@ip1981 ip1981 commented May 15, 2017

Multiple nixpkgs look like an overkill to me. Usually overriding individual packages and using custom modules is enough.

@grahamc
Copy link
Member Author

@grahamc grahamc commented May 15, 2017

Depends on the size and variety of the network. Three critical use cases of mine:

  1. Half x86-64 nodes (stable), half aarch64 nodes (a patched master). This requires completely separate nixpkgs versions.

  2. Canary and beta deployments, allowing rolling out a nixpkgs update in a staged / predictable pattern.

  3. Modules which carry patches and can't be upstreamed

I'm practically unable to use nixops without this patch, which I'm doing already:

(pkgs.nixops.overrideAttrs (x: {
      patches = [
        (pkgs.fetchpatch {
          # Allow specifying custom nixpkgs for a machine
          url = "https://github.com/NixOS/nixops/pull/665.patch";
          sha256 = "0q7sk3fq3x7r2sh6f853hcbykm74px9i0m5bqhg2fi0s4nckj5x0";
        })
      ];
    }))
@@ -103,6 +103,13 @@ in
'';
};

deployment.nix_path = mkOption {
default = {};
type = types.attrsOf types.str;

This comment has been minimized.

@ip1981

ip1981 May 16, 2017
Contributor

Maybe path is more adequate?

This comment has been minimized.

@edolstra

edolstra May 16, 2017
Member

It should be nixPath. (NixOS already has a nix.nixPath option.)

Arguably the type should be types.listOf types.str, like nix.nixPath. Nix search path entries are not required to have a prefix.

@ip1981
Copy link
Contributor

@ip1981 ip1981 commented May 16, 2017

Does it work well with modules, overrides, etc.? I mean, if I use some custom modules overriding some pkgs, which pkg set will be overridden?

@ip1981
Copy link
Contributor

@ip1981 ip1981 commented May 16, 2017

And, of course, there is a plain workaround - different deployments with different nix path :)

@grahamc
Copy link
Member Author

@grahamc grahamc commented May 16, 2017

modules

It works well with export NIXOS_EXTRA_MODULE_PATH=...

overrides

like pkgs.my-package.overrideAttrs ? Can you provide an example of this use so I can be sure?

different deployments with different nix path :)

Yes, different deployments or nixops deploy with different NIX_PATH in the environment variables / --exclude / --include parameters, but I think I can interpret your :) as not needing an explanation on why that is painful :P / not very nice.

@ip1981
Copy link
Contributor

@ip1981 ip1981 commented May 16, 2017

like pkgs.my-package.overrideAttrs ? Can you provide an example of this use so I can be sure?

I mean nixpkgs.config.packageOverrides for a while:

{
  # ...
  machineFoo = {config, ...} : {
    imports = [ <my-modules> ]; # with `nixpkgs.config.packageOverrides`, probably recursive.
    # ...
  };
  # ...
}

My point is to be able to use hierarchy of modules (via imports) in which topmost modules (who imports) can override packages defined in underlying modules (who is imported). At all levels. So no one is locked to nixpkgs.

@ip1981
Copy link
Contributor

@ip1981 ip1981 commented May 16, 2017

As far as I understand, you have to evaluate machine's spec in order to get it's specific nix path. And once evaluated, new nix path will override all packages overridden during evaluation. Correct?

P. S. Probably I'm wrong :)


importSources =
(concatMap (module:
if (!builtins.isFunction module

This comment has been minimized.

@ip1981

ip1981 May 16, 2017
Contributor

Why can't it be a function? Is module a "machine" definition? If yes, it usually a function.

This comment has been minimized.

@grahamc

grahamc May 16, 2017
Author Member

See #665 (comment): we can't introspect a function in this way.

@grahamc
Copy link
Member Author

@grahamc grahamc commented May 16, 2017

As far as I understand, you have to evaluate machine's spec in order to get it's specific nix path.

No, we don't evaluate any functions to find the nix path. This also answers your question about why it can't be a function: We can't call a machine's function without passing in a pkgs, and we don't have that when we're looking for which nixpkgs to include.

For this reason, the imports and overrides all work just fine.

Examples, this works:

{
  machine = {
    deployment.nix_path.nixpkgs = "https://github.com/nixos/nixpkgs/archives/master.tar.gz";
    nixpkgs.config.packageOverrides = {};
  };
}

This won't work, because it is a function and we can't fetch the nix_path without first executing the function:

{
  machine = { ... }: {
    deployment.nix_path.nixpkgs = "https://github.com/nixos/nixpkgs/archives/master.tar.gz";
  };
}

This will work:

{
  machine = {
    deployment.nix_path.nixpkgs = "https://github.com/nixos/nixpkgs/archives/master.tar.gz";
    imports = [{ ... }: {
      nixpkgs.config.packageOverrides = {};
    }];
  };
}
@ip1981
Copy link
Contributor

@ip1981 ip1981 commented May 16, 2017

Ok, I think I understood.

What you are doing is exactly nixops ... -I ... --include ..., but you do it in Nix, not in Bash :)
But since you do it in Nix with modules and options it might suggest, that one can use all the power of modules. But I believe that is not true. You have to evaluate modules to extract nix path, then evaluated again with new path. At least this is what I'd expect from modules. If you don't evaluate modules to extract nix path it's dirty cheating :)

I remember a try to make nixops modular with a means to define nix path. This didn't end well. See https://github.com/zalora/nixops.

I think wrappers of nixops is more flexible, safer and cleaner solution (nixops ... -I ... --include ...)

@ip1981
Copy link
Contributor

@ip1981 ip1981 commented May 16, 2017

NixOS modules is an extremely cool and powerful mechanism, maybe underestimated yet. So I'd say no to features that do not integrate flawlessly into module system (like not working with functions) ¯\_(ツ)_/¯

@grahamc
Copy link
Member Author

@grahamc grahamc commented May 16, 2017

But I believe that is not true.

Yes, that is true :( a mkForce inside a module utilized from a function will not work. That is not very nice.

What if we took advantage of the network nixops attrset key:

{
  network = {
    description = "Load balancer test";
    default_nix_path = {
      nixpkgs = "./stable-nixpkgs";
    };
    nix_path_overrides = {
      machineB = {
        nixpkgs = "./unstable-nixpkgs";
      };
    };
  };

  machineB = { ... }: {};
}

Now it is out of band as far as the machine is aware, yet declared in the file.

@domenkozar
Copy link
Member

@domenkozar domenkozar commented May 16, 2017

Maybe network.nixpath and deployment.nixpath?

@grahamc
Copy link
Member Author

@grahamc grahamc commented May 16, 2017

@ip1981
Copy link
Contributor

@ip1981 ip1981 commented May 16, 2017

Well, yes. To me, the only feasible way is to have nix path defined somewhere out of machine's "mega-module". I don't really care if it would be bash or nix :).

You can define you path.nix mapping machines to nix path, and then ... build a wrapper or whatever :) Even make use of nix-shell.


__nixPath = importSources;

machineImport = builtins.scopedImport {

This comment has been minimized.

@edolstra

edolstra May 16, 2017
Member

IIRC, scopedImport slows down evaluation because it disables the file cache in the evaluator. If that's still the case, machineImport should be something like machineImport = if __nixPath == [] then import else builtins.scopedImport ....

@edolstra
Copy link
Member

@edolstra edolstra commented May 16, 2017

Semantically, it would be cleanest to move the NixOS configuration of a machine to an option below the machine, so we can add options like nixPath to the machine that are not part of the NixOS configuration. E.g.

machines.foo =
  { nixPath = ...;
    nixosConfig = { config, pkgs, ... }:
      { environment.systemPackages = [ pkgs.foo ];
      };
  };

This allows machines (and the entire NixOps network specification) to be defined using the module system, e.g.

machines = mkOption {
  type = types.attrsOf (submodule {
    options = {
      nixPath = mkOption { ... };
      nixosConfig = mkOption { ... };
      ...
    };
  };
};

This is more elegant than having a separate network.nixPathOverrides option.

However it's not clear how to do this in a backwards-compatible way...

@grahamc
Copy link
Member Author

@grahamc grahamc commented May 16, 2017

If I manage to make a PR implementing @edolstra's suggestion, can that possibly merge? (cc @rbvermaa)

@moretea
Copy link
Contributor

@moretea moretea commented Jun 14, 2017

We could deprecate the current way, and add a compatibility layer.

This compatibility layer will just map over the attr/value pairs, and convert them to a format that is compatible with the module based version.

Would that be acceptable @edolstra @rbvermaa @grahamc?

@mickours
Copy link

@mickours mickours commented Apr 13, 2018

I'm really interested in this feature. Any chance this would be merged?

@timclassic
Copy link

@timclassic timclassic commented Oct 15, 2018

I am also interested in this feature. @grahamc, did you ever move forward with your PR attempt?

@grahamc
Copy link
Member Author

@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.

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