feat: Added support for openshift-ai-addon version 416 and 417 #99
Conversation
Scenario-1:
Scenario-2:
Scenario-3:
|
Scenario-4:
Scenario-5:
|
|
@zikra-iqbal - Have you tested all these combinations? Can see some cases in above screenshots. It would good to update the table with the result outcomes. Thanks.
|
|
There should not be any provision to show OCP versions less than If using 4.15, it should be clearly mentioned that this will only provision 
|
|
Also, as the |
|
Please include a validation here to allow only |
|
Did you tried setting the value of The documentation mentions about openshift ai version 418 but it is currently not present on the openshift-ai page. We need to keep a track of this as well if it is not working now. If working, we can include this as well. 
|
|
|
|
|
|
|
@imprateeksh, tested this combination as well. It's supporting addon versions |
I added the validation: https://github.com/terraform-ibm-modules/terraform-ibm-ocp-ai/pull/99/files#diff-e437423e7929a42cec4915226ae17f2e2c07bb2abec8f081c6243a1e9c983b77R162 |
imprateeksh
left a comment
imprateeksh
left a comment
There was a problem hiding this comment.
I have two suggestions here. Please check once.
| } | ||
| } | ||
|
|
||
| validation { |
There was a problem hiding this comment.
I do not see any issues with the validations but I have a suggestion to future proof the validation logic.
This can be achieved by having a local map of ocp-ai version associated with openshift version and use one validation block.
This way we can address future versions of ocp-ai by simply adding them to the map.
Here is what I am suggesting -
locals {
ocp_ai_addons = {
"416" = { min_ocp_ver = 16, max_ocp_ver = 18 } # i.e >=4.16.x and <4.18 as per constraints given.
"417" = { min_ocp_ver = 17, max_ocp_ver = 19 } # i.e. >=4.17.x and <4.19
# "418" = { min_ocp_ver = 18, max_ocp_ver = 20 } # For other versions, just add an entry here.
}
}
# Now use one validation block to handle this -
validation {
condition = (
var.addons.openshift-ai == null || !contains(keys(local.ocp_ai_addons), var.addons.openshift-ai.version) ||
(
can(regex("^\\d+\\.\\d+(\\.\\d+)?$", var.openshift_version)) && tonumber(split(".", var.openshift_version)[0]) == 4 &&
tonumber(split(".", var.openshift_version)[1]) >=local.ocp_ai_addons[var.addons.openshift-ai.version].min_ocp_ver &&
tonumber(split(".", var.openshift_version)[1]) < local.ocp_ai_addons[var.addons.openshift-ai.version].max_ocp_ver
)
)There was a problem hiding this comment.
Updated. Thank you!
| } | ||
| } | ||
|
|
||
| validation { |
There was a problem hiding this comment.
Also, noticed there is a possibility if user has not given version inside openshift-ai map, something like -
default = {
openshift-ai = {
version = ""
}
}So another simple validation can be added to ignore it same way we ignore if openshift-ai == null i.e. below can be appended to above condition
var.addons.openshift-ai.version == null Please check for this as well -
default = {
openshift-ai = {}
}
|
Please note - The tests should also be updated to check both |
As discussed, it's not required. The |
Description
Added support for openshift-ai-addon versions:
Documentation link
Release required?
x.x.X)x.X.x)X.x.x)Release notes content
Added support for openshift-ai-addon versions 416 and 417.
Run the pipeline
If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.
Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:
Checklist for reviewers
For mergers