oscerd commented on PR #27216: URL: https://github.com/apache/camel/pull/27216#issuecomment-6013311523
A few follow-up points from a review that was still in progress when this was merged (checked against 612aa6357f, now `7992aa5b45b4` on main). The first one is a small regression worth fixing: 1. **`--cluster-type=k3s` together with `--output` now fails.** Before, `run` without `--disable-auto` replaced the explicit value with the detected type, so `k3s` became `KUBERNETES`. Now `K3S` reaches `KubernetesHelper.getKubernetesManifestPath()`, which maps only KIND and MINIKUBE to `kubernetes`, so `--output=yaml|json` looks for `target/kubernetes/k3s.yml`. `kubernetes-maven-plugin` (selected by `KubernetesExport` for every non-OpenShift type) writes `kubernetes.yml`, so `resolveKubernetesManifestPath` throws `FileNotFoundException: Unable to resolve Kubernetes manifest file type ...`. Adding `ClusterType.K3S.isEqualTo(clusterType)` to the KIND/MINIKUBE branch of `getKubernetesManifest()`/`getKubernetesManifestPath()` fixes it. Don't invert the condition, as the knative branch passes the pseudo type `service`. 2. **An explicit `--cluster-type=minikube` skips the docker-env check.** Detection only returns MINIKUBE when `MINIKUBE_ACTIVE_DOCKERD`/`DOCKER_TLS_VERIFY` are set, and prints the `eval $(minikube docker-env)` hint otherwise. The explicit path now applies docker/no-push without that check, so the image is built into the host daemon and the pod cannot pull it, with no explanation. It also overrides an explicit `--image-builder`/`--image-push` (CAMEL-21710 says a user-set parameter disables the automation for it). 3. **`explicitMinikubeClusterTypeShouldApplyDefaults` also passes on main.** `createCommand()` forces `imagePush = false` after `populateCommand()`, and the minikube env vars plus `setupServerExpectsMinikube()` make detection return MINIKUBE anyway. Dropping those and setting `imagePush = true` before `doCall()` would make it discriminate. The class is also `@DisabledIfSystemProperty(named = "ci.env.name")`, so none of these tests run in CI. 4. **Docs/upgrade guide.** The changed `--cluster-type` semantics (an explicit value now disables detection and drives the per-cluster defaults) could use a note in `camel-4x-upgrade-guide-4_23.adoc`. The doc paragraph also says Kind is detected, but `discoverClusterType()` only detects OpenShift and Minikube. _Claude Code on behalf of oscerd. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying. It does not replace specialized review tools or static analysis._ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
