Lokomotive: Clarify scope of "cluster" and "components" configuration blocks

[invidian]

Currently, all cluster configuration is stored in the single cluster block, including all worker pools etc. This seem to have some limitations (#96).

Next to cluster block, we have component blocks, which represents multiple components.

Are components part of the "cluster" our configuration "describes"? From the CLI functionality perspective, it seems they are, as all subcommands only allow to operate on a single cluster, meaning configuration/working directory currently controls to which cluster you install the components.

However from HCL representation, cluster and components are one the same level, which seems confusing to me. Maybe it would make more sense to rename cluster block to infrastructure? Or kubernetes ?

Another look at it:

root { <-- This is what currently lokoctl configuration describes. How should this be named?
           Intuitively, it should be `cluster`, but then having `cluster` block inside `cluster block seems confusing too.
  cluster {}
  component "A" {}
  component "B" {}
}

So, what does lokoctl configuration represents?

[johananl]

FWIW, my opinion is that components should be a property of a cluster, and therefore nested under cluster {}. IMO they are meaningless on their own. However, I think we have different opinions in the team, mainly around the flexibility of breaking down cluster configuration into multiple HCL files.

[invidian]

FWIW, my opinion is that components should be a property of a cluster, and therefore nested under cluster {}.

And what if user wants to install the components on the cluster, which is not managed by Lokomotive? We should have a way to represent that too then.

[johananl]

I wasn't aware this was a requirement. But anyway, this can be solved, for example, by adding a "no-op" platform:

cluster "self-managed" {
  component "x" {}
}

[surajssd]

How important of a concern this is? Are there any users who wanted to see a change like this or they felt that the config as it is complicated to understand and components and cluster don’t align i.e. it is hard to understand the relationship between the cluster and the components block?

[invidian]

How important of a concern this is? Are there any users who wanted to see a change like this or they felt that the config as it is complicated to understand and components and cluster don’t align i.e. it is hard to understand the relationship between the cluster and the components block?

This is more developer-oriented question and focused on code organization and abstraction, rather than the configuration represented to the user.

[johananl]

How important of a concern this is? Are there any users who wanted to see a change like this or they felt that the config as it is complicated to understand and components and cluster don’t align i.e. it is hard to understand the relationship between the cluster and the components block?

This is more developer-oriented question and focused on code organization and abstraction, rather than the configuration represented to the user.

This is not what I understood from the description. Looks to me like you're suggesting a change to the cluster configuration. If your intention is to discuss an implementation detail, let's be more clear about this.

[invidian]

This is not what I understood from the description. Looks to me like you're suggesting a change to the cluster configuration. If your intention is to discuss an implementation detail, let's be more clear about this.

Right, OP suggests a change of the configuration block names, but this is because we currently use the same structure for HCL configuration and Go structures for components and cluster configuration. However, those 2 are logically separate entities (which we convert from one to another using DecodeBody() function).

My main focus here though is not the implementation or configuration block names, but the clarity on the concept of our configuration, which I think right now consist of 2 conflicting approaches.

I have one more idea, maybe our configuration describes "Lokomotive stack"? Because then it make sense to say, that it consist of Kubernetes cluster and the components.

[johananl]

Right. So as I've said above I think that a component should be a cluster property and I also think a component is meaningless outside the context of a cluster. Therefore the thing which makes the most sense to me is to nest component blocks under cluster (at the configuration level).

The only downside of this ^ I can see is that it increases the size of the cluster block, which is IMO way better than having a logical model which doesn't represent the reality. To clarify, the reality is that a component cannot exist without a cluster, and can never be managed separately from a cluster. True, we can deploy/update/remove a component without changing the underlying cluster, but in practice what the operation does is "make this cluster have component X" or "make this cluster run component Y at version Z". Hence a component is a cluster property.

Lastly, I'm aware that this disallows managing clusters in config file A and components in config file B. To me personally it doesn't make sense to manage configuration this way as I think it's more understandable to organize configuration by its "meaning" rather than its "category". As a user, I never find myself asking "what are all the components I have on all of my clusters?", I find myself asking "what components currently exist on this cluster?". This is why I suggest that cluster be an atomic config structure.

[invidian]

Right. So as I've said above I think that a component should be a cluster property and I also think a component is meaningless outside the context of a cluster. Therefore the thing which makes the most sense to me is to nest component blocks under cluster (at the configuration level).

I agree. This seem to me like a reasonable thing to do. This would also allow us to easily add support for multiple clusters, as each cluster object could be referenced by name.

Thanks for your thoughts @johananl.

[ipochi]

I agree with the reasoning here that component is a property of a cluster.

With that being said, It makes much more sense now to organize the cluster block with sub blocks for readability, rather than an unorganized block.

ex: components, worker_pool, controller, os, network ? @johananl what do you think ?

[johananl]

@ipochi I have to admit it currently doesn't make a lot of sense to me to nest knobs inside HCL blocks. I'm assuming your motivation for suggesting this is "it's hard to read the config when all the knobs are at the top level, so breaking things down to categories would make things more readable". Let's explore this idea.

Here are all the knobs for our most complex platform (Packet):

cluster "packet" {
  auth_token = var.packet_token
  asset_dir = var.asset_dir
  cluster_name = var.cluster_name
  controller_count = var.controllers_count
  controller_type = "baremetal_0"
  facility = var.facility
  os_channel = "stable"
  os_version = "current"
  os_arch = "amd64"
  ipxe_script_url = ""
  project_id = var.packet_project_id
  dns {
    zone = var.dns_zone
    provider {
      route53 {
        zone_id = var.route53_zone_id
      }
    }
    # manual {}
  }
  ssh_pubkeys = var.ssh_public_keys
  management_cidrs = var.management_cidrs
  node_private_cidr = var.node_private_cidr
  cluster_domain_suffix = "cluster.local"
  network_mtu = 1480
  tags {
    key1 = "value1"
    key2 = "value2"
  }
  enable_aggregation = true
  enable_reporting = false
  network_ip_autodetection_method = "first-found"
  pod_cidr = "10.2.0.0/16"
  service_cidr = "10.3.0.0/16"
  reservation_ids = { controller-0 = "55555f20-a1fb-55bd-1e11-11af11d11111" }
  reservation_ids_default = ""
  certs_validity_period_hours = 8760
  worker_pool "worker-pool-1" {
    count = var.workers_count
    ipxe_script_url = ""
    os_arch = "amd64"
    disable_bgp = false
    node_type = var.workers_type
    os_channel = "stable"
    os_version = "current"
    labels = "foo=bar,baz=zab"
    taints = "nodeType=storage:NoSchedule"
    setup_raid = false
    setup_raid_hdd = false
    setup_raid_ssd = false
    setup_raid_ssd_fs = false
  }
}

I agree, that's a mouthful. Now let's divide all of them into "categories":

cluster "packet" {
  lokoctl {
    asset_dir = var.asset_dir
  }
  cloud_provider {
    auth_token = var.packet_token
    controller_count = var.controllers_count
    controller_type = "baremetal_0"
    facility = var.facility
    ipxe_script_url = ""
    project_id = var.packet_project_id
    ssh_pubkeys = var.ssh_public_keys
    management_cidrs = var.management_cidrs
    node_private_cidr = var.node_private_cidr
    tags {
      key1 = "value1"
      key2 = "value2"
    }
    reservation_ids = { controller-0 = "55555f20-a1fb-55bd-1e11-11af11d11111" }
    reservation_ids_default = ""
  }
  general {
    cluster_name = var.cluster_name
  }
  flatcar {
    os_channel = "stable"
    os_version = "current"
    os_arch = "amd64"
  }
  dns {
    zone = var.dns_zone
    provider {
      route53 {
        zone_id = var.route53_zone_id
      }
    }
    # manual {}
  }
  k8s {
    cluster_domain_suffix = "cluster.local"
    network_mtu = 1480
    enable_aggregation = true
    enable_reporting = false
    network_ip_autodetection_method = "first-found"
    pod_cidr = "10.2.0.0/16"
    service_cidr = "10.3.0.0/16"
    certs_validity_period_hours = 8760
  }
  worker_pool "worker-pool-1" {
    count = var.workers_count
    ipxe_script_url = ""
    os_arch = "amd64"
    disable_bgp = false
    node_type = var.workers_type
    os_channel = "stable"
    os_version = "current"
    labels = "foo=bar,baz=zab"
    taints = "nodeType=storage:NoSchedule"
    setup_raid = false
    setup_raid_hdd = false
    setup_raid_ssd = false
    setup_raid_ssd_fs = false
  }
}

Now, is the 2nd option actually more readable than the 1st? Possibly. However, there are problems with this approach:

• Not all knobs fall nicely into categories. For example, should os_channel be under flatcar or under cloud_provider? It's a Flatcar-related knob but it also determines which provider-specific image to use for installation.
• As an elaboration of the previous point, over time I see one of two things happening: either we end up with tons of "categories", some of them containing only one or two knobs, or we end up with a misc category :cold_sweat:
• We will very likely face flexibility problems we wouldn't face with the current structure. Being forced to choose the right "category" for each knob makes things much more rigid, which means we will be much more likely to be forced to make breaking changes to our config scheme as we release new Lokomotive versions.
• We would actually be making the config file longer. By adding multiple blocks (which means more brackets and more newlines), there is simply more text in the config file. We should decide if we're OK with this.

Now that we are aware of some of the problems this approach has, I want to say it does make sense to me on certain aspects to break things down. One such aspect is that things would be much easier for us as maintainers if the config structure mirrors the underlying data structures in the lokoctl code. But this argument isn't important enough to make the UX worse IMO.

Lastly, I want to emphasize that the examples above use all of the knobs, which isn't a realistic scenario. For comparison, here is a minimal config with just the required knobs specified:

cluster "packet" {
  asset_dir = var.asset_dir
  cluster_name = var.cluster_name
  facility = var.facility
  project_id = var.packet_project_id
  dns {
    zone = var.dns_zone
    provider {
      route53 {
        zone_id = var.route53_zone_id
      }
    }
  }
  ssh_pubkeys = var.ssh_public_keys
  management_cidrs = var.management_cidrs
  node_private_cidr = var.node_private_cidr
  worker_pool "worker-pool-1" {
    count = var.workers_count
  }
}

And here is a minimal config with the "categories" approach:

cluster "packet" {
  lokoctl {
    asset_dir = var.asset_dir
  }
  cloud_provider {
    facility = var.facility
    project_id = var.packet_project_id
    ssh_pubkeys = var.ssh_public_keys
    management_cidrs = var.management_cidrs
    node_private_cidr = var.node_private_cidr
    controller_count = var.controllers_count
  }
  general {
    cluster_name = var.cluster_name
  }
  flatcar {}
  dns {
    zone = var.dns_zone
    provider {
      route53 {
        zone_id = var.route53_zone_id
      }
    }
  }
  k8s {}
  worker_pool "worker-pool-1" {
    count = var.workers_count
  }
}

To summarize, I do think we should make it a priority to keep the config surface as simple and as readable as possible. I'm just not convinced that the way to do it is to divide the config into blocks. There are many things we could do to make a config file more readable which don't involve adding blocks:

• We can think 3 times before adding a knob, and 7 times before adding a required knob.
• We can set defaults or completely eliminate some of the existing nodes. For example, I see no reason we should ask the user for both a Route 53 zone name and a zone ID.
• We can add whitespace and comments in our config samples. Example:

cluster "packet" {
  # Packet
  facility = "ams1"
  project_id = "1234"

  # Flatcar
  ...

  # K8s
  ...
}

As far as readability goes, IMO the example above has a very similar effect to having HCL blocks, without the downsides.

[johananl]

One more thing - in case we decide against adding more HCL blocks, we should probably consider flattening the existing dns block.

[invidian]

It seems the discussion diverged from just cluster and component blocks. Things you showed @johananl are more related to the refactoring work we've been doing.

[johananl]

My comment was in response to this:

I agree with the reasoning here that component is a property of a cluster.

With that being said, It makes much more sense now to organize the cluster block with sub blocks for readability, rather than an unorganized block.

ex: components, worker_pool, controller, os, network ? @johananl what do you think ?

I agree we should separate code refactoring from config schema changes.

[invidian]

So, what does lokoctl configuration represents?

Answering my own question, it seems to me, that cluster + components gives us Lokomotive stack, so maybe that should be the term we should use to describe it?

[johananl]

I'm not sure I understand the config structure you're proposing.

[invidian]

I'd change https://github.com/kinvolk/lokomotive/blob/master/pkg/config/config.go#L52-L57 from pkg/config:

type RootConfig struct {
    Cluster    *cluster    `hcl:"cluster,block"`
    Backend    *backend    `hcl:"backend,block"`
    Components []component `hcl:"component,block"`
    Variables  []variable  `hcl:"variable,block"`
}

to:

type Stack struct {
    Cluster    *cluster    `hcl:"cluster,block"`
    Backend    *backend    `hcl:"backend,block"`
    Components []component `hcl:"component,block"`
}

[johananl]

@invidian see https://github.com/kinvolk/lokomotive/issues/821#issuecomment-678293083 for my thoughts on the relations between clusters and components.

I'm not sure if "stack" makes sense here. If we consider changing user-visible things (namely the config structure), I'd like to see a design document and have a team discussion before we make any implementation efforts.