Skip to content

Fix LauncherConfig CRD to accept metadata in podTemplate#434

Open
MikeSpreitzer wants to merge 3 commits intollm-d-incubation:mainfrom
MikeSpreitzer:fix-433
Open

Fix LauncherConfig CRD to accept metadata in podTemplate#434
MikeSpreitzer wants to merge 3 commits intollm-d-incubation:mainfrom
MikeSpreitzer:fix-433

Conversation

@MikeSpreitzer
Copy link
Copy Markdown
Collaborator

@MikeSpreitzer MikeSpreitzer commented Apr 16, 2026

Summary

  • Replace corev1.PodTemplateSpec with custom EmbeddedPodTemplateSpec that uses EmbeddedObjectMeta with explicit Labels and Annotations fields
  • Update docs/cluster-sharing.md, declaring that changes like this are OK
  • Update BuildLauncherPodFromTemplate to accept the new type and convert metadata fields
  • Regenerate CRD, DeepCopy, and apply configurations

The generated CRD schema now includes proper labels and annotations properties under podTemplate.metadata, so strict decoding accepts them.

Fixes #433

Test plan

  • go build ./... passes
  • go test ./... passes
  • Generated CRD confirms labels/annotations properties under podTemplate.metadata
  • Manual: kubectl apply a LauncherConfig with spec.podTemplate.metadata.labels — no strict decoding error

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings April 16, 2026 17:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the LauncherConfig API/CRD to allow spec.podTemplate.metadata.labels and spec.podTemplate.metadata.annotations under strict decoding by replacing corev1.PodTemplateSpec with a schema-friendly embedded type, and wiring the controller to translate that embedded metadata into real Pods.

Changes:

  • Introduce EmbeddedObjectMeta / EmbeddedPodTemplateSpec and switch LauncherConfigSpec.PodTemplate to the new type.
  • Update launcher Pod construction to read the embedded metadata and set Pod labels/annotations accordingly.
  • Regenerate CRD schema, deepcopy code, and server-side-apply configurations for the new types.

Reviewed changes

Copilot reviewed 4 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/generated/applyconfiguration/utils.go Registers applyconfig types for EmbeddedObjectMeta and EmbeddedPodTemplateSpec.
pkg/generated/applyconfiguration/fma/v1alpha1/launcherconfigspec.go Switches applyconfig podTemplate field to the new embedded template apply type.
pkg/generated/applyconfiguration/fma/v1alpha1/embeddedpodtemplatespec.go Adds applyconfig for the new embedded pod template type.
pkg/generated/applyconfiguration/fma/v1alpha1/embeddedobjectmeta.go Adds applyconfig for embedded metadata (labels/annotations).
pkg/controller/utils/pod-helper.go Updates BuildLauncherPodFromTemplate to accept embedded template and map metadata into metav1.ObjectMeta.
config/crd/fma.llm-d.ai_launcherconfigs.yaml Regenerates CRD schema so podTemplate.metadata.labels/annotations are structurally defined.
api/fma/v1alpha1/zz_generated.deepcopy.go Adds deepcopy implementations for the new embedded types and updates LauncherConfigSpec deepcopy.
api/fma/v1alpha1/launcherconfig_types.go Defines embedded types and switches LauncherConfigSpec.PodTemplate to EmbeddedPodTemplateSpec.

Comment on lines +148 to +151
ObjectMeta: metav1.ObjectMeta{
Labels: template.Metadata.Labels,
Annotations: template.Metadata.Annotations,
},
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

template.Metadata.Labels / Annotations are assigned directly into the Pod's ObjectMeta, but this function later mutates pod.Labels and pod.Annotations. Because maps are reference types, this will also mutate the LauncherConfig's cached spec.podTemplate.metadata maps (from the informer/lister) when users supply labels/annotations, which can cause surprising controller behavior and potential races. Clone the maps (or deep-copy the template metadata) before mutating the Pod's labels/annotations so the LauncherConfig object remains immutable in memory.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

@MikeSpreitzer
Copy link
Copy Markdown
Collaborator Author

/ok-to-test

@github-actions
Copy link
Copy Markdown

🚀 E2E tests triggered by /ok-to-test

View the OpenShift E2E workflow run

MikeSpreitzer and others added 3 commits April 16, 2026 14:34
controller-gen renders embedded metav1.ObjectMeta as a bare
`type: object` with no sub-properties, causing strict decoding to
reject labels and annotations on spec.podTemplate.metadata.

Replace corev1.PodTemplateSpec with custom EmbeddedPodTemplateSpec
that explicitly declares Labels and Annotations fields so the CRD
schema admits them.

Fixes llm-d-incubation#433

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Mike Spreitzer <mspreitz@us.ibm.com>
Signed-off-by: Mike Spreitzer <mspreitz@us.ibm.com>
Previously the deploy script skipped `kubectl apply` for any CRD that
already existed, which meant spec changes (e.g. new fields) were never
picked up. Now the script compares the live `.spec` against the desired
one and re-applies when they differ.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Mike Spreitzer <mspreitz@us.ibm.com>
@MikeSpreitzer
Copy link
Copy Markdown
Collaborator Author

/ok-to-test

@github-actions
Copy link
Copy Markdown

🚀 E2E tests triggered by /ok-to-test

View the OpenShift E2E workflow run

@MikeSpreitzer
Copy link
Copy Markdown
Collaborator Author

The first try of the E2E test on OpenShift failed due to flake #435.

@MikeSpreitzer
Copy link
Copy Markdown
Collaborator Author

The retry of the E2E test on OpenShift succeeded.

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.

[Bug]: LauncherConfig does not admit metadata in the launcher template

2 participants