feat: add OCP version compatibility check - #151
Conversation
Add a validate_ocp_version role that reads ocp_versions from pattern-metadata.yaml and checks the target cluster version before installation. Explicitly unsupported versions cause a hard fail; versions below the minimum trigger a warning. Patterns without ocp_versions metadata are unaffected (graceful skip). Override with SKIP_OCP_VERSION_CHECK=true for testing. Signed-off-by: Przemyslaw Roguski <proguski@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
Hi @p-rog, thanks for the PR, nice work! Can we extend the schema for pattern-metadata yaml here https://github.com/validatedpatterns/pattern-ui-catalog/blob/main/pattern.schema.json so we have it in the reference place. Note that right now this code won't run when doing "make install" it will only run when running "make validate-cluster" You will need to add |
|
Could you also do a |
mlorenzofr
left a comment
There was a problem hiding this comment.
Good job. I think the implementation is quite good, although one task needs to be removed as it doesn't really make sense since we're comparing a k8s version with an ocp version, which have different versioning
- Rename SKIP_OCP_VERSION_CHECK to DISABLE_OCP_VERSION_CHECK (VP convention) - Remove Kubernetes version extraction (k8s version != OCP version) - Make ClusterVersion CVO fetch unconditional with assertion - Replace errors='ignore' with stat + slurp + rescue for proper handling of missing vs malformed pattern-metadata.yaml Signed-off-by: Przemyslaw Roguski <proguski@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Sure. Here is PR: @mbaldessari please review it.
@mbaldessari so, do you think we should update the common and add validate-cluster target in common/Makefile like it's done currently with the |
mhjacks
left a comment
There was a problem hiding this comment.
The set_fact/rescue bit has to change.
I'm curious what others think of the text box output - I could be persuaded on this point.
The note on changing pattern.sh is a change I think we would make and propogate.
|
|
||
| - name: OCP version check disabled | ||
| ansible.builtin.debug: | ||
| msg: "OCP version compatibility check disabled (DISABLE_OCP_VERSION_CHECK=true)." |
There was a problem hiding this comment.
Note: we'll need to add this to pattern.sh's list of env vars to pass through to the utlity container.
There was a problem hiding this comment.
We can add it later to the patternizer repo once this PR merges.
| ansible.builtin.fail: | ||
| msg: | | ||
|
|
||
| ╔══════════════════════════════════════════════════════════════════╗ |
There was a problem hiding this comment.
I would prefer to not have the "text box" output. It seems to depend on using the null STDOUT callback (which is default) but does not look good e.g. with the normal stdout callback. I think from a UX standpoint halting the installation with instructions on how to recover it is sufficient.
There was a problem hiding this comment.
Personally I like "text boxes" because it's very clear that this is the core information you have to read. If you don't mind I would prefer to leave text boxes.
| msg: | | ||
|
|
||
| ┌──────────────────────────────────────────────────────────────────┐ | ||
| │ WARNING │ |
There was a problem hiding this comment.
Same note on text box formatting as above.
Let's stick with leaving it in the validated-cluster, maybe some readme (iirc) might need some tweaking here and there since it is not being run by default during install. We can always add it later to the install target |
Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
validate_ocp_versionAnsible role that checks the target cluster's OCP version against compatibility metadata defined inpattern-metadata.yamlvalidate_clusterplaybook so it runs automatically duringmake installocp_versionsmetadata are unaffected — the check is gracefully skippedHow it works
Each pattern can declare an
ocp_versionssection inpattern-metadata.yaml:Override the hard fail for testing:
SKIP_OCP_VERSION_CHECK=true ./pattern.sh make installCompanion PR
Context
This originated from a ZTVP-specific implementation (layered-zero-trust#163) that was closed in favor of a framework-level solution after team discussion. The agreed approach was to use
pattern-metadata.yamlfor version data and therhvp.cluster_utilscollection for validation logic.Test plan
ocp_versionsmetadata on a supported OCP version — should show OKminimum— should show warning, proceedSKIP_OCP_VERSION_CHECK=true— should skip the check entirelyocp_versionsin metadata — should skip gracefullyMade with Cursor