rancher / distros-test-framework

5 stars 9 forks source link

Update helm repo for rancher 2.7 #85

Closed mdrahman-suse closed 5 months ago

mdrahman-suse commented 5 months ago

Proposed Changes

This change adds the ability to add helm args on deploy rancher cluster creation. It is needed to enable creating rancher cluster with user provided helm args.

Types of Changes

Testing

Checklist:

  1. If your PR changes anything on or related to Jenkins, run it pointing to your branch to make sure it's okay.

    • Ok
  2. Verify code lint; we should not have errors.

    • Ok
  3. Update the documentation if needed.

    • TBA
  4. Update makefile and docker run if adding new tests.

    • Ok
  5. Run your tests at least 4 times with all configurations needed and possible.

    • TBA
  6. If needed test with different os types.

    • TBA

Linked Issues

Further Comments

mdrahman-suse commented 5 months ago

ahh I think we still want the rancherImageVersion for the cases when we need to pull, for example, v2.8.3 for the chart, but use v2.8-head for the rancher image. But the change you made for the removal of the extra args bit for the one specific version looks good to me.

We can add that as the rancherVersion parameter. So for charts we have chartsVersion and for rancher we have rancherVersion

rancher-max commented 5 months ago

ahh I think we still want the rancherImageVersion for the cases when we need to pull, for example, v2.8.3 for the chart, but use v2.8-head for the rancher image. But the change you made for the removal of the extra args bit for the one specific version looks good to me.

We can add that as the rancherVersion parameter. So for charts we have chartsVersion and for rancher we have rancherVersion

rancherVersion is the helm chart version, but not necessarily the image version used in the resulting deployment 😬 It generally is, but not always -- for example in the case of v2.8-head, there is no helm chart version for v2.8-head, so you have to use v2.8.3 helmchart and specify the image to be v2.8-head

mdrahman-suse commented 5 months ago

I didn't recognize the variable name changes 🤦‍♂️ This looks great! Ignore my previous comment 😄

No worries that is my bad, shouldve mentioned on the description 🙇