Skip to content

Conversation

f4z4on
Copy link

@f4z4on f4z4on commented Jun 16, 2023

InnoDBCluster resource can have zero router instances. However, Helm
considers zero to be an empty value (see default function), so we
cannot rely on coalesce Helm function to source spec.router.instances
from one of the following values:

  1. routerInstances
  2. router.instances

We change implementation which relies on Helm functions which use
“emptiness” to the implementation which checks if a value is defined in
parent dict. We do not introduce any additional validation like checking
if the value is an integer zero or any other value or type. This has
been validated by Kubernetes using CRD so far, Helm properly reports
this back, and we do not change this.

We also fix validation of source for spec.router.instances which can be of different types by converting them to strings.

f4z4on added 2 commits June 16, 2023 18:06
This field is sourced from value “routerInstances” or
“router.instances”. If “router.instances” is set, we check if it is the
same as “routerInstances”.

We do this with “ne” Helm function. This function cannot compare
different types. This is is quite typical when mixing values from
different sources (for example default values.yaml file and command-line
arguments).

Perhaps more strangely, when values YAML file contains an integer value,
it is treated as float64 by Helm, so even when we use default
values.yaml and set “router.instances” on command line, we get a
strangely looking error message as a result of “printf” trying to print
float64 as int64 (on the lastest version Helm as of this
writing—3.12.1).

We solve both issues by converting both values to strings.
InnoDBCluster resource can have zero router instances. However, Helm
considers zero to be an empty value (see “default” function†), so we
cannot rely on “coalesce” Helm function to source spec.router.instances
from one of the following values:

1. routerInstances
2. router.instances

We change implementation which relies on Helm functions which use
“emptiness” to the implementation which checks if a value is defined in
parent dict. We do not introduce any additional validation like checking
if the value is an integer zero or any other value or type. This has
been validated by Kubernetes using CRD so far, Helm properly reports
this back, and we do not change this.

† https://helm.sh/docs/chart_template_guide/function_list/#default
@mysql-oca-bot
Copy link

Hi, thank you for submitting this pull request. In order to consider your code we need you to sign the Oracle Contribution Agreement (OCA). Please review the details and follow the instructions at https://oca.opensource.oracle.com/
Please make sure to include your MySQL bug system user (email) in the returned form.
Thanks

@f4z4on
Copy link
Author

f4z4on commented Jun 19, 2023

Valid OCA on place. I have not submitted anything to MySQL bug system, it is just this PR. But OCA has my GitHub username.

@mysql-oca-bot
Copy link

Hi, thank you for your contribution. Please confirm this code is submitted under the terms of the OCA (Oracle's Contribution Agreement) you have previously signed by cutting and pasting the following text as a comment:
"I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it."
Thanks

@f4z4on
Copy link
Author

f4z4on commented Jun 19, 2023

I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

@f4z4on
Copy link
Author

f4z4on commented Jun 22, 2023

I have submitted a bug to MySQL bug system—bug # 111533. It’s basically a copy of this PR with links here and slightly different structure / wording.

@mysql-oca-bot
Copy link

Hi, thank you for your contribution. Your code has been assigned to an internal queue. Please follow
bug http://bugs.mysql.com/bug.php?id=111537 for updates.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants