Add label parameter to cluster and tasks

[karol-kokoszka]

Fixes #3219

cc: @d-helios @zimnx

---

Please make sure that:

• Code is split to commits that address a single change
• Commit messages are informative
• Commit titles have module prefix
• Commit titles have issue nr. suffix

[Michal-Leszczynski]

@karol-kokoszka this PR introduces additional string label for cluster/tasks, but I think that operator team requested it to be a map:

Proposed solution is to add a labels field to the clsuters. User defined key-values which might be useful for filtering, tagging or putting arbitrary values like hash of entire cluster we put in API.

The only thing we're requesting here is enriching the cluster/task structs with user-defined key-value pairs, allowing the users to annotate the objects - akin to https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/.

[karol-kokoszka]

We won't introduce any map, the field is expected to serve general purpose. DevOps wants to use it as well to add their labels.
If operator wants to have the map, then can use JSON.

[zimnx]

What's the problem with having a map for DevOps?
Single text fields is not extensible, doesn't allow for advanced querying or filtering.
While with maps, users could search for clusters having particular key or set of keys.

[rzetelskik]

We won't introduce any map, the field is expected to serve general purpose. DevOps wants to use it as well to add their labels.
If operator wants to have the map, then can use JSON.

They can use a map for a single key, we can't use a label for a map - we'd collide with a user whenever they'd try to amend it.

[d-helios]

What's the problem with having a map for DevOps? Single text fields is not extensible, doesn't allow for advanced querying or filtering. While with maps, users could search for clusters having particular key or set of keys.

just to let you know that map for us is also ok.

[karol-kokoszka]

@zimnx @rzetelskik I don't understand you guys.
Here is a comment from the issue that is expected to be closed with this PR #3219 (comment)
It's talking about label text field.

Why you cannot use JSON ?
Single text field is extensible, you can put whatever you want.

While with maps, users could search for clusters having particular key or set of keys.

There is no functionality in manager to search for clusters using particular key or set of keys basing on this new label.
Nothing prevents you from building this kind of functionality on top of what SM returns.
Besides, the label field is not part of the key in the backing DB, you need to scan to get the data and just filter.

It's still manager client app that must implement the search.

[rzetelskik]

Here is a comment from the issue that is expected to be closed with this PR #3219 (comment)

It's your comment, not ours. We specifically ask for a key-value store there.

Why you cannot use JSON ?
Single text field is extensible, you can put whatever you want.

As long as the user doesn't fiddle with the label directly, sure. If they do, we lose all of the metadata and no longer know if this cluster/task should be controlled by the operator. And this is the only use case I care about personally here.

I don't really understand the pushback - it seem like a pretty standard approach? Grafana, prometheus, kubernetes all seem to use maps for labels.

[karol-kokoszka]

As long as the user doesn't fiddle with the label directly, sure. If they do, we lose all of the metadata and no longer know if this cluster/task should be controlled by the operator. And this is the only use case I care about personally here.

Makes sense, but the current implementation (in this PR), just allows to put all or nothing.
If there is a map of label fields, than it implies that there is completely new CLI and API in sctool to manage these labels.
It's achievable, but it won't be a part of 3.2.8.
It's because it's manager who will not only need to update the label field, but just update the diff.

The plan for 3.2.8 is to release it by 6th of May and the release is driven by #3767

Guys, if the label as a single text field is no go, then we must postpone it for a couple of weeks.

[zimnx]

It's a no go for us. We can wait for it until next release.