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

Make classifications a first class citizen #549

Open
michaeljs1990 opened this issue May 23, 2017 · 5 comments
Open

Make classifications a first class citizen #549

michaeljs1990 opened this issue May 23, 2017 · 5 comments

Comments

@michaeljs1990
Copy link
Contributor

michaeljs1990 commented May 23, 2017

I haven't looked into what this would take to implement yet but wanted to get some feedback before going down this road.

Currently classifications are a second class citizen of sorts.

  1. It's not easy/possible to query for them from the command line or at least not obvious to me.
  2. In order to search for new assets you first have to find the classification or a node that is similar and then click unallocated. The performance of this seems to be rather slow (in the 5s range) although it's not a big concern of mine.
  3. The values do not support full CQL syntax making it slightly inflexible. The case I ran into was a hardware vendor being so nice and sending us the exact same motherboards with one have a -U appended to it. It could be argued we should fix this during intake however.
  4. Some strangeness currently happens if you have an asset that you are string matching against such as BASE_PRODUCT. If you have one BASE_PRODUCT of SYS and one of SYSC and set a classification to match SYS when searching for unallocated or all by clicking the generated link in the GUI it will bring back SYS and SYSC however if you click on an asset with a product of SYSC the classification is not set.

I personally think if we can solve 1 & 4 it would make classifications much more useful and then a few more tasks might come out of this when doing those.

@byxorna
Copy link
Contributor

byxorna commented May 27, 2017

I agree with 1, 3, 4. I dont recall 2 being an issue, so that should probably be broken into a new ticket and profiled. It is totally possible its doing some stupid N+1 query type behavior.

The current classification system is pretty obtuse, even with the docs. Discovering which LSHW attributes are available, and what their non-human-ified value is (collins converts numbers to GB for some attributes) pretty complicated. I am not sure what a better system would look like, but... perhaps something like a "stored query" could emulate a classification system? It would enable pure CQL, be easily testable, and make referencing a commonly used set of assets (i.e. unallocated webs, or production mesos nodes, or DB class hardware that is unallocated) super simple. This could be integrated into the CLI/API pretty simply. Thoughts? (I envision something like phab's stored queries for maniphest).

Additionally, a new stored query system would allow us to maintain legacy classifications in lifesupport mode, and let us implement the new system greenfield.

@michaeljs1990
Copy link
Contributor Author

I like this idea and think that it reduces a lot of feature creep in the future such as we want to match in X way such as matching against multiple product numbers. Just to clarify would you still see that saved queries adding a "classification" on a nodeclass if it matches? I do rather like the ability to limit what machines nodeclasses can be provisioned as although it's not greatly flexible atm.

@byxorna
Copy link
Contributor

byxorna commented Jun 2, 2017

We added a way to limit provisioning to specific classifications (see https://tumblr.github.io/collins/configuration.html#provisioner%20profile and allowed_classes) a while back. I dont know how the saved queries thing would wrap into that, but I agree that it seems more flexible.

@michaeljs1990
Copy link
Contributor Author

O yeah I have been heavily pushing allowed_classes for about a month now with most nodes having them set at this point just to stop the madness that is provisioning a DB onto a GPU node. Extending the flexibility of saved queries to provisioning_profiles would be awesome but just not sure without putting some more thought into it how that would look.

@byxorna
Copy link
Contributor

byxorna commented Jun 3, 2017

Yea, it would be pretty easy to make provisioning profiles able to be restricted to specific saved queries

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

2 participants