terraform-provider-iterative icon indicating copy to clipboard operation
terraform-provider-iterative copied to clipboard

`task` follow-up 3

Open casperdcl opened this issue 2 years ago • 5 comments

minor nitpicks from https://github.com/iterative/terraform-provider-iterative/issues/258#issuecomment-1051948425:

p2-nice-to-have

  • [ ] Escape rclone connection strings properly https://github.com/rclone/rclone/issues/4996#issuecomment-778628943
  • [ ] Refactor orchestrator events & status as separate data sources
  • [ ] Chain attributes read to the outer task structs with pointers
  • [ ] Use https://github.com/openlyinc/pointy or https://github.com/samber/lo on every provider
  • [ ] Rename regions to locations so it's valid for zones and node selectors?

casperdcl avatar Feb 26 '22 19:02 casperdcl

Rename regions to locations so it's valid for zones and node selectors

I'd advise against. regions and zones are important concepts for cloud compute and have delicate meaning for HA and performance for different cloud providers. Those are distinctly different from k8s' node affinity (or anti-) and node selectors - their meaning depends on the topology of the k8s cluster and the syntax is different as well I suggest to keep original terminology. Supporting node affinity/anti-affinity is a necessary next step for running anything on an existing k8s cluster with the necessary customizability and it then the new "abstraction" will break anyways

omesser avatar Jun 19 '22 13:06 omesser

regions and zones are important concepts for cloud compute and have delicate meaning for HA and performance for different cloud providers.

Given the availability and performance[^1] requirements of the use cases this tool covers (i.e. experimentation and not serving), the current behavior seems appropriate: specifying a region causes the task to pick any zone with enough available resources, and providing a zone letter constrains the placement to that specific zone.

[^1]: Which kind of performance are we talking about? 🤔

0x2b3bfa0 avatar Jun 19 '22 13:06 0x2b3bfa0

Those are distinctly different from k8s' node affinity (or anti-) and node selectors - their meaning depends on the topology of the k8s cluster and the syntax is different as well

Definitely. Kubernetes syntax is too rich for a single string variable; trying to fit everything into a single parameter can be detrimental.

0x2b3bfa0 avatar Jun 19 '22 13:06 0x2b3bfa0

I suggest to keep original terminology.

Agreed for affinity & selectors, not quite for regions and zones.

0x2b3bfa0 avatar Jun 19 '22 13:06 0x2b3bfa0

@0x2b3bfa0

Which kind of performance are we talking about? 🤔

Latency between client machine and the hosted compute resources. (There's also a cost consideration when considering regions)

None of this translates nicely to k8s node selection - it CAN, if you set up your node groups or use labels in a very specific way, such that you actually pick nodes in specific cloud zone, maybe cross-node-groups, but that's besides the point.

Anything that has to do with selecting nodes on k8s is only meaningful in the context of how that specific cluster and its node groups are set up. On the other hand - for cloud providers, regions/zones is a common terminology I believe that inventing another abstraction here is not helpful - it will add confusion, people will be looking for familiar terms (rightfully so). Also if/when node affinity/anti-affinity is added - we would want that api to be consistent with node selectors (inside a k8s specific block, probably 😉 ).

omesser avatar Jun 24 '22 02:06 omesser