davsclaus commented on code in PR #27591:
URL: https://github.com/apache/camel/pull/27591#discussion_r4227344488
##########
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:
With only `--image-builder=jib` on Minikube (detected or explicit),
`imagePush` still gets the Minikube default `false`
(`detectedMinikubeKeepsImageBuilderSetByUser` asserts this). The export then
sets `jkube.build.strategy=jib` and `jkube.skip.push=true`. As far as I know,
JKube's Jib strategy builds an image tarball and does not load it into
Minikube's Docker daemon, so the image ends up neither in the daemon nor in a
registry, and the pod cannot pull it. Before this PR, jib was replaced by
docker, which works with docker-env. The `push=false` default only makes sense
with the docker builder, which builds straight into Minikube's daemon, and
CAMEL-21710 says a user-set option disables the automation. Could the push
default follow the resolved builder?
```java
if (imagePush == null && "docker".equals(imageBuilder)) {
this.imagePush = false;
}
```
Then jib falls through to `true` at the end. The test assertion and the doc
sentence in `camel-jbang-kubernetes.adoc` would change accordingly.
##########
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:
Optional: the hint is also printed when no image is built (with `--output`,
which runs with `-Djkube.skip.build=true`, and with `--image-build=false`), and
with an explicit Minikube it is repeated on every dev-mode reload:
```suggestion
if (explicitClusterType && imageBuild && output == null &&
"docker".equals(imageBuilder) && !imagePush
&& !KubernetesHelper.isMinikubeDockerEnv()) {
```
##########
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:
"nothing is pushed (unless you set `--image-builder` or `--image-push`)":
with the current code, setting `--image-builder` alone still pushes nothing,
because `imagePush` keeps the Minikube default `false`. If the change on
`KubernetesRun` is adopted, this sentence becomes accurate; otherwise please
reword it.
--
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]
