Skip to content

Conversation

@jmontleon
Copy link
Member

@jmontleon jmontleon commented Sep 23, 2025

If you start out using one provider, lets say openai, the deployment will be created with the environment:

        - name: OPENAI_API_KEY
          valueFrom:
            secretKeyRef:
              key: OPENAI_API_KEY
              name: kai-api-keys

If you then switch to another lets say bedrock and change the secret you'll end up with

    - name: AWS_BEARER_TOKEN_BEDROCK
      valueFrom:
        secretKeyRef:
          key: AWS_BEARER_TOKEN_BEDROCK
          name: kai-api-keys

But with the default strategic-merge option the deployments openai env var is not cleaned up. It still expects the OPENAI_API_KEY to exist in the secret, even if you're no longer using it.

Switching to the merge merge_type will ensure that the deployment stays in sync with the keys in the secret.

Summary by CodeRabbit

  • Chores
    • Updated Kai API deployment behavior to merge updates with existing Kubernetes resources, improving compatibility with cluster-managed fields and reducing the risk of disruptive changes during updates.
    • Service creation remains unchanged.
    • Users should experience smoother, more reliable rollouts with fewer interruptions during deployments.

@jmontleon jmontleon requested a review from fabianvf September 23, 2025 15:42
@coderabbitai
Copy link

coderabbitai bot commented Sep 23, 2025

Walkthrough

Updated Ansible task in roles/tackle/tasks/kai.yml to set merge_type: merge for the k8s module when creating the Kai API deployment. No other tasks or files were changed.

Changes

Cohort / File(s) Summary of Changes
Kai deployment task update
roles/tackle/tasks/kai.yml
Added merge_type: merge to the k8s task for "Create Kai API deployment" to merge the provided manifest with any existing resource. No changes to the subsequent Service task.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

I thump the ground—deployments surge,
A gentle merge, not harshly purge.
YAML winds now softly weave,
Kai stands steady, configs cleave.
Ears up high, I sip my tea—
Small tweak made, ship calmly. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Fix changing providers" is concise and directly relates to the PR's intent to address issues when switching provider-related secrets and environment variables, though it does not call out the specific implementation change (using merge_type on the deployment). It communicates the primary user-facing problem being fixed.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jmontleon jmontleon force-pushed the fix-changing-providers branch from 1bfd618 to 8be3cc8 Compare September 23, 2025 15:43
@jmontleon jmontleon changed the title Fix changing providers 🐛 fix deployment env var handling when switching providers Sep 23, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
roles/tackle/tasks/kai.yml (1)

75-75: Use a list for merge_type

kubernetes.core.k8s declares merge_type as a list; change to an explicit list (requirements.yml pins kubernetes.core 2.4.0).

-        merge_type: merge
+        merge_type: ["merge"]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 166e2cd and 8be3cc8.

📒 Files selected for processing (1)
  • roles/tackle/tasks/kai.yml (1 hunks)
🔇 Additional comments (1)
roles/tackle/tasks/kai.yml (1)

75-75: Replace JSON Merge Patch with server-side apply to avoid dropping injected sidecars

merge_type: merge uses a JSON Merge Patch that can replace arrays (e.g., spec.template.spec.containers) and drop webhook-injected sidecars (Istio/Linkerd/OTel). Use server-side apply to preserve controller/webhook-owned fields while removing stale env vars.

File: roles/tackle/tasks/kai.yml (line 75)

-        merge_type: merge
+        apply: true
+        field_manager: konveyor-operator
+        force: true

Verify in a mesh-enabled cluster that sidecars remain after rerunning the role and that obsolete env vars are removed as intended.

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.

2 participants