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 machine selector config to allow kubelet arg list #1181

Merged
merged 2 commits into from Aug 8, 2023

Conversation

a-blender
Copy link
Contributor

@a-blender a-blender commented Jul 28, 2023

Issue: #1074

Problem

Machine Selector Config kubelet-arg values could not be passed via Terraform to downstream machines and would not show up in the rancher UI.

Solution

My solution is to convert machine_selector_config.config from Type.Map (string map) to Type.String that supports yaml like Machine Global Config + state migration logic to handle the schema update. This allows for users to input both strings and lists, which allows subfield kubelet-arg to be passed as a list which is what Rancher is expecting. It also prevents a potential regression caused by 1) only allowing string args or lists which will reduce the flexibility of Machine Selector Config or 2) defining kubelet-arg as a subfield which will break other config fields that are already expected in the Rancher backend such as private-default-registry.

Before, passing kubelet-arg under Machine Selector Config would not work. This feature now works with the following configuration

machine_selector_config {
  config = <<EOF
    protect-kernel-default: true
    kubelet-arg:
      - key1=value1
      - key2=value2
  EOF
}

Testing

Engineering Testing

Manual Testing

Test plan

  • Prov rke2 cluster with Machine Selector Config and 2 kubelet args set. Verify cluster provisions successfully and args are displayed correctly in the rancher ui
  • Add/remove a kubelet-arg via tf and verify list is updated in the ui
  • Prov rke2 cluster with tf 3.1.0. Add machine selector config with 2 kubelet args via the rancher ui
  • Upgrade tf to rc version with fix
  • Verify no errors appear on terraform plan/apply or updates to Machine Selector Config.

Automated Testing

Tests updated. Ran go test -v ./rancher2 to make sure all automated tests pass.

QA Testing Considerations

Regressions Considerations

@a-blender a-blender marked this pull request as ready for review July 31, 2023 21:34
Copy link
Contributor

@jakefhyde jakefhyde left a comment

Choose a reason for hiding this comment

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

Consider this a lgtm once @felipe-colussi's comments are addressed.

@a-blender a-blender dismissed jakefhyde’s stale review August 8, 2023 14:43

Lgtm once other comments were addressed

@a-blender a-blender merged commit 7b9d01c into rancher:master Aug 8, 2023
1 check passed
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.

None yet

4 participants