Bump kubernetes-client-bom from 6.7.2 to 6.8.1

[manusa]

• Kubernetes Client 6.8.0 was just released: https://github.com/fabric8io/kubernetes-client/releases/tag/v6.8.0
• Kubernetes Client 6.8.1 was just released: https://github.com/fabric8io/kubernetes-client/releases/tag/v6.8.1

This will require bumping the Fabric8 Kubernetes Client version in Dekorate first.

• Bump Kubernetes Client to 6.8.0 dekorateio/dekorate#1233
• Release 4.0.0 ? dekorateio/dekorate#1235

The Dekorate dependency is also bumped to latest 4.0.0 which includes the 6.8.0 compatible Kubernetes Client dependency.

/cc @metacosm

[metacosm]

Tried this PR locally, this is definitely not a backwards compatible change: some of my code doesn't compile anymore.

[manusa]

Could you provide more details about your specific compilation failures?

Is it related to the change of syntax in builders? Or in intermediate type signatures (fluents)?

[metacosm]

Yes, issues with intermediate signatures with fluents, things like NamedInstallStrategyFluent.SpecNested<ClusterServiceVersionSpecFluent.InstallNested<ClusterServiceVersionFluent.SpecNested<ClusterServiceVersionBuilder>>> used as a method parameter…

[manusa]

You'll notice how these can be fixed if you check the changes in this PR.

I'm not sure how we should document this in Quarkus to help user's migration

[metacosm]

Not that easy in my case… Then again, I can't already not read or understand the explicit type as it is. I applied the suggestion from my IDE and the type got changed to:

NamedInstallStrategyFluent<ClusterServiceVersionSpecFluent<ClusterServiceVersionFluent<ClusterServiceVersionBuilder>.SpecNested<ClusterServiceVersionBuilder>>.InstallNested<ClusterServiceVersionFluent<ClusterServiceVersionBuilder>.SpecNested<ClusterServiceVersionBuilder>>>.SpecNested<ClusterServiceVersionSpecFluent<ClusterServiceVersionFluent<ClusterServiceVersionBuilder>.SpecNested<ClusterServiceVersionBuilder>>.InstallNested<ClusterServiceVersionFluent<ClusterServiceVersionBuilder>.SpecNested<ClusterServiceVersionBuilder>>>

I mean, come on… that's completely not understandable (not that the original type was) but now it's significantly worse. I mean I wouldn't be able to make that change without assistance from the IDE. Not sure what we can do here but we're definitely not going in the right direction on this particular issue.

[metacosm]

I managed to get it dealt with by using NamedInstallStrategyFluent<?>.SpecNested<?> instead, which I guess makes sense.

[metacosm]

Where do we stand on this? Has dekorate 4.0.0 been released as I don't see it in the list of releases? /cc @manusa @iocanel

[manusa]

From my standing point the pull request is ready (it included the 4.0.0 Dekorate bump one week ago) , not sure if it's expected for something else to be added.

[metacosm]

Can we get this merged then? @iocanel @geoand

[geoand]

I have no objections, but I would like the PR rebased onto main since the last time we had a CI run was over a week ago.

[geoand]

I assume this is a somewhat breaking change, right?

[metacosm]

I assume this is a somewhat breaking change, right?

Yes, so probably for 3.5.

[metacosm]

@manusa minor nit picking but the commit message for the second commit is wrong: it should say dekorate, not kubernetes client bom.

[manusa]

I assume this is a somewhat breaking change, right?

There are breaking changes with Sundrio generated classes (builders).

They can be mostly tackled by using type inference or by following the approach I did in this PR (#34956 (comment)) (for intermediate variables).

Also some builder methods are no longer available. However, they were already deprecated.

I probably asked this internally (as I can't find the comment here). Maybe we want to add some entries for the migration process.

[quarkus-bot]

Failing Jobs - Building cac2df1

Status	Name	Step	Failures	Logs	Raw logs	Build scan
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 17	Build	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 20

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 17 #

- Failing: extensions/smallrye-graphql/deployment 
! Skipped: extensions/micrometer-registry-prometheus/deployment extensions/micrometer/deployment extensions/opentelemetry/deployment and 38 more

📦 extensions/smallrye-graphql/deployment

✖ io.quarkus.smallrye.graphql.deployment.RequestLeakDetectionTest.testWithConcurrentCalls line 55 - More details - Source on GitHub

BUG! exception in phase 'class generation' in source unit 'Script1.groovy' unexpected NullPointerException
	at org.codehaus.groovy.control.CompilationUnit$IPrimaryClassNodeOperation.doPhaseOperation(CompilationUnit.java:942)
	at org.codehaus.groovy.control.CompilationUnit.processPhaseOperations(CompilationUnit.java:692)

[michalvavrik]

After this PR, when I use io.fabric8.kubernetes.client.impl.KubernetesClientImpl#load to load my template

---
apiVersion: "v1"
kind: "List"
items:
- apiVersion: "v1"
  kind: "Service"
  metadata:
    name: "app"
  spec:
    ports:
    - name: "http"
      port: 8080
      targetPort: 8080
    selector:
      app.kubernetes.io/name: "app"
    type: "ClusterIP"
- apiVersion: "v1"
  kind: "Service"
  metadata:
    name: "app-management"
  spec:
    ports:
      - name: "management"
        port: 9000
        targetPort: 8080
    selector:
      app.kubernetes.io/name: "app"
    type: "ClusterIP"
- apiVersion: "apps.openshift.io/v1"
  kind: "DeploymentConfig"
  metadata:
    labels:
      app.openshift.io/runtime: "quarkus"
      app.kubernetes.io/name: "app"
    name: "app"
  spec:
    replicas: 1
    selector:
      app.kubernetes.io/name: "app"
    template:
      metadata:
        labels:
          app.openshift.io/runtime: "quarkus"
          app.kubernetes.io/name: "app"
      spec:
        containers:
        - env:
          - name: "KUBERNETES_NAMESPACE"
            valueFrom:
              fieldRef:
                fieldPath: "metadata.namespace"
          image: "my-registry"
          name: "app"
          ports:
            - containerPort: 8080
              name: "http"
              protocol: "TCP"
              ports:
            - containerPort: 8080
              name: "management"
              protocol: "TCP"

my deployment config spec contains empty triggers and then when I adjust stuff and serialize it as KubernetesList, I can see newly template contains triggers: []

- apiVersion: "apps.openshift.io/v1"
  kind: "DeploymentConfig"
  metadata:
    labels:
      app.openshift.io/runtime: "quarkus"
      app.kubernetes.io/name: "app"
      scenarioId: "OpenShiftUsingContainerRegistryPingPongResourceIT-1695036746"
    name: "app"
    namespace: "mvavrik-debug"
  spec:
    replicas: 1
    selector:
      app.kubernetes.io/name: "app"
    template:
      metadata:
        labels:
          app.openshift.io/runtime: "quarkus"
          app.kubernetes.io/name: "app"
          tsLogWatch: "app"
          scenarioId: "OpenShiftUsingContainerRegistryPingPongResourceIT-1695036746"
        namespace: "mvavrik-debug"
      spec:
        containers:
        - env:
          - name: "KUBERNETES_NAMESPACE"
            valueFrom:
              fieldRef:
                fieldPath: "metadata.namespace"
          - name: "quarkus.log.console.format"
            value: "%d{HH:mm:ss,SSS} %s%e%n"
          image: "my-registry"
          name: "app"
          ports:
          - containerPort: 8080
            name: "http"
            protocol: "TCP"
            ports: null
          - containerPort: 8080
            name: "management"
            protocol: "TCP"
    triggers: []

which prevents my rollout with oc: error: #1 is already in progress (Pending).. This is new behavior, why are triggers: [] added, please?

[michalvavrik]

nope, I got rid of triggers and still having issue (with the bump only), so it's not related to the triggers.

[manusa]

cc/ @shawkins

There were some changes in the serialization default behavior, let me try to find some references.

[manusa]

Fix #5262: Built-in resources were in-consistent with respect to their serialization or empty collections. In many circumstances this was confusing behavior. In order to be consistent all built-in resources will omit empty collections by default. This is a breaking change if you are relying on an empty collection in a json merge or a strategic merge where the list has a patchStrategy of atomic. In these circumstances the empty collection will no longer be serialized. You may instead use a json patch, server side apply instead, or modify the serialized form of the patch.

fabric8io/kubernetes-client#5262

AFAIR setting the triggers to null (when "adjusting stuff") should fix your problem.

https://github.com/manusa/yakd/blob/f750188140c3706ed92ce51f111a7087484b7b56/src/main/java/com/marcnuri/yakd/deploymentconfigs/DeploymentConfigService.java#L89-L93

[michalvavrik]

Fix #5262: Built-in resources were in-consistent with respect to their serialization or empty collections. In many circumstances this was confusing behavior. In order to be consistent all built-in resources will omit empty collections by default. This is a breaking change if you are relying on an empty collection in a json merge or a strategic merge where the list has a patchStrategy of atomic. In these circumstances the empty collection will no longer be serialized. You may instead use a json patch, server side apply instead, or modify the serialized form of the patch.

fabric8io/kubernetes-client#5262

AFAIR setting the triggers to null (when "adjusting stuff") should fix your problem

yeah, it did. and after that, diff on previously serialized and newly (after bump) serialized deployment config doesn't show changes. I have some other issue caused by this bump though, I don't know where so probably not right time to get you involved :-)

Thanks for prompt answer.