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

RKE2 Cluster Templates: Some fields (i.e. ec2 rootSize) need to be wrapped in quotes, otherwise an error is thrown #40128

Open
slickwarren opened this issue Aug 5, 2021 · 10 comments
Assignees
Labels
kind/bug-qa Issues that have not yet hit a real release. Bugs introduced by a new feature or enhancement kind/tech-debt Formerly issues labeled as ci-improvements, tasks, flaky tests, and post-release. QA/XS release-note Note this issue in the milestone's release notes status/release-note-added team/hostbusters The team that is responsible for provisioning/managing downstream clusters + K8s version support

Comments

@slickwarren
Copy link
Contributor

on 2.6.0-rc1:
When provisioning using a cluster template, the rootSize for aws EC2 provisioners does not currently take an integer when it should. Instead an error is thrown when the below is used.
Screen Shot 2021-08-05 at 11 42 17 AM
wrapping the number i.e. "16" is a workaround for this issue.

repro steps:

  • add an rke2 template that can use amazonec2 provisioner
  • provision using the template -> under nodepools ensure that the rootSize option has an integer value
  • chart will install to fail
@slickwarren slickwarren self-assigned this Aug 5, 2021
@slickwarren slickwarren changed the title RKE2 Cluster Templates: Some fields (i.e. ec2 rootSize need to be wrapped in quotes, otherwise an error is thrown RKE2 Cluster Templates: Some fields (i.e. ec2 rootSize) need to be wrapped in quotes, otherwise an error is thrown Aug 6, 2021
@richard-cox
Copy link
Member

@slickwarren Are you using https://github.com/slickwarren/cluster-template-examples repo main branch and default-template? I've just tried that myself and the values we send look correct (v1/catalog.cattle.io.clusterrepos/cluster-templates?action=install charts.0.values.nodepool.rootSize: "16g"
The field itself is a generic question component which is created via the questions.yaml from the repo (contains type: string for nodepool.rootSize)

@slickwarren
Copy link
Contributor Author

slickwarren commented Aug 10, 2021

I was using @StrongMonkey 's chart. Would the default value in questions.yaml not being a string, even though the default type is string cause this issue that rancher's end can't really fix @richard-cox ?

@richard-cox
Copy link
Member

So values missing the g might be mistaken as a number instead of string

@slickwarren
Copy link
Contributor Author

ah, I see it now in the values-aws.yaml. However, I just checked the request sent when provisioning an RKE2 cluster using the node driver (not template) and it does not have a g in it @richard-cox @StrongMonkey
Screen Shot 2021-08-10 at 2 05 48 PM
so we would need this to format the number as a string afaik

@nwmac
Copy link
Member

nwmac commented Aug 19, 2021

@slickwarren Where are we with this? What is the cluster template git repo that you are using that has the problem? I tried the https://github.com/StrongMonkey/cluster-template-example but this does not presents form to edit

@slickwarren
Copy link
Contributor Author

@nwmac it should be available there https://github.com/StrongMonkey/cluster-template-examples , use branch new. when provisioning the cluster with the template, select amazonec2 as the infrastructure provider in order to view rootSize in the UI (nodepools will appear after selecting amazonec2)
Screen Shot 2021-08-19 at 10 41 13 AM

@gaktive gaktive added this to the v2.6.1 milestone Aug 20, 2021
@neillsom
Copy link

@slickwarren This does not appear to be a UI issue.
The UI form interprets the input (whether 16 or '16') as a string and submits it as such.
The issue is that once the form is submitted it is templated into yaml, and even though we are sending the value as a string, yaml converts it to an integer.
Updating the template chart should solve the problem

@slickwarren
Copy link
Contributor Author

hmm. I've tried updating the values in yaml to "16", Even in the yaml (viewed through the UI) the number is wrapped. But it is giving me the same error.
Screen Shot 2021-10-22 at 10 32 32 AM
but the UI still shows it without the quotes

Screen Shot 2021-10-22 at 10 34 21 AM

The only way I could get it to work properly was to do it through the UI field "16".

This seems like a UI problem still since the quotes are stripped in the UI view. If that's not the root cause, would this be a backend issue?

@gaktive gaktive removed this from the v2.6.3 milestone Nov 19, 2021
@gaktive gaktive added release-note Note this issue in the milestone's release notes kind/bug-qa Issues that have not yet hit a real release. Bugs introduced by a new feature or enhancement labels Nov 19, 2021
@zube zube bot added team/hostbusters The team that is responsible for provisioning/managing downstream clusters + K8s version support and removed team/hostbusters The team that is responsible for provisioning/managing downstream clusters + K8s version support labels Jan 26, 2022
@nwmac nwmac added this to the v2.6.5 milestone Mar 2, 2022
@nwmac nwmac modified the milestones: v2.6.5, v2.6.6 Mar 29, 2022
@gaktive gaktive modified the milestones: v2.6.6, v2.7.0 May 25, 2022
@nwmac nwmac removed this from the v2.7.0 milestone Sep 22, 2022
@catherineluse catherineluse self-assigned this Nov 7, 2022
@catherineluse
Copy link

catherineluse commented Nov 7, 2022

I believe this issue should be transferred to the back end.

This seems to be an issue with Helm. As Neill noted above, the UI always sends the value as a string regardless of whether it is wrapped in quotes or not. But that value is then passed to Helm, which converts it to an integer when there are no letters in it. helm/helm#1694

I'm pretty sure a small PR to the cluster template repo (https://github.com/rancher/cluster-template-examples) ought to force Helm to keep the value in string format, but I didn't figure out exactly how to do it.

In the helm chart values.yaml, I tried wrapping the value in quotes like this:

rootSize: "{{ $nodepool.rootSize }}"

and like this:

rootSize: {{ $nodepool.rootSize | quote }}

But neither worked. In any case, it's probably more of a matter for the back end.

@gaktive gaktive added the kind/tech-debt Formerly issues labeled as ci-improvements, tasks, flaky tests, and post-release. label Nov 7, 2022
@gaktive gaktive transferred this issue from rancher/dashboard Jan 10, 2023
@gaktive
Copy link
Member

gaktive commented Jan 10, 2023

Transferring to backend.

@Jono-SUSE-Rancher this isn't a high priority, barring anything that QA brings up, but it's a solid tech debt issue to consider. This should be able to leverage form usage in other parts of Helm chart validation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug-qa Issues that have not yet hit a real release. Bugs introduced by a new feature or enhancement kind/tech-debt Formerly issues labeled as ci-improvements, tasks, flaky tests, and post-release. QA/XS release-note Note this issue in the milestone's release notes status/release-note-added team/hostbusters The team that is responsible for provisioning/managing downstream clusters + K8s version support
Projects
None yet
Development

No branches or pull requests