Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump Kubernetes Client to 6.8.0 #1233

Merged
merged 2 commits into from
Aug 14, 2023

Conversation

manusa
Copy link
Collaborator

@manusa manusa commented Jul 24, 2023

  • deps: Bump Kubernetes Client to 6.8.0
  • deps: Bump sundrio to 0.100.1

Prior to bumping the Kubernetes Client version in Quarkus, we need to align the Fabric8 Kubernetes Client in Dekorate.

I'm trying to update. However, I'm reaching two problems (for now):

  • If keeping the Sundrio version intact, the prometheus-annotations module (and probably others) won't work/compile since its buildables rely on other Kubernetes Client buildables (such as ObjectMeta) that are not compatible accross sundrio versions.
  • If updating the Sundrio version too, then there seeems to be a problem with the option-annotations which makes use of the @Pojo generator. I think that since it has a Buildable superClass (in this case Configuration) the template uses a getXxx method which is not OK. My impression is that the problem is in Sundrio and that this wasn't considered when removing the Fluent interfaces (the Pojo tests don't include this scenario)

I'm kind of blocked at the moment.

/cc @iocanel

@iocanel
Copy link
Member

iocanel commented Jul 24, 2023

@manusa: I think that we should definitely update both at the same time.

@iocanel
Copy link
Member

iocanel commented Jul 24, 2023

I am a bit worried though about the impact that this will have downstream.

@manusa
Copy link
Collaborator Author

manusa commented Jul 24, 2023

I also bootstrapped the Quarkus PR (quarkusio/quarkus#34956) to keep track of anything.

I don't think there's any rush, so we can work on this with calm.

At the moment I can't look into the Sundrio problem. If there are no more updates, I'll try to look into it at the end of the week.

@iocanel
Copy link
Member

iocanel commented Jul 27, 2023

.My impression is that the problem is in Sundrio and that this wasn't considered when removing the Fluent interfaces (the Pojo tests don't include this scenario)

At closer inspection:
This is not something that has not been considered.
It's a common case of having trouble recognizing as Buildable a class coming from a dependency.
In this particular case the annotaiton prossecor processing JvmOptions fails to recognize Project as buildalbe.

Using something like:

@Buildable(builderPackage = "io.fabric8.kubernetes.api.builder", refs = @BuildableReference(Project.class))

fixes things.

Unfortunately, this needs to be added pretty much to all modules, which is lot of work.

I've been pondering for a while an alternative mechanism of detecting buildables and via class lookup (e.g. if there is an XXXFluent.class and XXXBuilder.class, then XXX is buildable). Maybe its time to put it to the test.

@manusa
Copy link
Collaborator Author

manusa commented Aug 4, 2023

I added the BuildableReferences which seems to take care of the problem.
However, now I have another issue with the generated KubernetesConfig builder generated from the KubernetesApplication annotation-pojo-generator.
Since BaseConfig has many of the fields KubernetesConfig has, I'm running into variable xxx is already defined in constructor EditableKubernetesConfig....
I'm trying to reproduce this on sundrio itself (sundrio/sundrio#411) but it's working fine (although there I'm not extending a generated Pojo).
Any ideas on how to work around this?

@metacosm
Copy link
Collaborator

metacosm commented Aug 7, 2023

This is definitely needed before we update the client in Quarkus as this is causing errors downstream as seen in https://github.com/quarkiverse/quarkus-operator-sdk/actions/runs/5779716289/job/15662390642#step:8:3612 for example.

@manusa manusa marked this pull request as ready for review August 8, 2023 13:28
@manusa
Copy link
Collaborator Author

manusa commented Aug 8, 2023

It's compiling locally now

@manusa
Copy link
Collaborator Author

manusa commented Aug 9, 2023

Not sure if this is due to the change in the Sundrio Builders, annotations, or anything else.

However, the S2i built image is not able to run the artifact.

The jar file doesn't contain a manifest main attribute and is not able to launch the jar file.

The fabric8/s2i-java image does include stuff to explode a Spring Boot Jar file and infer the BOOT-INF information, however this isn't present either in the Jar artifact.

So I'm assuming that there must be a problem with whatever logic you have in place to determine what's the main class (I've seen the SpringBootApplicationXxx classes have something there) and the new sundrio generated Pojos and builders.

deps: Bump sundrio to 0.100.3
@manusa
Copy link
Collaborator Author

manusa commented Aug 10, 2023

As discussed internally, the current behavior is OK.

Some of the OpenShift integration tests aren't fine, this is why they are flaky at the moment.
It appears that despite the Pods not being able to run (no main attribute in Jar manifest), they go into the Running state for a brief period of time.

o-name-1-deploy   0/1       ContainerCreating   0         4m
o-name-1-deploy   1/1       Running             0         4m
o-name-1-fsmqv    0/1       Pending             0         4m
o-name-1-fsmqv    0/1       Pending             0         4m
o-name-1-fsmqv    0/1       ContainerCreating   0         4m
o-name-1-fsmqv    0/1       ContainerCreating   0         4m
o-name-1-fsmqv    1/1       Running             0         4m
o-name-1-fsmqv    0/1       Error               0         4m
o-name-1-deploy   0/1       Completed           0         4m
o-name-1-deploy   0/1       Terminating         0         4m
o-name-1-fsmqv    0/1       Terminating         0         4m

If the OpenShiftExtension detects the "Readiness" then the tests will pass.

LOGGER.info("Waiting until ready (" + config.getReadinessTimeout() + " ms)...");
waitUntilCondition(context, waitables, i -> OpenshiftReadiness.isReady(i), config.getReadinessTimeout(),

If, however, the client doesn't detect the brief readiness, the test preparation will throw an exception and the test will fail.

//Display the item status
waitables.stream().map(r -> client.resource(r).fromServer().get())
.forEach(i -> {
if (!OpenshiftReadiness.isReady(i)) {
readinessFailed(context);
LOGGER.warning(i.getKind() + ":" + i.getMetadata().getName() + " not ready!");
}
});
if (hasReadinessFailed(context)) {
throw new IllegalStateException("Readiness Failed");
}

As agreed, I'll add a further commit to fix the test artifacts that aren't OK.

@sonarcloud
Copy link

sonarcloud bot commented Aug 10, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@manusa
Copy link
Collaborator Author

manusa commented Aug 11, 2023

https://pipelines.actions.githubusercontent.com/serviceHosts/b12c677e-945a-40a8-bddc-04656f7a6480/_apis/pipelines/1/runs/3228/signedlogcontent/14?urlExpires=2023-08-11T03%3A20%3A49.7565121Z&urlSigningMethod=HMACV1&urlSignature=QEu%2FFW%2BR%2FAgrG9Vp2LmU8p3lN2D%2BB6RIly0S8CbBRZM%3D

2023-08-10T15:50:04.5559285Z [INFO] Performing s2i build.
2023-08-10T15:50:04.8434503Z Uploading directory "/home/runner/work/dekorate/dekorate/examples/spring-boot-with-groovy-on-openshift-example/target" as binary input for the build ...
2023-08-10T15:51:04.8632355Z ............
2023-08-10T15:51:04.8633144Z Uploading finished
2023-08-10T15:51:04.8635026Z Error from server (Timeout): the server was unable to return a response in the time allotted, but may still be processing the request (post buildconfigs.build.openshift.io spring-boot-with-groovy-on-openshift-example)
2023-08-10T15:51:04.8747059Z 15:51:04.870 [main] DEBUG io.fabric8.kubernetes.client.Config - Trying to configure client from Kubernetes config...
2023-08-10T15:51:04.8748251Z 15:51:04.870 [main] DEBUG io.fabric8.kubernetes.client.Config - Found for Kubernetes config at: [/home/runner/.kube/config].
2023-08-10T15:51:05.0572049Z [INFO] Created: DeploymentConfig name:spring-boot-with-groovy-on-openshift-example.
2023-08-10T15:51:05.0958426Z [INFO] Created: Service name:spring-boot-with-groovy-on-openshift-example.

The new failure looks more like just a flake. Seems that the OpenShift S2I build timed out for whatever reason (maybe not enough resources)

Same test run locally completes successfully:

[INFO] ------< io.dekorate:spring-boot-with-groovy-on-openshift-example >------
[INFO] Building Dekorate :: Examples :: Spring Boot with Groovy on Openshift 3.7-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
# ...
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 173.35 s - in io.dekorate.example.SpringBootOnOpenshiftIT
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] 
[INFO] --- maven-failsafe-plugin:3.0.0-M3:verify (default) @ spring-boot-with-groovy-on-openshift-example ---

@iocanel Could you please re-trigger the pipeline?

@metacosm
Copy link
Collaborator

@iocanel what's preventing this from being merged?

@iocanel
Copy link
Member

iocanel commented Aug 14, 2023

@iocanel what's preventing this from being merged?

Mostly, that I am on vacation. 😄

Copy link
Member

@iocanel iocanel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iocanel iocanel merged commit 8dd2868 into dekorateio:main Aug 14, 2023
8 checks passed
@metacosm
Copy link
Collaborator

@iocanel what's preventing this from being merged?

Mostly, that I am on vacation. 😄

Sorry, it wasn't in any of our shared calendars…

@manusa manusa deleted the deps/kubernetes-client branch August 21, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants