⚠️ Add metrics to infrastructure.json #1562
Conversation
Signed-off-by: Shlomit Koyfman <shlomitk@il.ibm.com>
Signed-off-by: Shlomit Koyfman <shlomitk@il.ibm.com>
Signed-off-by: Shlomit Koyfman <shlomitk@il.ibm.com>
Signed-off-by: Shlomit Koyfman <shlomitk@il.ibm.com>
pkg/optimizer/datapath_csp.go
Outdated
| infraElement := dpc.env.AttributeManager.GetAttribute(attr, cluster.Name) | ||
| if infraElement == nil { | ||
| return "", "", fmt.Errorf("attribute %s is not defined for cluster %s", attr, cluster.Name) | ||
| for _, instanceType := range instanceTypes { |
There was a problem hiding this comment.
This is probably not how we want to handle bandwidth-like attributes. We have to check len(instanceTypes). If it's 1, then it's a simple optimization goal and the current code is ok. If it's 2, then it's bandwidth-like optimization goal and we have to return "", "", nil
There was a problem hiding this comment.
This is not related to bandwidth-like attributes. The use-case is a single "cost" attribute defined for both clusters and storage-accounts. When this cost should be minimized it means minimizing everything on the data path that has "cost".
I saw that the code fills an array, so it should not be a problem, or should it?
There was a problem hiding this comment.
Policies could be about storage cost, cluster cost, or all costs. Minimization should be done on the relevant costs indicated in the policy. Right?
There was a problem hiding this comment.
Policies could be about storage cost, cluster cost, or all costs. Minimization should be done on the relevant costs indicated in the policy. Right?
Yes. Minimization policy is related to an attribute. The same attribute can be defined for storages, clusters,...
It is also possible to define separately storage-cost and cluster-cost as different attributes, in which case policy will define whether to minimize any of them, or both.
Signed-off-by: Shlomit Koyfman <shlomitk@il.ibm.com>
Signed-off-by: Shlomit Koyfman <shlomitk@il.ibm.com>
There was a problem hiding this comment.
I'm afraid the suggested change to setSimpleGoalVarArray() is not going to work.
We really need a different paramArray for each instance type, because the i-th entry in the paramArray means the attribute-value for module/cluster/storage number i. So we cannot just put them all into one param array, bur rather need to call getAttributeMapping() for each instanceType.
In fact, we need to call setSimpleGoalVarArray() for each instanceType. Each such call will result in a variable-array holding the costs of the selected clusters/modules/storage-accounts along the data-path. We then need to sum each such array, and finally sum the sums 😢.
I suggest to delay this "total cost" feature for a later PR. For now, forbid this feature and open a new issue.
Signed-off-by: Shlomit Koyfman <shlomitk@il.ibm.com>
OK. I have left the array in GetInstanceTypes but only the first element is used in the solver. |
|
@roytman This PR changes infrastructure.json format as well as the corresponding validation schema and taxonomy json. |
Signed-off-by: Shlomit Koyfman shlomitk@il.ibm.com
Fixes #1536
Fixes #1550