Code Review

This pull request enables templateRef support for LeaderWorkerSet and LeaderWorkerPattern workloads by updating validation rules and the RoleInstanceSet reconciler. The reconciler now resolves a base template from either a referenced RoleTemplate (applying a templatePatch if present) or an inline definition. Key feedback highlights the necessity of using DeepCopy() when retrieving and patching templates to avoid mutating the controller's cache or leaking state between leader and worker configurations. There is also a suggestion to refactor the template resolution logic into a shared utility to improve maintainability.

Pull request overview

Adds support for using templateRef (RoleTemplates) with LeaderWorkerPattern when constructing RoleInstanceSet leader/worker pod templates, enabling reuse of shared base pod templates while still applying leader/worker-specific patches.

Changes:

• Resolve a baseTemplate for LeaderWorkerPattern from either inline template or templateRef + templatePatch, then apply leaderTemplatePatch/workerTemplatePatch.
• Update RoleInstanceSet leader/worker template construction to use applyStrategicMergePatch and pass templates by value.
• Remove controller-side validation that previously rejected templateRef for LeaderWorkerPattern (and also for LeaderWorkerSet workloads).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

		if role.Workload.Kind == "InstanceSet" {
			return fmt.Errorf(
				"spec.roles[%d].templateRef: not supported for InstanceSet workloads",
				index,
			)
		}

		// Cross-resource check: referenced template must exist.
		templateRef := role.GetTemplateRef()
		if !validTemplateNames[templateRef.Name] {
			return fmt.Errorf(
				"spec.roles[%d].templateRef.name: template %q not found in spec.roleTemplates",
				index, templateRef.Name,
			)
		}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.