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

Introduce Container Linux config additive customizations #140

Closed
6 of 7 tasks
dghubble opened this issue Feb 19, 2018 · 13 comments · Fixed by #269
Closed
6 of 7 tasks

Introduce Container Linux config additive customizations #140

dghubble opened this issue Feb 19, 2018 · 13 comments · Fixed by #269

Comments

@dghubble
Copy link
Member

dghubble commented Feb 19, 2018

Feature

Introduce the ability for users to supply Container Linux Config "snippets" for controllers or workers. Snippets are valid Container Linux Configs which are additively merged into the base Container Linux Configs that Typhoon uses to provision controllers and workers. Configs are validated, merged into a single config for use in instance user-data. Validation errors are shown with line numbers during terraform plan.

This is a major feature and has been long awaited.

  • Allows users to add custom systemd units, network configs, files, raid arrays, or other customizations related to on-host disk provisioning as supported by Container Linux Configs (at Ignition v2.0.0).
  • Reduces user temptation to add a Terraform variable for every option. This helps keep Typhoon modules readable and maintainable over the long run.
  • See the Container Linux Config spec and examples

I've made changes to the terraform-provider-ct provider plugin in v0.2.1 to enable this feature.

Caveats:

  • There are limits to what additive merging (technically append) will allow. You cannot remove bits of the base config added by Typhoon. This is part of the reason Typhoon focuses on minimalism.
  • Typhoon cannot make guarantees about different end-user customizations. We'll continue to design, build, and test clusters as usual, but cannot entertain reports concerning end-user's customizations (e.g. "I customized X and now Y is broken").

Components

  • Introductory Docs
  • Update "customization" docs. We now have a better story.
  • Guide for updating terraform-provider-ct plugin to v0.2.1
  • AWS
  • Digital Ocean
  • Google Cloud
  • Bare-Metal

Problems

  • terraform-provider-ct is not currently an official terraform provider. This actually presents substantial migration difficulties. Admins who already manage a bunch of clusters and then swap the provider binary on their system will notice diff's to those existing clusters. Applying those diffs can destroy clusters as terraform believes its task is to replace every node. I'm still investigating this.
@dghubble
Copy link
Member Author

dghubble commented Feb 26, 2018

The issue with terraform-provider-ct patch#22 causing Terraform diffs was determined to be related to the provider updating the Ignition version. See the full description.

In short, users can safely update between terraform-provider-ct v0.2.x series releases, which stick to the same Ignition version and won't cause a diff to existing provisioned clusters. I've released terraform-provider-ct v0.2.1 which contains the required snippets feature. Typhoon users should be able to drop-in replace their old terraform-provider-ct v0.2.0 plugin (referenced in ~/.terraformrc) with terraform-provider-ct v0.2.1. This removes the migration blocker.

@alban
Copy link

alban commented Feb 26, 2018

Typhoon users should be able to drop-in replace their old terraform-provider-ct v0.2.0 plugin (referenced in ~/.terraformrc) with terraform-provider-ct v0.2.1. This removes the migration blocker.

Btw, the current documentation explains how to install this in /usr/local/bin/terraform-provider-ct but recent terraform suggests to use ~/.terraform.d/plugins/OS_ARCH (~/.terraform.d/plugins/linux_amd64) instead of .terraformrc.

@dghubble
Copy link
Member Author

dghubble commented Mar 19, 2018

Thanks @alban, may incorporate that suggestion soon. For the moment, since we instructed users to use the ~/.terraformrc approach, during the migration to the new version I'd like to stick with that.

@dghubble
Copy link
Member Author

Container Linux Config snippets should be shipping for AWS, Digital Ocean, and Google Cloud in the next release (v1.9.5). I need to put some more work and thought into the bare-metal variant, its harder.

@bcg62
Copy link

bcg62 commented Jun 1, 2018

any progress for bare metal? it would be great to be able to natively set things like http/https proxy or custom CA's via this functionality. Its the main reason why I have to maintain a fork.

@dghubble
Copy link
Member Author

dghubble commented Jun 1, 2018

There are two paths forward I've been pondering for some time.

Background:

The story for cloud providers is simpler. Workers are homogenous, they are identical. Controllers are homogenous (at least, I feel ok accepting clc snippets that apply equally to all controllers). Any need for different groups of workers (e.g. stable vs alpha channel, instance type, GPU) is effectively handled by worker pools, groups of homogenous nodes that accept the same clc_snippets. Feature requests to treat any individual node as special and have a special clc_snippet for that node can be shot down.

On bare-metal there are plenty of legitimate use-cases where the clc snippet customizations must vary by node. For example, different nodes have different numbers of disks so you'd create disk arrays differently per-node. Or different nodes with different NIC arrangements (some of mine have 4 bonded NICs, some just have 2, some just have 1). In the same way we ask users to supply an ordered list of names, domains, mac addresses, to support customization on bare-metal properly we can't take just a list of snippets.

# insufficient on bare-metal
variable "controller_clc_snippets" {
  type        = "list"
  description = "Controller Container Linux Config snippets"
  default     = []
}

Flavor 1

To support real-world bare-metal use-cases where there are clusters with heterogenous hardware, we'd the clc snippet mechanism to be an optional list of lists, the outer list being sorted. So the first element is the list of snippets for the 0th controller, etc. It becomes tricky to read, explain to people, and reason about so I'm still kinda unhappy with what that would look like. Its not a question of whether we could, its whether there is a better way.

I'd like to revisit schemes to try to organize these fields as a list of maps if possible. It was unreasonable to represent in Terraform last I checked, so still in the "wait until it is right" mode.

Flavor 2

The 2nd path is to treat bare-metal like cloud providers and not permit per-node customization. Controllers must all have the same customization. Workers must all have the same customizations. This would be the fastest route to adding the feature today, but one that I'm quite unlikely to take. Bare-metal is different from cloud. This approach misses many real-world use-cases and this feels like the wrong approach.

I do care about this issue too. For one of my specialized bare-metal clusters (heterogeneous hardware), I too have a small fork https://github.com/dghubble/typhoon/tree/disk-retention and want to see the right solution in place.

@bcg62
Copy link

bcg62 commented Jun 4, 2018

Thanks for the detailed update.

I agree with the complexity of Flavor 1. I currently leverage controller_networkds/worker_networkds in this fashion to build bonds.

My hardware is heterogeneous, but luckily with systemd network wildcarding I can define them in a way that works on all hosts and having the list is somewhat redundant as each value is the same.

The same applies for the rest of my additional config. While being able to apply special configs per node can be handy, I often find there are ways to make a single unified definition work across the fleet. Granted I am not doing anything disk or storage related as you have mentioned.

At the moment Flavor 2 would be enough for me to throw away my branch.

@grozan
Copy link

grozan commented Jun 6, 2018

Very much looking forward to having this feature available on bare metal as well!

Maybe there is a third option, consisting of leveraging the fact that config templates can contain Go templates elements?

If it's made easy, using variables, to define node-specific metadata (that goes in the matchbox group file for that node), then I think it could be acceptable for all nodes to share the same snippets. It's in the snippet that, if required, some logic is put to behave differently based on the node's metadata. That's the user's responsibility to write them.

Example: I have nodes with 3 NICs, and on those I want to statically assign IPs in the 3 different subnets they are connected to. I run terraform apply to generate all the items, including the matchbox items.

Using controller_networkds, today, the following custom static snippet is pushed to ALL controllers

units:
  {{ $network_hostid := .network_hostid }}
  {{ range $i, $v := .network_interfaces }}
    {{ if ne $i 0 }}
    - name: 00-{{ $v }}.network
      contents: |
        [Match]
        Name={{ $v }}

        [Network]
        Address=10.100.{{ $i }}.{{ $network_hostid }}/24
        {{ if eq $i 1 }}
        DNS=10.100.1.100
        Gateway=10.100.1.1
        {{ end }}
    {{ end }}
  {{ end }}

The only thing I have left to do is, before booting a node, to edit its generated matchbox group file and add a metadata section in there, like so

{
  "id": "kube-dev-kube-dev-master1",
  "profile": "kube-dev-controller-kube-dev-master1",
  "selector": {
    "mac": "1a:3f:e9:09:b1:a1",
    "os": "installed"
  },
  "metadata": {
    "network_hostid": "34",
    "network_interfaces": ["lo","eth0","eth1","eth2"]
  }
}

This way I have host-specific config which is generated, based on the metadata associated with each node, even if the snippet is shared by all nodes.

An easy way to pass this metadata to Terraform is all that is missing, at least for that kind of use cases.

@grozan
Copy link

grozan commented Jun 10, 2018

I made a little skeleton of another potential implementation, which I think would be fairly simple to use from a end-user's perspective.

I applied it to custom networkd fragments only, using the existing controller_networkds and worker_networkds.

Of course something similar could be applied to other parts of the genereted ignition templates. Could be used to generate custom metadata as well (put in the matchbox group files)

It goes something like this.

A new variable is introduced, to tell if you want to define custom networkd config fragments or not.
If you do, you set it the the path of the folder containing those fragments.

In this folder, must be present one fragment per node (if a node does not require any custom config, an empty fragment called like the node must still exist)
Also, as nodes are usually not too much different within a cluster, there is quite a chance that the user might want to push the same customization to all his controllers, or all his workers.
To avoid repetition, two additional files exist in the fragments folder: controllers and workers, appended to all controller and worker nodes, respectively.

And that's basically it. If you activate the feature, you drop fragments in the folder (one fragment per node) and they're put in the generated ignition templates for the respective nodes.

Implementation

new variable

variable "networkds_dir" {
  type        = "string"
  default     = ""
  description = "The path to the folder containing the networkds snippets. If defined, EVERY machine must have one (can be empty)"
}

generate the list of fragments

data "template_file" "controller_networkds_config" {
  template = "${join("",list("${file("${var.networkds_dir}/${element(var.controller_names,count.index)}")}","${file("${var.networkds_dir}/controllers")}"))}"
  count = "${length(var.networkds_dir) == 0 ? 0 : length(var.controller_names)}"
}

data "template_file" "worker_networkds_config" {
  template = "${join("",list("${file("${var.networkds_dir}/${element(var.worker_names,count.index)}")}","${file("${var.networkds_dir}/workers")}"))}"
  count = "${length(var.networkds_dir) == 0 ? 0 : length(var.worker_names)}"
}

and finally pass them along to Typhoon

module "kube-dev" {
  source = "git::https://github.com/poseidon/typhoon//bare-metal/container-linux/kubernetes?ref=v1.10.3"
  ...
  ...
  controller_networkds = [ "${data.template_file.controller_networkds_config.*.rendered}" ]
  worker_networkds = [ "${data.template_file.worker_networkds_config.*.rendered}" ]
}

What do you think?

@dghubble
Copy link
Member Author

dghubble commented Jul 26, 2018

I wanted a solution that supported merging multiple Container Linux config snippets, with potentially different sets for each bare-metal machine. Much more real-world testing is needed, but I believe #269 will achieve this goal.

Rejected Approaches

A list of lists turns out to be more difficult than imaged. Even working around the usual HCL type system problems (conditionals evaluate both sides of expressions which surprises most, empty lists therefore need a concat), Terraform element may only be used with flat lists. This idea appears to work at first glance (default no custom snippets, allows some machines to be customized, some not, etc.) but if you try to provide multiple snippets to a single machine, any after the first are silently ignored because element doesn't understand lists of lists.

# Hack to permit element(list, index) on empty list since Terraform evaluates regardless of
# condition. Warning, Terraform element(list, N+1) wraps around. Provide 0 or N sets of
# snippets where N is the number of workers.
snippets = ["${length(var.controller_clc_snippets) > 0 ? element(concat(var.controller_clc_snippets, list("")), count.index) : "\n"}"]

A map of lists keyed by machine name seems promising. There is a lot of subtlety here too. First we merge with a dummy map to workaround the issue where lookups on an empty map fail and conditionals evaluate both sides. Next, lookup can only be used with flat maps and will intentionally error may only be used with flat maps, this map contains elements of type list.

locals {
  # Hack to workaround https://github.com/hashicorp/terraform/issues/17251
  worker_clcs = "${merge(var.worker_clc, map("donotuse", "donotuse"))}"
}
snippets = ["${lookup(local.worker_clcs, element(var.worker_names, count.index), "\n")}"]

However, using bracket notation local.worker_clcs[machine_name] can handle non-flat maps. It gets the list of snippets for the machine. This was a promising mechanism. There is the additional complication that bracket lookups must never fail.

Candidate

#269 uses a map of lists with clever defaulting and overriding. I used a zipmap approach to ensure there is a key for every controller/worker with a default (that must be list("\n")). User customizations are done by filling out the controller_clc_snippets or worker_clc_snippets map (as opposed to list of clouds) to override the default empty snippet. This supports the default of do nothing and allows individual machines to have one or more snippets applied. If you care, give #269 a whirl.

@dghubble
Copy link
Member Author

@grozan with the approach in #269, any valid Container Linux Config snippets are accepted - they can come from local files or from string outputs from your own modules or something you devise, such as using Go templating.

@Malinskiy
Copy link

@dghubble Hey. This is an awesome feature for bare-bone clusters. However sometimes one requires the same snippets applied to coreos-installer also, for example if the OS drive name should be the same on all the machines, but some hardware has NVMe and some SATA, hence the need to write custom udev rules, but it's not possible currently to cleanly implement this.

Basically it would be awesome to have the snippets support for installer configs also.

@dghubble
Copy link
Member Author

On bare-metal, for common snippets, you can put them in a list and reference that list using existing Terraform mechanisms. #279 (comment)

Snippets aren't supportable in the install phase today. Snippets use the Matchbox raw Ignition API (snippet merges/validation are now done where you run terraform) rather than the Container Linux Config API, raw Ignition serving doesn't evaluate Matchbox-specific variables, and the .request.raw_request is a Matchbox-specific variable thats essential for gluing together iPXE and later phases. Years ago, Matchbox performed templating, but now Terraform now performs that, except that one variable still has to be at runtime.

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

Successfully merging a pull request may close this issue.

5 participants