[draft] feat: Add labels on machines (with cli for manual add/rm/ls)

[jabr]

Initial draft at implementing machine labels (#5).

For context, this is more of a proposed building block for future features based on the labels. I thought a draft/spike might be useful to think through the approach to ensure it will work for everything we want to do with them.

For example, how would we incorporate auto-generated or implicit labels, like ingress, platform (arm64/amd64), etc. Are those actually materialized labels in this set, or inserted dynamically by the API returning the set? And if materialized, due we need some namespacing standard to distinguish internal and user labels? i:ingress, i:arm64, u:eu, u:worker, etc?

[psviderski]

how would we incorporate auto-generated or implicit labels, like ingress, platform (arm64/amd64), etc. Are those actually materialized labels in this set, or inserted dynamically by the API returning the set? And if materialized, due we need some namespacing standard to distinguish internal and user labels? i:ingress, i:arm64, u:eu, u:worker, etc?

Tbh, I haven't thought much about the labels design yet. I was delaying it until we have to implement a feature that requires them. This way we can understand the requirements better and likely make better decisions.

I was thinking that we might want labels to be a map (key/value pairs), not a list of strings. This will be more in line with the docker labels design and the labels configuration in the Compose spec.

Regarding materialisation, I think that materialising auto-generated labels such as machine arch makes it easier to query them later and integrate in the workflows such as container scheduling. We can have a logic in the uncloudd daemon to update its own labels in the cluster store on the daemon start or on schedule.

I don't have any strong opinion on the label format yet. Maybe we shouldn't enforce any at least for now. Like in Docker and k8s there could be a naming convention with some reserved prefix(es) which isn't strictly enforced: https://docs.docker.com/engine/manage-resources/labels/#key-format-recommendations

The com.docker., io.docker., and org.dockerproject.* namespaces are reserved by Docker for internal use.

[psviderski]

What do you think about managing machine labels as part of rpc UpdateMachine(UpdateMachineRequest)?
Unless I'm missing something, it seems labels is just a property similar to name or endpoints.

[jabr]

I did it as a field on the UpdateMachine request initially, but it felt a bit incongruous that way. The other fields are "set" operations, replacing the corresponding machine value, but the label commands felt like they should be "add"/"remove" individual labels to/from the existing set.

So assuming that intuition is true (the workflow will typically be add/remove, not set), matching the "set" behavior of the rest of UpdateMachine meant doing a fetch all, add/remove locally, then push the whole set back, and that felt wrong, too.

Then I considered labelsToAdd and labelsToRemove in the UpdateMachineRequest and that felt like a mismatch with the rest of internal/machine/cluster/cluster.go#UpdateMachine as well as making it a pretty long function.

So I ended up with the current form and two new methods, which I felt had clearer semantics that matched the new cli commands and expected workflow better.

Happy to change if you think there's a better approach, but what are your thoughts on the above?

[jabr]

I was delaying it until we have to implement a feature that requires them. This way we can understand the requirements better and likely make better decisions.

The ingress (#8), arch/platform (#146), and general placement constraint (#3) issues related to push/deploy all struck me as something concrete that would benefit from having them now.

It also seems like a feature that will find more uses in other ways once we have it, too, like filtering service/container/machine list based on them. Sort of a "build it and they will come" situation. 😆

I was thinking that we might want labels to be a map (key/value pairs), not a list of strings. This will be more in line with the docker labels design and the labels configuration in the Compose spec.

That's a good point. I'm not super familiar with docker labels, and I was thinking of them more like "tags". The map approach solves my namespacing (internal vs user) question, too.

One clarification, though: are docker labels a map or a multimap? Can a key, say role, have more than one value associated? Like role=webapp and role=worker?

That was one of the reasons I went with the simple list of "tags" approach, so a user could do something like :role/webapp and :role/worker on a machine (using whatever structure they wanted in the tags themselves).

And actually, even the docker labels form could fit in the simple "tag" list, too. It looks like the docker compose spec even supports that syntax:

labels:
  - "com.example.description=Accounting webapp"
  - "com.example.department=Finance"
  - "com.example.label-with-empty-value"

Wondering if there's any real benefit to having a map data structure underneath it... I suppose some lookup value by key use case? Though that's the same as a "namespace" prefix filter...

[psviderski]

One clarification, though: are docker labels a map or a multimap? Can a key, say role, have more than one value associated? Like role=webapp and role=worker?

It's a map, can't have duplicate keys.

That was one of the reasons I went with the simple list of "tags" approach, so a user could do something like :role/webapp and :role/worker on a machine (using whatever structure they wanted in the tags themselves).

Docker uses an empty value in this case, see https://docs.docker.com/reference/cli/docker/container/run/#label

Wondering if there's any real benefit to having a map data structure underneath it... I suppose some lookup value by key use case?

Yeah, I think it's just a bit more convenient to not deal with string processing and having an implicit convention on the separator when doing key lookups. I believe most of the time labels will be key/values any way. For example, used for placement constraints https://docs.docker.com/reference/cli/docker/service/create/#constraint

[jabr]

Updated to use key-value map for labels instead of simple list.

[miekg]

volumes already support Docker-style labels as do images when build with the LABEL command.

For prometheus support (#304) labels would be nice, as I can then autodetect what and where need to be scaped:
uncloud.prometheus:/metrics or some such