allthingssecurity commented on code in PR #27591:
URL: https://github.com/apache/camel/pull/27591#discussion_r4227457911


##########
dsl/camel-jbang/camel-jbang-plugin-kubernetes/src/main/java/org/apache/camel/dsl/jbang/core/commands/kubernetes/KubernetesRun.java:
##########
@@ -820,16 +825,33 @@ private void detectCluster() {
             this.clusterType = ClusterType.KUBERNETES.name();
         }
         if (!disableAuto) {
-            // Apply per-cluster defaults for the resolved cluster type 
(explicit or detected)
+            // Apply per-cluster defaults for the resolved cluster type 
(explicit or detected),
+            // but keep the image options the user has set
             if (ClusterType.MINIKUBE.isEqualTo(clusterType)) {
-                this.imageBuilder = "docker";
-                this.imagePush = false;
+                if (imageBuilder == null) {
+                    this.imageBuilder = "docker";
+                }
+                if (imagePush == null) {
+                    this.imagePush = false;
+                }

Review Comment:
   Agreed and applied in ebcbeedcc139: the Minikube `--image-push=false` 
default now applies only when the resolved builder is `docker`, so 
`--image-builder=jib` alone falls through to pushing. 
`detectedMinikubeKeepsImageBuilderSetByUser` now asserts `imagePush` is true, 
and the `--image-push` description and the upgrade-guide note say the no-push 
default is for the docker builder.
   
   _Claude Code on behalf of allthingssecurity_



##########
docs/user-manual/modules/ROOT/pages/camel-jbang-kubernetes.adoc:
##########
@@ -288,7 +290,10 @@ eval $(minikube -p minikube docker-env)
 camel kubernetes run * --runtime=spring-boot
 ----
 
-On Minikube the image is built with Docker straight into the Minikube Docker 
daemon, so nothing is pushed.
+On Minikube the image is built with Docker straight into the Minikube Docker 
daemon, so nothing is pushed
+(unless you set `--image-builder` or `--image-push`).

Review Comment:
   With the builder-dependent push default (ebcbeedcc139) I reworded it to be 
exact: nothing is pushed unless you set `--image-push`, and with another 
`--image-builder` such as `jib` the image is pushed as on any other cluster.
   
   _Claude Code on behalf of allthingssecurity_



##########
dsl/camel-jbang/camel-jbang-plugin-kubernetes/src/main/java/org/apache/camel/dsl/jbang/core/commands/kubernetes/KubernetesRun.java:
##########
@@ -820,16 +825,33 @@ private void detectCluster() {
             this.clusterType = ClusterType.KUBERNETES.name();
         }
         if (!disableAuto) {
-            // Apply per-cluster defaults for the resolved cluster type 
(explicit or detected)
+            // Apply per-cluster defaults for the resolved cluster type 
(explicit or detected),
+            // but keep the image options the user has set
             if (ClusterType.MINIKUBE.isEqualTo(clusterType)) {
-                this.imageBuilder = "docker";
-                this.imagePush = false;
+                if (imageBuilder == null) {
+                    this.imageBuilder = "docker";
+                }
+                if (imagePush == null) {
+                    this.imagePush = false;
+                }
+                // the detection only returns Minikube when its Docker 
environment is active, and tells the user otherwise
+                if (explicitClusterType && "docker".equals(imageBuilder) && 
!imagePush
+                        && !KubernetesHelper.isMinikubeDockerEnv()) {

Review Comment:
   Applied in ebcbeedcc139 (`imageBuild && output == null`, with 
`Boolean.FALSE.equals(imagePush)` since the option is now a `Boolean`), so no 
hint with `--output` or `--image-build=false`. Two new tests in 
`KubernetesRunClusterTypeTest` cover both cases.
   
   _Claude Code on behalf of allthingssecurity_



-- 
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]

Reply via email to