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

Include Classes Based on AND-combinations of Facts #174

Open
srueg opened this issue Sep 8, 2020 · 3 comments
Open

Include Classes Based on AND-combinations of Facts #174

srueg opened this issue Sep 8, 2020 · 3 comments
Labels
enhancement New feature or request RFC Request for Comments

Comments

@srueg
Copy link
Contributor

srueg commented Sep 8, 2020

Context

Currently it's possible to include classes based on facts (currently hard-coded, dynamically with #93). This only allows for OR-combinations of facts though: For example I can only have a class which applies for all Rancher clusters and one for all cloudscale.ch clusters. But not a class which applies to clusters of type Rancher AND cloudscale.ch.

Possible Solution

Enable the ignore class not found feature we already use during component discovery (#138) also during catalog rendering. With this we could include all possible combinations of facts, regardless if the actual class exists or not. For example, with #93 implemented:

classes:
- global.distribution.${cluster:dist}
- global.distribution.${cluster:dist}.${cloud:provider}
- global.cloud.${cloud:provider}
- global.cloud.${cloud:provider}.${cluster:dist}
- global.cloud.${cloud:provider}.${cloud:region}
- ${customer:name}.${cluster:name}

This approach would allow to also configure this aspect of the hierarchy. The downside is that we might run into issues with classes which should exist but are ignored (e.g. typos, yml vs yaml, etc.).

Alternative

Implement a feature in Commodore to remove classes which don't exist. This should only auto-remove classes which are allowed to be missing, for example such combination classes.

This approach would still allow to configure the hierarchy as implemented in #93 and additionally skip missing classes. The exact rule which classes are allowed to be missing needs careful thought.

@srueg srueg added enhancement New feature or request RFC Request for Comments labels Sep 8, 2020
@corvus-ch
Copy link
Contributor

With #195, is this still relevant? At least it should be reworded to account for that change.

@srueg
Copy link
Contributor Author

srueg commented Oct 8, 2020

It's still the same problem, yes. I think it already is worded properly, with references to #93 ?

@simu
Copy link
Member

simu commented Apr 8, 2022

There's no easy way to actually implement this sanely in a generic fashion. Internally, we've implemented selected combinations of AND-combinations of facts as documented in https://kb.vshn.ch/vshnsyn/how-tos/create-missing-cloud-dist.html#_make_a_distribution_cloud_aware

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request RFC Request for Comments
Projects
None yet
Development

No branches or pull requests

3 participants