nix-community / nixops-gce

NixOps Google Cloud Backend
25 stars 7 forks source link

Network and image fixes #12

Closed talyz closed 3 years ago

talyz commented 3 years ago

Fix the following issues:

tewfik-ghariani commented 3 years ago

@talyz The current logic has been laid out in a way that the bootstrap image supports a gce-image to be used via the name attribute of an image object.

The "name" attribute in the imageOptions submodule accepts either the name of an existing image or an image-resource

Ref : https://github.com/nix-community/nixops-gce/pull/9/commits/3b6f24aafadeda27d8b5bc1d18ab2401382ee70a

So something like this should work :

machine = {
  deployment.gce = {
    bootstrapImage.name = resources.gceImages.custom-image;
  };
}

This should be enough since "family" and "project" default values are null

talyz commented 3 years ago

Yes, but it doesn't work, unfortunately, and it breaks backwards compatibility.

I also find it a rather confusing workflow to assign an image to the name attribute of imageOptions; you would assign the image to name, but then have to specify the project manually, even if that info is available in the image. imageOptions seems like a good alternative when you don't have a gce-image, but only one should be needed imo.

tewfik-ghariani commented 3 years ago

The project attribute is required when you would like to deploy from an image that is in a different project than the one used.

Basically, introducing the imageOptions and public bootstrap has the intention of using publicly available ready-to-use NixOs images created in the NixOs GCP project without the need for every end user to create their proper image ( similar to AWS EC2 AMIs )

More background details about these new concepts can be found in these PRs :

In case you would need to bootstrap your machines from a custom gce-image that you baked, either in your own project or a different one, you should theoretically simply specify the resource name in the name attribute of a given image

I believe to make this work, we should return the image object itself right here : https://github.com/nix-community/nixops-gce/blob/master/nixops_gcp/gcp_common.py#L81-L82 if it was of type 'gceImageResource'

Can you please give it a try and let me know how it goes?

talyz commented 3 years ago

I pushed an attempt at handling this at the python end instead. It also fixes various other bugs I encountered on the way. It's definitely still a bit broken, though: the type system is behaving really odd - the img is an ImageOptions object in gce_disks.py, but a ResourceEval in gce.py. I also tried changing the type to Union[ImageOption, GceImageOptions] in options.py, etc, but that resulted in img being a ResourceEval in gce_disks.py too and NixOps exiting with a TypeError:

TypeError: type of image must be one of (ImageOptions, GceImageOptions); got nixops.resources.ResourceEval instead
talyz commented 3 years ago

Also, I should add that I'm not at all opposed to the imageOptions submodule at all, if that's what it sounded like - I just think that it and the gce-image resource should be alternatives and specifying a gce-image for the name attribute of the submodule feels a bit strange. I was definitely confused by this moving from NixOps 1.7 to 2.0 and the error was very cryptic.

adisbladis commented 3 years ago

@tewfik-ghariani @AmineChikhaoui Could either of you take a look at this? I don't feel qualified to comment on the GCE specifics.

tewfik-ghariani commented 3 years ago

Sure, I'm working on a quick fix for the bootstrap image and I'll submit a PR soon