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

Fix msc old schema to have correct type #1223

Merged
merged 2 commits into from
Sep 27, 2023

Conversation

a-blender
Copy link
Contributor

@a-blender a-blender commented Sep 8, 2023

Issue: #1074

Problem

Solution

  • Update the machine_selector_config.config old schema to original type Map since it will only be used in the schema migration on tf upgrade. Won't affect newly created clusters.
  • Update docs

Testing

Engineering Testing

Manual Testing

Tested provisioning an RKE2 cluster on the latest version of tf rancher2 with MachineConfigSelector with config defined. Successful, new schema is used, no errors.

Tested upgrade case from 3.1.0 -> latest. State is upgraded to the correct type.

Example

Old state

"machine_selector_config": [
                  {
                    "config": {
                      "protect-kernel-default": "true"
                    },
                    "machine_label_selector": []
                  }
                ],

New state

"machine_selector_config": [
                  {
                    "config": "protect-kernel-default: \"true\"\n",
                    "machine_label_selector": []
                  }
                ],

Automated Testing

QA Testing Considerations

Regressions Considerations

@HarrisonWAffel
Copy link
Contributor

@a-blender does this mean that prior versions of the provider which used TypeString for this field are not properly migrating? or have we not released a version where the field uses TypeString?

@a-blender a-blender requested review from thatmidwesterncoder and removed request for jiaqiluo September 11, 2023 21:14
@a-blender
Copy link
Contributor Author

a-blender commented Sep 11, 2023

@a-blender Possible, it worked fine before but I had tested the old TF version by adding MSC kubelet args via the rancher UI because this field originally was broken in the provider and wasn't passing values correctly so it didn't make logical sense to test a use case that users couldn't even use themselves. I re-tested that a migration works correctly via TF input only for this.

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.

3 participants