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

Auto-tagging from kubernetes/openshift labels #7460

Closed
cben opened this issue Mar 21, 2016 · 24 comments
Closed

Auto-tagging from kubernetes/openshift labels #7460

cben opened this issue Mar 21, 2016 · 24 comments

Comments

@cben
Copy link
Contributor

cben commented Mar 21, 2016

Basic goal: since containers and related entities come and go quickly, manual tagging is futile (but it works). We want to automatically create tags based on existing kubernetes/openshift labels. All labels generating tags might be too much noise, so we probably want a "policy" by which one can define desired label->tag mapping.

UPDATE

Originally implemented in #7605; multiple bugs fixed, especially by big rewrite #11806; UI added by Zohar in #11591. See #11591 for screenshots.
Documentation BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1389129

I'm keeping this issue for the whole feature to track loose ends, untested corners, bugs.

There is also ongoing work to support direct use of labels in MiqExpression & reports (without mapping them to tags) - https://bugzilla.redhat.com/show_bug.cgi?id=1382720...
I've come to appreciate the wisdom of @Fryguy's comments that this is what we should have done from the beginning. Unfortunately there are corners — especially chargeback reports — that can't easily benefit from it, while tags are already supported, so we still need mapping to tags.


I was hoping to a proof-of-concept PR by now, but all I have is some understanding and questions.
@simon3z please cc whoever relevant.


Notes to self: What I learned about labels & tags

Labels are key=value pairs. Keys have example.com/foo form when system-assigned (eg. kubernetes.io/hostname); custom labels may use unprefixed foo form. Values have some constraints but are basically unstructured.
We store labels under custom_attributes polymorphic table where section="labels". (Other sections we have are "selectors" and "node_selectors". Don't think I care about those, although conceptually they represent label-dependent object sets.)
See https://gist.github.com/cben/2619583f777ddb480a10#file-labels-txt for examples.

Possible tags are defined via classifications and tags tables.
They are applied to objects through polymorphic taggings table. Many many types say acts_as_miq_taggable which gives them .taggings, the higher-level .tags shortcut and a bunch of ActsAsTaggable methods.

Tag and Classification live in a bewildering, mostly 1:1 symbiosis. Their models have a ton of methods, calling each other. => Apparently the higher-level concept is tags.
Tag names are organized hierarchically in "/namespace/category/name" structure. namespace is normally /managed.
See https://gist.github.com/cben/2619583f777ddb480a10#file-tags-txt for examples.
Classification stores metadata on a tag or category (hierarchy via parent_id), including human description.
See https://gist.github.com/cben/2619583f777ddb480a10#file-classification-txt for examples.

There is some code supporting /virtual/... tags, not sure if it's used.


Basic questions

  • How does one currently define possible tags? Out of the box we have things like "Location: London", coming from db/fixtures/classifications.yml, but how are they set in real life use? Is there some UI? Is it done via Automate?
  • How will we expect the user to edit the "label->tag mapping policy"? Config file / code / UI editing a table?
    • What data do we want: blacklist / whitelist? Keys only or also values? Should we bother user with giving pretty descriptions (I think not)?

Tag space pollution

Labels are key,value with an open-ended set of values; for tags we pre-configure all possible values.
This implies we'll need to auto-define tags for newly encountered values, which over time will pollute the Classification & Tag tables...
It's not obvious to me how to garbage-collect unused auto-defined tags.

  • Timely GC of unused tags is useful for UI — after picking category you get a dropdown with possible values.
    • There is a deeper UI problem: for "do this when you'll see a tag Foo: Bar" scenarios, it's useful to specify a value Bar that's not yet (and maybe never was) present in the system!

In any case, they should probably live in a different namespace from /managed.

Materialize tags vs virtual view?

  • What happens when user adds a new label->tag mapping? Should existing labels be immediately processed or is mapping only on refresh OK?
  • The deeper question is: We already have all labels in the DB. We essentially want the ability to use [some] labels as if they were tags. That doesn't mean they must be stored as actual taggings. It could be a model or a UI logic directly quering the labels. That might be more work(?) or less efficient but has consistency benefits.
    • Even if we do store actual tags, can we treat that as a cache/denormalization of what is conceptually a pure function of labels [and mapping policy]? Or am I missing something?
@cben
Copy link
Contributor Author

cben commented Mar 21, 2016

miq-bot add_label providers/containers, enhancement, question

@simon3z
Copy link
Contributor

simon3z commented Mar 21, 2016

@miq-bot add_label providers/containers, enhancement, question

@Fryguy
Copy link
Member

Fryguy commented Mar 22, 2016

See also a related discussion / argument, which will probably be relevant, here: http://talk.manageiq.org/t/aws-tag-synchronization/291/6

@simon3z
Copy link
Contributor

simon3z commented Mar 22, 2016

How does one currently define possible tags? Out of the box we have things like "Location: London", coming from db/fixtures/classifications.yml, but how are they set in real life use? Is there some UI?

@cben generally it's done through the UI (in the region settings: Configuration => Settings => Region ...). You can also import new tags.

How will we expect the user to edit the "label->tag mapping policy"? Config file / code / UI editing a table?

@cben in the end the user will configure the mapping in the UI but I think you should concentrate on the backend first.

I think we said that we would have created the tags based on the labels name and value, and I believe there is something similar already for VMs, e.g. tags as /managed/folder_path_blue/datacenters:red:vm and /managed/folder_path_blue/datacenters:blue:vm which I believe are automatically generated by the datacenters that are present in the provider.

Check this code.

@cben
Copy link
Contributor Author

cben commented Mar 23, 2016

Thanks, these are helpful.

See also a related discussion / argument, which will probably be relevant, here: http://talk.manageiq.org/t/aws-tag-synchronization/291/6

Hmm. Your stance there was that we should not auto-tag, that if really desired by a customer it should be done in Automate, and that we should rather make improve ability to use custom_attributes directly.
Has any of that changed?

@cben
Copy link
Contributor Author

cben commented Mar 23, 2016

region settings: Configuration => Settings => Region ...).

Oh, I see. Clicking the top-level "CFME Region: Region 0 [0]" in the outline on the left shows tabs with settings whose existance can't be guessed from the outline. Not the best UX imho.

@simon3z
Copy link
Contributor

simon3z commented Mar 23, 2016

According to @blomquisg, @gmcculloug could help you here @cben.

@gmcculloug is my comment correct about dynamic creation of tags?

@cben
Copy link
Contributor Author

cben commented Mar 30, 2016

Threw current code at #7605.

@cben
Copy link
Contributor Author

cben commented May 10, 2016

Backend got merged. (Some small tasks remain there.)

We are now missing the frontend to deliver a complete solution.
It should be possible to add/edit/remove rules.

Central UX question: should user have to create tag/category [at Settings->Configuration->Region 0->My Company Tags] and then create a mapping [at some other place TBD],
or should the mappings UI be able to also create the tag?
The latter sounds better IMHO but it should also support picking an existing tag as target of a new mapping.

@simon3z
Copy link
Contributor

simon3z commented May 10, 2016

@cben let's start simple and give the ability to pick from the already existing tags.
Creation of new tags can build on that if needed.

@simon3z
Copy link
Contributor

simon3z commented May 12, 2016

@miq-bot assign cben

@cben
Copy link
Contributor Author

cben commented May 17, 2016

@miq-bot remove_label question

@miq-bot miq-bot removed the question label May 17, 2016
@cben
Copy link
Contributor Author

cben commented May 23, 2016

  • Allow auto-tagging to trigger policy events like "Tag Request" and/or cancel the tagging?

Currently I believe it doesn't happen — I'm directly adding to entity.tags; there are a higher level methods in Classification that go through policy when is_request is true.

@cben
Copy link
Contributor Author

cben commented Jun 23, 2016

Discovered in docs:
https://access.redhat.com/documentation/en/red-hat-cloudforms/4.1-beta/general-configuration/chapter-3-configuration#tags

=> Company tags which you will see under My Company Tags for a resource. Create company tags by navigating to Settings → Configuration, then clicking on the Settings, then selecting Region, then the My Company Tags tab. A selection of company tags is provided to you by default as samples. These can be deleted if you do not need them, but are not recreated by CloudForms Management Engine.

=> System tags are assigned automatically by CloudForms Management Engine.

If this is a real concept, sounds like I want the tags I assign to show up as "System tags".
Currently the auto-assigned tags on an entity view show under "My Company tags":
https://cloud.githubusercontent.com/assets/273688/14157383/67471120-f6d3-11e5-9fa4-d2ab6bc9a1c1.png
Will ask people what are these System tags...

@cben
Copy link
Contributor Author

cben commented Jul 27, 2016

ManageIQ doesn't cope well with tons of tags due to session object size: #10074
This might become easy to hit with auto-tagging.

@cben
Copy link
Contributor Author

cben commented Sep 30, 2016

We discussed yesterday multiple problems with the design. Will open separate issues but for now an overview:

  • Any-value mapping given labels with similar values e.g. key=VaL-ue vs key=val_Ue which collide after Classification.sanitize_name crashes on attept to create an existing tag.
  • The mapping is not concurrency-safe due to caching + the fact I'm adding specific-value mappings back into the mapping table while mapping. Multiple container providers encountering same label (key and value) that has an any-value mapping can also cause a crash.
  • Adding mappings back into the table is also confusing for UI ("I didn't enter those, where did they come from?!"). Instead will lookup tags by deterministic name.
  • The currently indefinite cache is unacceptable once we have UI to edit the mapping table. Dropping the cache at start of every refresh should fix this.
  • Until we have more confidence in this, should isolate the rest of refresh from exceptions in the mapping code.

WIP #11806
UPDATE: merged, covers all these.
Some additional contsraints enforced by UI #11591: only any-value mappings, separate target category per mapping, ...

@cben
Copy link
Contributor Author

cben commented Oct 28, 2016

Where can one use auto-tagged read_only tags:

  • Reports - fixing in Reports: include read_only tag categories #10459
  • Policies for tag [un]assigned events won't fire (and can't prevent tagging) because we don't go through "classify is_request: true" mechanism. Is this OK?
  • Policy "assign tag" action - TOCHECK if it's allowed, probably shouldn't be.
  • Custom action "inherit parent tag" - shows read_only tags; TOCHECK do they work?, probably shouldn't?
  • ...

EXTRACTED TO #15861

@cben
Copy link
Contributor Author

cben commented Dec 12, 2016

  • check what happens with build / build pod labels. IIRC build pods are seen as both a ContainerGroup and ContainerBuildPod entities, with different labels, and I only mapped one of them to tags (?)

@cben
Copy link
Contributor Author

cben commented Dec 12, 2016

EXTRACTED TO: ManageIQ/manageiq-ui-classic#2840

Dumping my paper notes on UX loose ends (cc @zgalor):
Most of these reflect actual misunderstandings I saw QE and Doc team have deciphering the UI...

  • Mapped tags are listed under "My Company Tags" on an entity page, despite being distinct from the manually assigned tags you configure in "My Company Categories/Tags" settings.

  • When you see some mapped tags and try "Edit Tags", they're not there. This is intentional but confusing, would be better to include them but greyed out (and not allow changes).

  • Currently it's unclear what to type in "Label" field. Nothing there suggests it's the key part of a label. (Users don't necessarily have a clear mental model of labels having key, value parts!)

    • Label name should be dropdown offering existing label keys. Will be faster to use, reduce typos, and make it clearer.
    • Perhaps should also call the field "Label key" instead of "Label"?
  • "Choose a category" may sound like you should type name of an existing category. Something like "Category name to create" could be clearer.

  • Flash message confirming mapping was created/updated should explain that it will take effect only on next refresh.

  • Can't map a given label to a given category for 2 entity types (eg. only pods & images). Can 1 type, or all of them. Can map 2 types to different categories.

  • If you have labels with inconsistent key case ("KEY=..." vs "key=..."), you can't map them both at all. UI won't let you even map them to different categories, and what you probably want is mapping to same category.
    We wanted this to keep simpler predictable category names. We could keep that but coerce all keys to lowercase (so a "key" mapping would always also cover "KeY" etc) — I think that'd be better than

    Before touching anything wrt. case handling, it'd be nice to understand how case works with direct use of labels in search/policy/reports — https://bugzilla.redhat.com/show_bug.cgi?id=1382720.

@simon3z
Copy link
Contributor

simon3z commented Dec 12, 2016

"Choose a category" may sound like you should type name of an existing category. Something like "Category name to create" could be clearer.

Flash message confirming mapping was created/updated should explain that it will take effect only on next refresh.

@cben these are easy right? Just labels...

Can't map a given label to a given category for 2 entity types (eg. only pods & images). Can 1 type, or all of them. Can map 2 types to different categories.

@cben how hard is this to fix?

@cben
Copy link
Contributor Author

cben commented Dec 12, 2016

Yeah, first few ones are low-hanging tweaks. I hoped to already have some PRs but never found the time...

Can't map a given label to a given category for 2 entity types (eg. only pods & images). Can 1 type, or all of them.

This one is a bit harder. UI supports "map 2 types to different categories" by naming the categories kubernetes:container_group:key vs kubernetes:container_image:key (or something like that).
Would have to break one mapping : one category assumption (which made garbage collection of categories & tags trivial).

@miq-bot miq-bot added the stale label Sep 23, 2017
@miq-bot
Copy link
Member

miq-bot commented Sep 23, 2017

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

@Fryguy
Copy link
Member

Fryguy commented Nov 27, 2017

@cben Is this already implemented, and if so can this be closed?

@cben
Copy link
Contributor Author

cben commented Nov 27, 2017

Yeah, long implemented and on 2nd or 3rd rewrite now :-)

Closing, extracting todos I kept here to other issues.

@cben cben closed this as completed Nov 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants