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

AWS VPC resources + Diff engine #697

Merged
merged 151 commits into from Jan 2, 2018
Merged

AWS VPC resources + Diff engine #697

merged 151 commits into from Jan 2, 2018

Conversation

AmineChikhaoui
Copy link
Member

@AmineChikhaoui AmineChikhaoui commented Jul 6, 2017

This PR adds integration for AWS VPC resources and an implementation of the ideas discussed in #648 .
The suggested way of implementing resources is by using a diff engine that can be instantiated using nixops.resources.ec2_common.setup_diff_engine() this diff engine will handle the comparison between the state and the target configuration and then generate a plan which is composed of the various handlers that needs to be called to realize the target state.
A resource can basically define the handlers and setup the right dependencies between them e.g self.handle_entries = Handler(['entries'], after=[self.handle_create_network_acl], handle=self.realize_entries_change)
This means that this handler will handle the entries option changes and will run after create_network_acl if they're invoked in the same deploy operation and will use the method realize_entries_change to realize the required changes.
There is an experimental option --plan-only (supported only for the VPC resources which can be added to the nixops deploy cmd to show the changes that will be made and exit.

Note that this doesn't implement a global DAG for the resources in a network but rather for the attributes handlers per resource.

For the VPC resources, what was already implemented is:

  • VPC
  • VPC subnet
  • VPC internet gateway
  • VPC egress only internet gateway
  • VPC NAT gateway
  • VPC DHCP options
  • VPC network ACL
  • Had to also rewrite the elastic IP resource to handle the vpc domain.
  • VPC customer gateway
  • VPC network interface
  • VPC network interface attachement
  • VPC route table
  • VPC route
  • VPC route table association
  • VPC endpoint
  • VPN connection/VPN gateway
  • Update ec2 instances + old resources to be able to accept a vpc resource in the vpcId options

The remaining work:

  • VPC peering connection (will be done in a separate PR due to some complexity in the resource state transitions)
  • Add examples/documentation.

I would appreciate some initial feedback on the suggested implementation of #648 though to be able to make appropriate changes if needed.

  - Experimenting with a different state represention based on NixOS#648
  - Experimenting with a diff structure that allows showing resource
    attributes changes before deploying if NIXOPS_PLAN env var is set.
* Resource VPC dhcp options: initial nix options
need to associate the default one before deleting the dhcp options.
the dhcp options. Use the new diff structure maybe ?
* Introducing a seperate Handler class, that allows a handler to
define a set of keys/options that it can manage and have an abstract
method handle to be implemented by developers adding resources. A
handler can also set the different dependencies that it needs to wait
for before being called.
* During plan operation, the diff engine chooses the minimal set of
handlers that are needed to realize the change then performs a topological
sort of the chosen handlers by representing them as a DAG based on the
dependencies between them.
inherit region accessKeyId;
vpcId = resources.vpc.vpc-nixops;
rules = [
{ toPort = 22; fromPort = 22; sourceIp = "41.231.120.171/32"; }
Copy link
Member

@Mic92 Mic92 Oct 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a check if fromPort is set otherwise the following error message is thrown:

AttributeError: 'NoneType' object has no attribute 'get'

from

from_port = int(rule_xml.find("attr[@name='fromPort']/int").get("value"))

UPDATE this problem is probably independent from this pull request. please ignore.

@Mic92
Copy link
Member

Mic92 commented Oct 20, 2017

Can you add an example how to add an elastic ip to a machine in a VPC subnet?

I tested the following: https://gist.github.com/Mic92/f262cbd1c386a15e8ffa166fda8a861f

a nixops deploy cmd is interrupted the user can run nixops check
or nixops deploy --check to have a consistent state.
@Mic92
Copy link
Member

Mic92 commented Nov 8, 2017

Removing ip addresses also does not work yet.

@AmineChikhaoui
Copy link
Member Author

@Mic92 can you share more details on the issue you had ? is this during a machine destroy or elastic ip destroy ?

@AmineChikhaoui
Copy link
Member Author

@domenkozar @rbvermaa I'm ok to merge this if there are no objections. I tried to address some of Eelco's comments and I'll create few issues for the remaining improvements to work on them later.
But I think globally it would be nice if we can merge this so that we can use VPC resources and use the diff engine for any new resource as it make it much easier to handle the state.
Let me know if you have any comments :)

@Mic92
Copy link
Member

Mic92 commented Nov 13, 2017

I have a machine, that I only start for benchmarks. As I have not found a way to remove an elastic ip, when the machine was stopped, I have removed it manually in the web interface. To assign a new ip I created a new resource in nixops each time. Now I have a number of obsolete nix-ip$i resources, I cannot get rid off:

$ nixops deploy -d cntr --kill-obsolete --show-trace --debug
resource ‘cntr-ip3’ is obsolete
resource ‘cntr-ip2’ is obsolete
resource ‘cntr-ip4’ is obsolete
resource ‘cntr-ip’ is obsolete
...
error: Multiple exceptions: list index out of range, list index out of range, list index out of range, list index out of range
------------------------------
Traceback (most recent call last):
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/parallel.py", line 41, in thread_fun
    result_queue.put((worker_fun(t), None))
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/deployment.py", line 1066, in worker
    if m.destroy(wipe=wipe): self.delete_resource(m)
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/resources/elastic_ip.py", line 109, in destroy
    eip = self.describe_eip()
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/resources/elastic_ip.py", line 103, in describe_eip
    return response['Addresses'][0]
IndexError: list index out of range
------------------------------
Traceback (most recent call last):
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/parallel.py", line 41, in thread_fun
    result_queue.put((worker_fun(t), None))
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/deployment.py", line 1066, in worker
    if m.destroy(wipe=wipe): self.delete_resource(m)
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/resources/elastic_ip.py", line 109, in destroy
    eip = self.describe_eip()
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/resources/elastic_ip.py", line 103, in describe_eip
    return response['Addresses'][0]
IndexError: list index out of range
------------------------------
Traceback (most recent call last):
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/parallel.py", line 41, in thread_fun
    result_queue.put((worker_fun(t), None))
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/deployment.py", line 1066, in worker
    if m.destroy(wipe=wipe): self.delete_resource(m)
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/resources/elastic_ip.py", line 109, in destroy
    eip = self.describe_eip()
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/resources/elastic_ip.py", line 103, in describe_eip
    return response['Addresses'][0]
IndexError: list index out of range
------------------------------
Traceback (most recent call last):
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/parallel.py", line 41, in thread_fun
    result_queue.put((worker_fun(t), None))
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/deployment.py", line 1066, in worker
    if m.destroy(wipe=wipe): self.delete_resource(m)
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/resources/elastic_ip.py", line 109, in destroy
    eip = self.describe_eip()
  File "/home/joerg/.homesick/repos/dotfiles/nixos/vms/nixops/nixops/resources/elastic_ip.py", line 103, in describe_eip
    return response['Addresses'][0]
IndexError: list index out of range

 eip api call doesn't throw a client error in case the eip doesn't
 exist and only return an empty list.
@AmineChikhaoui
Copy link
Member Author

@Mic92 thanks should be fixed in f6de2a9.

@Mic92
Copy link
Member

Mic92 commented Nov 13, 2017

@AmineChikhaoui thanks. that solved the issue.

@AmineChikhaoui
Copy link
Member Author

@domenkozar @rbvermaa Would you mind looking at this again. I've added few basic tests to tests/functional/vpc.py. Let me know if there are any specific tests you think should be added.

@edolstra edolstra merged commit f790182 into NixOS:master Jan 2, 2018
@AmineChikhaoui
Copy link
Member Author

yay 🎆 , thanks @edolstra

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants