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

Linode support for NixOps. #721

Closed
wants to merge 2 commits into from
Closed

Linode support for NixOps. #721

wants to merge 2 commits into from

Conversation

@GlennS
Copy link
Contributor

@GlennS GlennS commented Aug 23, 2017

Linode support. Fixes #198.

It works as follows:

  1. Deploy Debian using a standard Linux kernel (not one of Linode's special modifed ones).
  2. SSH in and run nixos-infect.
  3. Set the machine to boot from the disk you just installed Nixos on, then restart.

Current Status According to Me

Not ready for merge:

  • One failing test (tests/functional/test_send_keys_sends_keys.py)
  • Need to stop maintaining a separate copy of nixos-infect, and maintain a patch instead. Depends on: #820
  • Linode changed their API, so it's broken at the moment. Fix in: NixOS/nixpkgs#33928

Tests

I have run the tests on Linux using python2 tests.py -A linode. To run them you need to set your personal API key, which you can do in one of two ways:

  • edit tests/functional/single_machine_linode_base.nix to have my personalAPIKey set
  • set environment variable LINODE_PERSONAL_API_KEY

The tests take a lot of time to run.

@dhess
Copy link
Contributor

@dhess dhess commented Aug 24, 2017

I think current practice (followed by the ec2 and digitalocean backends, anyway) is not to store the API key in the NixOps state database, but to require the user to set the appropriate environment variable when creating/destroying instances.

@GlennS GlennS force-pushed the cse-bristol:linode branch from 42c1076 to 62a4d6f Aug 24, 2017
@GlennS
Copy link
Contributor Author

@GlennS GlennS commented Aug 24, 2017

I thought it might be something like that.

I've made a change to not store the Linode personal API key in the database.

Instead it will look first in deployment.linode.personalAPIKey, and then in env variable LINODE_PERSONAL_API_KEY.

To make this work, the MachineState has to call depl.evaluate() on MachineState.__init__, to force it to load all the properties from the user supplied definition files.

@domenkozar
Copy link
Member

@domenkozar domenkozar commented Oct 3, 2017

Is this ready for testing?

@GlennS
Copy link
Contributor Author

@GlennS GlennS commented Oct 9, 2017

domenkozar: it works for me at the moment if you want to try it out.

@rbvermaa
Copy link
Member

@rbvermaa rbvermaa commented Nov 14, 2017

I will run the tests today on my linode account and merge if everything is working.

@mhsjlw
Copy link

@mhsjlw mhsjlw commented Dec 20, 2017

See #820 to discuss nixos-infect and nixos-infect.linode

@GlennS GlennS force-pushed the cse-bristol:linode branch from 62a4d6f to 6487470 Jan 12, 2018
@GlennS GlennS force-pushed the cse-bristol:linode branch from 6487470 to 9bc8207 Jan 16, 2018
Also now waits when provisioning and starting up, since I was getting 'API not ready' errors.
@GlennS
Copy link
Contributor Author

@GlennS GlennS commented Jan 16, 2018

I don't really understand test_send_keys_sends_keys.py.

single_machine_secret_key.nix defines a key, but says storeKeysOnMachine = false;.

The first thing the test does after deployment is check if that key is stored on the machine using test -f /run/keys/secret.key.

Can anyone explain?

@rbvermaa
Copy link
Member

@rbvermaa rbvermaa commented Jan 22, 2018

Getting the following stack trace when running tests:

Traceback (most recent call last):
  File "/nix/store/bmkc8xrrrdi59sp0487rwj3cs1wrwcyc-python2.7-nose-1.3.7/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/rbvermaa/src/nixops/tests/functional/single_machine_test.py", line 59, in test_linode
    self.run_check()
  File "/home/rbvermaa/src/nixops/tests/functional/test_rollback_rollsback.py", line 22, in run_check
    self.depl.deploy()
  File "/home/rbvermaa/src/nixops/nixops/deployment.py", line 1036, in deploy
    self.run_with_notify('deploy', lambda: self._deploy(**kwargs))
  File "/home/rbvermaa/src/nixops/nixops/deployment.py", line 1028, in run_with_notify
    f()
  File "/home/rbvermaa/src/nixops/nixops/deployment.py", line 1036, in <lambda>
    self.run_with_notify('deploy', lambda: self._deploy(**kwargs))
  File "/home/rbvermaa/src/nixops/nixops/deployment.py", line 975, in _deploy
    nixops.parallel.run_tasks(nr_workers=-1, tasks=self.active_resources.itervalues(), worker_fun=worker)
  File "/home/rbvermaa/src/nixops/nixops/parallel.py", line 44, in thread_fun
    result_queue.put((worker_fun(t), None, t.name))
  File "/home/rbvermaa/src/nixops/nixops/deployment.py", line 948, in worker
    r.create(self.definitions[r.name], check=check, allow_reboot=allow_reboot, allow_recreate=allow_recreate)
  File "/home/rbvermaa/src/nixops/nixops/backends/linode_backend.py", line 259, in create
    plan = LinodeState.get_plan(client, defn.type_id)
  File "/home/rbvermaa/src/nixops/nixops/backends/linode_backend.py", line 72, in get_plan
    for t in client.linode.get_types():
TypeError: 'bool' object is not iterable

Any idea why? get_types() seems to return False

@GlennS
Copy link
Contributor Author

@GlennS GlennS commented Jan 22, 2018

Hi rbvermaa, sorry my fault for not mentioning. This is because it currently also depends on this change to nixpkgs NixOS/nixpkgs#33928

Linode changed the behaviour of the HTTP endpoint for their API, breaking the previous version.

@GlennS
Copy link
Contributor Author

@GlennS GlennS commented Apr 5, 2018

Status update:

Linode have released their final set of breaking changes to their HTTP API. I'm waiting for them to update their Python library that calls that API, then I'll finish this up.

@rbrewer123
Copy link

@rbrewer123 rbrewer123 commented Apr 20, 2018

I tested this PR running nixops master on NixOS 18.03. I used the linode-api package from the nixos-unstable channel which is at version 4.1.8b1 of the upstream linode-api for Python.

I was able to create the single-node webserver network from the beginning of the nixops manual. I tried create, deploy, check, ssh, destroy, and delete. It all worked pretty well and I was able to browse to the webserver after deploying.

I noticed 2 minor issues compared to running the webserver tutorial with the virtualbox backend. One is that some parts of the deployment seemed to be pulling from 17.09 instead of 18.03. The other is that nixops check showed one of the systemd units failed (binfmt something?). Unfortunately I didn't write it down at the time.

This looks very promising and I hope it can be merged soon. I'm already finding it useful.

@tomberek
Copy link
Contributor

@tomberek tomberek commented May 4, 2018

Anything that needs to be tested?

@attente
Copy link

@attente attente commented Dec 28, 2018

Hello, I was curious about this feature and came across your PR. Is there still a chance this will be merged?

@GlennS
Copy link
Contributor Author

@GlennS GlennS commented Dec 31, 2018

Hi attente, the answer is 'maybe', depending on whether I can find some time to do the work needed to finish it.

The Linode API looks like it has settled down a bit now, so what I need to do is:

  • Update the Python Linode API package to a recent version.
  • Probably rebase this pull request to a more recent version of NixOps.
  • Look again at the copy of the nixos-infect script and see if it needs any tidying. (Probably won't merge this in with the DigitalOcean one after all).
  • Read the latest documentation for the Linode API and compare to the work I did to see if there are any changes.
  • Run the tests again and make sure everything is working.
@kristoff3r
Copy link

@kristoff3r kristoff3r commented Jan 7, 2019

I have used this work a bit, and there doesn't seem to be a lot left to do.

  • The rebase with master is trivial, and everything builds and works after adding linode to setup.cfg
  • The Linode API only has one minor and one patch version bumps since the latest NixOS version, so I guess the changes should be minor?
  • The DigitalOcean nixos-infect hasn't changed since this pull request was last updated. It does seem to need some tidying, but I personally haven't had any problems using it.

It would be nice to get this over the finish line. I'd like to help, but I'm pretty new to both nixops internals and linode, so I don't really know how to test it comprehensively.

@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
@GlennS
Copy link
Contributor Author

@GlennS GlennS commented Mar 26, 2020

To be fair, I should've finished this thing 2 years ago.

I guess I'm going to be spending some time at home for a while, so maybe I can look at redoing this for the new plugin architecture.

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.

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