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

Terraform rancher2 agent customization #1137

Merged
merged 4 commits into from
Jun 21, 2023

Conversation

a-blender
Copy link
Contributor

@a-blender a-blender commented Jun 1, 2023

Issue:

#1097

Problem

Cluster/Fleet agent deployment customization is not supported by the TFP.

Solution

Add cluster_agent_deployment_customization and fleet_agent_deployment_customization (and nested) fields tolerations, affinity, and resource requirements to the rancher2 provider so users will be able to define those values for RKE, RKE2/K3s, and EKS clusters.

Error handling

For agent deployment customization, we have a unique scenario with error handling. Most tf resources do not add error handling in the expander, but for rke clusters I have to marshal v1 Tolerations and Affinity strings into objects that exist on the rancher backend. They are being passed as strings because 1) it's too complex to struct out all the nested fields, 2) this is how terraform harvester encodes node affinity. If the Toleration or Affinity string passed via the tf config is invalid, expanding the field will fail. I argue that this should cause a cluster create/update to also fail.

My solution to this is to add error handling to the expander. If expanding an agent deployment customization returns nil with no error, customization is not defined and a create/update will pass. If it returns nil with an error, a create/update will fail and the user needs to fix their config.

Testing

6/2 update: Not fully tested yet, but requesting preliminary review of backend logic for the rke case.

6/13 update: Finished testing rke with all new fields and fixing bugs. Working on adding EKS, testing v2 prov and EKS and updating tests.

6/16 update: Added Toleration v2 and ResourceRequirement resources, updated backend logic. Added/updated tests which turned out to be tricky. Need to re-test

Engineering Testing

Manual Testing

Smoke testing

  • rke single all-role node + agent customization
    • remove agent customization, verify removed
    • add it back, verify customization is added
    • update customization, verified it got updated on the v3 cluster spec and the agent deployment yaml
  • rke2 (v2 prov) single all-role node + agent customization
    • same as above
  • eks 2 node + agent customization
    • same as above

Note: you may not see cluster go into Updating state in the UI for added/updated agent customization or minor updates because the redeploy is fast.

Automated Testing

QA Testing Considerations

Regressions Considerations

@a-blender a-blender force-pushed the cluster-agent-customization branch 2 times, most recently from 2b744c3 to f7dd78e Compare June 5, 2023 16:31
@a-blender a-blender force-pushed the cluster-agent-customization branch 3 times, most recently from 831c191 to 794be56 Compare June 5, 2023 21:07
@a-blender a-blender force-pushed the cluster-agent-customization branch from 794be56 to ea1bee1 Compare June 13, 2023 00:30
@a-blender a-blender changed the title Cluster agent customization Terraform rancher2 cluster agent customization Jun 13, 2023
@a-blender a-blender changed the title Terraform rancher2 cluster agent customization Terraform rancher2 agent customization Jun 13, 2023
- Update error handling and go mod
- Address comments
@a-blender a-blender force-pushed the cluster-agent-customization branch 4 times, most recently from e7d43da to 68198d5 Compare June 16, 2023 21:02
@a-blender a-blender force-pushed the cluster-agent-customization branch from 68198d5 to b214863 Compare June 20, 2023 22:36
@a-blender a-blender marked this pull request as ready for review June 20, 2023 22:37
@a-blender a-blender requested a review from a team June 20, 2023 22:37
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.

Few nits

rancher2/structure_cluster_v2_test.go Outdated Show resolved Hide resolved
rancher2/structure_agent_deployment_customization.go Outdated Show resolved Hide resolved
rancher2/structure_resource_requirements.go Show resolved Hide resolved
rancher2/structure_resource_requirements.go Show resolved Hide resolved
rancher2/structure_resource_requirements_v2.go Outdated Show resolved Hide resolved
@a-blender a-blender requested a review from a team June 21, 2023 16:09
@a-blender a-blender merged commit 3343210 into rancher:master Jun 21, 2023
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