OKP: Filter by product version - #9
Conversation
|
Tested locally and also ran the kuttl tests: |
12a8154 to
df48e55
Compare
|
|
||
| logger := h.GetLogger() | ||
|
|
||
| ocpVersion, err := DetectOCPVersion(ctx, h) |
There was a problem hiding this comment.
Minor: I can see the DetectOCPVersion has been called multiple times , does it make sense to detect the version once and then pass it where ever call is made ?
There was a problem hiding this comment.
I looked a bit into that when i was reviewing the code but, to be honest, this is a cheap k8s API call that should be completely fine in practice. I would keep it like this for now to keep the code clearer and have buildLCoreConfigYAML() and buildLlamaStackYAML() independently (they both call this common method)
In the future I believe we should compeltely remove the buildLlamaStackYAML() cause in LCORE there are ways of generating the LlamaStack (OGX) config from the LCORE config itself and keep things consistent. We should do that instead IMHO.
There was a problem hiding this comment.
But if you guys think this duplication is something to address, I can do it no problem
There was a problem hiding this comment.
I like the OGX config generation from LCORE config for the future.
|
|
||
| logger := h.GetLogger() | ||
|
|
||
| ocpVersion, err := DetectOCPVersion(ctx, h) |
There was a problem hiding this comment.
I like the OGX config generation from LCORE config for the future.
|
LGTM!! |
df48e55 to
4553ad0
Compare
lpiwowar
left a comment
lpiwowar
left a comment
There was a problem hiding this comment.
LGTM, thank you:). Just three super small nits.
Update the default OKP chunk_filter_query to include version-specific filters for both OpenStack and OpenShift. For OCP version detection, the code reuses the DetectOCPVersion() and for OpenStack a new detectOpenStackVersion() was introduced. The way the OpenStack version is determined is by mapping the OCP versions accordingly to RHOSO. For example, OCP <= 4.21 maps to OpenStack 18.0. In the future, once RHOSO 19.0 is out, we just need to add a new map entry for it. If no entry is mapped the code log a message and safely return a default version for RHOSO (right now, 18.0). The dev.okpChunkFilterQuery override continues to bypass detection entirely. Kuttl tests pin the query via that override to keep assertions deterministic across cluster environments. Unit-tests were added to cover those kuttl gaps. Signed-off-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
4553ad0 to
5ef9c77
Compare
4 participants
Update the default OKP chunk_filter_query to include version-specific filters for both OpenStack and OpenShift.
For OCP version detection, the code reuses the DetectOCPVersion() and for OpenStack a new detectOpenStackVersion() was introduced.
The way the OpenStack version is determined is by mapping the OCP versions accordingly to RHOSO. For example, OCP <= 4.21 maps to OpenStack 18.0. In the future, once RHOSO 19.0 is out, we just need to add a new map entry for it. If no entry is mapped the code log a message and safely return a default version for RHOSO (right now, 18.0).
The dev.okpChunkFilterQuery override continues to bypass detection entirely.
Kuttl tests pin the query via that override to keep assertions deterministic across cluster environments.