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

Update verify script to validate min and max versions supported for Openshift #87

Merged
merged 2 commits into from
May 17, 2022

Conversation

rajendraindukuri
Copy link
Collaborator

@rajendraindukuri rajendraindukuri commented May 16, 2022

Description

As part of OCP 4.10 support for csi-unity driver , fixed verify script which validates MIN and MAX versions of Openshift supported by the driver

GitHub Issues

GitHub Issue #
dell/csm#305

Checklist:

  • I have performed a self-review of my own code to ensure there are no formatting, vetting, linting, or security issues
  • I have verified that new and existing unit tests pass locally with my changes
  • I have not allowed coverage numbers to degenerate
  • I have maintained at least 90% code coverage
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • Backward compatibility is not broken

How Has This Been Tested?

Tested helm installation checks for positive and negative scenarios in OCP 4.10 env. Please find below screenshots for reference:

Min_version_check_410

max_version_check

@@ -11,7 +11,7 @@
# verify-csi-unity method
function verify-csi-unity() {
verify_k8s_versions "1.21" "1.23"
verify_openshift_versions "4.8" "4.9"
verify_openshift_versions "4.9" "4.10"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we removing 4.8 EUS support ? is that discussed in PO meeting ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @prablr79 , these are just inputs for Major and Minor versions. We don't validate for EUS versions as it is implicitly covered.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok thanks

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @prablr79 Raised PR s for all drivers repos except for powerstore as some of the other validations are getting overridden.. Passed on the info to team

@@ -224,7 +224,7 @@ function verify_openshift_versions() {
log arrow
log smart_step "Verifying minimum OpenShift version" "small"
error=0
if [[ ${V} < ${MIN} ]]; then
if (( ${V%%.*} < ${MIN%%.*} || ( ${V%%.*} == ${MIN%%.*} && ${V##*.} < ${MIN##*.} ) )) ; then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this change applicable across drivers ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @prablr79 . Thanks for bringing this up. I will raise a common PR for all drivers.

Copy link
Collaborator

@prablr79 prablr79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with another PR for across drivers to cover.

@rajendraindukuri rajendraindukuri merged commit c47b9a2 into main May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants