Fix bug with CloudRunner and K8s with Namespaces

[harry8525]

Changes

• K8S does not work if it is kicked off from a self hosted runner that runs in a different K8s namespace since the namespace defaults to the one runner is kicked from. Cloud Runner defaults to "default" namespace which if the self hosted runner is not that, it errors.

Successful Workflow Run Link

PRs don't have access to secrets so you will need to provide a link to a successful run of the workflows from your own
repo.

• ...

Checklist

• Read the contribution guide and accept the
  code of conduct
• Docs (If new inputs or outputs have been added or changes to behavior that should be documented. Please make a PR
  in the documentation repo)
• Readme (updated or not needed)
• Tests (added, updated or not needed)

Summary by CodeRabbit

• New Features
  • Added containerNamespace configuration for Kubernetes deployments. Users can now specify a custom namespace; if not provided, it defaults to 'default'. The namespace is properly applied across build operations and log retrieval.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[github-actions]

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds support for custom Kubernetes container namespaces. A new containerNamespace field is introduced to BuildParameters, populated from CloudRunnerOptions. The Kubernetes provider now uses this namespace during initialization, and kubectl log commands are updated to consistently include the namespace flag.

Changes

Cohort / File(s)	Summary
BuildParameters configuration src/model/build-parameters.ts	Added containerNamespace public field and mapped it from CloudRunnerOptions in the create() method.
CloudRunnerOptions provider src/model/cloud-runner/options/cloud-runner-options.ts	Added static getter containerNamespace that returns the input value or defaults to 'default'.
Kubernetes provider initialization src/model/cloud-runner/providers/k8s/index.ts	Updated constructor to set namespace from buildParameters.containerNamespace instead of always using 'default'.
Kubernetes task runner src/model/cloud-runner/providers/k8s/kubernetes-task-runner.ts	Modified kubectl logs extraFlags to consistently append namespace flag (-n ${namespace}) in both PodRunning true and false branches.

Sequence Diagram

sequenceDiagram
    participant opts as CloudRunnerOptions
    participant bp as BuildParameters
    participant k8s as K8s Provider
    participant kubectl as kubectl Task Runner

    opts->>opts: containerNamespace (static getter)
    opts-->>bp: return default or input value
    bp->>bp: create() with containerNamespace
    bp->>k8s: initialize with buildParameters
    k8s->>k8s: set namespace = containerNamespace
    k8s->>kubectl: namespace available
    kubectl->>kubectl: append -n ${namespace} to logs flags

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10-15 minutes

• Verify that the default value 'default' aligns with Kubernetes conventions
• Confirm the data flow from CloudRunnerOptions → BuildParameters → K8s provider is complete and consistent
• Ensure the kubectl logs command namespace flag is correctly formatted and applied in both conditional branches

Poem

🐰 A namespace for our containers bright,
From options passed to bearers of light,
Through BuildParameters it flows with grace,
K8s knows its rightful place,
kubectl flags now unified in might!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The PR description addresses the problem and changes but lacks specific details about the solution implementation and related documentation updates.	Clarify how the containerNamespace parameter works, explain the default behavior, and provide details about what documentation or testing has been completed.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix bug with CloudRunner and K8s with Namespaces' clearly and specifically describes the main change: fixing a namespace handling bug in CloudRunner's Kubernetes provider.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/model/cloud-runner/providers/k8s/index.ts (2)

48-48: Simplify the conditional expression.

The ternary operator can be simplified to use the logical OR operator for better readability.

Apply this diff:

-    this.namespace = buildParameters.containerNamespace ? buildParameters.containerNamespace : 'default';
+    this.namespace = buildParameters.containerNamespace || 'default';

---

48-48: Consider validating namespace existence and permissions.

To provide better error messages and fail fast, consider validating that:

1. The specified namespace exists in the cluster
2. The service account has necessary permissions in that namespace

This would prevent cryptic runtime errors during job creation or pod operations.

Example validation logic could be added after line 48:

// Validate namespace exists
try {
  await this.kubeClient.readNamespace(this.namespace);
  CloudRunnerLogger.log(`Using Kubernetes namespace: ${this.namespace}`);
} catch (error: any) {
  throw new Error(
    `Kubernetes namespace '${this.namespace}' does not exist or is not accessible. ` +
    `Please ensure the namespace exists and the service account has appropriate permissions.`
  );
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d4ee06 and f0234de.

⛔ Files ignored due to path filters (2)

• dist/index.js is excluded by !**/dist/**
• dist/index.js.map is excluded by !**/dist/**, !**/*.map

📒 Files selected for processing (4)

• src/model/build-parameters.ts (2 hunks)
• src/model/cloud-runner/options/cloud-runner-options.ts (1 hunks)
• src/model/cloud-runner/providers/k8s/index.ts (1 hunks)
• src/model/cloud-runner/providers/k8s/kubernetes-task-runner.ts (1 hunks)

🔇 Additional comments (5)

src/model/cloud-runner/options/cloud-runner-options.ts (1)

138-140: LGTM! Implementation follows established patterns.

The new containerNamespace getter correctly defaults to 'default' and follows the same pattern as other container configuration getters like containerCpu and containerMemory.

src/model/build-parameters.ts (2)

62-62: LGTM! Field declaration follows the established pattern.

The new containerNamespace field is properly declared and positioned logically near other Kubernetes-related configuration fields.

---

191-191: LGTM! Mapping correctly integrates the new parameter.

The field is properly mapped from CloudRunnerOptions.containerNamespace in the create() method, following the same pattern as other CloudRunner options.

src/model/cloud-runner/providers/k8s/kubernetes-task-runner.ts (1)

33-34: Excellent fix! Namespace flag now consistently specified.

This change ensures that kubectl logs commands always include the -n ${namespace} flag in both code paths (running pod with follow mode and previous/terminated pod mode). This directly addresses the bug where kubectl would otherwise default to the runner's current namespace context, causing failures when the runner is in a different namespace than the workload.

src/model/cloud-runner/providers/k8s/index.ts (1)

48-48: The review comment references code that does not exist in the repository.

The review comment points to line 48 showing this.namespace = buildParameters.containerNamespace ? buildParameters.containerNamespace : 'default'; but the actual code at line 49 hardcodes the namespace to 'default'. The containerNamespace property is not present in buildParameters anywhere in the codebase.

Currently, users cannot specify arbitrary Kubernetes namespaces—the namespace is fixed to 'default'. The security concerns about RBAC permissions and namespace validation do not apply since there is no user-configurable namespace selection mechanism in the current code.

Likely an incorrect or invalid review comment.